browser(firefox): handle the case when inner window is restored from …

…history (#2791) When innerWindow is restored from the history state, we do not receive content-document-global-created notification, but would still like to know that window is now using a different inner window to reset the state. This introduces a new notification juggler-dom-window-reused. At the same time, goBack()/goForward() sometimes do not initiate navigation synchronously, so our check for pendingNaivgationId() does not work. Instead, we rely on canGoBack, and assume that client will not need the navigationId synchronously.
microsoft · Jul 1, 2020 · c4e3ed8 · c4e3ed8
1 parent e467ea5
commit c4e3ed8
Show file tree

Hide file tree

Showing 5 changed files with 76 additions and 18 deletions.
diff --git a/browser_patches/firefox/BUILD_NUMBER b/browser_patches/firefox/BUILD_NUMBER
@@ -1 +1 @@
-1116
+1117
diff --git a/browser_patches/firefox/juggler/content/FrameTree.js b/browser_patches/firefox/juggler/content/FrameTree.js
@@ -53,6 +53,7 @@ class FrameTree {
                   Ci.nsIWebProgress.NOTIFY_FRAME_LOCATION;
     this._eventListeners = [
       helper.addObserver(this._onDOMWindowCreated.bind(this), 'content-document-global-created'),
+      helper.addObserver(this._onDOMWindowCreated.bind(this), 'juggler-dom-window-reused'),
       helper.addObserver(subject => this._onDocShellCreated(subject.QueryInterface(Ci.nsIDocShell)), 'webnavigation-create'),
       helper.addObserver(subject => this._onDocShellDestroyed(subject.QueryInterface(Ci.nsIDocShell)), 'webnavigation-destroy'),
       helper.addProgressListener(webProgress, this, flags),

diff --git a/browser_patches/firefox/juggler/content/PageAgent.js b/browser_patches/firefox/juggler/content/PageAgent.js
@@ -378,6 +378,8 @@ class PageAgent {
   }
 
   _onDOMContentLoaded(event) {
+    if (!event.target.ownerGlobal)
+      return;
     const docShell = event.target.ownerGlobal.docShell;
     const frame = this._frameTree.frameForDocShell(docShell);
     if (!frame)
@@ -528,25 +530,24 @@ class PageAgent {
     const frame = this._frameTree.frame(frameId);
     const docShell = frame.docShell().QueryInterface(Ci.nsIWebNavigation);
     docShell.reload(Ci.nsIWebNavigation.LOAD_FLAGS_NONE);
-    return {navigationId: frame.pendingNavigationId(), navigationURL: frame.pendingNavigationURL()};
   }
 
   async _goBack({frameId, url}) {
     const frame = this._frameTree.frame(frameId);
     const docShell = frame.docShell();
     if (!docShell.canGoBack)
-      return {navigationId: null, navigationURL: null};
+      return {success: false};
     docShell.goBack();
-    return {navigationId: frame.pendingNavigationId(), navigationURL: frame.pendingNavigationURL()};
+    return {success: true};
   }
 
   async _goForward({frameId, url}) {
     const frame = this._frameTree.frame(frameId);
     const docShell = frame.docShell();
     if (!docShell.canGoForward)
-      return {navigationId: null, navigationURL: null};
+      return {success: false};
     docShell.goForward();
-    return {navigationId: frame.pendingNavigationId(), navigationURL: frame.pendingNavigationURL()};
+    return {success: true};
   }
 
   async _adoptNode({frameId, objectId, executionContextId}) {

diff --git a/browser_patches/firefox/juggler/protocol/Protocol.js b/browser_patches/firefox/juggler/protocol/Protocol.js
@@ -712,27 +712,21 @@ const Page = {
         frameId: t.String,
       },
       returns: {
-        navigationId: t.Nullable(t.String),
-        navigationURL: t.Nullable(t.String),
-      }
+        success: t.Boolean,
+      },
     },
     'goForward': {
       params: {
         frameId: t.String,
       },
       returns: {
-        navigationId: t.Nullable(t.String),
-        navigationURL: t.Nullable(t.String),
-      }
+        success: t.Boolean,
+      },
     },
     'reload': {
       params: {
         frameId: t.String,
       },
-      returns: {
-        navigationId: t.String,
-        navigationURL: t.String,
-      }
     },
     'getBoundingBox': {
       params: {

diff --git a/browser_patches/firefox/patches/bootstrap.diff b/browser_patches/firefox/patches/bootstrap.diff
@@ -652,10 +652,60 @@ index e268e2bbe8add1b43f6e4d6507cc7810d707a344..a34a7a292a02ea8d94042475a43ae3a0
    dom::MediaCapabilities* MediaCapabilities();
    dom::MediaSession* MediaSession();
 diff --git a/dom/base/nsGlobalWindowOuter.cpp b/dom/base/nsGlobalWindowOuter.cpp
-index da9d56e843a2c762dc7d5527712cdd3d30418f7f..ea9d962513dfc02682d5dfd543dd72ba78ab1745 100644
+index da9d56e843a2c762dc7d5527712cdd3d30418f7f..aa3d2d6022b60ac3a263c2d1df4eb12714d8e728 100644
 --- a/dom/base/nsGlobalWindowOuter.cpp
 +++ b/dom/base/nsGlobalWindowOuter.cpp
-@@ -3861,6 +3861,14 @@ Maybe<CSSIntSize> nsGlobalWindowOuter::GetRDMDeviceSize(
+@@ -2430,7 +2430,7 @@ nsresult nsGlobalWindowOuter::SetNewDocument(Document* aDocument,
+         &nsGlobalWindowInner::FireOnNewGlobalObject));
+   }
+
+-  if (newInnerWindow && !newInnerWindow->mHasNotifiedGlobalCreated && mDoc) {
++  if (newInnerWindow && mDoc) {
+     // We should probably notify. However if this is the, arguably bad,
+     // situation when we're creating a temporary non-chrome-about-blank
+     // document in a chrome docshell, don't notify just yet. Instead wait
+@@ -2449,10 +2449,16 @@ nsresult nsGlobalWindowOuter::SetNewDocument(Document* aDocument,
+     }();
+
+     if (!isContentAboutBlankInChromeDocshell) {
+-      newInnerWindow->mHasNotifiedGlobalCreated = true;
+-      nsContentUtils::AddScriptRunner(NewRunnableMethod(
+-          "nsGlobalWindowOuter::DispatchDOMWindowCreated", this,
+-          &nsGlobalWindowOuter::DispatchDOMWindowCreated));
++      if (!newInnerWindow->mHasNotifiedGlobalCreated) {
++        newInnerWindow->mHasNotifiedGlobalCreated = true;
++        nsContentUtils::AddScriptRunner(NewRunnableMethod(
++            "nsGlobalWindowOuter::DispatchDOMWindowCreated", this,
++            &nsGlobalWindowOuter::DispatchDOMWindowCreated));
++      } else if (!reUseInnerWindow) {
++        nsContentUtils::AddScriptRunner(NewRunnableMethod(
++            "nsGlobalWindowOuter::JugglerDispatchDOMWindowReused", this,
++            &nsGlobalWindowOuter::JugglerDispatchDOMWindowReused));
++      }
+     }
+   }
+
+@@ -2609,6 +2615,19 @@ void nsGlobalWindowOuter::DispatchDOMWindowCreated() {
+   }
+ }
+
++void nsGlobalWindowOuter::JugglerDispatchDOMWindowReused() {
++  nsCOMPtr<nsIObserverService> observerService =
++      mozilla::services::GetObserverService();
++  if (observerService && mDoc) {
++    nsIPrincipal* principal = mDoc->NodePrincipal();
++    if (!principal->IsSystemPrincipal()) {
++      observerService->NotifyObservers(static_cast<nsIDOMWindow*>(this),
++                                      "juggler-dom-window-reused",
++                                      nullptr);
++    }
++  }
++}
++
+ void nsGlobalWindowOuter::ClearStatus() { SetStatusOuter(EmptyString()); }
+
+ void nsGlobalWindowOuter::SetDocShell(nsDocShell* aDocShell) {
+@@ -3861,6 +3880,14 @@ Maybe<CSSIntSize> nsGlobalWindowOuter::GetRDMDeviceSize(
        }
      }
    }
@@ -670,6 +720,18 @@ index da9d56e843a2c762dc7d5527712cdd3d30418f7f..ea9d962513dfc02682d5dfd543dd72ba
    return Nothing();
  }
 
+diff --git a/dom/base/nsGlobalWindowOuter.h b/dom/base/nsGlobalWindowOuter.h
+index d2a8f0219f21800ef7800ed637c1f61d454a99a6..7e88989437613cec54d6bbf551afd572e28c4221 100644
+--- a/dom/base/nsGlobalWindowOuter.h
++++ b/dom/base/nsGlobalWindowOuter.h
+@@ -319,6 +319,7 @@ class nsGlobalWindowOuter final : public mozilla::dom::EventTarget,
+
+   // Outer windows only.
+   void DispatchDOMWindowCreated();
++  void JugglerDispatchDOMWindowReused();
+
+   // Outer windows only.
+   virtual void EnsureSizeAndPositionUpToDate() override;
 diff --git a/dom/base/nsINode.cpp b/dom/base/nsINode.cpp
 index 7c0d4bd261bf053c7cab4091522dc1e52118eba1..e320e6bfb0402eb07ae53272264278bad9638519 100644
 --- a/dom/base/nsINode.cpp