Diff
Modified: trunk/Source/WebKit/ChangeLog (235986 => 235987)
--- trunk/Source/WebKit/ChangeLog 2018-09-13 20:11:01 UTC (rev 235986)
+++ trunk/Source/WebKit/ChangeLog 2018-09-13 20:44:43 UTC (rev 235987)
@@ -1,5 +1,21 @@
2018-09-13 Chris Dumez <[email protected]>
+ Add release logging to help debug PSON issues
+ https://bugs.webkit.org/show_bug.cgi?id=189562
+
+ Reviewed by Ryosuke Niwa.
+
+ Add release logging to help debug issues related to process swap on navigation.
+
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::decidePolicyForNavigationAction):
+ * UIProcess/WebProcessPool.cpp:
+ (WebKit::WebProcessPool::processForNavigation):
+ (WebKit::WebProcessPool::processForNavigationInternal):
+ * UIProcess/WebProcessPool.h:
+
+2018-09-13 Chris Dumez <[email protected]>
+
Add null check for drawing area in WebPage::didCompletePageTransition() after r235867
https://bugs.webkit.org/show_bug.cgi?id=189587
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (235986 => 235987)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2018-09-13 20:11:01 UTC (rev 235986)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2018-09-13 20:44:43 UTC (rev 235987)
@@ -4040,15 +4040,19 @@
}
if (policyAction == PolicyAction::Use && frame->isMainFrame()) {
- auto proposedProcess = process().processPool().processForNavigation(*this, *navigation, processSwapRequestedByClient, policyAction);
+ String reason;
+ auto proposedProcess = process().processPool().processForNavigation(*this, *navigation, processSwapRequestedByClient, policyAction, reason);
+ ASSERT(!reason.isNull());
if (proposedProcess.ptr() != &process()) {
+ RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "%p - WebPageProxy::decidePolicyForNavigationAction, swapping process %i with process %i for navigation, reason: %{public}s", this, processIdentifier(), proposedProcess->processIdentifier(), reason.utf8().data());
LOG(ProcessSwapping, "(ProcessSwapping) Switching from process %i to new process (%i) for navigation %" PRIu64 " '%s'", processIdentifier(), proposedProcess->processIdentifier(), navigation->navigationID(), navigation->loggingString());
RunLoop::main().dispatch([this, protectedThis = WTFMove(protectedThis), navigation = makeRef(*navigation), proposedProcess = WTFMove(proposedProcess)]() mutable {
continueNavigationInNewProcess(navigation.get(), WTFMove(proposedProcess));
});
- }
+ } else
+ RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "%p - WebPageProxy::decidePolicyForNavigationAction, keep using process %i for navigation, reason: %{public}s", this, processIdentifier(), reason.utf8().data());
}
receivedPolicyDecision(policyAction, navigation.get(), WTFMove(data), WTFMove(sender));
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (235986 => 235987)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2018-09-13 20:11:01 UTC (rev 235986)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2018-09-13 20:44:43 UTC (rev 235987)
@@ -2102,9 +2102,9 @@
m_swappedProcessesPerRegistrableDomain.remove(registrableDomain);
}
-Ref<WebProcessProxy> WebProcessPool::processForNavigation(WebPageProxy& page, const API::Navigation& navigation, ProcessSwapRequestedByClient processSwapRequestedByClient, PolicyAction& action)
+Ref<WebProcessProxy> WebProcessPool::processForNavigation(WebPageProxy& page, const API::Navigation& navigation, ProcessSwapRequestedByClient processSwapRequestedByClient, PolicyAction& action, String& reason)
{
- auto process = processForNavigationInternal(page, navigation, processSwapRequestedByClient, action);
+ auto process = processForNavigationInternal(page, navigation, processSwapRequestedByClient, action, reason);
if (m_configuration->alwaysKeepAndReuseSwappedProcesses() && process.ptr() != &page.process()) {
static std::once_flag onceFlag;
@@ -2120,40 +2120,56 @@
return process;
}
-Ref<WebProcessProxy> WebProcessPool::processForNavigationInternal(WebPageProxy& page, const API::Navigation& navigation, ProcessSwapRequestedByClient processSwapRequestedByClient, PolicyAction& action)
+Ref<WebProcessProxy> WebProcessPool::processForNavigationInternal(WebPageProxy& page, const API::Navigation& navigation, ProcessSwapRequestedByClient processSwapRequestedByClient, PolicyAction& action, String& reason)
{
- if (!m_configuration->processSwapsOnNavigation() && processSwapRequestedByClient == ProcessSwapRequestedByClient::No)
+ if (!m_configuration->processSwapsOnNavigation() && processSwapRequestedByClient == ProcessSwapRequestedByClient::No) {
+ reason = "Feature is disabled"_s;
return page.process();
+ }
- if (page.inspectorFrontendCount() > 0)
+ if (page.inspectorFrontendCount() > 0) {
+ reason = "A Web Inspector frontend is connected"_s;
return page.process();
+ }
- if (m_automationSession)
+ if (m_automationSession) {
+ reason = "An automation session is active"_s;
return page.process();
+ }
- if (!page.process().hasCommittedAnyProvisionalLoads())
+ if (!page.process().hasCommittedAnyProvisionalLoads()) {
+ reason = "Process has not yet committed any provisional loads"_s;
return page.process();
+ }
if (navigation.isCrossOriginWindowOpenNavigation()) {
- if (navigation.opener() && !m_configuration->processSwapsOnWindowOpenWithOpener())
+ if (navigation.opener() && !m_configuration->processSwapsOnWindowOpenWithOpener()) {
+ reason = "Browsing context has an opener"_s;
return page.process();
+ }
+ reason = "Initial navigation is cross-site in a newly opened window"_s;
action = ""
return createNewWebProcess(page.websiteDataStore());
}
// FIXME: We should support process swap when a window has an opener.
- if (navigation.opener())
+ if (navigation.opener()) {
+ reason = "Browsing context has an opener"_s;
return page.process();
+ }
// FIXME: We should support process swap when a window has opened other windows via window.open.
- if (navigation.hasOpenedFrames())
+ if (navigation.hasOpenedFrames()) {
+ reason = "Browsing context has opened other windows"_s;
return page.process();
+ }
if (auto* backForwardListItem = navigation.targetItem()) {
if (auto* suspendedPage = backForwardListItem->suspendedPage()) {
ASSERT(suspendedPage->process());
action = ""
+ reason = "Using target back/forward item's process"_s;
return *suspendedPage->process();
}
@@ -2160,20 +2176,28 @@
// If the target back/forward item and the current back/forward item originated
// in the same WebProcess then we should reuse the current WebProcess.
if (auto* fromItem = navigation.fromItem()) {
- if (fromItem->itemID().processIdentifier == backForwardListItem->itemID().processIdentifier)
+ if (fromItem->itemID().processIdentifier == backForwardListItem->itemID().processIdentifier) {
+ reason = "Source and target back/forward item originated in the same process"_s;
return page.process();
+ }
}
}
auto targetURL = navigation.currentRequest().url();
if (processSwapRequestedByClient == ProcessSwapRequestedByClient::No) {
- if (navigation.treatAsSameOriginNavigation())
+ if (navigation.treatAsSameOriginNavigation()) {
+ reason = "The treatAsSameOriginNavigation flag is set"_s;
return page.process();
+ }
auto url = "" { ParsedURLString, page.pageLoadState().url() };
- if (!url.isValid() || !targetURL.isValid() || url.isEmpty() || url.isBlankURL() || registrableDomainsAreEqual(url, targetURL))
+ if (!url.isValid() || !targetURL.isValid() || url.isEmpty() || url.isBlankURL() || registrableDomainsAreEqual(url, targetURL)) {
+ reason = "Navigation is same-site"_s;
return page.process();
- }
+ }
+ reason = "Navigation is cross-site"_s;
+ } else
+ reason = "Process swap was requested by the client"_s;
if (m_configuration->alwaysKeepAndReuseSwappedProcesses()) {
auto registrableDomain = toRegistrableDomain(targetURL);
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.h (235986 => 235987)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.h 2018-09-13 20:11:01 UTC (rev 235986)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.h 2018-09-13 20:44:43 UTC (rev 235987)
@@ -447,7 +447,7 @@
BackgroundWebProcessToken backgroundWebProcessToken() const { return BackgroundWebProcessToken(m_backgroundWebProcessCounter.count()); }
#endif
- Ref<WebProcessProxy> processForNavigation(WebPageProxy&, const API::Navigation&, ProcessSwapRequestedByClient, WebCore::PolicyAction&);
+ Ref<WebProcessProxy> processForNavigation(WebPageProxy&, const API::Navigation&, ProcessSwapRequestedByClient, WebCore::PolicyAction&, String& reason);
void registerSuspendedPageProxy(SuspendedPageProxy&);
void unregisterSuspendedPageProxy(SuspendedPageProxy&);
void didReachGoodTimeToPrewarm();
@@ -467,7 +467,7 @@
void platformInitializeWebProcess(WebProcessCreationParameters&);
void platformInvalidateContext();
- Ref<WebProcessProxy> processForNavigationInternal(WebPageProxy&, const API::Navigation&, ProcessSwapRequestedByClient, WebCore::PolicyAction&);
+ Ref<WebProcessProxy> processForNavigationInternal(WebPageProxy&, const API::Navigation&, ProcessSwapRequestedByClient, WebCore::PolicyAction&, String& reason);
RefPtr<WebProcessProxy> tryTakePrewarmedProcess(WebsiteDataStore&);
