[webkit-changes] [WebKit/WebKit] 30bf99: RELEASE_ASSERT(isSuspensionImminent == IsSuspensio...

[Chris Dumez]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 30bf9977849af414fb4571037893986af04b19ac
      https://github.com/WebKit/WebKit/commit/30bf9977849af414fb4571037893986af04b19ac
  Author: Chris Dumez <cdumez at apple.com>
  Date:   2023-07-17 (Mon, 17 Jul 2023)

  Changed paths:
    M Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp
    M Source/WebKit/UIProcess/ProcessThrottler.cpp
    M Source/WebKit/UIProcess/ProcessThrottler.h

  Log Message:
  -----------
  RELEASE_ASSERT(isSuspensionImminent == IsSuspensionImminent::Yes) under ProcessThrottler::sendPrepareToSuspendIPC()
https://bugs.webkit.org/show_bug.cgi?id=259287
rdar://110941932

Reviewed by Brent Fulgham.

Normally, when we try to send the PrepareToSuspend IPC and there is already one
in flight, it is because the assertion we were holding during the handshake was
invalidated by RunningBoard. We had an assertion to this effect in
ProcessThrottler::sendPrepareToSuspendIPC() which was getting hit in a very
specific scenario.

The scenario in question is:
1. A WebProcess is launching (we don't have a PID for it yet)
2. We detect that the UIProcess is about to suspend so we call
   ProcessThrottler::setAllowsActivities(false) which invalidates all
   activities
3. This causes us to try and send the prepareToSuspend IPC to the WebProcess
   which gets queued since the process is still launching.
4. Later on, the WebProcess finishes launching and we try to send the PrepareToSuspend
   IPC to the WebProcess. This causes AuxiliaryProcessProxy::wakeUpTemporarilyForIPC()
   to get called, which would create a new activity and potentially overwrite one it
   had already taken
5. Because new activities are not allowed, it would only clear the existing activity
   that was previously taken (but invalidated). ProcessThrottler::removeActivity() would
   fail to check if the activity was removed from the map and call
   updateThrottleStateIfNeeded() which would try to send the prepareToSuspend IPC again

This is when we fail the assertion since we're not in the imminent suspension case.
This is merely due to the bug in removeActivity() which called updateThrottleStateIfNeeded()
even though it did nothing.

* Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp:
(WebKit::AuxiliaryProcessProxy::wakeUpTemporarilyForIPC):
Check if we already have a pending activity to avoid constructing a new one unnecessarily.
This is not critical but it is nice to avoid churn for something that should be a no-op.

* Source/WebKit/UIProcess/ProcessThrottler.cpp:
(WebKit::ProcessThrottler::removeActivity):
In ProcessThrottler::addActivity(), we refuse to add the activity if `m_allowsActivities`
is true. However, we didn't have the same check in removeActivity(). As a result, we'd
try to remove the activity from the map (even though it is not in there), then we would
call updateThrottleStateIfNeeded() which could try to send the prepareToSuspend() again
unnecessarily since our state hasn't changed.

* Source/WebKit/UIProcess/ProcessThrottler.h:
(WebKit::ProcessThrottlerTimedActivity::activity const):

Canonical link: https://commits.webkit.org/266113@main