Skip to content

Commit

Permalink
Regression: Pages that update their title now get suspended when back…
Browse files Browse the repository at this point in the history
…grounded

https://bugs.webkit.org/show_bug.cgi?id=270813
rdar://124222280

Reviewed by Ben Nham.

Pages that update their title now get suspended when backgrounded since we've
stopped taking near-suspended assertions on both iOS and macOS. We used to
monitor pages in the background for 8 minutes to see if they update their title
while in the background, if they did, we would let them keep running in the
background.

Since we no longer take near-suspended assertions, we can no longer observe
pages in the background (since they'd get suspended as soon as backgrounded).
To address the issue, we now monitor title changes while in the foreground. If
the page updates its title, we now take a background assertion to let it keep
running after backgrounding.

This is not perfect but this addresses the regression for now. We should
revisit though because this is too permissive. Maybe we only want to consider
title changes without recent user interaction for e.g.

* Source/WebKit/UIProcess/ProcessThrottler.cpp:
(WebKit::ProcessThrottler::ProcessThrottler):
(WebKit::ProcessThrottler::dropNearSuspendedAssertionTimerFired):
(WebKit::m_shouldTakeUIBackgroundAssertion): Deleted.
(WebKit::ProcessThrottler::pageAllowedToRunInTheBackgroundToken): Deleted.
(WebKit::ProcessThrottler::numberOfPagesAllowedToRunInTheBackgroundChanged): Deleted.
* Source/WebKit/UIProcess/ProcessThrottler.h:
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::didCommitLoadForFrame):
(WebKit::WebPageProxy::didFailLoadForFrame):
(WebKit::WebPageProxy::didReceiveTitleForFrame):
(WebKit::WebPageProxy::resetStateAfterProcessExited):
* Source/WebKit/UIProcess/WebPageProxyInternals.h:

Canonical link: https://commits.webkit.org/275960@main
  • Loading branch information
cdumez committed Mar 12, 2024
1 parent 3d45c70 commit 3130b40
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 42 deletions.
25 changes: 1 addition & 24 deletions Source/WebKit/UIProcess/ProcessThrottler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ ProcessThrottler::ProcessThrottler(AuxiliaryProcessProxy& process, bool shouldTa
, m_prepareToSuspendTimeoutTimer(RunLoop::main(), this, &ProcessThrottler::prepareToSuspendTimeoutTimerFired)
, m_dropNearSuspendedAssertionTimer(RunLoop::main(), this, &ProcessThrottler::dropNearSuspendedAssertionTimerFired)
, m_prepareToDropLastAssertionTimeoutTimer(RunLoop::main(), this, &ProcessThrottler::prepareToDropLastAssertionTimeoutTimerFired)
, m_pageAllowedToRunInTheBackgroundCounter([this](RefCounterEvent) { numberOfPagesAllowedToRunInTheBackgroundChanged(); })
, m_shouldTakeUIBackgroundAssertion(shouldTakeUIBackgroundAssertion)
{
}
Expand Down Expand Up @@ -362,10 +361,7 @@ void ProcessThrottler::dropNearSuspendedAssertionTimerFired()
PROCESSTHROTTLER_RELEASE_LOG("dropNearSuspendedAssertionTimerFired: Removing near-suspended process assertion");
RELEASE_ASSERT(isHoldingNearSuspendedAssertion());
ASSERT(m_shouldDropNearSuspendedAssertionAfterDelay);
if (m_pageAllowedToRunInTheBackgroundCounter.value())
PROCESSTHROTTLER_RELEASE_LOG("dropNearSuspendedAssertionTimerFired: Not releasing near-suspended assertion because a page is allowed to run in the background");
else
clearAssertion();
clearAssertion();
}

void ProcessThrottler::processReadyToSuspend()
Expand Down Expand Up @@ -522,25 +518,6 @@ Ref<AuxiliaryProcessProxy> ProcessThrottler::protectedProcess() const
return m_process.get();
}

PageAllowedToRunInTheBackgroundCounter::Token ProcessThrottler::pageAllowedToRunInTheBackgroundToken()
{
return m_pageAllowedToRunInTheBackgroundCounter.count();
}

void ProcessThrottler::numberOfPagesAllowedToRunInTheBackgroundChanged()
{
if (m_pageAllowedToRunInTheBackgroundCounter.value())
return;

if (m_dropNearSuspendedAssertionTimer.isActive())
return;

if (isHoldingNearSuspendedAssertion()) {
PROCESSTHROTTLER_RELEASE_LOG("numberOfPagesAllowedToRunInTheBackgroundChanged: Releasing near-suspended assertion");
clearAssertion();
}
}

