Bug 1588220: Keep track of last active inner window when BrowsingContext is discarded. r=bzbarsky

Any number of outer windows may be attached to a BrowsingContext over its
lifetime. While the BrowsingContext is alive, it's easy to keep track of which
of these is active, and therefore which of its inner windows is active. After
it has been discarded, though, it discards its docShell reference, so all we
can tell about an inner window is whether it is active for its own outer
window, but not whether it should be considered active for its
BrowsingContext.

This patch updates the BrowsingContext detach logic to store a flag on the
current inner window recording that it was active when its BrowsingContext was
detached, and then later checks that flag to determine if it is the current
window for a detached BrowsingContext.

Differential Revision: https://phabricator.services.mozilla.com/D49032

--HG--
extra : moz-landing-system : lando

docshell/base/nsDocShell.cpp

@@ -4756,7 +4756,7 @@ nsDocShell::Destroy() {
   mCurrentURI = nullptr;
 
   if (mScriptGlobal) {
-    mScriptGlobal->DetachFromDocShell();
+    mScriptGlobal->DetachFromDocShell(!mSkipBrowsingContextDetachOnDestroy);
     mScriptGlobal = nullptr;
   }
 

dom/base/nsGlobalWindowInner.cpp

@@ -856,6 +856,7 @@ nsGlobalWindowInner::nsGlobalWindowInner(nsGlobalWindowOuter* aOuterWindow,
       mHasGamepad(false),
       mHasVREvents(false),
       mHasVRDisplayActivateEvents(false),
+      mWasCurrentInnerWindow(false),
       mHasSeenGamepadInput(false),
       mSuspendDepth(0),
       mFreezeDepth(0),
@@ -2548,19 +2549,13 @@ bool nsPIDOMWindowInner::IsCurrentInnerWindow() const {
   auto* bc = GetBrowsingContext();
   MOZ_ASSERT(bc);
 
-  nsPIDOMWindowOuter* outer;
-  // When a BC is discarded, it stops returning outer windows altogether. That
-  // doesn't work for this check, since we still want current inner window to be
-  // treated as current after that point. Simply falling back to `mOuterWindow`
-  // here isn't ideal, since it will start returning true for inner windows
-  // which were current before a remoteness switch once a BrowsingContext has
-  // been discarded, but it's not incorrect in a way which should cause
-  // significant issues.
-  if (!bc->IsDiscarded()) {
-    outer = bc->GetDOMWindow();
-  } else {
-    outer = mOuterWindow;
+  if (bc->IsDiscarded()) {
+    // If our BrowsingContext has been discarded, we consider ourselves
+    // still-current if we were current at the time it was discarded.
+    return WasCurrentInnerWindow();
   }
+
+  nsPIDOMWindowOuter* outer = bc->GetDOMWindow();
   return outer && outer->GetCurrentInnerWindow() == this;
 }
 

dom/base/nsGlobalWindowInner.h

@@ -1295,9 +1295,17 @@ class nsGlobalWindowInner final : public mozilla::dom::EventTarget,
 
   // Indicates whether this window wants VRDisplayActivate events
   bool mHasVRDisplayActivateEvents : 1;
+
+  // True if this was the currently-active inner window for a BrowsingContext at
+  // the time it was discarded.
+  bool mWasCurrentInnerWindow : 1;
+  void SetWasCurrentInnerWindow() { mWasCurrentInnerWindow = true; }
+  bool WasCurrentInnerWindow() const override { return mWasCurrentInnerWindow; }
+
+  bool mHasSeenGamepadInput : 1;
+
   nsCheapSet<nsUint32HashKey> mGamepadIndexSet;
   nsRefPtrHashtable<nsUint32HashKey, mozilla::dom::Gamepad> mGamepads;
-  bool mHasSeenGamepadInput;
 
   RefPtr<nsScreen> mScreen;
 

dom/base/nsGlobalWindowOuter.cpp

@@ -2458,7 +2458,7 @@ void nsGlobalWindowOuter::SetDocShell(nsDocShell* aDocShell) {
   SetIsBackgroundInternal(!docShellActive);
 }
 
-void nsGlobalWindowOuter::DetachFromDocShell() {
+void nsGlobalWindowOuter::DetachFromDocShell(bool aIsBeingDiscarded) {
   // DetachFromDocShell means the window is being torn down. Drop our
   // reference to the script context, allowing it to be deleted
   // later. Meanwhile, keep our weak reference to the script object
@@ -2520,6 +2520,14 @@ void nsGlobalWindowOuter::DetachFromDocShell() {
     mContext = nullptr;
   }
 
+  if (aIsBeingDiscarded) {
+    // If our BrowsingContext is being discarded, make a note that our current
+    // inner window was active at the time it went away.
+    if (GetCurrentInnerWindow()) {
+      GetCurrentInnerWindowInternal()->SetWasCurrentInnerWindow();
+    }
+  }
+
   mDocShell = nullptr;
   mBrowsingContext->ClearDocShell();
 

dom/base/nsGlobalWindowOuter.h

@@ -311,7 +311,7 @@ class nsGlobalWindowOuter final : public mozilla::dom::EventTarget,
   // Outer windows only.
   bool WouldReuseInnerWindow(Document* aNewDocument);
 
-  void DetachFromDocShell();
+  void DetachFromDocShell(bool aIsBeingDiscarded);
 
   virtual nsresult SetNewDocument(
       Document* aDocument, nsISupports* aState, bool aForceReuseInnerWindow,

dom/base/nsPIDOMWindow.h

@@ -170,6 +170,10 @@ class nsPIDOMWindowInner : public mozIDOMWindow {
   // Returns true if this window is the same as mTopInnerWindow
   inline bool IsTopInnerWindow() const;
 
+  // Returns true if this was the current window for its BrowsingContext when it
+  // was discarded.
+  virtual bool WasCurrentInnerWindow() const = 0;
+
   // Check whether a document is currently loading (really checks if the
   // load event has completed).  May not be reset to false on errors.
   inline bool IsLoading() const;

dom/base/test/file_current_inner_window.html

@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+  <meta charset="UTF-8">
+  <title></title>
+  <script>
+    function isCurrentWinnerWindow() {
+      // If we are the current inner window for our BrowsingContext, bare word
+      // access to `name` will return "". If we are not, it will throw.
+      // Note that this is *not* the same as accessing `window.name`, which
+      // will go through our WindowProxy, which will always forward it to the
+      // currently-active inner window.
+      try {
+        void name;
+        return true;
+      } catch (e) {
+        return false;
+      }
+    }
+  </script>
+</head>
+<body>
+</body>
+</html>

dom/base/test/mochitest.ini

@@ -131,6 +131,7 @@ support-files =
   file_bug945152.jar
   file_bug1274806.html
   file_bug1453693.html
+  file_current_inner_window.html
   file_domwindowutils_animation.html
   file_general_document.html
   file_history_document_open.html
@@ -653,6 +654,7 @@ skip-if = verify
 [test_document_importNode_document.html]
 [test_custom_element.html]
 [test_custom_element_reflector.html]
+[test_current_inner_window.html]
 [test_domparser_null_char.html]
 [test_domparsing.html]
 [test_domrequest.html]

dom/base/test/test_current_inner_window.html

@@ -0,0 +1,61 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <meta charset="utf-8">
+  <title>Test that current inner window checks are correct after navigations/discards</title>
+  <script src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+
+<iframe id="frame"></iframe>
+
+<script type="application/javascript">
+"use strict";
+
+const TEST_FILE = "file_current_inner_window.html";
+const BASE_PATH = location.pathname.replace(/[^\/]+$/, "");
+
+let frame = document.getElementById("frame");
+
+function loadInFrame(url) {
+  return new Promise(resolve => {
+    frame.addEventListener("load", resolve, { once: true });
+    frame.contentWindow.location = url;
+  });
+}
+
+add_task(async function() {
+  await loadInFrame(TEST_FILE);
+
+  // Store the check function from the window before we navigate. After that,
+  // its bare word property accesses will continue referring to the same inner
+  // window no matter how many times the frame navigates.
+  let check1 = frame.contentWindow.isCurrentWinnerWindow;
+  ok(check1(),
+     "Initial inner window should be current before we navigate away");
+
+  await loadInFrame(`http://example.com/${BASE_PATH}/${TEST_FILE}`);
+  ok(!check1(),
+     "Initial inner window should no longer be current after we navigate away");
+  await SpecialPowers.spawn(frame, [], () => {
+    Assert.ok(this.content.wrappedJSObject.isCurrentWinnerWindow(),
+              "Remote inner window should be current after before we navigate away");
+  });
+
+  await loadInFrame(TEST_FILE);
+  ok(!check1(),
+     "Initial inner window should still not be current after we back to current process");
+  let check2 = frame.contentWindow.isCurrentWinnerWindow;
+  ok(check2(),
+     "Second in-process inner window should be current before we remove the frame");
+
+  frame.remove();
+  ok(!check1(),
+     "Initial inner window should still not be current after we remove the frame");
+  ok(check2(),
+     "Second in-process inner window should still be current after we remove the frame");
+});
+</script>
+</body>
+</html>