Bug 1351808 - Part 2 - Exclude non-standard tab types from session store. r?sebastian draft
authorJan Henning <jh+bugzilla@buttercookie.de>
Sat, 08 Apr 2017 13:43:09 +0200
changeset 569605 84d995d9822831c10633f776ab309f2f55d4e4d8
parent 569604 9e1dde56b474a00c7f070385cdea1e0dcda6b136
child 569606 edd1af6d7027507ecd85c30f74881e1342d821c3
push id56232
push usermozilla@buttercookie.de
push dateThu, 27 Apr 2017 18:04:06 +0000
reviewerssebastian
bugs1351808, 1352997, 1346008
milestone55.0a1
Bug 1351808 - Part 2 - Exclude non-standard tab types from session store. r?sebastian Restoring anything other than normal browsing tabs (e.g. custom tabs, web apps) is more involved because those tabs - don't appear in our normal tabs UI - are opened in separate activities - when we're starting up, Android's task switcher might or might not still have available task entries corresponding to such tabs from the last session Therefore, for now, the session store will simply exclude those kinds of tabs from being saved in the session store data. Instead of a real restore, if the corresponding tab has been closed or Gecko stopped running, we just recreate the custom tab/web app based on the stored Activity intent data we have available (bug 1352997). Tab zombification while Gecko is running however remains fully supported, as we continue collecting session history data for all tab types, even if we don't necessarily save it to disk. Because custom tabs/web apps currently still share a common Gecko browser window with normal tabs, we also have to modify our selected tab tracking logic accordingly, so that selecting one of these special tab types doesn't overwrite the last selected normal browsing tab. To that effect, we now track the selected tab *ID* in memory and only convert that to a tab index when writing the data to disk. As the ID remains stable while Gecko is running, this makes tracking changes for a sub-group of tabs only easier, as we don't have to watch out for closing tabs of *any* kind affecting the tab index of everything behind them. Bug 1346008#c3 has some preliminary ideas on how session restoring for custom tabs/web apps could be made to work. MozReview-Commit-ID: 1q5Jtv0DKrE
mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
mobile/android/chrome/content/browser.js
mobile/android/components/SessionStore.js
--- a/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
+++ b/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
@@ -1938,16 +1938,21 @@ public abstract class GeckoApp
                 // Update all parent tab IDs ...
                 parser.updateParentId(tabs);
                 windowObject.put("tabs", tabs);
                 // ... and for recently closed tabs as well (if we've got any).
                 final JSONArray closedTabs = windowObject.optJSONArray("closedTabs");
                 parser.updateParentId(closedTabs);
                 windowObject.putOpt("closedTabs", closedTabs);
 
