Backed out 3 changesets (bug 1700963) for Browser-chrome failures in docshell/test/browser/browser_bug1309900_crossProcessHistoryNavigation.js. CLOSED TREE

Backed out changeset cda35e554327 (bug 1700963)
Backed out changeset fbafd75b06d3 (bug 1700963)
Backed out changeset 8b922e4f612b (bug 1700963)

browser/components/sessionstore/ContentSessionStore.jsm

@@ -522,7 +522,9 @@ class ContentSessionStore {
     this.contentRestoreInitialized = false;
 
     this.handlers = [this.messageQueue];
-    if (!Services.appinfo.sessionHistoryInParent) {
+    if (Services.appinfo.sessionHistoryInParent) {
+      this.mm.sendAsyncMessage("SessionStore:addSHistoryListener");
+    } else {
       this.handlers.push(new EventListener(this));
       this.handlers.push(new SessionHistoryListener(this));
 

browser/components/sessionstore/SessionStore.jsm

@@ -53,8 +53,6 @@ const OBSERVING = [
   "browser:purge-session-history-for-domain",
   "idle-daily",
   "clear-origin-attributes-data",
-  "browsing-context-did-set-embedder",
-  "browsing-context-discarded",
 ];
 
 // XUL Window properties to (re)store
@@ -97,6 +95,10 @@ const MESSAGES = [
 
   // The content script encountered an error.
   "SessionStore:error",
+
+  // The content script asks us to add the session history listener in the
+  // parent process when sessionHistory is in the parent process.
+  "SessionStore:addSHistoryListener",
 ];
 
 // The list of messages we accept from <xul:browser>s that have no tab
@@ -112,6 +114,9 @@ const NOTAB_MESSAGES = new Set([
 
   // For a description see above.
   "SessionStore:error",
+
+  // For a description see above.
+  "SessionStore:addSHistoryListener",
 ]);
 
 // The list of messages we accept without an "epoch" parameter.
@@ -122,6 +127,9 @@ const NOEPOCH_MESSAGES = new Set([
 
   // For a description see above.
   "SessionStore:error",
+
+  // For a description see above.
+  "SessionStore:addSHistoryListener",
 ]);
 
 // The list of messages we want to receive even during the short period after a
@@ -986,83 +994,40 @@ var SessionStoreInternal = {
           this._forgetTabsWithUserContextId(userContextId);
         }
         break;
-      case "browsing-context-did-set-embedder":
-        if (Services.appinfo.sessionHistoryInParent) {
-          if (
-            aSubject &&
-            aSubject === aSubject.top &&
-            aSubject.isContent &&
-            aSubject.embedderElement
-          ) {
-            this.addSHistoryListener(aSubject.embedderElement, aSubject);
-          }
-        }
-        break;
-      case "browsing-context-discarded":
-        if (Services.appinfo.sessionHistoryInParent) {
-          let listener = this._browserSHistoryListener.get(
-            aSubject.embedderElement?.permanentKey
-          );
-          if (listener) {
-            listener.uninstall();
-          }
-        }
-        break;
     }
   },
 
-  // Create a new SessionHistory listener on the provided browser element and
+  // Create a sHistoryListener and register it.
-  // save a reference to that listener on the _browserSHistoryListener map.
+  // We also need to save the SHistoryLister into this._browserSHistoryListener.
-  addSHistoryListener(aBrowser, aBrowsingContext) {
+  addSHistoryListener(aBrowser) {
-    class SHistoryListener {
+    function SHistoryListener(browser) {
-      constructor(browser, browsingContext) {
+      browser.browsingContext.sessionHistory.addSHistoryListener(this);
-        if (!browsingContext.sessionHistory) {
-          throw new Error("no SessionHistory object!");
-        }
 
-        this._permanentKey = browser.permanentKey;
+      this.browserId = browser.browsingContext.browserId;
-        this._browserId = browsingContext.browserId;
+      this._fromIdx = kNoIndex;
-        this._fromIdx = kNoIndex;
+      this._sHistoryChanges = false;
+      this._permanentKey = browser.permanentKey;
+      if (browser.currentURI && browser.ownerGlobal) {
+        this._lastKnownBody = browser.ownerGlobal.document.body;
+      }
+    }
+    SHistoryListener.prototype = {
+      QueryInterface: ChromeUtils.generateQI([
+        "nsISHistoryListener",
+        "nsISupportsWeakReference",
+      ]),
 
-        // Immediately collect data if we start with a non-empty SHistory.
+      notifySHistoryChanges(index) {
-        if (
-          browsingContext.currentURI?.spec !== "about:blank" ||
-          browsingContext.sessionHistory.count !== 0
-        ) {
-          browser.frameLoader.requestSHistoryUpdate(/* aImmediately */ true);
-        }
-
-        browsingContext.sessionHistory.addSHistoryListener(this);
-      }
-      uninstall() {
-        let bc = BrowsingContext.getCurrentTopByBrowserId(this._browserId);
-        if (bc?.sessionHistory) {
-          bc.sessionHistory.removeSHistoryListener(this);
-        }
-        SessionStoreInternal._browserSHistoryListener.delete(
-          this._permanentKey
-        );
-      }
-      reset() {
-        this._fromIdx = kNoIndex;
-      }
-      didCollect() {
-        return this._fromIdx !== kNoIndex;
-      }
-      collectFrom(index) {
         if (this._fromIdx <= index) {
-          // If we already know that we need to update history from index N we
+          // If we already know that we need to update history from index N we can ignore any changes
-          // can ignore any changes that happened with an element with index
+          // that happened with an element with index larger than N.
-          // larger than N.
+          // Note: initially we use kNoIndex which is MAX_SAFE_INTEGER which means we don't ignore anything
-          //
+          // here, and in case of navigation in the history back and forth we use kLastIndex which ignores
-          // Note: initially we use kNoIndex which is MAX_SAFE_INTEGER which
-          // means we don't ignore anything here, and in case of navigation in
-          // the history back and forth cases we use kLastIndex which ignores
           // only the subsequent navigations, but not any new elements added.
           return;
         }
 
-        let browser = BrowsingContext.getCurrentTopByBrowserId(this._browserId)
+        let browser = BrowsingContext.getCurrentTopByBrowserId(this.browserId)
           ?.embedderElement;
 
         if (!browser) {
@@ -1070,40 +1035,49 @@ var SessionStoreInternal = {
           return;
         }
 
-        if (!this.didCollect()) {
+        if (!this._sHistoryChanges) {
-          browser.frameLoader.requestSHistoryUpdate(/* aImmediately */ false);
+          browser.frameLoader.requestSHistoryUpdate(/*aImmediately*/ false);
+          this._sHistoryChanges = true;
         }
-
         this._fromIdx = index;
-      }
+        if (browser.currentURI && browser.ownerGlobal) {
-      OnHistoryNewEntry(newURI, oldIndex) {
+          this._lastKnownBody = browser.ownerGlobal.document.body;
-        // We use oldIndex - 1 to collect the current entry as well. This makes
+        }
-        // sure to collect any changes that were made to the entry while the
+      },
-        // document was active.
-        this.collectFrom(oldIndex == -1 ? oldIndex : oldIndex - 1);
-      }
-      OnHistoryGotoIndex() {
-        this.collectFrom(kLastIndex);
-      }
-      OnHistoryPurge() {
-        this.collectFrom(-1);
-      }
-      OnHistoryReload() {
-        this.collectFrom(-1);
-        return true;
-      }
-      OnHistoryReplaceEntry() {
-        this.collectFrom(-1);
-      }
-    }
-    SHistoryListener.prototype.QueryInterface = ChromeUtils.generateQI([
-      "nsISHistoryListener",
-      "nsISupportsWeakReference",
-    ]);
 
-    if (!aBrowser || !aBrowser.permanentKey) {
+      uninstall() {
-      return;
+        let bc = BrowsingContext.getCurrentTopByBrowserId(this.browserId);
-    }
+
+        if (bc?.sessionHistory) {
+          bc.sessionHistory.removeSHistoryListener(this);
+        }
+        SessionStoreInternal._browserSHistoryListener.delete(
+          this._permanentKey
+        );
+      },
+
+      OnHistoryNewEntry(newURI, oldIndex) {
+        // We use oldIndex - 1 to collect the current entry as well. This makes sure to
+        // collect any changes that were made to the entry while the document was active.
+        this.notifySHistoryChanges(oldIndex == -1 ? oldIndex : oldIndex - 1);
+      },
+
+      OnHistoryGotoIndex() {
+        this.notifySHistoryChanges(kLastIndex);
+      },
+      OnHistoryPurge() {
+        this.notifySHistoryChanges(-1);
+      },
+
+      OnHistoryReload() {
+        this.notifySHistoryChanges(-1);
+        return true;
+      },
+
+      OnHistoryReplaceEntry() {
+        this.notifySHistoryChanges(-1);
+      },
+    };
 
     // Don't bother registering another listener if we already have one for this
     // browser. We would've already updated the listener's state in response to
@@ -1112,8 +1086,20 @@ var SessionStoreInternal = {
       return;
     }
 
-    let listener = new SHistoryListener(aBrowser, aBrowsingContext);
+    // XXX: When can this happen?
+    if (!aBrowser.browsingContext?.sessionHistory) {
+      throw new Error("no SessionHistory object");
+    }
+
+    let listener = new SHistoryListener(aBrowser);
     this._browserSHistoryListener.set(aBrowser.permanentKey, listener);
+
+    // Collect data if we start with a non-empty shistory.
+    let uri = aBrowser.currentURI.displaySpec;
+    let history = aBrowser.frameLoader.browsingContext.sessionHistory;
+    if (uri != "about:blank" || history.count != 0) {
+      aBrowser.frameLoader.requestSHistoryUpdate(/*aImmediately*/ true);
+    }
   },
 
   /**
@@ -1249,45 +1235,62 @@ var SessionStoreInternal = {
       return;
     }
 
-    // Only collect history changes for toplevel contexts.
+    let sHistoryChangedInListener = false;
-    if (aBrowsingContext === aBrowsingContext.top) {
+    let listener = this._browserSHistoryListener.get(aBrowser.permanentKey);
-      let listener = this._browserSHistoryListener.get(aBrowser.permanentKey);
+    if (listener) {
+      sHistoryChangedInListener = listener._sHistoryChanges;
+    }
+
+    if (aData.sHistoryNeeded || sHistoryChangedInListener) {
       if (!listener) {
-        this.addSHistoryListener(aBrowser, aBrowsingContext);
+        debug(
+          "updateSessionStoreFromTablistener() with aData.sHistoryNeeded, but no SHlistener. Add again!!!"
+        );
+        this.addSHistoryListener(aBrowser);
         listener = this._browserSHistoryListener.get(aBrowser.permanentKey);
       }
-      if (!listener) {
-        throw new Error("no SHistoryListener!");
-      }
 
-      let needsFullCollect = !!aData.sHistoryNeeded;
+      if (listener) {
-      if (needsFullCollect || listener.didCollect()) {
+        if (!aData.sHistoryNeeded && listener._fromIdx == kNoIndex) {
-        if (!aBrowsingContext.sessionHistory) {
+          // No shistory changes needed.
-          throw new Error("no SessionHistory object!");
+          listener._sHistoryChanges = false;
+        } else if (aBrowsingContext.sessionHistory) {
+          let uri = aBrowsingContext.currentURI?.displaySpec;
+          let body =
+            aBrowser.ownerGlobal?.document.body ?? listener._lastKnownBody;
+          // If aData.sHistoryNeeded we need to collect all session
+          // history entries, because with SHIP this indicates that we
+          // either saw 'DOMTitleChanged' in
+          // mozilla::dom::TabListener::HandleEvent or
+          // 'OnDocumentStart/OnDocumentEnd' was called on
+          // mozilla::dom::ContentSessionStore, and both needs a full
+          // collect.
+          aData.data.historychange = SessionHistory.collectFromParent(
+            uri,
+            body,
+            aBrowsingContext.sessionHistory,
+            listener._sHistoryChanges && !aData.sHistoryNeeded
+              ? listener._fromIdx
+              : -1
+          );
+          listener._sHistoryChanges = false;
+          listener._fromIdx = kNoIndex;
+        } else {
+          debug(
+            "updateSessionStoreFromTablistener() with sHistoryNeeded, but no sessionHistory.\n"
+          );
         }
-
+      } else {
-        // If |needsFullCollect| we need to collect all session history entries,
+        debug(
-        // because with SHIP this indicates that we either saw 'DOMTitleChanged'
+          "updateSessionStoreFromTablistener() with sHistoryNeeded, but no sHlistener.\n"
-        // in mozilla::dom::TabListener::HandleEvent or 'OnDocument{Start,End}'
-        // was called on mozilla::dom::ContentSessionStore, and both need a full
-        // collect.
-        let fromIdx = -1;
-        if (listener.didCollect() && !needsFullCollect) {
-          fromIdx = listener._fromIdx;
-        }
-
-        aData.data.historychange = SessionHistory.collectFromParent(
-          aBrowsingContext.currentURI?.spec,
-          true, // Bug 1704574
-          aBrowsingContext.sessionHistory,
-          fromIdx
         );
-
-        listener.reset();
       }
     }
 
-    delete aData.sHistoryNeeded; // Avoid saving this to disk.
+    if ("sHistoryNeeded" in aData) {
+      delete aData.sHistoryNeeded;
+    }
+
     TabState.update(aBrowser, aData);
     let win = aBrowser.ownerGlobal;
     this.saveStateDelayed(win);
@@ -1329,6 +1332,9 @@ var SessionStoreInternal = {
     }
 
     switch (aMessage.name) {
+      case "SessionStore:addSHistoryListener":
+        this.addSHistoryListener(browser);
+        break;
       case "SessionStore:update":
         // |browser.frameLoader| might be empty if the browser was already
         // destroyed and its tab removed. In that case we still have the last

docshell/base/BrowsingContext.cpp

@@ -688,13 +688,6 @@ void BrowsingContext::SetEmbedderElement(Element* aEmbedder) {
   }
 
   mEmbedderElement = aEmbedder;
-
-  if (mEmbedderElement) {
-    if (nsCOMPtr<nsIObserverService> obs = services::GetObserverService()) {
-      obs->NotifyWhenScriptSafe(ToSupports(this),
-                                "browsing-context-did-set-embedder", nullptr);
-    }
-  }
 }
 
 void BrowsingContext::Embed() {

toolkit/modules/sessionstore/SessionHistory.jsm

@@ -42,13 +42,8 @@ var SessionHistory = Object.freeze({
     return SessionHistoryInternal.collect(docShell, aFromIdx);
   },
 
-  collectFromParent(uri, documentHasChildNodes, history, aFromIdx = -1) {
+  collectFromParent(uri, body, history, aFromIdx = -1) {
-    return SessionHistoryInternal.collectCommon(
+    return SessionHistoryInternal.collectCommon(uri, body, history, aFromIdx);
-      uri,
-      documentHasChildNodes,
-      history,
-      aFromIdx
-    );
   },
 
   restore(docShell, tabData) {
@@ -106,15 +101,10 @@ var SessionHistoryInternal = {
     let uri = webNavigation.currentURI.displaySpec;
     let body = webNavigation.document.body;
     let history = webNavigation.sessionHistory;
-    return this.collectCommon(
+    return this.collectCommon(uri, body, history.legacySHistory, aFromIdx);
-      uri,
-      body && body.hasChildNodes(),
-      history.legacySHistory,
-      aFromIdx
-    );
   },
 
-  collectCommon(uri, documentHasChildNodes, shistory, aFromIdx) {
+  collectCommon(uri, body, shistory, aFromIdx) {
     let data = {
       entries: [],
       requestedIndex: shistory.requestedIndex + 1,
@@ -152,7 +142,7 @@ var SessionHistoryInternal = {
       // or it's a blank tab that was modified (like a custom newtab page),
       // record it. For about:blank we explicitly want an empty array without
       // an 'index' property to denote that there are no history entries.
-      if (uri != "about:blank" || documentHasChildNodes) {
+      if (uri != "about:blank" || (body && body.hasChildNodes())) {
         data.entries.push({
           url: uri,
           triggeringPrincipal_base64: E10SUtils.SERIALIZED_SYSTEMPRINCIPAL,