Use same limit for page cache and suspended pages
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 5 Nov 2018 21:38:05 +0000 (21:38 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 5 Nov 2018 21:38:05 +0000 (21:38 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191266

Reviewed by Geoffrey Garen.

Source/WebKit:

Use same limit for page cache and suspended pages as they serve the same purpose.

* UIProcess/API/Cocoa/WKProcessPool.mm:
(-[WKProcessPool _maximumSuspendedPageCount]):
* UIProcess/API/Cocoa/WKProcessPoolPrivate.h:
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::updateMaxSuspendedPageCount):
(WebKit::WebProcessPool::setCacheModel):
(WebKit::WebProcessPool::addSuspendedPageProxy):
* UIProcess/WebProcessPool.h:

Tools:

Update API tests accordingly.

* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@237830 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm
Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h
Source/WebKit/UIProcess/WebProcessPool.cpp
Source/WebKit/UIProcess/WebProcessPool.h
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm

index aad7a54..69a1786 100644 (file)
@@ -1,3 +1,21 @@
+2018-11-05  Chris Dumez  <cdumez@apple.com>
+
+        Use same limit for page cache and suspended pages
+        https://bugs.webkit.org/show_bug.cgi?id=191266
+
+        Reviewed by Geoffrey Garen.
+
+        Use same limit for page cache and suspended pages as they serve the same purpose.
+
+        * UIProcess/API/Cocoa/WKProcessPool.mm:
+        (-[WKProcessPool _maximumSuspendedPageCount]):
+        * UIProcess/API/Cocoa/WKProcessPoolPrivate.h:
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::updateMaxSuspendedPageCount):
+        (WebKit::WebProcessPool::setCacheModel):
+        (WebKit::WebProcessPool::addSuspendedPageProxy):
+        * UIProcess/WebProcessPool.h:
+
 2018-11-05  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         [iOS] Allow WKWebView clients to customize bar button item groups in the accessory view when editing
index 79ba442..cf2811e 100644 (file)
@@ -506,6 +506,11 @@ static NSDictionary *policiesHashMapToDictionary(const HashMap<String, HashMap<S
 #endif
 }
 
