Regression(r238817) PSON Page Cache API tests are failing
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 4 Dec 2018 19:44:09 +0000 (19:44 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 4 Dec 2018 19:44:09 +0000 (19:44 +0000)
https://bugs.webkit.org/show_bug.cgi?id=192348

Reviewed by Alex Christensen.

Source/WebCore:

* page/MemoryRelease.cpp:
(WebCore::releaseCriticalMemory):
(WebCore::releaseMemory):
* page/MemoryRelease.h:

Source/WebKit:

Before suspending a WebProcess on iOS, we normally fake a memory pressure signal
so that the suspended process uses as little memory as possible while suspended.
Among other things, this will clear the page cache. This is an issue in the case
of process-swap on navigation because we keep suspended web processes around to
keep Page Cache functional.

To address the issue, when a WebProcess is about to get suspended, we check if
the process has any suspended WebPage (WebPage used for PSON PageCache support)
and we bypass the PageCache clearing if it does.

Our API tests did not catch this before r238817 because the NavigationState's
assertion was preventing the old WebProcesses from suspending for 3 seconds,
which was enough for those tests to complete.

* UIProcess/SuspendedPageProxy.cpp:
(WebKit::SuspendedPageProxy::SuspendedPageProxy):
(WebKit::SuspendedPageProxy::didFinishLoad):
* UIProcess/SuspendedPageProxy.h:
Take a background assertion until the suspension load is complete, to make sure
the suspension load has a chance to complete before the process gets suspended.

* WebProcess/WebProcess.cpp:
(WebKit::WebProcess::initializeWebProcess):
(WebKit::WebProcess::hasPageRequiringPageCacheWhileSuspended const):
(WebKit::WebProcess::actualPrepareToSuspend):
* WebProcess/WebProcess.h:

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

Source/WebCore/ChangeLog
Source/WebCore/page/MemoryRelease.cpp
Source/WebCore/page/MemoryRelease.h
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/SuspendedPageProxy.cpp
Source/WebKit/UIProcess/SuspendedPageProxy.h
Source/WebKit/WebProcess/WebProcess.cpp
Source/WebKit/WebProcess/WebProcess.h

index 1026412..e6ee9ed 100644 (file)
@@ -1,3 +1,15 @@
+2018-12-04  Chris Dumez  <cdumez@apple.com>
+
+        Regression(r238817) PSON Page Cache API tests are failing
+        https://bugs.webkit.org/show_bug.cgi?id=192348
+
+        Reviewed by Alex Christensen.
+
+        * page/MemoryRelease.cpp:
+        (WebCore::releaseCriticalMemory):
+        (WebCore::releaseMemory):
+        * page/MemoryRelease.h:
+
 2018-12-04  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r238838.
index 8875963..08f04a4 100644 (file)
@@ -75,11 +75,13 @@ static void releaseNoncriticalMemory()
     InlineStyleSheetOwner::clearCache();
 }
 
-static void releaseCriticalMemory(Synchronous synchronous)
+static void releaseCriticalMemory(Synchronous synchronous, MaintainPageCache maintainPageCache)
 {
     // Right now, the only reason we call release critical memory while not under memory pressure is if the process is about to be suspended.
-    PruningReason pruningReason = MemoryPressureHandler::singleton().isUnderMemoryPressure() ? PruningReason::MemoryPressure : PruningReason::ProcessSuspended;
-    PageCache::singleton().pruneToSizeNow(0, pruningReason);
+    if (maintainPageCache == MaintainPageCache::No) {
+        PruningReason pruningReason = MemoryPressureHandler::singleton().isUnderMemoryPressure() ? PruningReason::MemoryPressure : PruningReason::ProcessSuspended;
+        PageCache::singleton().pruneToSizeNow(0, pruningReason);
+    }
 
     MemoryCache::singleton().pruneLiveResourcesToSize(0, /*shouldDestroyDecodedDataForAllLiveResources*/ true);
 
@@ -111,14 +113,14 @@ static void releaseCriticalMemory(Synchronous synchronous)
     }
 }
 
