Diff
Modified: trunk/Source/WebKit/ChangeLog (260927 => 260928)
--- trunk/Source/WebKit/ChangeLog 2020-04-30 00:10:26 UTC (rev 260927)
+++ trunk/Source/WebKit/ChangeLog 2020-04-30 00:50:03 UTC (rev 260928)
@@ -1,3 +1,23 @@
+2020-04-29 Chris Dumez <[email protected]>
+
+ REGRESSION(r260791) Network process fails to suspend promptly
+ https://bugs.webkit.org/show_bug.cgi?id=211207
+ <rdar://problem/62620454>
+
+ Reviewed by Alex Christensen.
+
+ After r260791, all WebResourceLoadStatisticsStore instances share a single WorkQueue.
+ As a result, the logic to suspend WebResourceLoadStatisticsStore's WorkQueues in
+ NetworkProcess::prepareToSuspend() needs to get updated to reflect this.
+
+ * NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:
+ (WebKit::WebResourceLoadStatisticsStore::suspend):
+ (WebKit::WebResourceLoadStatisticsStore::resume):
+ * NetworkProcess/Classifier/WebResourceLoadStatisticsStore.h:
+ * NetworkProcess/NetworkProcess.cpp:
+ (WebKit::NetworkProcess::prepareToSuspend):
+ (WebKit::NetworkProcess::resume):
+
2020-04-29 Adrian Perez de Castro <[email protected]>
REGRESSION(r260889): TestContextMenu:/webkit/WebKitWebView/populate-menu no longer passes
Modified: trunk/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp (260927 => 260928)
--- trunk/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp 2020-04-30 00:10:26 UTC (rev 260927)
+++ trunk/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp 2020-04-30 00:50:03 UTC (rev 260928)
@@ -62,6 +62,10 @@
namespace WebKit {
using namespace WebCore;
+WebResourceLoadStatisticsStore::State WebResourceLoadStatisticsStore::suspendedState { WebResourceLoadStatisticsStore::State::Running };
+Lock WebResourceLoadStatisticsStore::suspendedStateLock;
+Condition WebResourceLoadStatisticsStore::suspendedStateChangeCondition;
+
const OptionSet<WebsiteDataType>& WebResourceLoadStatisticsStore::monitoredDataTypes()
{
static NeverDestroyed<OptionSet<WebsiteDataType>> dataTypes(std::initializer_list<WebsiteDataType>({
@@ -1381,43 +1385,38 @@
void WebResourceLoadStatisticsStore::suspend(CompletionHandler<void()>&& completionHandler)
{
- // Suspend is needed to manage the database store which is not used in ephemeral sessions.
- ASSERT(RunLoop::isMain() && !isEphemeral());
-
CompletionHandlerCallingScope completionHandlerCaller(WTFMove(completionHandler));
- Locker<Lock> stateLocker(m_stateLock);
- if (m_state != State::Running)
+ Locker<Lock> stateLocker(suspendedStateLock);
+ if (suspendedState != State::Running)
return;
- m_state = State::WillSuspend;
+ suspendedState = State::WillSuspend;
- postTask([this, completionHandler = completionHandlerCaller.release()] () mutable {
- Locker<Lock> stateLocker(m_stateLock);
- ASSERT(m_state != State::Suspended);
+ sharedStatisticsQueue()->dispatch([completionHandler = completionHandlerCaller.release()] () mutable {
- if (m_state != State::WillSuspend) {
+ Locker<Lock> stateLocker(suspendedStateLock);
+ ASSERT(suspendedState != State::Suspended);
+
+ if (suspendedState != State::WillSuspend) {
postTaskReply(WTFMove(completionHandler));
return;
}
- m_state = State::Suspended;
+ suspendedState = State::Suspended;
postTaskReply(WTFMove(completionHandler));
- while (m_state == State::Suspended)
- m_stateChangeCondition.wait(m_stateLock);
- ASSERT(m_state == State::Running);
+ while (suspendedState == State::Suspended)
+ suspendedStateChangeCondition.wait(suspendedStateLock);
+ ASSERT(suspendedState == State::Running);
});
}
void WebResourceLoadStatisticsStore::resume()
{
- // Resume is needed to manage the database store which is not used in ephemeral sessions.
- ASSERT(RunLoop::isMain() && !isEphemeral());
-
- Locker<Lock> stateLocker(m_stateLock);
- auto previousState = m_state;
- m_state = State::Running;
+ Locker<Lock> stateLocker(suspendedStateLock);
+ auto previousState = suspendedState;
+ suspendedState = State::Running;
if (previousState == State::Suspended)
- m_stateChangeCondition.notifyOne();
+ suspendedStateChangeCondition.notifyOne();
}
} // namespace WebKit
Modified: trunk/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.h (260927 => 260928)
--- trunk/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.h 2020-04-30 00:10:26 UTC (rev 260927)
+++ trunk/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.h 2020-04-30 00:50:03 UTC (rev 260928)
@@ -292,8 +292,8 @@
void resourceLoadStatisticsUpdated(Vector<WebCore::ResourceLoadStatistics>&&);
void requestStorageAccessUnderOpener(DomainInNeedOfStorageAccess&&, WebCore::PageIdentifier openerID, OpenerDomain&&);
void aggregatedThirdPartyData(CompletionHandler<void(Vector<WebResourceLoadStatisticsStore::ThirdPartyData>&&)>&&);
- void suspend(CompletionHandler<void()>&&);
- void resume();
+ static void suspend(CompletionHandler<void()>&&);
+ static void resume();
bool isEphemeral() const { return m_isEphemeral == WebCore::ResourceLoadStatistics::IsEphemeral::Yes; };
@@ -337,9 +337,9 @@
WillSuspend,
Suspended
};
- State m_state { State::Running };
- Lock m_stateLock;
- Condition m_stateChangeCondition;
+ static State suspendedState;
+ static Lock suspendedStateLock;
+ static Condition suspendedStateChangeCondition;
};
} // namespace WebKit
Modified: trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp (260927 => 260928)
--- trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2020-04-30 00:10:26 UTC (rev 260927)
+++ trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2020-04-30 00:50:03 UTC (rev 260928)
@@ -2218,12 +2218,7 @@
});
#if ENABLE(RESOURCE_LOAD_STATISTICS)
- forEachNetworkSession([&callbackAggregator](auto& networkSession) {
- if (auto* resourceLoadStatistics = networkSession.resourceLoadStatistics()) {
- if (!resourceLoadStatistics->isEphemeral())
- resourceLoadStatistics->suspend([callbackAggregator] { });
- }
- });
+ WebResourceLoadStatisticsStore::suspend([callbackAggregator] { });
#endif
platformPrepareToSuspend([callbackAggregator] { });
@@ -2265,12 +2260,7 @@
connection->endSuspension();
#if ENABLE(RESOURCE_LOAD_STATISTICS)
- forEachNetworkSession([](auto& networkSession) {
- if (auto* resourceLoadStatistics = networkSession.resourceLoadStatistics()) {
- if (!resourceLoadStatistics->isEphemeral())
- resourceLoadStatistics->resume();
- }
- });
+ WebResourceLoadStatisticsStore::resume();
#endif
#if ENABLE(SERVICE_WORKER)
Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm (260927 => 260928)
--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm 2020-04-30 00:10:26 UTC (rev 260927)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm 2020-04-30 00:50:03 UTC (rev 260928)
@@ -421,6 +421,13 @@
_processPool->sendNetworkProcessWillSuspendImminentlyForTesting();
}
+- (void)_sendNetworkProcessPrepareToSuspend:(void(^)(void))completionHandler
+{
+ _processPool->sendNetworkProcessPrepareToSuspendForTesting([completionHandler = makeBlockPtr(completionHandler)] {
+ completionHandler();
+ });
+}
+
- (void)_sendNetworkProcessDidResume
{
_processPool->sendNetworkProcessDidResume();
Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h (260927 => 260928)
--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h 2020-04-30 00:10:26 UTC (rev 260927)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h 2020-04-30 00:50:03 UTC (rev 260928)
@@ -88,6 +88,7 @@
// Test only. Should be called only while no web content processes are running.
- (void)_terminateNetworkProcess WK_API_AVAILABLE(macos(10.15), ios(13.0));
+- (void)_sendNetworkProcessPrepareToSuspend:(void(^)(void))completionHandler WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
- (void)_sendNetworkProcessWillSuspendImminently WK_API_AVAILABLE(macos(10.15), ios(13.0));
- (void)_sendNetworkProcessDidResume WK_API_AVAILABLE(macos(10.15), ios(13.0));
- (void)_terminateServiceWorkers WK_API_AVAILABLE(macos(10.14), ios(12.0));
Modified: trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h (260927 => 260928)
--- trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h 2020-04-30 00:10:26 UTC (rev 260927)
+++ trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h 2020-04-30 00:50:03 UTC (rev 260928)
@@ -226,6 +226,9 @@
void clearAppBoundSession(PAL::SessionID, CompletionHandler<void()>&&);
void getAppBoundDomains(PAL::SessionID, CompletionHandler<void(HashSet<WebCore::RegistrableDomain>&&)>&&);
+ // ProcessThrottlerClient
+ void sendPrepareToSuspend(IsSuspensionImminent, CompletionHandler<void()>&&) final;
+
private:
// AuxiliaryProcessProxy
ASCIILiteral processName() const final { return "Networking"_s; }
@@ -237,9 +240,6 @@
void networkProcessCrashed();
void clearCallbackStates();
- // ProcessThrottlerClient
- void sendPrepareToSuspend(IsSuspensionImminent, CompletionHandler<void()>&&) final;
-
// IPC::Connection::Client
void didReceiveMessage(IPC::Connection&, IPC::Decoder&) override;
void didReceiveSyncMessage(IPC::Connection&, IPC::Decoder&, std::unique_ptr<IPC::Encoder>&) override;
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (260927 => 260928)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2020-04-30 00:10:26 UTC (rev 260927)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2020-04-30 00:50:03 UTC (rev 260928)
@@ -1799,6 +1799,14 @@
process->terminate();
}
+void WebProcessPool::sendNetworkProcessPrepareToSuspendForTesting(CompletionHandler<void()>&& completionHandler)
+{
+ if (!m_networkProcess)
+ return completionHandler();
+
+ m_networkProcess->sendPrepareToSuspend(IsSuspensionImminent::No, WTFMove(completionHandler));
+}
+
void WebProcessPool::sendNetworkProcessWillSuspendImminentlyForTesting()
{
if (m_networkProcess)
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.h (260927 => 260928)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.h 2020-04-30 00:10:26 UTC (rev 260927)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.h 2020-04-30 00:50:03 UTC (rev 260928)
@@ -321,6 +321,7 @@
void clearCachedCredentials();
void terminateNetworkProcess();
void terminateAllWebContentProcesses();
+ void sendNetworkProcessPrepareToSuspendForTesting(CompletionHandler<void()>&&);
void sendNetworkProcessWillSuspendImminentlyForTesting();
void sendNetworkProcessDidResume();
void terminateServiceWorkers();
Modified: trunk/Tools/ChangeLog (260927 => 260928)
--- trunk/Tools/ChangeLog 2020-04-30 00:10:26 UTC (rev 260927)
+++ trunk/Tools/ChangeLog 2020-04-30 00:50:03 UTC (rev 260928)
@@ -1,3 +1,16 @@
+2020-04-29 Chris Dumez <[email protected]>
+
+ REGRESSION(r260791) Network process fails to suspend promptly
+ https://bugs.webkit.org/show_bug.cgi?id=211207
+ <rdar://problem/62620454>
+
+ Reviewed by Alex Christensen.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/ResourceLoadStatistics.mm:
+ (TEST):
+
2020-04-29 Kate Cheney <[email protected]>
Add better handling of an uncleared bundle identifier in WebKitTestRunner
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ResourceLoadStatistics.mm (260927 => 260928)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ResourceLoadStatistics.mm 2020-04-30 00:10:26 UTC (rev 260927)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ResourceLoadStatistics.mm 2020-04-30 00:50:03 UTC (rev 260928)
@@ -1549,3 +1549,50 @@
TestWebKitAPI::Util::run(&doneFlag);
}
+
+TEST(ResourceLoadStatistics, StoreSuspension)
+{
+ auto *sharedProcessPool = [WKProcessPool _sharedProcessPool];
+ auto *dataStore1 = [WKWebsiteDataStore defaultDataStore];
+ auto *dataStore2 = [[[WKWebsiteDataStore alloc] _initWithConfiguration:[[[_WKWebsiteDataStoreConfiguration alloc] init] autorelease]] autorelease];
+
+ [dataStore1 _setResourceLoadStatisticsEnabled:YES];
+ [dataStore2 _setResourceLoadStatisticsEnabled:YES];
+
+ static bool doneFlag = false;
+ [dataStore1 _setUseITPDatabase:true completionHandler: ^(void) {
+ doneFlag = true;
+ }];
+ TestWebKitAPI::Util::run(&doneFlag);
+
+ doneFlag = false;
+ [dataStore2 _setUseITPDatabase:true completionHandler: ^(void) {
+ doneFlag = true;
+ }];
+ TestWebKitAPI::Util::run(&doneFlag);
+
+ auto configuration1 = adoptNS([[WKWebViewConfiguration alloc] init]);
+ [configuration1 setProcessPool: sharedProcessPool];
+ configuration1.get().websiteDataStore = dataStore1;
+
+ auto configuration2 = adoptNS([[WKWebViewConfiguration alloc] init]);
+ [configuration2 setProcessPool: sharedProcessPool];
+ configuration2.get().websiteDataStore = dataStore2;
+
+ auto webView1 = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration1.get()]);
+ auto webView2 = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration2.get()]);
+
+ [webView1 loadHTMLString:@"WebKit Test" baseURL:[NSURL URLWithString:@"http://webkit.org"]];
+ [webView1 _test_waitForDidFinishNavigation];
+
+ [webView2 loadHTMLString:@"WebKit Test" baseURL:[NSURL URLWithString:@"http://webkit2.org"]];
+ [webView2 _test_waitForDidFinishNavigation];
+
+ doneFlag = false;
+ [sharedProcessPool _sendNetworkProcessPrepareToSuspend:^{
+ doneFlag = true;
+ }];
+ TestWebKitAPI::Util::run(&doneFlag);
+
+ [sharedProcessPool _sendNetworkProcessDidResume];
+}
