[webkit-changes] [238867] trunk/Source

Tue, 04 Dec 2018 11:45:27 -0800

Title: [238867] trunk/Source
Revision
238867
Author
cdu...@apple.com
Date
2018-12-04 11:44:09 -0800 (Tue, 04 Dec 2018)

Log Message

Regression(r238817) PSON Page Cache API tests are failing
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:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (238866 => 238867)


--- trunk/Source/WebCore/ChangeLog	2018-12-04 19:42:59 UTC (rev 238866)
+++ trunk/Source/WebCore/ChangeLog	2018-12-04 19:44:09 UTC (rev 238867)
@@ -1,3 +1,15 @@
+2018-12-04  Chris Dumez  <cdu...@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  <ryanhad...@apple.com>
 
         Unreviewed, rolling out r238838.

Modified: trunk/Source/WebCore/page/MemoryRelease.cpp (238866 => 238867)


--- trunk/Source/WebCore/page/MemoryRelease.cpp	2018-12-04 19:42:59 UTC (rev 238866)
+++ trunk/Source/WebCore/page/MemoryRelease.cpp	2018-12-04 19:44:09 UTC (rev 238867)
@@ -75,11 +75,13 @@
     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,7 +113,7 @@
     }
 }
 
-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));
 
@@ -118,7 +120,7 @@
     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();

Modified: trunk/Source/WebCore/page/MemoryRelease.h (238866 => 238867)


--- trunk/Source/WebCore/page/MemoryRelease.h	2018-12-04 19:42:59 UTC (rev 238866)
+++ trunk/Source/WebCore/page/MemoryRelease.h	2018-12-04 19:44:09 UTC (rev 238867)
@@ -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();

Modified: trunk/Source/WebKit/ChangeLog (238866 => 238867)


--- trunk/Source/WebKit/ChangeLog	2018-12-04 19:42:59 UTC (rev 238866)
+++ trunk/Source/WebKit/ChangeLog	2018-12-04 19:44:09 UTC (rev 238867)
@@ -1,3 +1,37 @@
+2018-12-04  Chris Dumez  <cdu...@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  <you...@apple.com>
 
         Device orientation may be wrong on page reload after crash

Modified: trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp (238866 => 238867)


--- trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp	2018-12-04 19:42:59 UTC (rev 238866)
+++ trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp	2018-12-04 19:44:09 UTC (rev 238867)
@@ -79,6 +79,9 @@
     , 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 @@
 
     m_process->send(Messages::WebProcess::UpdateActivePages(), 0);
 
+#if PLATFORM(IOS_FAMILY)
+    m_suspensionToken = nullptr;
+#endif
+
     if (auto finishedSuspendingHandler = WTFMove(m_finishedSuspendingHandler))
         finishedSuspendingHandler();
 }

Modified: trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h (238866 => 238867)


--- trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h	2018-12-04 19:42:59 UTC (rev 238866)
+++ trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h	2018-12-04 19:44:09 UTC (rev 238867)
@@ -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 @@
 
     bool m_finishedSuspending { false };
     CompletionHandler<void()> m_finishedSuspendingHandler;
+#if PLATFORM(IOS_FAMILY)
+    ProcessThrottler::BackgroundActivityToken m_suspensionToken;
+#endif
 };
 
 } // namespace WebKit

Modified: trunk/Source/WebKit/WebProcess/WebProcess.cpp (238866 => 238867)


--- trunk/Source/WebKit/WebProcess/WebProcess.cpp	2018-12-04 19:42:59 UTC (rev 238866)
+++ trunk/Source/WebKit/WebProcess/WebProcess.cpp	2018-12-04 19:44:09 UTC (rev 238867)
@@ -280,8 +280,9 @@
     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 @@
     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::actualPrepareToSuspend(ShouldAcknowledgeWhenReadyToSuspend shouldAcknowledgeWhenReadyToSuspend)
 {
+    SetForScope<bool> suspensionScope(m_isSuspending, true);
+
     if (!m_suppressMemoryPressureHandler)
         MemoryPressureHandler::singleton().releaseMemory(Critical::Yes, Synchronous::Yes);

Modified: trunk/Source/WebKit/WebProcess/WebProcess.h (238866 => 238867)


--- trunk/Source/WebKit/WebProcess/WebProcess.h	2018-12-04 19:42:59 UTC (rev 238866)
+++ trunk/Source/WebKit/WebProcess/WebProcess.h	2018-12-04 19:44:09 UTC (rev 238867)
@@ -348,6 +348,8 @@
     enum class ShouldAcknowledgeWhenReadyToSuspend { No, Yes };
     void actualPrepareToSuspend(ShouldAcknowledgeWhenReadyToSuspend);
 
+    bool hasPageRequiringPageCacheWhileSuspended() const;
+
     void ensureAutomationSessionProxy(const String& sessionIdentifier);
     void destroyAutomationSessionProxy();
 
@@ -476,6 +478,7 @@
 #if PLATFORM(WAYLAND)
     std::unique_ptr<WaylandCompositorDisplay> m_waylandCompositorDisplay;
 #endif
+    bool m_isSuspending { false };
 };
 
 } // namespace WebKit

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to