-void releaseMemory(Critical critical, Synchronous synchronous)
+void releaseMemory(Critical critical, Synchronous synchronous, MaintainPageCache maintainPageCache)
 {
     TraceScope scope(MemoryPressureHandlerStart, MemoryPressureHandlerEnd, static_cast<uint64_t>(critical), static_cast<uint64_t>(synchronous));
 
     if (critical == Critical::Yes) {
         // Return unused pages back to the OS now as this will likely give us a little memory to work with.
         WTF::releaseFastMallocFreeMemory();
-        releaseCriticalMemory(synchronous);
+        releaseCriticalMemory(synchronous, maintainPageCache);
     }
 
     releaseNoncriticalMemory();
index a372a5d..9b07861 100644 (file)
@@ -29,7 +29,9 @@
 
 namespace WebCore {
 
-WEBCORE_EXPORT void releaseMemory(Critical, Synchronous);
+enum class MaintainPageCache : bool { No, Yes };
+
+WEBCORE_EXPORT void releaseMemory(Critical, Synchronous, MaintainPageCache = MaintainPageCache::No);
 void platformReleaseMemory(Critical);
 void jettisonExpensiveObjectsOnTopLevelNavigation();
 WEBCORE_EXPORT void registerMemoryReleaseNotifyCallbacks();
index 99e9175..55dff1c 100644 (file)
@@ -1,3 +1,37 @@
+2018-12-04  Chris Dumez  <cdumez@apple.com>
+
+        Regression(r238817) PSON Page Cache API tests are failing
+        https://bugs.webkit.org/show_bug.cgi?id=192348
+
+        Reviewed by Alex Christensen.
+
+        Before suspending a WebProcess on iOS, we normally fake a memory pressure signal
+        so that the suspended process uses as little memory as possible while suspended.
+        Among other things, this will clear the page cache. This is an issue in the case
+        of process-swap on navigation because we keep suspended web processes around to
+        keep Page Cache functional.
+
+        To address the issue, when a WebProcess is about to get suspended, we check if
+        the process has any suspended WebPage (WebPage used for PSON PageCache support)
+        and we bypass the PageCache clearing if it does.
+
+        Our API tests did not catch this before r238817 because the NavigationState's
+        assertion was preventing the old WebProcesses from suspending for 3 seconds,
+        which was enough for those tests to complete.
+
+        * UIProcess/SuspendedPageProxy.cpp:
+        (WebKit::SuspendedPageProxy::SuspendedPageProxy):
+        (WebKit::SuspendedPageProxy::didFinishLoad):
+        * UIProcess/SuspendedPageProxy.h:
+        Take a background assertion until the suspension load is complete, to make sure
+        the suspension load has a chance to complete before the process gets suspended.
+
+        * WebProcess/WebProcess.cpp:
+        (WebKit::WebProcess::initializeWebProcess):
+        (WebKit::WebProcess::hasPageRequiringPageCacheWhileSuspended const):
+        (WebKit::WebProcess::actualPrepareToSuspend):
+        * WebProcess/WebProcess.h:
+
 2018-12-04  Youenn Fablet  <youenn@apple.com>
 
         Device orientation may be wrong on page reload after crash
index 1497361..0fea9a2 100644 (file)
@@ -79,6 +79,9 @@ SuspendedPageProxy::SuspendedPageProxy(WebPageProxy& page, Ref<WebProcessProxy>&
     , m_process(WTFMove(process))
     , m_mainFrameID(mainFrameID)
     , m_registrableDomain(toRegistrableDomain(URL(URL(), item.url())))
+#if PLATFORM(IOS_FAMILY)
+    , m_suspensionToken(m_process->throttler().backgroundActivityToken())
+#endif
 {
     item.setSuspendedPage(this);
     m_process->addMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_page.pageID(), *this);
@@ -129,6 +132,10 @@ void SuspendedPageProxy::didFinishLoad()
 
     m_process->send(Messages::WebProcess::UpdateActivePages(), 0);
 
+#if PLATFORM(IOS_FAMILY)
+    m_suspensionToken = nullptr;
+#endif
+
     if (auto finishedSuspendingHandler = WTFMove(m_finishedSuspendingHandler))
         finishedSuspendingHandler();
 }
index 1fdeece..f31d8ba 100644 (file)
@@ -26,6 +26,7 @@
 #pragma once
 
 #include "Connection.h"
+#include "ProcessThrottler.h"
 #include "WebBackForwardListItem.h"
 #include <WebCore/SecurityOriginData.h>
 #include <wtf/RefCounted.h>
@@ -68,6 +69,9 @@ private:
 
     bool m_finishedSuspending { false };
     CompletionHandler<void()> m_finishedSuspendingHandler;
+#if PLATFORM(IOS_FAMILY)
+    ProcessThrottler::BackgroundActivityToken m_suspensionToken;
+#endif
 };
 
 } // namespace WebKit