+                if (isExternalURL) {
+                    // Pass on the tab we would have selected if we weren't going to open an
+                    // external URL later on.
+                    windowObject.put("selectedTabId", parser.getStoredSelectedTabId());
+                }
                 sessionString = new JSONObject().put(
                         "windows", new JSONArray().put(windowObject)).toString();
             } catch (final JSONException e) {
                 throw new SessionRestoreException(e);
             }
         } else {
             if (parser.allTabsSkipped() || sessionDataValid) {
                 // If we intentionally skipped all tabs we've read from the session file, we
--- a/mobile/android/chrome/content/browser.js
+++ b/mobile/android/chrome/content/browser.js
@@ -3655,17 +3655,18 @@ Tab.prototype = {
       entries: [{
         url: aURL,
         title: truncate(title, MAX_TITLE_LENGTH)
       }],
       index: 1,
       desktopMode: this.desktopMode,
       isPrivate: isPrivate,
       tabId: this.id,
-      parentId: this.parentId
+      parentId: this.parentId,
+      type: this.type
     };
 
     if (aParams.delayLoad) {
       // If this is a zombie tab, mark the browser for delay loading, which will
       // restore the tab when selected using the session data added above
       this.browser.__SS_restore = true;
       this.browser.setAttribute("pending", "true");
     } else {
--- a/mobile/android/components/SessionStore.js
+++ b/mobile/android/components/SessionStore.js
@@ -560,17 +560,17 @@ SessionStore.prototype = {
 
     // Ignore non-browser windows and windows opened while shutting down
     if (aWindow.document.documentElement.getAttribute("windowtype") != "navigator:browser" || this._loadState <= STATE_QUITTING) {
       return;
     }
 
     // Assign it a unique identifier (timestamp) and create its data object
     aWindow.__SSID = "window" + Date.now();
-    this._windows[aWindow.__SSID] = { tabs: [], selected: 0, closedTabs: [] };
+    this._windows[aWindow.__SSID] = { tabs: [], selectedTabId: INVALID_TAB_ID, closedTabs: [] };
 
     // Perform additional initialization when the first window is loading
     if (this._loadState == STATE_STOPPED) {
       this._loadState = STATE_RUNNING;
       this._lastSaveTime = Date.now();
     }
 
     // Add tab change listeners to all already existing tabs
@@ -678,18 +678,30 @@ SessionStore.prototype = {
     log("onTabRemove() ran for tab " + aWindow.BrowserApp.getTabForBrowser(aBrowser).id +
         ", aNoNotification = " + aNoNotification);
     if (!aNoNotification) {
       this.saveStateDelayed();
     }
   },
 
   onTabClose: function ss_onTabClose(aWindow, aBrowser, aTabIndex) {
-    let data = aBrowser.__SS_data || {};
-    if (this._maxTabsUndo == 0 || this._sessionDataIsEmpty(data)) {
+    let data = aBrowser.__SS_data;
+    let tab = aWindow.BrowserApp.getTabForId(data.tabId);
+
+    let windowData = this._windows[aWindow.__SSID];
+    if (windowData.selectedTabId == tab.id) {
+      // Normally, we will first select another tab anyway before closing the previous tab, which
+      // would make this logic moot. However
+      // - we only update the selected tab when selecting a normal BROWSING-type tab, and
+      // - in conjunction with switching between activities, the event order as we see it can
+      //   become reversed.
+      windowData.selectedTabId = INVALID_TAB_ID;
+    }
+
+    if (this._maxTabsUndo == 0 || this._sessionDataIsEmpty(data) || tab.type != "BROWSING") {
       this._lastClosedTabIndex = INVALID_TAB_INDEX;
       return;
     }
 
     if (aWindow.BrowserApp.tabs.length > 0) {
       // Bundle this browser's data and extra data and save in the closedTabs
       // window property
       data.extData = aBrowser.__SS_extdata || {};
@@ -701,17 +713,17 @@ SessionStore.prototype = {
       }
 
       this._lastClosedTabIndex = aTabIndex;
 
       if (this._notifyClosedTabs) {
         this._sendClosedTabsToJava(aWindow);
       }
 
-      log("onTabClose() ran for tab " + aWindow.BrowserApp.getTabForBrowser(aBrowser).id);
+      log("onTabClose() ran for tab " + tab.id);
       let evt = new Event("SSTabCloseProcessed", {"bubbles":true, "cancelable":false});
       aBrowser.dispatchEvent(evt);
     }
   },
 
   _sessionDataIsEmpty: function ss_sessionDataIsEmpty(aData) {
     if (!aData || !aData.entries || aData.entries.length == 0) {
       return true;
@@ -794,22 +806,23 @@ SessionStore.prototype = {
     this._updateCrashReportURL(aWindow);
   },
 
   onTabSelect: function ss_onTabSelect(aWindow, aBrowser) {
     if (this._loadState != STATE_RUNNING) {
       return;
     }
 
-    let index = aWindow.BrowserApp.selectedTabIndex;
-    this._windows[aWindow.__SSID].selected = parseInt(index) + 1; // 1-based
-
     let tab = aWindow.BrowserApp.getTabForBrowser(aBrowser);
     let tabId = tab.id;
 
+    if (tab.type == "BROWSING") {
+      this._windows[aWindow.__SSID].selectedTabId = tabId;
+    }
+
     // Restore the resurrected browser
     if (tabId != this._keepAsZombieTabId) {
       this.restoreZombieTab(tab);
     } else {
       log("keeping as zombie tab " + tabId);
     }
     // The tab id passed through Tab:KeepZombified is valid for one TabSelect only.
     this._keepAsZombieTabId = INVALID_TAB_ID;
@@ -1047,33 +1060,40 @@ SessionStore.prototype = {
     log("_saveState() current state collected");
 
     for (let winIndex = 0; winIndex < data.windows.length; ++winIndex) {
       let win = data.windows[winIndex];
       let normalWin = {};
       for (let prop in win) {
         normalWin[prop] = data[prop];
       }
+      // This particular attribute will be converted to a tab index further down
+      // and stored in the appropriate (normal or private) window data.
+      delete normalWin.selectedTabId;
       normalWin.tabs = [];
 
       // Save normal closed tabs. Forget about private closed tabs.
       normalWin.closedTabs = win.closedTabs.filter(tab => !tab.isPrivate);
 
       normalData.windows.push(normalWin);
       privateData.windows.push({ tabs: [] });
 
       // Split the session data into private and non-private data objects.
       // Non-private session data will be saved to disk, and private session
       // data will be sent to Java for Android to hold it in memory.
       for (let i = 0; i < win.tabs.length; ++i) {
         let tab = win.tabs[i];
+        if (tab.type != "BROWSING") {
+          continue;
+        }
+
         let savedWin = tab.isPrivate ? privateData.windows[winIndex] : normalData.windows[winIndex];
         savedWin.tabs.push(tab);
-        if (win.selected == i + 1) {
-          savedWin.selected = savedWin.tabs.length;
+        if (win.selectedTabId === tab.tabId) {
+          savedWin.selected = savedWin.tabs.length; // 1-based index
         }
       }
     }
 
     // Write only non-private data to disk
     if (normalData.windows[0] && normalData.windows[0].tabs) {
       log("_saveState() writing normal data, " +
            normalData.windows[0].tabs.length + " tabs in window[0]");
@@ -1132,18 +1152,21 @@ SessionStore.prototype = {
     // Ignore windows not tracked by SessionStore
     if (!aWindow.__SSID || !this._windows[aWindow.__SSID]) {
       return;
     }
 
     let winData = this._windows[aWindow.__SSID];
     winData.tabs = [];
 
-    let index = aWindow.BrowserApp.selectedTabIndex;
-    winData.selected = parseInt(index) + 1; // 1-based
+    let selectedTab = aWindow.BrowserApp.selectedTab;
+
+    if (selectedTab != null && selectedTab.type == "BROWSING") {
+      winData.selectedTabId = selectedTab.id;
+    }
 
     let tabs = aWindow.BrowserApp.tabs;
     for (let i = 0; i < tabs.length; i++) {
       let browser = tabs[i].browser;
       if (browser.__SS_data) {
         let tabData = browser.__SS_data;
         if (browser.__SS_extdata) {
           tabData.extData = browser.__SS_extdata;
@@ -1428,23 +1451,34 @@ SessionStore.prototype = {
         tab.browser.__SS_restoreReloadPending = true;
 
         this._restoreTab(tabData, tab.browser);
         this._startupRestoreFinished = true;
         log("startupRestoreFinished = true");
 
         delete tab.browser.__SS_restore;
         tab.browser.removeAttribute("pending");
+
+        this._windows[window.__SSID].selectedTabId = tab.id;
       } else {
         // Mark the browser for delay loading
         tab.browser.__SS_restore = true;
         tab.browser.setAttribute("pending", "true");
       }
     }
 
+    if (state.windows[0].hasOwnProperty("selectedTabId") &&
+      this._windows[window.__SSID].selectedTabId == INVALID_TAB_ID) {
+      // If none of the restored tabs was the selected tab, we might be opening an URL from an
+      // external intent. If this new tab is a normal BROWSING tab, we'll catch its selection
+      // anyway, however if we've opened a custom tab/web app or anything like that we want to
+      // ignore it. So instead, we store the tab we would have selected from the session file.
+      this._windows[window.__SSID].selectedTabId = state.windows[0].selectedTabId;
+    }
+
     // Restore the closed tabs array on the current window.
     if (state.windows[0].closedTabs && this._maxTabsUndo > 0) {
       this._windows[window.__SSID].closedTabs = state.windows[0].closedTabs;
       log("_restoreWindow() loaded " + state.windows[0].closedTabs.length + " closed tabs");
     }
   },
 
   getClosedTabCount: function ss_getClosedTabCount(aWindow) {