Bug 719494 - Closing tabs is too slow sometimes. r=mbrubeck
authorMargaret Leibovic <margaret.leibovic@gmail.com>
Tue, 24 Jan 2012 09:15:41 -0800
changeset 86472 111b399b10e42a1d613589061090305ec98026a0
parent 86471 fd472718b66ebbedb5f783a057da8725de2c8232
child 86473 fcec3385530b922626e5a4d07ee37c22cc2c9917
push id805
push userakeybl@mozilla.com
push dateWed, 01 Feb 2012 18:17:35 +0000
treeherdermozilla-aurora@6fb3bf232436 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmbrubeck
bugs719494
milestone12.0a1
Bug 719494 - Closing tabs is too slow sometimes. r=mbrubeck * * * Bug 719494 - (Part 2) Add back closeTab in JS so that add-ons can use it
mobile/android/base/GeckoApp.java
mobile/android/base/Tabs.java
mobile/android/chrome/content/browser.js
--- a/mobile/android/base/GeckoApp.java
+++ b/mobile/android/base/GeckoApp.java
@@ -951,20 +951,20 @@ abstract public class GeckoApp
                 //GeckoApp.mAppContext.doCameraCapture(message.getString("path"));
                 doCameraCapture();
             } else if (event.equals("Tab:Added")) {
                 Log.i(LOGTAG, "Created a new tab");
                 Tab tab = handleAddTab(message);
                 Boolean selected = message.getBoolean("selected");
                 if (selected)
                     handleSelectTab(tab.getId());
-            } else if (event.equals("Tab:Closed")) {
-                Log.i(LOGTAG, "Destroyed a tab");
+            } else if (event.equals("Tab:Close")) {
                 int tabId = message.getInt("tabID");
-                handleCloseTab(tabId);
+                Tab tab = Tabs.getInstance().getTab(tabId);
+                Tabs.getInstance().closeTab(tab);
             } else if (event.equals("Tab:ScreenshotData")) {
                 int tabId = message.getInt("tabID");
                 Tab tab = Tabs.getInstance().getTab(tabId);
                 processThumbnail(tab, null, Base64.decode(message.getString("data").substring(22), Base64.DEFAULT));
             } else if (event.equals("Tab:Selected")) {
                 int tabId = message.getInt("tabID");
                 Log.i(LOGTAG, "Switched to tab: " + tabId);
                 handleSelectTab(tabId);
@@ -1225,30 +1225,16 @@ abstract public class GeckoApp
             public void run() {
                 mBrowserToolbar.updateTabs(Tabs.getInstance().getCount());
             }
         });
 
         return tab;
     }
 
