Bug 1451913 P0 Delay rebinding and freeing the old inner window until after set the new inner window. r=smaug

dom/base/nsGlobalWindowInner.cpp

@@ -2473,6 +2473,35 @@ nsGlobalWindowInner::NoteCalledRegisterForServiceWorkerScope(const nsACString& a
   mClientSource->NoteCalledRegisterForServiceWorkerScope(aScope);
 }
 
+void
+nsGlobalWindowInner::MigrateStateForDocumentOpen(nsGlobalWindowInner* aOldInner)
+{
+  MOZ_DIAGNOSTIC_ASSERT(aOldInner);
+  MOZ_DIAGNOSTIC_ASSERT(aOldInner != this);
+  MOZ_DIAGNOSTIC_ASSERT(mDoc);
+
+  // Make a copy of the old window's performance object on document.open.
+  // Note that we have to force eager creation of it here, because we need
+  // to grab the current document channel and whatnot before that changes.
+  aOldInner->CreatePerformanceObjectIfNeeded();
+  if (aOldInner->mPerformance) {
+    mPerformance =
+      Performance::CreateForMainThread(this,
+                                       mDoc->NodePrincipal(),
+                                       aOldInner->mPerformance->GetDOMTiming(),
+                                       aOldInner->mPerformance->GetChannel());
+  }
+
+  // Rebind DETH objects to the new global created by document.open().
+  // XXX: Is this correct?  We should consider if the spec and our
+  //      implementation should change to match other browsers by
+  //      just reusing the current window.  (Bug 1449992)
+  aOldInner->ForEachEventTargetObject(
+    [&] (DOMEventTargetHelper* aDETH, bool* aDoneOut) {
+      aDETH->BindToOwner(this->AsInner());
+    });
+}
+
 void
 nsGlobalWindowInner::UpdateTopInnerWindow()
 {

dom/base/nsGlobalWindowInner.h

@@ -1151,6 +1151,8 @@ private:
   CallState ShouldReportForServiceWorkerScopeInternal(const nsACString& aScope,
                                                       bool* aResultOut);
 
+  void
+  MigrateStateForDocumentOpen(nsGlobalWindowInner* aOldInner);
 
 public:
   // Timeout Functions

dom/base/nsGlobalWindowOuter.cpp

@@ -1725,7 +1725,7 @@ nsGlobalWindowOuter::SetNewDocument(nsIDocument* aDocument,
 
   mContext->WillInitializeContext();
 
-  nsGlobalWindowInner *currentInner = GetCurrentInnerWindowInternal();
+  RefPtr<nsGlobalWindowInner> currentInner = GetCurrentInnerWindowInternal();
 
   if (currentInner && currentInner->mNavigator) {
     currentInner->mNavigator->OnNavigation();
@@ -1739,6 +1739,9 @@ nsGlobalWindowOuter::SetNewDocument(nsIDocument* aDocument,
   nsCOMPtr<WindowStateHolder> wsh = do_QueryInterface(aState);
   NS_ASSERTION(!aState || wsh, "What kind of weird state are you giving me here?");
 
+  bool handleDocumentOpen = false;
+  bool doomCurrentInner = false;
+
   JS::Rooted<JSObject*> newInnerGlobal(cx);
   if (reUseInnerWindow) {
     // We're reusing the current inner window.
@@ -1822,32 +1825,13 @@ nsGlobalWindowOuter::SetNewDocument(nsIDocument* aDocument,
 
     if (currentInner && currentInner->GetWrapperPreserveColor()) {
       if (oldDoc == aDocument) {
-        // Make a copy of the old window's performance object on document.open.
-        // Note that we have to force eager creation of it here, because we need
-        // to grab the current document channel and whatnot before that changes.
-        currentInner->AsInner()->CreatePerformanceObjectIfNeeded();
-        if (currentInner->mPerformance) {
-          newInnerWindow->mPerformance =
-            Performance::CreateForMainThread(newInnerWindow->AsInner(),
-                                             aDocument->NodePrincipal(),
-                                             currentInner->mPerformance->GetDOMTiming(),
-                                             currentInner->mPerformance->GetChannel());
-        }
-
-        // Rebind DETH objects to the new global created by document.open().
-        // XXX: Is this correct?  We should consider if the spec and our
-        //      implementation should change to match other browsers by
-        //      just reusing the current window.  (Bug 1449992)
-        currentInner->ForEachEventTargetObject(
-          [&] (DOMEventTargetHelper* aDETH, bool* aDoneOut) {
-            aDETH->BindToOwner(newInnerWindow->AsGlobal());
-          });
+        handleDocumentOpen = true;
       }
 
       // Don't free objects on our current inner window if it's going to be
       // held in the bfcache.
       if (!currentInner->IsFrozen()) {
-        currentInner->FreeInnerObjects();
+        doomCurrentInner = true;
       }
     }
 
@@ -1998,6 +1982,19 @@ nsGlobalWindowOuter::SetNewDocument(nsIDocument* aDocument,
     newInnerWindow->mChromeEventHandler = mChromeEventHandler;
   }
 
+  // Handle any document.open() logic after we setup the new inner window
+  // so that any bound DETH objects can see the top window, document, etc.
+  if (handleDocumentOpen) {
+    newInnerWindow->MigrateStateForDocumentOpen(currentInner);
+  }
+
+  // We no longer need the old inner window.  Start its destruction if
+  // its not being reused and clear our reference.
+  if (doomCurrentInner) {
+    currentInner->FreeInnerObjects();
+  }
+  currentInner = nullptr;
+
   // Ask the JS engine to assert that it's valid to access our DocGroup whenever
   // it runs JS code for this compartment. We skip the check if this window is
   // for chrome JS or an add-on.