From 17d6e262f79b0d73f979320eeb565f17d5b8b66c Mon Sep 17 00:00:00 2001 From: Marian-Vasile Laza Date: Mon, 5 Jul 2021 17:12:13 +0300 Subject: [PATCH] Backed out changeset 55f827545de2 (bug 1701303) for causing bustages on ContentParent.cpp. CLOSED TREE --- .../privatebrowsing/test/browser/browser.ini | 1 - ...ng_last_private_browsing_context_exited.js | 66 --------------- docshell/base/BrowsingContext.cpp | 8 -- docshell/base/CanonicalBrowsingContext.cpp | 84 ------------------- docshell/base/CanonicalBrowsingContext.h | 7 -- docshell/base/nsDocShell.cpp | 74 ++++++++++++++++ docshell/base/nsDocShell.h | 1 + docshell/base/nsIDocShell.idl | 2 + dom/ipc/BrowserChild.cpp | 4 + dom/ipc/ContentParent.cpp | 32 +++++++ dom/ipc/ContentParent.h | 2 + dom/ipc/PContent.ipdl | 3 + dom/xul/ChromeObserver.cpp | 3 + dom/xul/nsXULElement.cpp | 23 +++++ dom/xul/nsXULElement.h | 2 + 15 files changed, 146 insertions(+), 166 deletions(-) delete mode 100644 browser/components/privatebrowsing/test/browser/browser_privatebrowsing_last_private_browsing_context_exited.js diff --git a/browser/components/privatebrowsing/test/browser/browser.ini b/browser/components/privatebrowsing/test/browser/browser.ini index 8d423be009db..37f8e8f9304c 100644 --- a/browser/components/privatebrowsing/test/browser/browser.ini +++ b/browser/components/privatebrowsing/test/browser/browser.ini @@ -42,7 +42,6 @@ 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] diff --git a/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_last_private_browsing_context_exited.js b/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_last_private_browsing_context_exited.js deleted file mode 100644 index 1fd28d4ca63a..000000000000 --- a/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_last_private_browsing_context_exited.js +++ /dev/null @@ -1,66 +0,0 @@ -/* 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"); -}); diff --git a/docshell/base/BrowsingContext.cpp b/docshell/base/BrowsingContext.cpp index 466c28d578ec..e0ec4ed9cc65 100644 --- a/docshell/base/BrowsingContext.cpp +++ b/docshell/base/BrowsingContext.cpp @@ -799,10 +799,6 @@ 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) { @@ -1592,10 +1588,6 @@ NS_IMETHODIMP BrowsingContext::SetPrivateBrowsing(bool aPrivateBrowsing) { if (IsContent()) { mOriginAttributes.SyncAttributesWithPrivateBrowsing(aPrivateBrowsing); } - - if (XRE_IsParentProcess()) { - Canonical()->AdjustPrivateBrowsingCount(aPrivateBrowsing); - } } AssertOriginAttributesMatchPrivateBrowsing(); diff --git a/docshell/base/CanonicalBrowsingContext.cpp b/docshell/base/CanonicalBrowsingContext.cpp index 808d1ef86e64..177d3135ba4c 100644 --- a/docshell/base/CanonicalBrowsingContext.cpp +++ b/docshell/base/CanonicalBrowsingContext.cpp @@ -27,7 +27,6 @@ #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" @@ -61,49 +60,6 @@ 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 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 { @@ -1133,30 +1089,6 @@ 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() { @@ -1440,22 +1372,6 @@ 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)) { diff --git a/docshell/base/CanonicalBrowsingContext.h b/docshell/base/CanonicalBrowsingContext.h index c0ddd5722405..a277c4ddedd6 100644 --- a/docshell/base/CanonicalBrowsingContext.h +++ b/docshell/base/CanonicalBrowsingContext.h @@ -335,13 +335,6 @@ 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, diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index ad6e575454c4..5f74b97bde24 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -273,6 +273,9 @@ 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) \ @@ -304,6 +307,33 @@ 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 obsvc = services::GetObserverService(); + if (obsvc) { + obsvc->NotifyObservers(nullptr, "last-pb-context-exited", nullptr); + } + } +} + static bool IsTopLevelDoc(BrowsingContext* aBrowsingContext, nsILoadInfo* aLoadInfo) { MOZ_ASSERT(aBrowsingContext); @@ -381,6 +411,7 @@ nsDocShell::nsDocShell(BrowsingContext* aBrowsingContext, mIsBeingDestroyed(false), mIsExecutingOnLoadHandler(false), mSavingOldViewer(false), + mAffectPrivateSessionLifetime(true), mInvisible(false), mHasLoadedNonBlankURI(false), mBlankTiming(false), @@ -1686,6 +1717,14 @@ nsDocShell::GetUsePrivateBrowsing(bool* aUsePrivateBrowsing) { void nsDocShell::NotifyPrivateBrowsingChanged() { MOZ_ASSERT(!mIsBeingDestroyed); + if (mAffectPrivateSessionLifetime) { + if (UsePrivateBrowsing()) { + IncreasePrivateDocShellCount(); + } else { + DecreasePrivateDocShellCount(); + } + } + nsTObserverArray::ForwardIterator iter(mPrivacyObservers); while (iter.HasMore()) { nsWeakPtr ref = iter.GetNext(); @@ -1738,6 +1777,35 @@ 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 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) { @@ -2533,6 +2601,8 @@ 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 @@ -4457,6 +4527,10 @@ nsDocShell::Destroy() { // to break the cycle between us and the timers. CancelRefreshURITimers(); + if (UsePrivateBrowsing() && mAffectPrivateSessionLifetime) { + DecreasePrivateDocShellCount(); + } + return NS_OK; } diff --git a/docshell/base/nsDocShell.h b/docshell/base/nsDocShell.h index 0be1c873204e..ee3d3b69cf8d 100644 --- a/docshell/base/nsDocShell.h +++ b/docshell/base/nsDocShell.h @@ -1282,6 +1282,7 @@ 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; diff --git a/docshell/base/nsIDocShell.idl b/docshell/base/nsIDocShell.idl index 352b70d12030..6ae176e22a80 100644 --- a/docshell/base/nsIDocShell.idl +++ b/docshell/base/nsIDocShell.idl @@ -605,6 +605,8 @@ 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. diff --git a/dom/ipc/BrowserChild.cpp b/dom/ipc/BrowserChild.cpp index 4441cde15347..00ba87d396a7 100644 --- a/dom/ipc/BrowserChild.cpp +++ b/dom/ipc/BrowserChild.cpp @@ -495,6 +495,10 @@ nsresult BrowserChild::Init(mozIDOMWindowProxy* aParent, NS_ENSURE_SUCCESS(rv, rv); } + docShell->SetAffectPrivateSessionLifetime( + mBrowsingContext->UsePrivateBrowsing() || + mChromeFlags & nsIWebBrowserChrome::CHROME_PRIVATE_LIFETIME); + #ifdef DEBUG nsCOMPtr loadContext = do_GetInterface(WebNavigation()); MOZ_ASSERT(loadContext); diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index d9cd6248866f..2290824be23c 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -1539,6 +1539,9 @@ already_AddRefed ContentParent::CreateBrowser( if (loadContext && loadContext->UseRemoteSubframes()) { chromeFlags |= nsIWebBrowserChrome::CHROME_FISSION_WINDOW; } + if (docShell->GetAffectPrivateSessionLifetime()) { + chromeFlags |= nsIWebBrowserChrome::CHROME_PRIVATE_LIFETIME; + } if (tabId == 0) { return nullptr; @@ -4763,6 +4766,35 @@ mozilla::ipc::IPCResult ContentParent::RecvScriptErrorInternal( return IPC_OK(); } +mozilla::ipc::IPCResult ContentParent::RecvPrivateDocShellsExist( + const bool& aExist) { + if (!sPrivateContent) { + sPrivateContent = MakeUnique>(); + 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 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); diff --git a/dom/ipc/ContentParent.h b/dom/ipc/ContentParent.h index c59adbb02801..6d62062decd0 100644 --- a/dom/ipc/ContentParent.h +++ b/dom/ipc/ContentParent.h @@ -1118,6 +1118,8 @@ class ContentParent final const ClonedMessageData* aStack = nullptr); public: + mozilla::ipc::IPCResult RecvPrivateDocShellsExist(const bool& aExist); + mozilla::ipc::IPCResult RecvCommitBrowsingContextTransaction( const MaybeDiscarded& aContext, BrowsingContext::BaseTransaction&& aTransaction, uint64_t aEpoch); diff --git a/dom/ipc/PContent.ipdl b/dom/ipc/PContent.ipdl index 4fc8c003e60c..763b37bd1934 100644 --- a/dom/ipc/PContent.ipdl +++ b/dom/ipc/PContent.ipdl @@ -1154,6 +1154,9 @@ 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(); diff --git a/dom/xul/ChromeObserver.cpp b/dom/xul/ChromeObserver.cpp index b0ded0486feb..afeb553df540 100644 --- a/dom/xul/ChromeObserver.cpp +++ b/dom/xul/ChromeObserver.cpp @@ -162,6 +162,9 @@ 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 xulElement = nsXULElement::FromNodeOrNull(aElement); + xulElement->MaybeUpdatePrivateLifetime(); } // title and drawintitlebar are settable on // any root node (windows, dialogs, etc) diff --git a/dom/xul/nsXULElement.cpp b/dom/xul/nsXULElement.cpp index db6707beca45..2bd8b738866d 100644 --- a/dom/xul/nsXULElement.cpp +++ b/dom/xul/nsXULElement.cpp @@ -134,6 +134,21 @@ nsXULElement::nsXULElement(already_AddRefed&& 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 docShell = win ? win->GetDocShell() : nullptr; + if (docShell) { + docShell->SetAffectPrivateSessionLifetime(false); + } +} + /* static */ nsXULElement* NS_NewBasicXULElement( already_AddRefed&& aNodeInfo) { @@ -219,6 +234,14 @@ already_AddRefed 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(); } diff --git a/dom/xul/nsXULElement.h b/dom/xul/nsXULElement.h index affe1d7f7906..40985432e05e 100644 --- a/dom/xul/nsXULElement.h +++ b/dom/xul/nsXULElement.h @@ -504,6 +504,8 @@ class nsXULElement : public nsStyledElement { bool IsInteractiveHTMLContent() const override; + void MaybeUpdatePrivateLifetime(); + protected: ~nsXULElement();