Diff
Modified: trunk/Source/WebKit/ChangeLog (237466 => 237467)
--- trunk/Source/WebKit/ChangeLog 2018-10-26 17:47:10 UTC (rev 237466)
+++ trunk/Source/WebKit/ChangeLog 2018-10-26 18:01:39 UTC (rev 237467)
@@ -1,3 +1,41 @@
+2018-10-26 Chris Dumez <[email protected]>
+
+ [PSON] Avoid tearing down the drawing area when suspending a WebPage due to process-swap
+ https://bugs.webkit.org/show_bug.cgi?id=190879
+
+ Reviewed by Antti Koivisto.
+
+ Avoid tearing down the drawing area when suspending a WebPage due to process-swap. We really only need to reset
+ the drawing area upon resuming the WebPage. There is no strict need to destroy the drawing area on suspension
+ and this has caused various crashes because code usually assumes we always have a drawing area.
+
+ This patch also drops various drawing area null checks that were added to address PSON crashes.
+
+ * UIProcess/SuspendedPageProxy.cpp:
+ (WebKit::SuspendedPageProxy::tearDownDrawingAreaInWebProcess): Deleted.
+ * UIProcess/SuspendedPageProxy.h:
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::suspendCurrentPageIfPossible):
+ (WebKit::WebPageProxy::receivedNavigationPolicyDecision):
+ (WebKit::WebPageProxy::didCompletePageTransition):
+ (WebKit::WebPageProxy::enterAcceleratedCompositingMode):
+ * UIProcess/WebPageProxy.h:
+ * WebProcess/WebCoreSupport/WebChromeClient.cpp:
+ (WebKit::WebChromeClient::invalidateContentsAndRootView):
+ (WebKit::WebChromeClient::invalidateContentsForSlowScroll):
+ (WebKit::WebChromeClient::contentsSizeChanged const):
+ * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
+ (WebKit::WebFrameLoaderClient::transitionToCommittedForNewPage):
+ * WebProcess/WebPage/RemoteLayerTree/RemoteScrollingCoordinator.mm:
+ (WebKit::RemoteScrollingCoordinator::scheduleTreeStateCommit):
+ * WebProcess/WebPage/WebPage.cpp:
+ (WebKit::WebPage::reinitializeWebPage):
+ (WebKit::WebPage::exitAcceleratedCompositingMode):
+ (WebKit::WebPage::setIsSuspended):
+ (WebKit::WebPage::tearDownDrawingAreaForSuspend): Deleted.
+ * WebProcess/WebPage/WebPage.h:
+ * WebProcess/WebPage/WebPage.messages.in:
+
2018-10-26 Antti Koivisto <[email protected]>
Use random() instead of begin() to limit cache sizes
Modified: trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp (237466 => 237467)
--- trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp 2018-10-26 17:47:10 UTC (rev 237466)
+++ trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp 2018-10-26 18:01:39 UTC (rev 237467)
@@ -103,11 +103,6 @@
});
}
-void SuspendedPageProxy::tearDownDrawingAreaInWebProcess()
-{
- m_process->send(Messages::WebPage::TearDownDrawingAreaForSuspend(), m_page.pageID());
-}
-
void SuspendedPageProxy::unsuspend()
{
ASSERT(m_isSuspended);
Modified: trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h (237466 => 237467)
--- trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h 2018-10-26 17:47:10 UTC (rev 237466)
+++ trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h 2018-10-26 18:01:39 UTC (rev 237467)
@@ -46,8 +46,6 @@
uint64_t mainFrameID() const { return m_mainFrameID; }
const WebCore::SecurityOriginData& origin() const { return m_origin; }
- void tearDownDrawingAreaInWebProcess();
-
void unsuspend();
#if !LOG_DISABLED
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (237466 => 237467)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2018-10-26 17:47:10 UTC (rev 237466)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2018-10-26 18:01:39 UTC (rev 237467)
@@ -724,10 +724,14 @@
}
auto suspendedPage = std::make_unique<SuspendedPageProxy>(*this, m_process.copyRef(), *currentItem, *mainFrameID);
+
+#if PLATFORM(MAC)
m_pageSuspendedDueToCurrentNavigation = makeWeakPtr(suspendedPage.get());
+#endif
+
+ LOG(ProcessSwapping, "WebPageProxy %" PRIu64 " created suspended page %s for process pid %i, back/forward item %s" PRIu64, pageID(), suspendedPage->loggingString(), m_process->processIdentifier(), currentItem->itemID().logString());
+
m_process->processPool().addSuspendedPageProxy(WTFMove(suspendedPage));
-
- LOG(ProcessSwapping, "WebPageProxy %" PRIu64 " created suspended page %s for process pid %i, back/forward item %s" PRIu64, pageID(), m_pageSuspendedDueToCurrentNavigation->loggingString(), m_process->processIdentifier(), currentItem->itemID().logString());
}
void WebPageProxy::swapToWebProcess(Ref<WebProcessProxy>&& process, API::Navigation& navigation, std::optional<uint64_t> mainFrameIDInPreviousProcess)
@@ -2491,9 +2495,11 @@
}
if (policyAction == PolicyAction::Use && frame.isMainFrame()) {
+#if PLATFORM(MAC)
// We're about to navigate so we need to reset m_pageSuspendedDueToCurrentNavigation. If we decide to process swap,
// continueNavigationInNewProcess() will take care of updating m_pageSuspendedDueToCurrentNavigation as needed.
m_pageSuspendedDueToCurrentNavigation = nullptr;
+#endif
String reason;
auto proposedProcess = process().processPool().processForNavigation(*this, *navigation, processSwapRequestedByClient, policyAction, reason);
@@ -3516,11 +3522,11 @@
void WebPageProxy::didCompletePageTransition(bool isInitialEmptyDocument)
{
+#if PLATFORM(MAC)
// Attach drawing area for the initial empty document only if this is not a process swap.
if (isInitialEmptyDocument && m_pageSuspendedDueToCurrentNavigation)
return;
-#if PLATFORM(MAC)
if (!m_drawingArea)
return;
@@ -3527,8 +3533,7 @@
// Drawing area for the suspended page will be torn down when the attach completes.
m_drawingArea->attachInWebProcess();
#else
- if (m_pageSuspendedDueToCurrentNavigation)
- m_pageSuspendedDueToCurrentNavigation->tearDownDrawingAreaInWebProcess();
+ UNUSED_PARAM(isInitialEmptyDocument);
#endif
}
@@ -6425,10 +6430,6 @@
void WebPageProxy::enterAcceleratedCompositingMode(const LayerTreeContext& layerTreeContext)
{
pageClient().enterAcceleratedCompositingMode(layerTreeContext);
-
- // We have completed the page transition and can tear down the layers in the suspended process.
- if (m_pageSuspendedDueToCurrentNavigation)
- m_pageSuspendedDueToCurrentNavigation->tearDownDrawingAreaInWebProcess();
}
void WebPageProxy::exitAcceleratedCompositingMode()
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.h (237466 => 237467)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.h 2018-10-26 17:47:10 UTC (rev 237466)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.h 2018-10-26 18:01:39 UTC (rev 237467)
@@ -2259,8 +2259,10 @@
std::optional<MonotonicTime> m_pageLoadStart;
+#if PLATFORM(MAC)
// FIXME: We should try and get rid of this data member.
WeakPtr<SuspendedPageProxy> m_pageSuspendedDueToCurrentNavigation;
+#endif
RunLoop::Timer<WebPageProxy> m_resetRecentCrashCountTimer;
unsigned m_recentCrashCount { 0 };
Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp (237466 => 237467)
--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp 2018-10-26 17:47:10 UTC (rev 237466)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp 2018-10-26 18:01:39 UTC (rev 237467)
@@ -523,8 +523,7 @@
return;
}
- if (auto* drawingArea = m_page.drawingArea())
- drawingArea->setNeedsDisplayInRect(rect);
+ m_page.drawingArea()->setNeedsDisplayInRect(rect);
}
void WebChromeClient::invalidateContentsForSlowScroll(const IntRect& rect)
@@ -542,8 +541,7 @@
return;
}
#endif
- if (auto* drawingArea = m_page.drawingArea())
- drawingArea->setNeedsDisplayInRect(rect);
+ m_page.drawingArea()->setNeedsDisplayInRect(rect);
}
void WebChromeClient::scroll(const IntSize& scrollDelta, const IntRect& scrollRect, const IntRect& clipRect)
@@ -597,8 +595,7 @@
m_page.send(Messages::WebPageProxy::DidChangeContentSize(size));
- if (auto* drawingArea = m_page.drawingArea())
- drawingArea->mainFrameContentSizeChanged(size);
+ m_page.drawingArea()->mainFrameContentSizeChanged(size);
if (frameView && !frameView->delegatesScrolling()) {
bool hasHorizontalScrollbar = frameView->horizontalScrollbar();
Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp (237466 => 237467)
--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp 2018-10-26 17:47:10 UTC (rev 237466)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp 2018-10-26 18:01:39 UTC (rev 237467)
@@ -1435,8 +1435,7 @@
m_frame->coreFrame()->view()->setProhibitsScrolling(shouldDisableScrolling);
m_frame->coreFrame()->view()->setVisualUpdatesAllowedByClient(!webPage->shouldExtendIncrementalRenderingSuppression());
#if PLATFORM(COCOA)
- if (webPage->drawingArea())
- m_frame->coreFrame()->view()->setViewExposedRect(webPage->drawingArea()->viewExposedRect());
+ m_frame->coreFrame()->view()->setViewExposedRect(webPage->drawingArea()->viewExposedRect());
#endif
#if PLATFORM(IOS_FAMILY)
m_frame->coreFrame()->view()->setDelegatesScrolling(true);
Modified: trunk/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteScrollingCoordinator.mm (237466 => 237467)
--- trunk/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteScrollingCoordinator.mm 2018-10-26 17:47:10 UTC (rev 237466)
+++ trunk/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteScrollingCoordinator.mm 2018-10-26 18:01:39 UTC (rev 237467)
@@ -61,8 +61,7 @@
void RemoteScrollingCoordinator::scheduleTreeStateCommit()
{
- if (auto* drawingArea = m_webPage->drawingArea())
- drawingArea->scheduleCompositingLayerFlush();
+ m_webPage->drawingArea()->scheduleCompositingLayerFlush();
}
bool RemoteScrollingCoordinator::coordinatesScrollingForFrameView(const FrameView& frameView) const
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (237466 => 237467)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2018-10-26 17:47:10 UTC (rev 237466)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2018-10-26 18:01:39 UTC (rev 237467)
@@ -653,7 +653,14 @@
void WebPage::reinitializeWebPage(WebPageCreationParameters&& parameters)
{
- if (!m_drawingArea) {
+ ASSERT(m_drawingArea);
+
+ if (m_shouldResetDrawingArea) {
+ // Make sure we destroy the previous drawing area before constructing the new one as DrawingArea registers / unregisters
+ // itself as an IPC::MesssageReceiver in its constructor / destructor.
+ m_drawingArea = nullptr;
+ m_shouldResetDrawingArea = false;
+
m_drawingArea = DrawingArea::create(*this, parameters);
m_drawingArea->setPaintingEnabled(false);
m_drawingArea->setShouldScaleViewToFitDocument(parameters.shouldScaleViewToFitDocument);
@@ -1155,8 +1162,7 @@
void WebPage::exitAcceleratedCompositingMode()
{
- if (m_drawingArea)
- m_drawingArea->setRootCompositingLayer(0);
+ m_drawingArea->setRootCompositingLayer(0);
}
void WebPage::close()
@@ -6128,16 +6134,12 @@
m_isSuspended = suspended;
+ if (!m_isSuspended)
+ m_shouldResetDrawingArea = true;
+
setLayerTreeStateIsFrozen(true);
}
-void WebPage::tearDownDrawingAreaForSuspend()
-{
- if (!m_isSuspended)
- return;
- m_drawingArea = nullptr;
-}
-
void WebPage::frameBecameRemote(uint64_t frameID, GlobalFrameIdentifier&& remoteFrameIdentifier, GlobalWindowIdentifier&& remoteWindowIdentifier)
{
RefPtr<WebFrame> frame = WebProcess::singleton().webFrame(frameID);
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.h (237466 => 237467)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.h 2018-10-26 17:47:10 UTC (rev 237466)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.h 2018-10-26 18:01:39 UTC (rev 237467)
@@ -1424,7 +1424,6 @@
void urlSchemeTaskDidComplete(uint64_t handlerIdentifier, uint64_t taskIdentifier, const WebCore::ResourceError&);
void setIsSuspended(bool);
- void tearDownDrawingAreaForSuspend();
RefPtr<WebImage> snapshotAtSize(const WebCore::IntRect&, const WebCore::IntSize& bitmapSize, SnapshotOptions);
RefPtr<WebImage> snapshotNode(WebCore::Node&, SnapshotOptions, unsigned maximumPixelCount = std::numeric_limits<unsigned>::max());
@@ -1449,6 +1448,7 @@
WebCore::IntSize m_viewSize;
std::unique_ptr<DrawingArea> m_drawingArea;
+ bool m_shouldResetDrawingArea { false };
HashSet<PluginView*> m_pluginViews;
bool m_hasSeenPlugin { false };
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.messages.in (237466 => 237467)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.messages.in 2018-10-26 17:47:10 UTC (rev 237466)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.messages.in 2018-10-26 18:01:39 UTC (rev 237467)
@@ -512,7 +512,6 @@
URLSchemeTaskDidComplete(uint64_t handlerIdentifier, uint64_t taskIdentifier, WebCore::ResourceError error)
SetIsSuspended(bool suspended)
- TearDownDrawingAreaForSuspend();
#if ENABLE(RESOURCE_LOAD_STATISTICS)
StorageAccessResponse(bool wasGranted, uint64_t contextId)
