RELEASE_ASSERT(isSuspensionImminent == IsSuspensionImminent::Yes) und…

…er 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

Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp

@@ -332,7 +332,8 @@ void AuxiliaryProcessProxy::wakeUpTemporarilyForIPC()
 {
     // If we keep trying to send IPC to a suspended process, the outgoing message queue may grow large and result
     // in increased memory usage. To avoid this, we wake up the process for a bit so we can drain the messages.
-    m_timedActivityForIPC = throttler().backgroundActivity("IPC sending due to large outgoing queue"_s);
+    if (!ProcessThrottler::isValidBackgroundActivity(m_timedActivityForIPC.activity()))
+        m_timedActivityForIPC = throttler().backgroundActivity("IPC sending due to large outgoing queue"_s);
 }
 #endif
 

Source/WebKit/UIProcess/ProcessThrottler.cpp

@@ -85,10 +85,21 @@ bool ProcessThrottler::addActivity(Activity& activity)
 void ProcessThrottler::removeActivity(Activity& activity)
 {
     ASSERT(isMainRunLoop());
+    if (!m_allowsActivities) {
+        ASSERT(m_foregroundActivities.isEmpty());
+        ASSERT(m_backgroundActivities.isEmpty());
+        return;
+    }
+
+    bool wasRemoved;
     if (activity.isForeground())
-        m_foregroundActivities.remove(&activity);
+        wasRemoved = m_foregroundActivities.remove(&activity);
     else
-        m_backgroundActivities.remove(&activity);
+        wasRemoved = m_backgroundActivities.remove(&activity);
+    ASSERT(wasRemoved);
+    if (!wasRemoved)
+        return;
+
     updateThrottleStateIfNeeded();
 }
 

Source/WebKit/UIProcess/ProcessThrottler.h

@@ -93,6 +93,7 @@ class ProcessThrottlerTimedActivity {
     public:
         explicit ProcessThrottlerTimedActivity(Seconds timeout, ActivityVariant&& = nullptr);
         ProcessThrottlerTimedActivity& operator=(ActivityVariant&&);
+        const ActivityVariant& activity() const { return m_activity; }
 
     private:
         void activityTimedOut();