-    void handleCloseTab(final int tabId) {
-        final Tab tab = Tabs.getInstance().getTab(tabId);
-        Tabs.getInstance().removeTab(tabId);
-        tab.removeAllDoorHangers();
-
-        mMainHandler.post(new Runnable() { 
-            public void run() {
-                onTabsChanged(tab);
-                mBrowserToolbar.updateTabs(Tabs.getInstance().getCount());
-                mDoorHangerPopup.updatePopup();
-            }
-        });
-    }
-
     void handleSelectTab(int tabId) {
         final Tab tab = Tabs.getInstance().selectTab(tabId);
         if (tab == null)
             return;
 
         if (tab.getURL().equals("about:home"))
             showAboutHome();
         else
@@ -1661,17 +1647,17 @@ abstract public class GeckoApp
         GeckoAppShell.registerGeckoEventListener("DOMWindowClose", GeckoApp.mAppContext);
         GeckoAppShell.registerGeckoEventListener("log", GeckoApp.mAppContext);
         GeckoAppShell.registerGeckoEventListener("Content:LocationChange", GeckoApp.mAppContext);
         GeckoAppShell.registerGeckoEventListener("Content:SecurityChange", GeckoApp.mAppContext);
         GeckoAppShell.registerGeckoEventListener("Content:StateChange", GeckoApp.mAppContext);
         GeckoAppShell.registerGeckoEventListener("Content:LoadError", GeckoApp.mAppContext);
         GeckoAppShell.registerGeckoEventListener("onCameraCapture", GeckoApp.mAppContext);
         GeckoAppShell.registerGeckoEventListener("Tab:Added", GeckoApp.mAppContext);
-        GeckoAppShell.registerGeckoEventListener("Tab:Closed", GeckoApp.mAppContext);
+        GeckoAppShell.registerGeckoEventListener("Tab:Close", GeckoApp.mAppContext);
         GeckoAppShell.registerGeckoEventListener("Tab:Selected", GeckoApp.mAppContext);
         GeckoAppShell.registerGeckoEventListener("Tab:ScreenshotData", GeckoApp.mAppContext);
         GeckoAppShell.registerGeckoEventListener("Doorhanger:Add", GeckoApp.mAppContext);
         GeckoAppShell.registerGeckoEventListener("Doorhanger:Remove", GeckoApp.mAppContext);
         GeckoAppShell.registerGeckoEventListener("Menu:Add", GeckoApp.mAppContext);
         GeckoAppShell.registerGeckoEventListener("Menu:Remove", GeckoApp.mAppContext);
         GeckoAppShell.registerGeckoEventListener("Gecko:Ready", GeckoApp.mAppContext);
         GeckoAppShell.registerGeckoEventListener("Toast:Show", GeckoApp.mAppContext);
@@ -1992,17 +1978,17 @@ abstract public class GeckoApp
         GeckoAppShell.unregisterGeckoEventListener("DOMWindowClose", GeckoApp.mAppContext);
         GeckoAppShell.unregisterGeckoEventListener("log", GeckoApp.mAppContext);
         GeckoAppShell.unregisterGeckoEventListener("Content:LocationChange", GeckoApp.mAppContext);
         GeckoAppShell.unregisterGeckoEventListener("Content:SecurityChange", GeckoApp.mAppContext);
         GeckoAppShell.unregisterGeckoEventListener("Content:StateChange", GeckoApp.mAppContext);
         GeckoAppShell.unregisterGeckoEventListener("Content:LoadError", GeckoApp.mAppContext);
         GeckoAppShell.unregisterGeckoEventListener("onCameraCapture", GeckoApp.mAppContext);
         GeckoAppShell.unregisterGeckoEventListener("Tab:Added", GeckoApp.mAppContext);
-        GeckoAppShell.unregisterGeckoEventListener("Tab:Closed", GeckoApp.mAppContext);
+        GeckoAppShell.unregisterGeckoEventListener("Tab:Close", GeckoApp.mAppContext);
         GeckoAppShell.unregisterGeckoEventListener("Tab:Selected", GeckoApp.mAppContext);
         GeckoAppShell.unregisterGeckoEventListener("Tab:ScreenshotData", GeckoApp.mAppContext);
         GeckoAppShell.unregisterGeckoEventListener("Doorhanger:Add", GeckoApp.mAppContext);
         GeckoAppShell.unregisterGeckoEventListener("Menu:Add", GeckoApp.mAppContext);
         GeckoAppShell.unregisterGeckoEventListener("Menu:Remove", GeckoApp.mAppContext);
         GeckoAppShell.unregisterGeckoEventListener("Gecko:Ready", GeckoApp.mAppContext);
         GeckoAppShell.unregisterGeckoEventListener("Toast:Show", GeckoApp.mAppContext);
         GeckoAppShell.unregisterGeckoEventListener("ToggleChrome:Hide", GeckoApp.mAppContext);
--- a/mobile/android/base/Tabs.java
+++ b/mobile/android/base/Tabs.java
@@ -32,25 +32,26 @@
  * and other provisions required by the GPL or the LGPL. If you do not delete
  * the provisions above, a recipient may use your version of this file under
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
 package org.mozilla.gecko;
 
-import java.util.*;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.Iterator;
+
+import org.json.JSONException;
+import org.json.JSONObject;
 
 import android.content.ContentResolver;
-import android.graphics.drawable.*;
 import android.util.Log;
 
-import org.json.JSONObject;
-import org.json.JSONException;
-
 public class Tabs implements GeckoEventListener {
     private static final String LOGTAG = "GeckoTabs";
 
     private static int selectedTab = -1;
     private HashMap<Integer, Tab> tabs;
     private ArrayList<Tab> order;
     private ContentResolver resolver;
 
@@ -139,18 +140,35 @@ public class Tabs implements GeckoEventL
         closeTab(tab, getNextTab(tab));
     }
 
     /** Close tab and then select nextTab */
     public void closeTab(Tab tab, Tab nextTab) {
         if (tab == null || nextTab == null)
             return;
 
+        // TODO: Clean up the Tab:Select/Tab:Selected message passing so that we can
+        // immediately update the Java UI here (bug 719493)
         GeckoAppShell.sendEventToGecko(new GeckoEvent("Tab:Select", String.valueOf(nextTab.getId())));
-        GeckoAppShell.sendEventToGecko(new GeckoEvent("Tab:Close", String.valueOf(tab.getId())));
+
+        int tabId = tab.getId();
+        removeTab(tabId);
+        tab.removeAllDoorHangers();
+
+        final Tab closedTab = tab;
+        GeckoApp.mAppContext.mMainHandler.post(new Runnable() { 
+            public void run() {
+                GeckoApp.mAppContext.onTabsChanged(closedTab);
+                GeckoApp.mBrowserToolbar.updateTabs(Tabs.getInstance().getCount());
+                GeckoApp.mDoorHangerPopup.updatePopup();
+            }
+        });
+
+        // Pass a message to Gecko to update tab state in BrowserApp
+        GeckoAppShell.sendEventToGecko(new GeckoEvent("Tab:Closed", String.valueOf(tabId)));
     }
 
     /** Return the tab that will be selected by default after this one is closed */
     public Tab getNextTab(Tab tab) {
         Tab selectedTab = getSelectedTab();
         if (selectedTab != tab)
             return selectedTab;
 
--- a/mobile/android/chrome/content/browser.js
+++ b/mobile/android/chrome/content/browser.js
@@ -203,17 +203,17 @@ var BrowserApp = {
     BrowserEventHandler.init();
     ViewportHandler.init();
 
     getBridge().setDrawMetadataProvider(MetadataProvider);
 
     Services.obs.addObserver(this, "Tab:Add", false);
     Services.obs.addObserver(this, "Tab:Load", false);
     Services.obs.addObserver(this, "Tab:Select", false);
-    Services.obs.addObserver(this, "Tab:Close", false);
+    Services.obs.addObserver(this, "Tab:Closed", false);
     Services.obs.addObserver(this, "Tab:Screenshot", false);
     Services.obs.addObserver(this, "Session:Back", false);
     Services.obs.addObserver(this, "Session:Forward", false);
     Services.obs.addObserver(this, "Session:Reload", false);
     Services.obs.addObserver(this, "Session:Stop", false);
     Services.obs.addObserver(this, "SaveAs:PDF", false);
     Services.obs.addObserver(this, "Browser:Quit", false);
     Services.obs.addObserver(this, "Preferences:Get", false);
@@ -467,17 +467,37 @@ var BrowserApp = {
 
     let evt = document.createEvent("UIEvents");
     evt.initUIEvent("TabOpen", true, false, window, null);
     newTab.browser.dispatchEvent(evt);
 
     return newTab;
   },
 
+  // Use this method to close a tab from JS. This method sends a message
+  // to Java to close the tab in the Java UI (we'll get a Tab:Closed message
+  // back from Java when that happens).
   closeTab: function closeTab(aTab) {
+    if (!aTab) {
+      Cu.reportError("Error trying to close tab (tab doesn't exist)");
+      return;
+    }
+
+    let message = {
+      gecko: {
+        type: "Tab:Close",
+        tabID: aTab.id
+      }
+    };
+    sendMessageToJava(message);
+  },
+
+  // Calling this will update the state in BrowserApp after a tab has been
+  // closed in the Java UI.
+  _handleTabClosed: function _handleTabClosed(aTab) {
     if (aTab == this.selectedTab)
       this.selectedTab = null;
 
     let evt = document.createEvent("UIEvents");
     evt.initUIEvent("TabClose", true, false, window, null);
     aTab.browser.dispatchEvent(evt);
 
     aTab.destroy();
@@ -836,18 +856,18 @@ var BrowserApp = {
         params.showProgress = false;
 
       if (aTopic == "Tab:Add")
         this.addTab(url, params);
       else
         this.loadURI(url, browser, params);
     } else if (aTopic == "Tab:Select") {
       this.selectTab(this.getTabForId(parseInt(aData)));
-    } else if (aTopic == "Tab:Close") {
-      this.closeTab(this.getTabForId(parseInt(aData)));
+    } else if (aTopic == "Tab:Closed") {
+      this._handleTabClosed(this.getTabForId(parseInt(aData)));
     } else if (aTopic == "Tab:Screenshot") {
       this.screenshotTab(aData);
     } else if (aTopic == "Browser:Quit") {
       this.quit();
     } else if (aTopic == "SaveAs:PDF") {
       this.saveAsPDF(browser);
     } else if (aTopic == "Preferences:Get") {
       this.getPreferences(aData);
@@ -1417,24 +1437,16 @@ Tab.prototype = {
     // not stable when panels are removed.
     let selectedPanel = BrowserApp.deck.selectedPanel;
     BrowserApp.deck.removeChild(this.vbox);
     BrowserApp.deck.selectedPanel = selectedPanel;
 
     this.browser = null;
     this.vbox = null;
     this.documentIdForCurrentViewport = null;
-    let message = {
-      gecko: {
-        type: "Tab:Closed",
-        tabID: this.id
-      }
-    };
-
-    sendMessageToJava(message);
   },
 
   set active(aActive) {
     if (!this.browser)
       return;
 
     if (aActive) {
       this.browser.setAttribute("type", "content-primary");