Bug 1701303 - Move counting of private browsing contexts to parent process. r=smaug,johannh

Move the counting of private browsing contexts to the parent
process. Also change to only consider non-chrome browsing contexts
when counting private contexts. The latter is possible due to bug
1528115, because we no longer need to support hidden private windows.

With counting in the parent process we can make sure that when we're
changing remoteness on a private browsing context the private browsing
context count never drops to zero. This fixes an issue with Fission,
where we remoteness changes could transiently have a zero private
browsing context count, that would be mistaken for the last private
browsing context going away.

Changing to only count non-chrome browsing contexts makes us only fire
'last-pb-context-exited' once, and since we count them in the parent
there is no missing information about contexts that makes us wait for
a content process about telling us about insertion or removal of
browsing contexts.

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

browser/components/privatebrowsing/test/browser/browser.ini

@@ -42,6 +42,7 @@ skip-if = verify
 [browser_privatebrowsing_downloadLastDir_toggle.js]
 [browser_privatebrowsing_favicon.js]
 [browser_privatebrowsing_history_shift_click.js]
+[browser_privatebrowsing_last_private_browsing_context_exited.js]
 [browser_privatebrowsing_lastpbcontextexited.js]
 [browser_privatebrowsing_localStorage.js]
 [browser_privatebrowsing_localStorage_before_after.js]

browser/components/privatebrowsing/test/browser/browser_privatebrowsing_last_private_browsing_context_exited.js

@@ -0,0 +1,66 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+add_task(async function test_no_notification_when_pb_autostart() {
+  let observedLastPBContext = false;
+  let observerExited = {
+    observe(aSubject, aTopic, aData) {
+      observedLastPBContext = true;
+    },
+  };
+  Services.obs.addObserver(observerExited, "last-pb-context-exited");
+
+  await SpecialPowers.pushPrefEnv({
+    set: [["browser.privatebrowsing.autostart", true]],
+  });
+
+  let win = await BrowserTestUtils.openNewBrowserWindow();
+
+  let browser = win.gBrowser.selectedTab.linkedBrowser;
+  ok(browser.browsingContext.usePrivateBrowsing, "should use private browsing");
+
+  await BrowserTestUtils.closeWindow(win);
+
+  await SpecialPowers.popPrefEnv();
+  Services.obs.removeObserver(observerExited, "last-pb-context-exited");
+  ok(!observedLastPBContext, "No last-pb-context-exited notification seen");
+});
+
+add_task(async function test_notification_when_about_preferences() {
+  let observedLastPBContext = false;
+  let observerExited = {
+    observe(aSubject, aTopic, aData) {
+      observedLastPBContext = true;
+    },
+  };
+  Services.obs.addObserver(observerExited, "last-pb-context-exited");
+
+  let win = await BrowserTestUtils.openNewBrowserWindow({ private: true });
+
+  let browser = win.gBrowser.selectedTab.linkedBrowser;
+  ok(browser.browsingContext.usePrivateBrowsing, "should use private browsing");
+  ok(browser.browsingContext.isContent, "should be content browsing context");
+
+  let tab = await BrowserTestUtils.addTab(win.gBrowser, "about:preferences");
+  ok(
+    tab.linkedBrowser.browsingContext.usePrivateBrowsing,
+    "should use private browsing"
+  );
+  ok(
+    tab.linkedBrowser.browsingContext.isContent,
+    "should be content browsing context"
+  );
+
+  let tabClose = BrowserTestUtils.waitForTabClosing(win.gBrowser.selectedTab);
+  BrowserTestUtils.removeTab(win.gBrowser.selectedTab);
+  await tabClose;
+
+  ok(!observedLastPBContext, "No last-pb-context-exited notification seen");
+
+  await BrowserTestUtils.closeWindow(win);
+
+  Services.obs.removeObserver(observerExited, "last-pb-context-exited");
+  ok(observedLastPBContext, "No last-pb-context-exited notification seen");
+});

docshell/base/BrowsingContext.cpp

@@ -799,6 +799,10 @@ void BrowsingContext::Attach(bool aFromIPC, ContentParent* aOriginProcess) {
     obs->NotifyWhenScriptSafe(ToSupports(this), "browsing-context-attached",
                               nullptr);
   }
