Diff
Modified: trunk/Source/WebKit/ChangeLog (244074 => 244075)
--- trunk/Source/WebKit/ChangeLog 2019-04-09 10:21:03 UTC (rev 244074)
+++ trunk/Source/WebKit/ChangeLog 2019-04-09 14:20:12 UTC (rev 244075)
@@ -1,3 +1,32 @@
+2019-04-09 Chris Dumez <cdu...@apple.com>
+
+ Loads using loadHTMLString() cause flashing when process-swapping
+ https://bugs.webkit.org/show_bug.cgi?id=196714
+ <rdar://problem/49637354>
+
+ Reviewed by Antti Koivisto.
+
+ Our logic to decide if we should construct a SuspendedPageProxy on process-swap was assuming
+ a SuspendedPageProxy is only useful for PageCache and would therefore not create one if PageCache
+ is disabled or if there is no associated WebBackForwardListItem. However, constructing a
+ SuspendedPageProxy is also useful to prevent flashing when process-swapping as we need to keep
+ displaying the layer of the previous process until there is something meaningful to show in the
+ new process.
+
+ This patch makes it so that we now construct a SuspendedPageProxy on process-swap, even if
+ PageCache is disabled or if there is no associated WebBackForwardListItem. The process in
+ question will not be useful for PageCache but it will avoid flashing. The SuspendedPageProxy's
+ process may also get used for future navigations to the same site (as demonstrated by the
+ API test) which is beneficial for performance.
+
+ * UIProcess/SuspendedPageProxy.cpp:
+ (WebKit::SuspendedPageProxy::SuspendedPageProxy):
+ * UIProcess/SuspendedPageProxy.h:
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::suspendCurrentPageIfPossible):
+ * UIProcess/WebProcessPool.cpp:
+ (WebKit::WebProcessPool::findReusableSuspendedPageProcess):
+
2019-04-08 Don Olmstead <don.olmst...@sony.com>
[CMake][WinCairo] Separate copied headers into different directories
Modified: trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp (244074 => 244075)
--- trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp 2019-04-09 10:21:03 UTC (rev 244074)
+++ trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp 2019-04-09 14:20:12 UTC (rev 244075)
@@ -78,17 +78,15 @@
}
#endif
-SuspendedPageProxy::SuspendedPageProxy(WebPageProxy& page, Ref<WebProcessProxy>&& process, WebBackForwardListItem& item, uint64_t mainFrameID)
+SuspendedPageProxy::SuspendedPageProxy(WebPageProxy& page, Ref<WebProcessProxy>&& process, uint64_t mainFrameID)
: m_page(page)
, m_process(WTFMove(process))
, m_mainFrameID(mainFrameID)
- , m_registrableDomain(URL(URL(), item.url()))
, m_suspensionTimeoutTimer(RunLoop::main(), this, &SuspendedPageProxy::suspensionTimedOut)
#if PLATFORM(IOS_FAMILY)
, m_suspensionToken(m_process->throttler().backgroundActivityToken())
#endif
{
- item.setSuspendedPage(this);
m_process->incrementSuspendedPageCount();
m_process->addMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_page.pageID(), *this);
Modified: trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h (244074 => 244075)
--- trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h 2019-04-09 10:21:03 UTC (rev 244074)
+++ trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h 2019-04-09 14:20:12 UTC (rev 244075)
@@ -41,13 +41,12 @@
class SuspendedPageProxy final: public IPC::MessageReceiver, public CanMakeWeakPtr<SuspendedPageProxy> {
WTF_MAKE_FAST_ALLOCATED;
public:
- SuspendedPageProxy(WebPageProxy&, Ref<WebProcessProxy>&&, WebBackForwardListItem&, uint64_t mainFrameID);
+ SuspendedPageProxy(WebPageProxy&, Ref<WebProcessProxy>&&, uint64_t mainFrameID);
~SuspendedPageProxy();
WebPageProxy& page() const { return m_page; }
WebProcessProxy& process() { return m_process.get(); }
uint64_t mainFrameID() const { return m_mainFrameID; }
- const WebCore::RegistrableDomain& registrableDomain() const { return m_registrableDomain; }
bool failedToSuspend() const { return m_suspensionState == SuspensionState::FailedToSuspend; }
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (244074 => 244075)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-04-09 10:21:03 UTC (rev 244074)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-04-09 14:20:12 UTC (rev 244075)
@@ -741,11 +741,6 @@
if (!mainFrameID)
return false;
- if (!m_preferences->usesPageCache()) {
- RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "suspendCurrentPageIfPossible: Not suspending current page for process pid %i because page cache is disabled", m_process->processIdentifier());
- return false;
- }
-
// If the client forced a swap then it may not be Web-compatible to suspend the previous page because other windows may have an opener link to the page.
if (processSwapRequestedByClient == ProcessSwapRequestedByClient::Yes) {
RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "suspendCurrentPageIfPossible: Not suspending current page for process pid %i because the swap was requested by the client", m_process->processIdentifier());
@@ -763,19 +758,15 @@
}
auto* fromItem = navigation.fromItem();
- if (!fromItem) {
- RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "suspendCurrentPageIfPossible: Not suspending current page for process pid %i because the navigation does not have a fromItem", m_process->processIdentifier());
- return false;
- }
// If the source and the destination back / forward list items are the same, then this is a client-side redirect. In this case,
// there is no need to suspend the previous page as there will be no way to get back to it.
- if (fromItem == m_backForwardList->currentItem()) {
+ if (fromItem && fromItem == m_backForwardList->currentItem()) {
RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "suspendCurrentPageIfPossible: Not suspending current page for process pid %i because this is a client-side redirect", m_process->processIdentifier());
return false;
}
- if (fromItem->url() != pageLoadState().url()) {
+ if (fromItem && fromItem->url() != pageLoadState().url()) {
RELEASE_LOG_ERROR_IF_ALLOWED(ProcessSwapping, "suspendCurrentPageIfPossible: Not suspending current page for process pid %i because fromItem's URL does not match the page URL.", m_process->processIdentifier());
ASSERT_NOT_REACHED();
return false;
@@ -782,10 +773,13 @@
}
RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "suspendCurrentPageIfPossible: Suspending current page for process pid %i", m_process->processIdentifier());
- auto suspendedPage = std::make_unique<SuspendedPageProxy>(*this, m_process.copyRef(), *fromItem, *mainFrameID);
+ auto suspendedPage = std::make_unique<SuspendedPageProxy>(*this, m_process.copyRef(), *mainFrameID);
- LOG(ProcessSwapping, "WebPageProxy %" PRIu64 " created suspended page %s for process pid %i, back/forward item %s" PRIu64, pageID(), suspendedPage->loggingString(), m_process->processIdentifier(), fromItem->itemID().logString());
+ LOG(ProcessSwapping, "WebPageProxy %" PRIu64 " created suspended page %s for process pid %i, back/forward item %s" PRIu64, pageID(), suspendedPage->loggingString(), m_process->processIdentifier(), fromItem ? fromItem->itemID().logString() : 0);
+ if (fromItem && m_preferences->usesPageCache())
+ fromItem->setSuspendedPage(suspendedPage.get());
+
m_process->processPool().addSuspendedPage(WTFMove(suspendedPage));
return true;
}
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (244074 => 244075)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2019-04-09 10:21:03 UTC (rev 244074)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2019-04-09 14:20:12 UTC (rev 244075)
@@ -2356,7 +2356,7 @@
RefPtr<WebProcessProxy> WebProcessPool::findReusableSuspendedPageProcess(const WebCore::RegistrableDomain& registrableDomain, WebPageProxy& page, WebsiteDataStore& dataStore)
{
auto it = m_suspendedPages.findIf([&](auto& suspendedPage) {
- return suspendedPage->registrableDomain() == registrableDomain && &suspendedPage->process().websiteDataStore() == &dataStore;
+ return suspendedPage->process().registrableDomain() == registrableDomain && &suspendedPage->process().websiteDataStore() == &dataStore;
});
if (it == m_suspendedPages.end())
return nullptr;
Modified: trunk/Tools/ChangeLog (244074 => 244075)
--- trunk/Tools/ChangeLog 2019-04-09 10:21:03 UTC (rev 244074)
+++ trunk/Tools/ChangeLog 2019-04-09 14:20:12 UTC (rev 244075)
@@ -1,3 +1,15 @@
+2019-04-09 Chris Dumez <cdu...@apple.com>
+
+ Loads using loadHTMLString() cause flashing when process-swapping
+ https://bugs.webkit.org/show_bug.cgi?id=196714
+ <rdar://problem/49637354>
+
+ Reviewed by Antti Koivisto.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
2019-04-09 Carlos Garcia Campos <cgar...@igalia.com>
Unreviewed. Fix ATK accessibility tests after r244059.
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (244074 => 244075)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2019-04-09 10:21:03 UTC (rev 244074)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2019-04-09 14:20:12 UTC (rev 244075)
@@ -2346,6 +2346,7 @@
TEST(ProcessSwap, ReuseSuspendedProcess)
{
auto processPoolConfiguration = psonProcessPoolConfiguration();
+ processPoolConfiguration.get().usesWebProcessCache = NO;
auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
@@ -2394,6 +2395,55 @@
EXPECT_EQ(applePID, [webView _webProcessIdentifier]);
}
+TEST(ProcessSwap, ReuseSuspendedProcessLoadHTMLString)
+{
+ auto processPoolConfiguration = psonProcessPoolConfiguration();
+ processPoolConfiguration.get().usesWebProcessCache = NO;
+ auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+
+ auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+ [webViewConfiguration setProcessPool:processPool.get()];
+ auto handler = adoptNS([[PSONScheme alloc] init]);
+ [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
+
+ auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+ auto delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
+ [webView setNavigationDelegate:delegate.get()];
+
+ NSString *htmlString = @"<html><body>TEST</body></html>";
+ [webView loadHTMLString:htmlString baseURL:[NSURL URLWithString:@"pson://www.webkit.org/main1.html"]];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto webkitPID = [webView _webProcessIdentifier];
+
+ [webView loadHTMLString:htmlString baseURL:[NSURL URLWithString:@"pson://www.apple.com/main1.html"]];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto applePID = [webView _webProcessIdentifier];
+
+ EXPECT_NE(webkitPID, applePID);
+
+ [webView loadHTMLString:htmlString baseURL:[NSURL URLWithString:@"pson://www.webkit.org/main2.html"]];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ // We should have gone back to the webkit.org process for this load since we reuse SuspendedPages' process when possible.
+ EXPECT_EQ(webkitPID, [webView _webProcessIdentifier]);
+
+ [webView loadHTMLString:htmlString baseURL:[NSURL URLWithString:@"pson://www.apple.com/main2.html"]];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ // We should have gone back to the apple.com process for this load since we reuse SuspendedPages' process when possible.
+ EXPECT_EQ(applePID, [webView _webProcessIdentifier]);
+}
+
static const char* failsToEnterPageCacheTestBytes = R"PSONRESOURCE(
<body>
<script>