bool ProcessThrottler::isSuspended() const
{
return m_isConnectedToProcess && !m_assertion;
Expand Down
10 changes: 0 additions & 10 deletions Source/WebKit/UIProcess/ProcessThrottler.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,6 @@ enum ProcessSuppressionDisabledCounterType { };
using ProcessSuppressionDisabledCounter = RefCounter<ProcessSuppressionDisabledCounterType>;
using ProcessSuppressionDisabledToken = ProcessSuppressionDisabledCounter::Token;

enum PageAllowedToRunInTheBackgroundCounterType { };
using PageAllowedToRunInTheBackgroundCounter = RefCounter<PageAllowedToRunInTheBackgroundCounterType>;

enum class IsSuspensionImminent : bool { No, Yes };
enum class ProcessThrottleState : uint8_t { Suspended, Background, Foreground };
enum class ProcessThrottlerActivityType : bool { Background, Foreground };
Expand Down Expand Up @@ -124,11 +121,6 @@ class ProcessThrottler : public CanMakeWeakPtr<ProcessThrottler> {
static bool isValidBackgroundActivity(const ActivityVariant&);
static bool isValidForegroundActivity(const ActivityVariant&);

// If any page holds one of these tokens, we will never release the "suspended" assertion which
// means that the page will not be suspended when in the background, except if the application
// also gets backgrounded.
PageAllowedToRunInTheBackgroundCounter::Token pageAllowedToRunInTheBackgroundToken();

using TimedActivity = ProcessThrottlerTimedActivity;

void didConnectToProcess(AuxiliaryProcessProxy&);
Expand Down Expand Up @@ -168,7 +160,6 @@ class ProcessThrottler : public CanMakeWeakPtr<ProcessThrottler> {
void assertionWasInvalidated();

void clearPendingRequestToSuspend();
void numberOfPagesAllowedToRunInTheBackgroundChanged();
void clearAssertion();

class ProcessAssertionCache;
Expand All @@ -186,7 +177,6 @@ class ProcessThrottler : public CanMakeWeakPtr<ProcessThrottler> {
WeakHashSet<Activity> m_backgroundActivities;
std::optional<uint64_t> m_pendingRequestToSuspendID;
ProcessThrottleState m_state { ProcessThrottleState::Suspended };
PageAllowedToRunInTheBackgroundCounter m_pageAllowedToRunInTheBackgroundCounter;
bool m_shouldDropNearSuspendedAssertionAfterDelay { false };
const bool m_shouldTakeUIBackgroundAssertion { false };
bool m_shouldTakeNearSuspendedAssertion { true };
Expand Down
14 changes: 7 additions & 7 deletions Source/WebKit/UIProcess/WebPageProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6221,7 +6221,7 @@ void WebPageProxy::didCommitLoadForFrame(FrameIdentifier frameID, FrameInfoData&
if (auto* drawingAreaProxy = dynamicDowncast<RemoteLayerTreeDrawingAreaProxy>(*m_drawingArea))
internals().firstLayerTreeTransactionIdAfterDidCommitLoad = drawingAreaProxy->nextLayerTreeTransactionID();
#endif
internals().pageAllowedToRunInTheBackgroundToken = nullptr;
internals().pageAllowedToRunInTheBackgroundActivityDueToTitleChanges = nullptr;
internals().pageAllowedToRunInTheBackgroundActivityDueToNotifications = nullptr;
}

Expand Down Expand Up @@ -6477,7 +6477,7 @@ void WebPageProxy::didFailLoadForFrame(FrameIdentifier frameID, FrameInfoData&&

if (isMainFrame) {
internals().pageLoadState.didFailLoad(transaction);
internals().pageAllowedToRunInTheBackgroundToken = nullptr;
internals().pageAllowedToRunInTheBackgroundActivityDueToTitleChanges = nullptr;
internals().pageAllowedToRunInTheBackgroundActivityDueToNotifications = nullptr;
}

Expand Down Expand Up @@ -6654,12 +6654,12 @@ void WebPageProxy::didReceiveTitleForFrame(FrameIdentifier frameID, const String

if (frame->isMainFrame()) {
internals().pageLoadState.setTitle(transaction, title);
if (!isViewVisible() && !frame->title().isNull() && frame->title() != title) {
WEBPAGEPROXY_RELEASE_LOG(ViewState, "didReceiveTitleForFrame: This page changes its title in the background and is allowed to run in the background");
if (!frame->title().isNull() && frame->title() != title) {
WEBPAGEPROXY_RELEASE_LOG(ViewState, "didReceiveTitleForFrame: This page updates its title and is now allowed to run in the background");
// This page updates its title in the background and is thus able to communicate with
// the user while in the background. Allow it to run in the background.
if (!internals().pageAllowedToRunInTheBackgroundToken)
internals().pageAllowedToRunInTheBackgroundToken = process().throttler().pageAllowedToRunInTheBackgroundToken();
if (!internals().pageAllowedToRunInTheBackgroundActivityDueToTitleChanges)
internals().pageAllowedToRunInTheBackgroundActivityDueToTitleChanges = process().throttler().backgroundActivity("Page updates its title"_s).moveToUniquePtr();
}
}

Expand Down Expand Up @@ -9780,7 +9780,7 @@ void WebPageProxy::resetStateAfterProcessExited(ProcessTerminationReason termina

internals().pageIsUserObservableCount = nullptr;
internals().visiblePageToken = nullptr;
internals().pageAllowedToRunInTheBackgroundToken = nullptr;
internals().pageAllowedToRunInTheBackgroundActivityDueToTitleChanges = nullptr;
internals().pageAllowedToRunInTheBackgroundActivityDueToNotifications = nullptr;

m_hasRunningProcess = false;
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/UIProcess/WebPageProxyInternals.h
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ struct WebPageProxy::Internals final : WebPopupMenuProxy::Client
WebCore::IntRect visibleScrollerThumbRect;
WebCore::PageIdentifier webPageID;
WindowKind windowKind { WindowKind::Unparented };
PageAllowedToRunInTheBackgroundCounter::Token pageAllowedToRunInTheBackgroundToken;
std::unique_ptr<ProcessThrottlerActivity> pageAllowedToRunInTheBackgroundActivityDueToTitleChanges;
std::unique_ptr<ProcessThrottlerActivity> pageAllowedToRunInTheBackgroundActivityDueToNotifications;

WebPageProxyMessageReceiverRegistration messageReceiverRegistration;
Expand Down

0 comments on commit 3130b40

Please sign in to comment.