From 0c7b4c828a6c103e90d326e99e8c834c85a292fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 5 Jul 2021 17:31:49 +0000 Subject: [PATCH] Bug 1717983 - Don't consider a browser active if the tab is inactive but we're preserving layers. r=nika GeckoView always calls preserveLayers(true) on all elements, which causes the puppet widget to always be considered visible. Given how the code worked before, aBrowsingContext.isActive = false after that call would deactivate the pres shell, but after my patch it stops doing so. We don't really want to un-throttle the refresh driver etc just because we're preserving layers, so propagate the state to the child process and account for that in the logic to determine PresShell activeness. Depends on D118703 Differential Revision: https://phabricator.services.mozilla.com/D118884 --- dom/ipc/BrowserChild.cpp | 54 ++++++++++++++++++++++----------------- dom/ipc/BrowserChild.h | 7 +++++ dom/ipc/BrowserParent.cpp | 12 ++++++--- dom/ipc/BrowserParent.h | 2 +- dom/ipc/PBrowser.ipdl | 6 +++++ layout/base/PresShell.cpp | 11 +++++++- 6 files changed, 63 insertions(+), 29 deletions(-) diff --git a/dom/ipc/BrowserChild.cpp b/dom/ipc/BrowserChild.cpp index 1a8835254935..86ecea17bdaf 100644 --- a/dom/ipc/BrowserChild.cpp +++ b/dom/ipc/BrowserChild.cpp @@ -341,6 +341,7 @@ BrowserChild::BrowserChild(ContentChild* aManager, const TabId& aTabId, #endif mShouldSendWebProgressEventsToParent(false), mRenderLayers(true), + mIsPreservingLayers(false), mPendingDocShellIsActive(false), mPendingDocShellReceivedMessage(false), mPendingRenderLayers(false), @@ -2960,21 +2961,7 @@ void BrowserChild::MakeVisible() { mPuppetWidget->Show(true); } - if (nsCOMPtr docShell = do_GetInterface(WebNavigation())) { - // The browser / tab-switcher is responsible of fixing the browsingContext - // state up explicitly via SetDocShellIsActive, which propagates to children - // automatically. - // - // We need it not to be observable, as this used via RecvRenderLayers and co., - // for stuff like async tab warming. - // - // We don't want to go through the docshell because we don't want to change - // the visibility state of the document, which has side effects like firing - // events to content, unblocking media playback, unthrottling timeouts... - if (RefPtr presShell = docShell->GetPresShell()) { - presShell->ActivenessMaybeChanged(); - } - } + PresShellActivenessMaybeChanged(); } void BrowserChild::MakeHidden() { @@ -2994,15 +2981,36 @@ void BrowserChild::MakeHidden() { mPuppetWidget->Show(false); } - if (nsCOMPtr docShell = do_GetInterface(WebNavigation())) { - // We don't use BrowserChildBase::GetPresShell() here because that would - // create a content viewer if one doesn't exist yet. Creating a content - // viewer can cause JS to run, which we want to avoid. - // nsIDocShell::GetPresShell returns null if no content viewer exists yet. - if (RefPtr presShell = docShell->GetPresShell()) { - presShell->ActivenessMaybeChanged(); - } + PresShellActivenessMaybeChanged(); +} + +IPCResult BrowserChild::RecvPreserveLayers(bool aPreserve) { + mIsPreservingLayers = true; + + PresShellActivenessMaybeChanged(); + + return IPC_OK(); +} + +void BrowserChild::PresShellActivenessMaybeChanged() { + // We don't use BrowserChildBase::GetPresShell() here because that would + // create a content viewer if one doesn't exist yet. Creating a content + // viewer can cause JS to run, which we want to avoid. + // nsIDocShell::GetPresShell returns null if no content viewer exists yet. + // + // When this method is called we don't want to go through the browsing context + // because we don't want to change the visibility state of the document, which + // has side effects like firing events to content, unblocking media playback, + // unthrottling timeouts... PresShell activeness has a lot less side effects. + nsCOMPtr docShell = do_GetInterface(WebNavigation()); + if (!docShell) { + return; } + RefPtr presShell = docShell->GetPresShell(); + if (!presShell) { + return; + } + presShell->ActivenessMaybeChanged(); } NS_IMETHODIMP diff --git a/dom/ipc/BrowserChild.h b/dom/ipc/BrowserChild.h index 51f725a47d10..23c6feff7451 100644 --- a/dom/ipc/BrowserChild.h +++ b/dom/ipc/BrowserChild.h @@ -486,6 +486,7 @@ class BrowserChild final : public nsMessageManagerScriptExecutor, nsTArray& aCommands); bool IsVisible(); + bool IsPreservingLayers() const { return mIsPreservingLayers; } /** * Signal to this BrowserChild that it should be made visible: @@ -495,6 +496,7 @@ class BrowserChild final : public nsMessageManagerScriptExecutor, MOZ_CAN_RUN_SCRIPT void UpdateVisibility(); MOZ_CAN_RUN_SCRIPT void MakeVisible(); void MakeHidden(); + void PresShellActivenessMaybeChanged(); ContentChild* Manager() const { return mManager; } @@ -713,6 +715,8 @@ class BrowserChild final : public nsMessageManagerScriptExecutor, mozilla::ipc::IPCResult RecvRenderLayers( const bool& aEnabled, const layers::LayersObserverEpoch& aEpoch); + mozilla::ipc::IPCResult RecvPreserveLayers(bool); + mozilla::ipc::IPCResult RecvNavigateByKey(const bool& aForward, const bool& aForDocumentNavigation); @@ -889,6 +893,9 @@ class BrowserChild final : public nsMessageManagerScriptExecutor, // Whether we are rendering to the compositor or not. bool mRenderLayers; + // Whether we're artificially preserving layers. + bool mIsPreservingLayers; + // In some circumstances, a DocShell might be in a state where it is // "blocked", and we should not attempt to change its active state or // the underlying PresShell state until the DocShell becomes unblocked. diff --git a/dom/ipc/BrowserParent.cpp b/dom/ipc/BrowserParent.cpp index fd8ffc248ea5..05d77937f986 100644 --- a/dom/ipc/BrowserParent.cpp +++ b/dom/ipc/BrowserParent.cpp @@ -222,7 +222,7 @@ BrowserParent::BrowserParent(ContentParent* aManager, const TabId& aTabId, mMarkedDestroying(false), mIsDestroyed(false), mRemoteTargetSetsCursor(false), - mPreserveLayers(false), + mIsPreservingLayers(false), mRenderLayers(true), mHasLayers(false), mHasPresented(false), @@ -3377,7 +3377,7 @@ bool BrowserParent::GetRenderLayers() { return mRenderLayers; } void BrowserParent::SetRenderLayers(bool aEnabled) { if (aEnabled == mRenderLayers) { - if (aEnabled && mHasLayers && mPreserveLayers) { + if (aEnabled && mHasLayers && mIsPreservingLayers) { // RenderLayers might be called when we've been preserving layers, // and already had layers uploaded. In that case, the MozLayerTreeReady // event will not naturally arrive, which can confuse the front-end @@ -3396,7 +3396,7 @@ void BrowserParent::SetRenderLayers(bool aEnabled) { // Preserve layers means that attempts to stop rendering layers // will be ignored. - if (!aEnabled && mPreserveLayers) { + if (!aEnabled && mIsPreservingLayers) { return; } @@ -3420,7 +3420,11 @@ void BrowserParent::SetRenderLayersInternal(bool aEnabled) { } void BrowserParent::PreserveLayers(bool aPreserveLayers) { - mPreserveLayers = aPreserveLayers; + if (mIsPreservingLayers == aPreserveLayers) { + return; + } + mIsPreservingLayers = aPreserveLayers; + Unused << SendPreserveLayers(aPreserveLayers); } void BrowserParent::NotifyResolutionChanged() { diff --git a/dom/ipc/BrowserParent.h b/dom/ipc/BrowserParent.h index b34b1cf99d45..80e4d055e26c 100644 --- a/dom/ipc/BrowserParent.h +++ b/dom/ipc/BrowserParent.h @@ -950,7 +950,7 @@ class BrowserParent final : public PBrowserParent, // If this flag is set, then the tab's layers will be preserved even when // the tab's docshell is inactive. - bool mPreserveLayers : 1; + bool mIsPreservingLayers : 1; // Holds the most recent value passed to the RenderLayers function. This // does not necessarily mean that the layers have finished rendering diff --git a/dom/ipc/PBrowser.ipdl b/dom/ipc/PBrowser.ipdl index bf1c3752320d..9750219fa46a 100644 --- a/dom/ipc/PBrowser.ipdl +++ b/dom/ipc/PBrowser.ipdl @@ -889,6 +889,12 @@ child: * PaintWhileInterruptingJS). */ async RenderLayers(bool aEnabled, LayersObserverEpoch aEpoch); + + /** + * Communicates the child that we want layers to be preserved even when the + * browser is inactive. + */ + async PreserveLayers(bool aPreserve); child: /** * Notify the child that it shouldn't paint the offscreen displayport. diff --git a/layout/base/PresShell.cpp b/layout/base/PresShell.cpp index 8628b3103b69..d166d7ed18de 100644 --- a/layout/base/PresShell.cpp +++ b/layout/base/PresShell.cpp @@ -10797,7 +10797,16 @@ bool PresShell::ShouldBeActive() const { // throttling code (in-process throttling for non-visible iframes lives // right now in Document::ShouldThrottleFrameRequests(), but that only // throttles rAF). - return browserChild->IsVisible(); + if (!browserChild->IsVisible()) { + return false; + } + + // If the browser is visible but just due to be preserving layers + // artificially, we do want to fall back to the browsing context activeness + // instead. Otherwise we do want to be active for the use cases above. + if (!browserChild->IsPreservingLayers()) { + return true; + } } BrowsingContext* bc = doc->GetBrowsingContext();