Branch: refs/heads/main Home: https://github.com/WebKit/WebKit Commit: 30bf9977849af414fb4571037893986af04b19ac https://github.com/WebKit/WebKit/commit/30bf9977849af414fb4571037893986af04b19ac Author: Chris Dumez <cdu...@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 _______________________________________________ webkit-changes mailing list webkit-changes@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-changes