index 8322274..2831aaa 100644 (file)
@@ -280,8 +280,9 @@ void WebProcess::initializeWebProcess(WebProcessCreationParameters&& parameters)
     m_suppressMemoryPressureHandler = parameters.shouldSuppressMemoryPressureHandler;
     if (!m_suppressMemoryPressureHandler) {
         auto& memoryPressureHandler = MemoryPressureHandler::singleton();
-        memoryPressureHandler.setLowMemoryHandler([] (Critical critical, Synchronous synchronous) {
-            WebCore::releaseMemory(critical, synchronous);
+        memoryPressureHandler.setLowMemoryHandler([this] (Critical critical, Synchronous synchronous) {
+            auto maintainPageCache = m_isSuspending && hasPageRequiringPageCacheWhileSuspended() ? WebCore::MaintainPageCache::Yes : WebCore::MaintainPageCache::No;
+            WebCore::releaseMemory(critical, synchronous, maintainPageCache);
         });
 #if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MAX_ALLOWED >= 101200) || PLATFORM(GTK) || PLATFORM(WPE)
         memoryPressureHandler.setShouldUsePeriodicMemoryMonitor(true);
@@ -429,6 +430,15 @@ void WebProcess::initializeWebProcess(WebProcessCreationParameters&& parameters)
     RELEASE_LOG(Process, "%p - WebProcess::initializeWebProcess: Presenting process = %d", this, WebCore::presentingApplicationPID());
 }
 
+bool WebProcess::hasPageRequiringPageCacheWhileSuspended() const
+{
+    for (auto& page : m_pageMap.values()) {
+        if (page->isSuspended())
+            return true;
+    }
+    return false;
+}
+
 void WebProcess::markIsNoLongerPrewarmed()
 {
 #if PLATFORM(MAC)
@@ -1376,6 +1386,8 @@ void WebProcess::resetAllGeolocationPermissions()
 
 void WebProcess::actualPrepareToSuspend(ShouldAcknowledgeWhenReadyToSuspend shouldAcknowledgeWhenReadyToSuspend)
 {
+    SetForScope<bool> suspensionScope(m_isSuspending, true);
+
     if (!m_suppressMemoryPressureHandler)
         MemoryPressureHandler::singleton().releaseMemory(Critical::Yes, Synchronous::Yes);
 
index fb78e63..9172ee1 100644 (file)
@@ -348,6 +348,8 @@ private:
     enum class ShouldAcknowledgeWhenReadyToSuspend { No, Yes };
     void actualPrepareToSuspend(ShouldAcknowledgeWhenReadyToSuspend);
 
+    bool hasPageRequiringPageCacheWhileSuspended() const;
+
     void ensureAutomationSessionProxy(const String& sessionIdentifier);
     void destroyAutomationSessionProxy();
 
@@ -476,6 +478,7 @@ private:
 #if PLATFORM(WAYLAND)
     std::unique_ptr<WaylandCompositorDisplay> m_waylandCompositorDisplay;
 #endif
+    bool m_isSuspending { false };
 };
 
 } // namespace WebKit