+- (NSUInteger)_maximumSuspendedPageCount
+{
+    return _processPool->maxSuspendedPageCount();
+}
+
 - (size_t)_serviceWorkerProcessCount
 {
 #if ENABLE(SERVICE_WORKER)
index a6744d9..4d3a191 100644 (file)
 - (void)_syncNetworkProcessCookies WK_API_AVAILABLE(macosx(10.13), ios(11.0));
 - (void)_makeNextWebProcessLaunchFailForTesting WK_API_AVAILABLE(macosx(10.14), ios(12.0));
 - (void)_makeNextNetworkProcessLaunchFailForTesting WK_API_AVAILABLE(macosx(10.14), ios(12.0));
+- (NSUInteger)_maximumSuspendedPageCount WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
 
 // Test only. Returns web processes running web pages (does not include web processes running service workers)
 - (size_t)_webPageContentProcessCount WK_API_AVAILABLE(macosx(10.13.4), ios(11.3));
index 19860e8..e4a8770 100644 (file)
@@ -125,7 +125,6 @@ using namespace WebCore;
 DEFINE_DEBUG_ONLY_GLOBAL(WTF::RefCountedLeakCounter, processPoolCounter, ("WebProcessPool"));
 
 const Seconds serviceWorkerTerminationDelay { 5_s };
-const unsigned maximumSuspendedPagesCount { 3 };
 
 static uint64_t generateListenerIdentifier()
 {
@@ -301,6 +300,8 @@ WebProcessPool::WebProcessPool(API::ProcessPoolConfiguration& configuration)
 #endif
 
     notifyThisWebProcessPoolWasCreated();
+
+    updateMaxSuspendedPageCount();
 }
 
 WebProcessPool::~WebProcessPool()
@@ -1470,9 +1471,24 @@ void WebProcessPool::registerURLSchemeAsCanDisplayOnlyIfCanRequest(const String&
     sendToNetworkingProcess(Messages::NetworkProcess::RegisterURLSchemeAsCanDisplayOnlyIfCanRequest(urlScheme));
 }
 
+void WebProcessPool::updateMaxSuspendedPageCount()
+{
+    unsigned dummy = 0;
+    Seconds dummyInterval;
+    unsigned pageCacheSize = 0;
+    calculateMemoryCacheSizes(m_configuration->cacheModel(), dummy, dummy, dummy, dummyInterval, pageCacheSize);
+
+    m_maxSuspendedPageCount = pageCacheSize;
+
+    while (m_suspendedPages.size() > m_maxSuspendedPageCount)
+        m_suspendedPages.removeFirst();
+}
+
 void WebProcessPool::setCacheModel(CacheModel cacheModel)
 {
     m_configuration->setCacheModel(cacheModel);
+    updateMaxSuspendedPageCount();
+
     sendToAllProcesses(Messages::WebProcess::SetCacheModel(cacheModel));
 
     if (m_networkProcess)
@@ -2234,7 +2250,7 @@ Ref<WebProcessProxy> WebProcessPool::processForNavigationInternal(WebPageProxy&
 
 void WebProcessPool::addSuspendedPageProxy(std::unique_ptr<SuspendedPageProxy>&& suspendedPage)
 {
-    if (m_suspendedPages.size() >= maximumSuspendedPagesCount)
+    if (m_suspendedPages.size() >= m_maxSuspendedPageCount)
         m_suspendedPages.removeFirst();
 
     m_suspendedPages.append(WTFMove(suspendedPage));
index c911ee2..d3a8d8e 100644 (file)
@@ -450,6 +450,7 @@ public:
     void removeAllSuspendedPageProxiesForPage(WebPageProxy&);
     std::unique_ptr<SuspendedPageProxy> takeSuspendedPageProxy(SuspendedPageProxy&);
     bool hasSuspendedPageProxyFor(WebProcessProxy&) const;
+    unsigned maxSuspendedPageCount() const { return m_maxSuspendedPageCount; }
 
     void didReachGoodTimeToPrewarm();
 
@@ -539,6 +540,8 @@ private:
 
     void tryPrewarmWithDomainInformation(WebProcessProxy&, const WebCore::URL&);
 
+    void updateMaxSuspendedPageCount();
+
     Ref<API::ProcessPoolConfiguration> m_configuration;
 
     IPC::MessageReceiverMap m_messageReceiverMap;
@@ -713,6 +716,8 @@ private:
 #endif
 
     Deque<std::unique_ptr<SuspendedPageProxy>> m_suspendedPages;
+    unsigned m_maxSuspendedPageCount { 0 };
+
     HashMap<String, RefPtr<WebProcessProxy>> m_swappedProcessesPerRegistrableDomain;
 
     HashMap<String, std::unique_ptr<WebCore::PrewarmInformation>> m_prewarmInformationPerRegistrableDomain;
index 20fa6a2..abe33f4 100644 (file)
@@ -1,3 +1,14 @@
+2018-11-05  Chris Dumez  <cdumez@apple.com>
+
+        Use same limit for page cache and suspended pages
+        https://bugs.webkit.org/show_bug.cgi?id=191266
+
+        Reviewed by Geoffrey Garen.
+
+        Update API tests accordingly.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
 2018-11-05  Basuke Suzuki  <Basuke.Suzuki@sony.com>
 
         [style] Exclude style check for auto generated files.
index c64f353..4b5a946 100644 (file)
@@ -522,7 +522,6 @@ TEST(ProcessSwap, NoSwappingForeTLDPlus2)
     EXPECT_EQ(numberOfDecidePolicyCalls, 2);
 }
 
-// We currently keep 3 suspended pages around so we can go back 3 times and use the page cache for each load.
 TEST(ProcessSwap, Back)
 {
     auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
@@ -544,6 +543,8 @@ TEST(ProcessSwap, Back)
     auto delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
     [webView setNavigationDelegate:delegate.get()];
 
+    EXPECT_GT([processPool _maximumSuspendedPageCount], 0U);
+
     NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
     [webView loadRequest:request];
 
@@ -599,31 +600,30 @@ TEST(ProcessSwap, Back)
     done = false;
 
     auto pidAfterSecondBackNavigation = [webView _webProcessIdentifier];
-    EXPECT_EQ(applePID, pidAfterSecondBackNavigation);
-
-    [webView goBack]; // Back to webkit.org.
-
-    TestWebKitAPI::Util::run(&receivedMessage);
-    receivedMessage = false;
-    TestWebKitAPI::Util::run(&done);
-    done = false;
+    if ([processPool _maximumSuspendedPageCount] > 1)
+        EXPECT_EQ(applePID, pidAfterSecondBackNavigation);
+    else {
+        EXPECT_NE(applePID, pidAfterSecondBackNavigation);
+        EXPECT_NE(googlePID, pidAfterSecondBackNavigation);
+    }
 
-    auto pidAfterThirdBackNavigation = [webView _webProcessIdentifier];
-    EXPECT_EQ(webkitPID, pidAfterThirdBackNavigation);
 
-    // 7 loads, 7 decidePolicy calls (e.g. any load that performs a process swap should not have generated an
+    // 6 loads, 6 decidePolicy calls (e.g. any load that performs a process swap should not have generated an
     // additional decidePolicy call as a result of the process swap)
-    EXPECT_EQ(7, numberOfDecidePolicyCalls);
+    EXPECT_EQ(6, numberOfDecidePolicyCalls);
 
-    EXPECT_EQ(6u, [receivedMessages count]);
+    EXPECT_EQ(5u, [receivedMessages count]);
     EXPECT_WK_STREQ(@"PageShow called. Persisted: false, and window.history.state is: null", receivedMessages.get()[0]);
     EXPECT_WK_STREQ(@"PageShow called. Persisted: false, and window.history.state is: null", receivedMessages.get()[1]);
     EXPECT_WK_STREQ(@"PageShow called. Persisted: false, and window.history.state is: null", receivedMessages.get()[2]);
     EXPECT_WK_STREQ(@"PageShow called. Persisted: true, and window.history.state is: onloadCalled", receivedMessages.get()[3]);
-    EXPECT_WK_STREQ(@"PageShow called. Persisted: true, and window.history.state is: onloadCalled", receivedMessages.get()[4]);
-    EXPECT_WK_STREQ(@"PageShow called. Persisted: true, and window.history.state is: onloadCalled", receivedMessages.get()[5]);
 
-    EXPECT_EQ(4u, seenPIDs.size());
+    // The number of suspended pages we keep around is determined at runtime.
+    if ([processPool _maximumSuspendedPageCount] > 1) {
+        EXPECT_WK_STREQ(@"PageShow called. Persisted: true, and window.history.state is: onloadCalled", receivedMessages.get()[4]);
+        EXPECT_EQ(4u, seenPIDs.size());
+    } else
+        EXPECT_EQ(5u, seenPIDs.size());
 }
 
 #if PLATFORM(MAC)
@@ -1348,7 +1348,8 @@ static void runClientSideRedirectTest(ShouldEnablePSON shouldEnablePSON)
     EXPECT_FALSE(didPerformClientRedirect);
 
     auto pidAfterBackNavigation = [webView _webProcessIdentifier];
-    EXPECT_EQ(webkitPID, pidAfterBackNavigation);
+    if ([processPool _maximumSuspendedPageCount] > 1)
+        EXPECT_EQ(webkitPID, pidAfterBackNavigation);
 
     // Validate Back/Forward list.
     currentItem = backForwardList.currentItem;
@@ -1658,7 +1659,7 @@ TEST(ProcessSwap, MainFramesOnly)
     EXPECT_EQ(1u, seenPIDs.size());
 }
 
-TEST(ProcessSwap, ThreePreviousProcessesRemains)
+TEST(ProcessSwap, SuspendedPageLimit)
 {
     auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
     [processPoolConfiguration setProcessSwapsOnNavigation:YES];
@@ -1673,6 +1674,9 @@ TEST(ProcessSwap, ThreePreviousProcessesRemains)
     auto delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
     [webView setNavigationDelegate:delegate.get()];
 
+    auto maximumSuspendedPageCount = [processPool _maximumSuspendedPageCount];
+    EXPECT_GT(maximumSuspendedPageCount, 0U);
+
     NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
     [webView loadRequest:request];
 
@@ -1706,8 +1710,8 @@ TEST(ProcessSwap, ThreePreviousProcessesRemains)
     // Navigations to 5 different domains, we expect to have seen 5 different PIDs
     EXPECT_EQ(5u, seenPIDs.size());
 
-    // But only 4 of those processes should still be alive (1 visible, 3 suspended).
-    EXPECT_EQ(4u, [processPool _webProcessCountIgnoringPrewarmed]);
+    // But not all of those processes should still be alive (1 visible, maximumSuspendedPageCount suspended).
+    EXPECT_EQ([processPool _webProcessCountIgnoringPrewarmed], (1U + maximumSuspendedPageCount));
 }
 
 TEST(ProcessSwap, PageCache1)