- Revision
- 261034
- Author
- [email protected]
- Date
- 2020-05-01 15:57:00 -0700 (Fri, 01 May 2020)
Log Message
[iOS] ProcessThrottler fails to re-take ProcessAssertion if the previous one was invalidated
https://bugs.webkit.org/show_bug.cgi?id=211297
<rdar://problem/62542463>
Reviewed by Jer Noble.
Our ProcessAssertions may get invalidated upon backgrounding of the app. When the app becomes
foreground and the ProcessThrottler tries to take a Foreground assertion as a result, it would
incorrectly think it already had such assertion and not do anything, even though the previous
one is no longer valid. As a result, the child processes would stay suspended even though the
app was foregrounded.
To address the issue, add a isValid() method to ProcessAssertion() and check it in
ProcessThrottler::setAssertionType() to determine if we need to re-take an assertion or not.
We also invalidate all pending ProcessThrottler activities upon ProcessAssertion invalidation
for good measure. This way, the holders of these activities will be able to rely on
Activity::isValid() to determine if they need to re-take their activities or not.
* Platform/spi/ios/AssertionServicesSPI.h:
* Platform/spi/ios/RunningBoardServicesSPI.h:
* UIProcess/ProcessAssertion.cpp:
(WebKit::ProcessAssertion::isValid const):
* UIProcess/ProcessAssertion.h:
(WebKit::ProcessAssertion::validity const): Deleted.
* UIProcess/ProcessThrottler.cpp:
(WebKit::ProcessThrottler::setAssertionType):
(WebKit::ProcessThrottler::assertionWasInvalidated):
* UIProcess/ProcessThrottler.h:
* UIProcess/ios/ProcessAssertionIOS.mm:
(WebKit::ProcessAssertion::processAssertionWasInvalidated):
(WebKit::ProcessAssertion::isValid const):
(WebKit::ProcessAndUIAssertion::updateRunInBackgroundCount):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (261033 => 261034)
--- trunk/Source/WebKit/ChangeLog 2020-05-01 22:55:19 UTC (rev 261033)
+++ trunk/Source/WebKit/ChangeLog 2020-05-01 22:57:00 UTC (rev 261034)
@@ -1,5 +1,40 @@
2020-05-01 Chris Dumez <[email protected]>
+ [iOS] ProcessThrottler fails to re-take ProcessAssertion if the previous one was invalidated
+ https://bugs.webkit.org/show_bug.cgi?id=211297
+ <rdar://problem/62542463>
+
+ Reviewed by Jer Noble.
+
+ Our ProcessAssertions may get invalidated upon backgrounding of the app. When the app becomes
+ foreground and the ProcessThrottler tries to take a Foreground assertion as a result, it would
+ incorrectly think it already had such assertion and not do anything, even though the previous
+ one is no longer valid. As a result, the child processes would stay suspended even though the
+ app was foregrounded.
+
+ To address the issue, add a isValid() method to ProcessAssertion() and check it in
+ ProcessThrottler::setAssertionType() to determine if we need to re-take an assertion or not.
+ We also invalidate all pending ProcessThrottler activities upon ProcessAssertion invalidation
+ for good measure. This way, the holders of these activities will be able to rely on
+ Activity::isValid() to determine if they need to re-take their activities or not.
+
+ * Platform/spi/ios/AssertionServicesSPI.h:
+ * Platform/spi/ios/RunningBoardServicesSPI.h:
+ * UIProcess/ProcessAssertion.cpp:
+ (WebKit::ProcessAssertion::isValid const):
+ * UIProcess/ProcessAssertion.h:
+ (WebKit::ProcessAssertion::validity const): Deleted.
+ * UIProcess/ProcessThrottler.cpp:
+ (WebKit::ProcessThrottler::setAssertionType):
+ (WebKit::ProcessThrottler::assertionWasInvalidated):
+ * UIProcess/ProcessThrottler.h:
+ * UIProcess/ios/ProcessAssertionIOS.mm:
+ (WebKit::ProcessAssertion::processAssertionWasInvalidated):
+ (WebKit::ProcessAssertion::isValid const):
+ (WebKit::ProcessAndUIAssertion::updateRunInBackgroundCount):
+
+2020-05-01 Chris Dumez <[email protected]>
+
Unreviewed, reverting r261015.
Seems to have broken clean builds
Modified: trunk/Source/WebKit/Platform/spi/ios/AssertionServicesSPI.h (261033 => 261034)
--- trunk/Source/WebKit/Platform/spi/ios/AssertionServicesSPI.h 2020-05-01 22:55:19 UTC (rev 261033)
+++ trunk/Source/WebKit/Platform/spi/ios/AssertionServicesSPI.h 2020-05-01 22:57:00 UTC (rev 261034)
@@ -86,6 +86,7 @@
@property (nonatomic, assign) BKSProcessAssertionFlags flags;
- (id)initWithPID:(pid_t)pid flags:(BKSProcessAssertionFlags)flags reason:(BKSProcessAssertionReason)reason name:(NSString *)name withHandler:(BKSProcessAssertionAcquisitionHandler)handler;
+@property (nonatomic, readonly) BOOL valid;
@property (nonatomic, copy) BKSProcessAssertionInvalidationHandler invalidationHandler;
- (void)invalidate;
@end
Modified: trunk/Source/WebKit/Platform/spi/ios/RunningBoardServicesSPI.h (261033 => 261034)
--- trunk/Source/WebKit/Platform/spi/ios/RunningBoardServicesSPI.h 2020-05-01 22:55:19 UTC (rev 261033)
+++ trunk/Source/WebKit/Platform/spi/ios/RunningBoardServicesSPI.h 2020-05-01 22:57:00 UTC (rev 261034)
@@ -50,6 +50,8 @@
- (void)invalidate;
- (void)addObserver:(id <RBSAssertionObserving>)observer;
- (void)removeObserver:(id <RBSAssertionObserving>)observer;
+
+@property (nonatomic, readonly, assign, getter=isValid) BOOL valid;
@end
@protocol RBSAssertionObserving <NSObject>
Modified: trunk/Source/WebKit/UIProcess/ProcessAssertion.cpp (261033 => 261034)
--- trunk/Source/WebKit/UIProcess/ProcessAssertion.cpp 2020-05-01 22:55:19 UTC (rev 261033)
+++ trunk/Source/WebKit/UIProcess/ProcessAssertion.cpp 2020-05-01 22:57:00 UTC (rev 261034)
@@ -40,6 +40,11 @@
ProcessAssertion::~ProcessAssertion() = default;
+bool ProcessAssertion::isValid() const
+{
+ return true;
+}
+
} // namespace WebKit
#endif // !PLATFORM(IOS_FAMILY)
Modified: trunk/Source/WebKit/UIProcess/ProcessAssertion.h (261033 => 261034)
--- trunk/Source/WebKit/UIProcess/ProcessAssertion.h 2020-05-01 22:55:19 UTC (rev 261033)
+++ trunk/Source/WebKit/UIProcess/ProcessAssertion.h 2020-05-01 22:57:00 UTC (rev 261034)
@@ -59,7 +59,9 @@
class Client {
public:
virtual ~Client() { }
+
virtual void uiAssertionWillExpireImminently() = 0;
+ virtual void assertionWasInvalidated() = 0;
};
ProcessAssertion(ProcessID, const String& reason, ProcessAssertionType);
@@ -71,11 +73,10 @@
ProcessAssertionType type() const { return m_assertionType; }
ProcessID pid() const { return m_pid; }
+ bool isValid() const;
+
#if PLATFORM(IOS_FAMILY)
protected:
- enum class Validity { No, Yes, Unset };
- Validity validity() const { return m_validity; }
-
virtual void processAssertionWasInvalidated();
#endif
@@ -86,7 +87,6 @@
RetainPtr<RBSAssertion> m_rbsAssertion;
RetainPtr<WKRBSAssertionDelegate> m_delegate;
RetainPtr<BKSProcessAssertion> m_bksAssertion; // Legacy.
- Validity m_validity { Validity::Unset };
#endif
Client* m_client { nullptr };
};
Modified: trunk/Source/WebKit/UIProcess/ProcessThrottler.cpp (261033 => 261034)
--- trunk/Source/WebKit/UIProcess/ProcessThrottler.cpp 2020-05-01 22:55:19 UTC (rev 261033)
+++ trunk/Source/WebKit/UIProcess/ProcessThrottler.cpp 2020-05-01 22:57:00 UTC (rev 261034)
@@ -124,7 +124,7 @@
void ProcessThrottler::setAssertionType(ProcessAssertionType newType)
{
- if (m_assertion && m_assertion->type() == newType)
+ if (m_assertion && m_assertion->isValid() && m_assertion->type() == newType)
return;
PROCESSTHROTTLER_RELEASE_LOG("setAssertionType: Updating process assertion type to %u (foregroundActivities: %u, backgroundActivities: %u)", newType, m_foregroundActivities.size(), m_backgroundActivities.size());
@@ -225,6 +225,12 @@
m_prepareToSuspendTimeoutTimer.stop();
}
+void ProcessThrottler::assertionWasInvalidated()
+{
+ PROCESSTHROTTLER_RELEASE_LOG("assertionWasInvalidated:");
+ invalidateAllActivities();
+}
+
bool ProcessThrottler::isValidBackgroundActivity(const ProcessThrottler::ActivityVariant& activity)
{
if (!WTF::holds_alternative<UniqueRef<ProcessThrottler::BackgroundActivity>>(activity))
Modified: trunk/Source/WebKit/UIProcess/ProcessThrottler.h (261033 => 261034)
--- trunk/Source/WebKit/UIProcess/ProcessThrottler.h 2020-05-01 22:55:19 UTC (rev 261033)
+++ trunk/Source/WebKit/UIProcess/ProcessThrottler.h 2020-05-01 22:57:00 UTC (rev 261034)
@@ -137,7 +137,8 @@
String assertionName(ProcessAssertionType) const;
// ProcessAssertionClient
- void uiAssertionWillExpireImminently() override;
+ void uiAssertionWillExpireImminently() final;
+ void assertionWasInvalidated() final;
void clearPendingRequestToSuspend();
Modified: trunk/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm (261033 => 261034)
--- trunk/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm 2020-05-01 22:55:19 UTC (rev 261033)
+++ trunk/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm 2020-05-01 22:57:00 UTC (rev 261034)
@@ -442,12 +442,24 @@
ASSERT(RunLoop::isMain());
RELEASE_LOG(ProcessSuspension, "%p - ProcessAssertion::processAssertionWasInvalidated() PID: %d", this, m_pid);
- m_validity = Validity::No;
+ if (auto* client = this->client())
+ client->assertionWasInvalidated();
}
+bool ProcessAssertion::isValid() const
+{
+ if (m_rbsAssertion)
+ return m_rbsAssertion.get().valid;
+
+ if (m_bksAssertion)
+ return m_bksAssertion.get().valid;
+
+ return false;
+}
+
void ProcessAndUIAssertion::updateRunInBackgroundCount()
{
- bool shouldHoldBackgroundTask = validity() != Validity::No && type() != ProcessAssertionType::Suspended;
+ bool shouldHoldBackgroundTask = isValid() && type() != ProcessAssertionType::Suspended;
if (m_isHoldingBackgroundTask == shouldHoldBackgroundTask)
return;
