Attachment 356455 Details for Bug 192348 – Patch

[patch] Patch

bug-192348-20181203210416.patch (text/plain), 10.18 KB, created by Chris Dumez on 2018-12-03 21:04:18 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2018-12-03 21:04:18 PST

Size: 10.18 KB

patch

obsolete

>Subversion Revision: 238817
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index de7f5756093d8b3bb35324656c248abf7ddafaf5..8b8f20f7b6dd486ed1c8bebad2d8d6065787ab42 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,15 @@
>+2018-12-03  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 NOBODY (OOPS!).
>+
>+        * page/MemoryRelease.cpp:
>+        (WebCore::releaseCriticalMemory):
>+        (WebCore::releaseMemory):
>+        * page/MemoryRelease.h:
>+
> 2018-12-03  Zalan Bujtas  <zalan@apple.com>
> 
>         [iOS] Unreviewed build fix.
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 4906483351a01cbd0671a94157901b22c9f28d98..900be53e543c893690735e0ab96757780dbf53f2 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,37 @@
>+2018-12-03  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 NOBODY (OOPS!).
>+
>+        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-03  Alex Christensen  <achristensen@webkit.org>
> 
>         Update process assertions after swapping processes
>diff --git a/Source/WebCore/page/MemoryRelease.cpp b/Source/WebCore/page/MemoryRelease.cpp
>index 887596315257ad672dbf60080694a88fe6ee8a7c..08f04a46dcbcfd422d3983fb0a3b55f3837a093b 100644
>--- a/Source/WebCore/page/MemoryRelease.cpp
>+++ b/Source/WebCore/page/MemoryRelease.cpp
>@@ -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();
>diff --git a/Source/WebCore/page/MemoryRelease.h b/Source/WebCore/page/MemoryRelease.h
>index a372a5dc6c291f747bd8c5ac17b6975421f41825..f9c4a68e067a8f1e15e22889dc9be27af304fcdd 100644
>--- a/Source/WebCore/page/MemoryRelease.h
>+++ b/Source/WebCore/page/MemoryRelease.h
>@@ -29,7 +29,9 @@
> 
> namespace WebCore {
> 
>-WEBCORE_EXPORT void releaseMemory(Critical, Synchronous);
>+enum class MaintainPageCache { No, Yes };
>+
>+WEBCORE_EXPORT void releaseMemory(Critical, Synchronous, MaintainPageCache = MaintainPageCache::No);
> void platformReleaseMemory(Critical);
> void jettisonExpensiveObjectsOnTopLevelNavigation();
> WEBCORE_EXPORT void registerMemoryReleaseNotifyCallbacks();
>diff --git a/Source/WebKit/UIProcess/SuspendedPageProxy.cpp b/Source/WebKit/UIProcess/SuspendedPageProxy.cpp
>index 1497361393d532853dd1df8666c8ab5ba70e9836..0fea9a2b920b170d7ca76ca1f25ad210aa4690fe 100644
>--- a/Source/WebKit/UIProcess/SuspendedPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/SuspendedPageProxy.cpp
>@@ -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();
> }
>diff --git a/Source/WebKit/UIProcess/SuspendedPageProxy.h b/Source/WebKit/UIProcess/SuspendedPageProxy.h
>index 1fdeece2c35f58d2d6a931bbe88f178e4db33d39..f31d8ba2a5ed7775d51a568f534881e7941a3a33 100644
>--- a/Source/WebKit/UIProcess/SuspendedPageProxy.h
>+++ b/Source/WebKit/UIProcess/SuspendedPageProxy.h
>@@ -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
>diff --git a/Source/WebKit/WebProcess/WebProcess.cpp b/Source/WebKit/WebProcess/WebProcess.cpp
>index 83222740e253dfbced6b3474a0731d6b3fba0b4a..2831aaa3b379d3319a297fda4cfcb410eb376154 100644
>--- a/Source/WebKit/WebProcess/WebProcess.cpp
>+++ b/Source/WebKit/WebProcess/WebProcess.cpp
>@@ -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);
> 
>diff --git a/Source/WebKit/WebProcess/WebProcess.h b/Source/WebKit/WebProcess/WebProcess.h
>index fb78e63a2756e22b5b39aee1c4af5e95bbfc6cf0..9172ee1c7a631127e392102a9b523cc7f3e692ae 100644
>--- a/Source/WebKit/WebProcess/WebProcess.h
>+++ b/Source/WebKit/WebProcess/WebProcess.h
>@@ -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

Actions: View | Formatted Diff | Diff

Attachments on bug 192348: 356455 | 356516 | 356518