+
+  if (XRE_IsParentProcess()) {
+    Canonical()->CanonicalAttach();
+  }
 }
 
 void BrowsingContext::Detach(bool aFromIPC) {
@@ -1588,6 +1592,10 @@ NS_IMETHODIMP BrowsingContext::SetPrivateBrowsing(bool aPrivateBrowsing) {
     if (IsContent()) {
       mOriginAttributes.SyncAttributesWithPrivateBrowsing(aPrivateBrowsing);
     }
+
+    if (XRE_IsParentProcess()) {
+      Canonical()->AdjustPrivateBrowsingCount(aPrivateBrowsing);
+    }
   }
   AssertOriginAttributesMatchPrivateBrowsing();
 

docshell/base/CanonicalBrowsingContext.cpp

@@ -27,6 +27,7 @@
 #include "mozilla/NullPrincipal.h"
 #include "mozilla/StaticPrefs_docshell.h"
 #include "mozilla/StaticPrefs_fission.h"
+#include "mozilla/Telemetry.h"
 #include "nsIWebNavigation.h"
 #include "mozilla/MozPromiseInlines.h"
 #include "nsDocShell.h"
@@ -60,6 +61,49 @@ extern mozilla::LazyLogModule gSHIPBFCacheLog;
 #define AUTOPLAY_LOG(msg, ...) \
   MOZ_LOG(gAutoplayPermissionLog, LogLevel::Debug, (msg, ##__VA_ARGS__))
 
+static mozilla::LazyLogModule sPBContext("PBContext");
+
+// Global count of canonical browsing contexts with the private attribute set
+static uint32_t gNumberOfPrivateContexts = 0;
+
+static void IncreasePrivateCount() {
+  gNumberOfPrivateContexts++;
+  MOZ_LOG(sPBContext, mozilla::LogLevel::Debug,
+          ("%s: Private browsing context count %d -> %d", __func__,
+           gNumberOfPrivateContexts - 1, gNumberOfPrivateContexts));
+  if (gNumberOfPrivateContexts > 1) {
+    return;
+  }
+
+  static bool sHasSeenPrivateContext = false;
+  if (!sHasSeenPrivateContext) {
+    sHasSeenPrivateContext = true;
+    mozilla::Telemetry::ScalarSet(
+        mozilla::Telemetry::ScalarID::DOM_PARENTPROCESS_PRIVATE_WINDOW_USED,
+        true);
+  }
+}
+
+static void DecreasePrivateCount() {
+  MOZ_ASSERT(gNumberOfPrivateContexts > 0);
+  gNumberOfPrivateContexts--;
+
+  MOZ_LOG(sPBContext, mozilla::LogLevel::Debug,
+          ("%s: Private browsing context count %d -> %d", __func__,
+           gNumberOfPrivateContexts + 1, gNumberOfPrivateContexts));
+  if (!gNumberOfPrivateContexts &&
+      !mozilla::Preferences::GetBool("browser.privatebrowsing.autostart")) {
+    nsCOMPtr<nsIObserverService> observerService =
+        mozilla::services::GetObserverService();
+    if (observerService) {
+      MOZ_LOG(sPBContext, mozilla::LogLevel::Debug,
+              ("%s: last-pb-context-exited fired", __func__));
+      observerService->NotifyObservers(nullptr, "last-pb-context-exited",
+                                       nullptr);
+    }
+  }
+}
+
 namespace mozilla {
 namespace dom {
 
@@ -1089,6 +1133,30 @@ void CanonicalBrowsingContext::CanonicalDiscard() {
   }
 
   CancelSessionStoreUpdate();
+
+  if (UsePrivateBrowsing() && EverAttached() && IsContent()) {
+    DecreasePrivateCount();
+  }
+}
+
+void CanonicalBrowsingContext::CanonicalAttach() {
+  if (UsePrivateBrowsing() && IsContent()) {
+    IncreasePrivateCount();
+  }
+}
+
+void CanonicalBrowsingContext::AdjustPrivateBrowsingCount(
+    bool aPrivateBrowsing) {
+  if (IsDiscarded() || !EverAttached() || IsChrome()) {
+    return;
+  }
+
+  MOZ_DIAGNOSTIC_ASSERT(aPrivateBrowsing == UsePrivateBrowsing());
+  if (aPrivateBrowsing) {
+    IncreasePrivateCount();
+  } else {
+    DecreasePrivateCount();
+  }
 }
 
 void CanonicalBrowsingContext::NotifyStartDelayedAutoplayMedia() {
@@ -1372,6 +1440,22 @@ nsresult CanonicalBrowsingContext::PendingRemotenessChange::FinishTopContent() {
   MOZ_RELEASE_ASSERT(frameLoaderOwner,
                      "embedder browser must be nsFrameLoaderOwner");
 
+  // If we're process switching a browsing context in private browsing
+  // mode we might decrease the private browsing count to '0', which
+  // would make us fire "last-pb-context-exited" and drop the private
+  // session. To prevent that we artificially increment the number of
+  // private browsing contexts with '1' until the process switch is done.
+  bool usePrivateBrowsing = mTarget->UsePrivateBrowsing();
+  if (usePrivateBrowsing) {
+    IncreasePrivateCount();
+  }
+
+  auto restorePrivateCount = MakeScopeExit([usePrivateBrowsing]() {
+    if (usePrivateBrowsing) {
+      DecreasePrivateCount();
+    }
+  });
+
   // Tell frontend code that this browser element is about to change process.
   nsresult rv = browser->BeforeChangeRemoteness();
   if (NS_FAILED(rv)) {

docshell/base/CanonicalBrowsingContext.h

@@ -335,6 +335,13 @@ class CanonicalBrowsingContext final : public BrowsingContext {
   // Called when the browsing context is being discarded.
   void CanonicalDiscard();
 
+  // Called when the browsing context is being attached.
+  void CanonicalAttach();
+
+  // Called when the browsing context private mode is changed after
+  // being attached, but before being discarded.
+  void AdjustPrivateBrowsingCount(bool aPrivateBrowsing);
+
   using Type = BrowsingContext::Type;
   CanonicalBrowsingContext(WindowContext* aParentWindow,
                            BrowsingContextGroup* aGroup,

docshell/base/nsDocShell.cpp

@@ -273,9 +273,6 @@ static NS_DEFINE_CID(kAppShellCID, NS_APPSHELL_CID);
 // Number of documents currently loading
 static int32_t gNumberOfDocumentsLoading = 0;
 
-// Global count of docshells with the private attribute set
-static uint32_t gNumberOfPrivateDocShells = 0;
-
 static mozilla::LazyLogModule gCharsetMenuLog("CharsetMenu");
 
 #define LOGCHARSETMENU(args) \
@@ -307,33 +304,6 @@ static void FavorPerformanceHint(bool aPerfOverStarvation) {
   }
 }
 
-static void IncreasePrivateDocShellCount() {
-  gNumberOfPrivateDocShells++;
-  if (gNumberOfPrivateDocShells > 1 || !XRE_IsContentProcess()) {
-    return;
-  }
-
-  mozilla::dom::ContentChild* cc = mozilla::dom::ContentChild::GetSingleton();
-  cc->SendPrivateDocShellsExist(true);
-}
-
-static void DecreasePrivateDocShellCount() {
-  MOZ_ASSERT(gNumberOfPrivateDocShells > 0);
-  gNumberOfPrivateDocShells--;
-  if (!gNumberOfPrivateDocShells) {
-    if (XRE_IsContentProcess()) {
-      dom::ContentChild* cc = dom::ContentChild::GetSingleton();
-      cc->SendPrivateDocShellsExist(false);
-      return;
-    }
-
-    nsCOMPtr<nsIObserverService> obsvc = services::GetObserverService();
-    if (obsvc) {
-      obsvc->NotifyObservers(nullptr, "last-pb-context-exited", nullptr);
-    }
-  }
-}
-
 static bool IsTopLevelDoc(BrowsingContext* aBrowsingContext,
                           nsILoadInfo* aLoadInfo) {
   MOZ_ASSERT(aBrowsingContext);
@@ -411,7 +381,6 @@ nsDocShell::nsDocShell(BrowsingContext* aBrowsingContext,
       mIsBeingDestroyed(false),
       mIsExecutingOnLoadHandler(false),
       mSavingOldViewer(false),
-      mAffectPrivateSessionLifetime(true),
       mInvisible(false),
       mHasLoadedNonBlankURI(false),
       mBlankTiming(false),
@@ -1717,14 +1686,6 @@ nsDocShell::GetUsePrivateBrowsing(bool* aUsePrivateBrowsing) {
 void nsDocShell::NotifyPrivateBrowsingChanged() {
   MOZ_ASSERT(!mIsBeingDestroyed);
 
-  if (mAffectPrivateSessionLifetime) {
-    if (UsePrivateBrowsing()) {
-      IncreasePrivateDocShellCount();
-    } else {
-      DecreasePrivateDocShellCount();
-    }
-  }
-
   nsTObserverArray<nsWeakPtr>::ForwardIterator iter(mPrivacyObservers);
   while (iter.HasMore()) {
     nsWeakPtr ref = iter.GetNext();
@@ -1777,35 +1738,6 @@ nsDocShell::SetRemoteSubframes(bool aUseRemoteSubframes) {
   return mBrowsingContext->SetRemoteSubframes(aUseRemoteSubframes);
 }
 
-NS_IMETHODIMP
-nsDocShell::SetAffectPrivateSessionLifetime(bool aAffectLifetime) {
-  MOZ_ASSERT(!mIsBeingDestroyed);
-
-  bool change = aAffectLifetime != mAffectPrivateSessionLifetime;
-  if (change && UsePrivateBrowsing()) {
-    if (aAffectLifetime) {
-      IncreasePrivateDocShellCount();
-    } else {
-      DecreasePrivateDocShellCount();
-    }
-  }
-  mAffectPrivateSessionLifetime = aAffectLifetime;
-
-  for (auto* child : mChildList.ForwardRange()) {
-    nsCOMPtr<nsIDocShell> shell = do_QueryObject(child);
-    if (shell) {
-      shell->SetAffectPrivateSessionLifetime(aAffectLifetime);
-    }
-  }
-  return NS_OK;
-}
-
-NS_IMETHODIMP
-nsDocShell::GetAffectPrivateSessionLifetime(bool* aAffectLifetime) {
-  *aAffectLifetime = mAffectPrivateSessionLifetime;
-  return NS_OK;
-}
-
 NS_IMETHODIMP
 nsDocShell::AddWeakPrivacyTransitionObserver(
     nsIPrivacyTransitionObserver* aObserver) {
@@ -2601,8 +2533,6 @@ nsresult nsDocShell::SetDocLoaderParent(nsDocLoader* aParent) {
       value = false;
     }
     SetAllowDNSPrefetch(mAllowDNSPrefetch && value);
-    SetAffectPrivateSessionLifetime(
-        parentAsDocShell->GetAffectPrivateSessionLifetime());
 
     // We don't need to inherit metaViewportOverride, because the viewport
     // is only relevant for the outermost nsDocShell, not for any iframes
@@ -4527,10 +4457,6 @@ nsDocShell::Destroy() {
   // to break the cycle between us and the timers.
   CancelRefreshURITimers();
 
-  if (UsePrivateBrowsing() && mAffectPrivateSessionLifetime) {
-    DecreasePrivateDocShellCount();
-  }
-
   return NS_OK;
 }
 

docshell/base/nsDocShell.h

@@ -1282,7 +1282,6 @@ class nsDocShell final : public nsDocLoader,
   // should be passed a SHEntry to save itself into.
   bool mSavingOldViewer : 1;
 
-  bool mAffectPrivateSessionLifetime : 1;
   bool mInvisible : 1;
   bool mHasLoadedNonBlankURI : 1;
 

docshell/base/nsIDocShell.idl

@@ -605,8 +605,6 @@ interface nsIDocShell : nsIDocShellTreeItem
    */
   [noscript, notxpcom] bool pluginsAllowedInCurrentDoc();
 
-  [noscript, infallible] attribute boolean affectPrivateSessionLifetime;
-
   /**
    * Indicates whether the UI may enable the character encoding menu. The UI
    * must disable the menu when this property is false.

dom/ipc/BrowserChild.cpp

@@ -495,10 +495,6 @@ nsresult BrowserChild::Init(mozIDOMWindowProxy* aParent,
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
-  docShell->SetAffectPrivateSessionLifetime(
-      mBrowsingContext->UsePrivateBrowsing() ||
-      mChromeFlags & nsIWebBrowserChrome::CHROME_PRIVATE_LIFETIME);
-
 #ifdef DEBUG
   nsCOMPtr<nsILoadContext> loadContext = do_GetInterface(WebNavigation());
   MOZ_ASSERT(loadContext);

dom/ipc/ContentParent.cpp

@@ -1539,9 +1539,6 @@ already_AddRefed<RemoteBrowser> ContentParent::CreateBrowser(
   if (loadContext && loadContext->UseRemoteSubframes()) {
     chromeFlags |= nsIWebBrowserChrome::CHROME_FISSION_WINDOW;
   }
-  if (docShell->GetAffectPrivateSessionLifetime()) {
-    chromeFlags |= nsIWebBrowserChrome::CHROME_PRIVATE_LIFETIME;
-  }
 
   if (tabId == 0) {
     return nullptr;
@@ -4766,35 +4763,6 @@ mozilla::ipc::IPCResult ContentParent::RecvScriptErrorInternal(
   return IPC_OK();
 }
 
-mozilla::ipc::IPCResult ContentParent::RecvPrivateDocShellsExist(
-    const bool& aExist) {
-  if (!sPrivateContent) {
-    sPrivateContent = MakeUnique<nsTArray<ContentParent*>>();
-    if (!sHasSeenPrivateDocShell) {
-      sHasSeenPrivateDocShell = true;
-      Telemetry::ScalarSet(
-          Telemetry::ScalarID::DOM_PARENTPROCESS_PRIVATE_WINDOW_USED, true);
-    }
-  }
-  if (aExist) {
-    sPrivateContent->AppendElement(this);
-  } else {
-    sPrivateContent->RemoveElement(this);
-
-    // Only fire the notification if we have private and non-private
-    // windows: if privatebrowsing.autostart is true, all windows are
-    // private.
-    if (!sPrivateContent->Length() &&
-        !Preferences::GetBool("browser.privatebrowsing.autostart")) {
-      nsCOMPtr<nsIObserverService> obs =
-          mozilla::services::GetObserverService();
-      obs->NotifyObservers(nullptr, "last-pb-context-exited", nullptr);
-      sPrivateContent = nullptr;
-    }
-  }
-  return IPC_OK();
-}
-
 bool ContentParent::DoLoadMessageManagerScript(const nsAString& aURL,
                                                bool aRunInGlobalScope) {
   MOZ_ASSERT(!aRunInGlobalScope);

dom/ipc/ContentParent.h

@@ -1118,8 +1118,6 @@ class ContentParent final
       const ClonedMessageData* aStack = nullptr);
 
  public:
-  mozilla::ipc::IPCResult RecvPrivateDocShellsExist(const bool& aExist);
-
   mozilla::ipc::IPCResult RecvCommitBrowsingContextTransaction(
       const MaybeDiscarded<BrowsingContext>& aContext,
       BrowsingContext::BaseTransaction&& aTransaction, uint64_t aEpoch);

dom/ipc/PContent.ipdl

@@ -1154,9 +1154,6 @@ parent:
     sync GetIconForExtension(nsCString aFileExt, uint32_t aIconSize)
         returns (uint8_t[] bits);
 
-    // Notify the parent of the presence or absence of private docshells
-    async PrivateDocShellsExist(bool aExist);
-
     // Tell the parent that the child has gone idle for the first time.
     async FirstIdle();
 

dom/xul/ChromeObserver.cpp

@@ -162,9 +162,6 @@ void ChromeObserver::AttributeChanged(dom::Element* aElement,
       HideWindowChrome(value->Equals(u"true"_ns, eCaseMatters));
     } else if (aName == nsGkAtoms::chromemargin) {
       SetChromeMargins(value);
-    } else if (aName == nsGkAtoms::windowtype && aElement->IsXULElement()) {
-      RefPtr<nsXULElement> xulElement = nsXULElement::FromNodeOrNull(aElement);
-      xulElement->MaybeUpdatePrivateLifetime();
     }
     // title and drawintitlebar are settable on
     // any root node (windows, dialogs, etc)

dom/xul/nsXULElement.cpp

@@ -134,21 +134,6 @@ nsXULElement::nsXULElement(already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo)
 
 nsXULElement::~nsXULElement() = default;
 
-void nsXULElement::MaybeUpdatePrivateLifetime() {
-  if (AttrValueIs(kNameSpaceID_None, nsGkAtoms::windowtype,
-                  u"navigator:browser"_ns, eCaseMatters) ||
-      AttrValueIs(kNameSpaceID_None, nsGkAtoms::windowtype,
-                  u"navigator:geckoview"_ns, eCaseMatters)) {
-    return;
-  }
-
-  nsPIDOMWindowOuter* win = OwnerDoc()->GetWindow();
-  nsCOMPtr<nsIDocShell> docShell = win ? win->GetDocShell() : nullptr;
-  if (docShell) {
-    docShell->SetAffectPrivateSessionLifetime(false);
-  }
-}
-
 /* static */
 nsXULElement* NS_NewBasicXULElement(
     already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo) {
@@ -234,14 +219,6 @@ already_AddRefed<nsXULElement> nsXULElement::CreateFromPrototype(
       }
     }
 
-    if (aIsRoot && aPrototype->mNodeInfo->Equals(nsGkAtoms::window)) {
-      for (size_t i = 0; i < aPrototype->mAttributes.Length(); ++i) {
-        if (aPrototype->mAttributes[i].mName.Equals(nsGkAtoms::windowtype)) {
-          element->MaybeUpdatePrivateLifetime();
-        }
-      }
-    }
-
     return baseElement.forget().downcast<nsXULElement>();
   }
 

dom/xul/nsXULElement.h

@@ -504,8 +504,6 @@ class nsXULElement : public nsStyledElement {
 
   bool IsInteractiveHTMLContent() const override;
 
-  void MaybeUpdatePrivateLifetime();
-
  protected:
   ~nsXULElement();