Rename webProcessDidCrashWithReason callback to webProcessDidTerminate and stop calling webProcessDidCrash for client terminations
https://bugs.webkit.org/show_bug.cgi?id=171624
Reviewed by Dan Bernstein.
Source/WebKit2:
Follow-up fixes after r216129 based on feedback I have received:
- Rename webProcessDidCrashWithReason callback function to webProcessDidTerminate given that this is called
for non-crashes (e.g. terminations requested by the client).
- Rename WKProcessCrashReason / ProcessCrashReason to WKProcessTerminationReason / ProcessTerminationReason
for consistency with the new naming.
- Stop calling processDidCrash / webProcessDidCrash for terminations requested by the client, to maintain
pre-r216129 behavior. Those are not crashes (The client used an API such as WKPageTerminateProcess()).
webProcessDidTerminate will still be called though.
- Fix a bug where - for terminations due to resource limits - WebPageProxy::processDidCrash() was getting
called twice: First by WebProcessProxy::requestTermination() with reason "RequestedByClient" then a
second time by WebProcessProxy::terminateProcessDueToResourceLimits() with the proper reason.
* Shared/ProcessTerminationReason.h: Renamed from Source/WebKit2/Shared/ProcessCrashReason.h.
* UIProcess/API/APINavigationClient.h:
(API::NavigationClient::processDidTerminate):
* UIProcess/API/C/WKAPICast.h:
(WebKit::toAPI):
* UIProcess/API/C/WKPage.cpp:
(WKPageTerminate):
(WKPageSetPageNavigationClient):
* UIProcess/API/C/WKPageNavigationClient.h:
* UIProcess/API/C/WKProcessTerminationReason.h: Renamed from Source/WebKit2/UIProcess/API/C/WKProcessCrashReason.h.
* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _killWebContentProcessAndResetState]):
* UIProcess/Cocoa/NavigationState.h:
* UIProcess/Cocoa/NavigationState.mm:
(WebKit::NavigationState::NavigationClient::processDidTerminate):
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::processDidTerminate):
* UIProcess/WebPageProxy.h:
* UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::didClose):
(WebKit::WebProcessProxy::requestTermination):
(WebKit::WebProcessProxy::logDiagnosticMessageForResourceLimitTermination):
(WebKit::WebProcessProxy::didExceedActiveMemoryLimit):
(WebKit::WebProcessProxy::didExceedInactiveMemoryLimit):
(WebKit::WebProcessProxy::didExceedBackgroundCPULimit):
* UIProcess/WebProcessProxy.h:
* WebKit2.xcodeproj/project.pbxproj:
Tools:
Extend API test coverage to cover crashes in addition to terminations requested by the client.
* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/Tests/WebKit2/ProcessDidTerminate.cpp: Renamed from Tools/TestWebKitAPI/Tests/WebKit2/ProcessDidCrashWithReason.cpp.
(TestWebKitAPI::webProcessWasTerminatedByClient):
(TestWebKitAPI::webProcessCrashed):
(TestWebKitAPI::TEST):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@216249 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebKit2/ChangeLog b/Source/WebKit2/ChangeLog
index e51b620..c503ac9 100644
--- a/Source/WebKit2/ChangeLog
+++ b/Source/WebKit2/ChangeLog
@@ -1,3 +1,50 @@
+2017-05-05 Chris Dumez <cdumez@apple.com>
+
+ Rename webProcessDidCrashWithReason callback to webProcessDidTerminate and stop calling webProcessDidCrash for client terminations
+ https://bugs.webkit.org/show_bug.cgi?id=171624
+
+ Reviewed by Dan Bernstein.
+
+ Follow-up fixes after r216129 based on feedback I have received:
+ - Rename webProcessDidCrashWithReason callback function to webProcessDidTerminate given that this is called
+ for non-crashes (e.g. terminations requested by the client).
+ - Rename WKProcessCrashReason / ProcessCrashReason to WKProcessTerminationReason / ProcessTerminationReason
+ for consistency with the new naming.
+ - Stop calling processDidCrash / webProcessDidCrash for terminations requested by the client, to maintain
+ pre-r216129 behavior. Those are not crashes (The client used an API such as WKPageTerminateProcess()).
+ webProcessDidTerminate will still be called though.
+ - Fix a bug where - for terminations due to resource limits - WebPageProxy::processDidCrash() was getting
+ called twice: First by WebProcessProxy::requestTermination() with reason "RequestedByClient" then a
+ second time by WebProcessProxy::terminateProcessDueToResourceLimits() with the proper reason.
+
+ * Shared/ProcessTerminationReason.h: Renamed from Source/WebKit2/Shared/ProcessCrashReason.h.
+ * UIProcess/API/APINavigationClient.h:
+ (API::NavigationClient::processDidTerminate):
+ * UIProcess/API/C/WKAPICast.h:
+ (WebKit::toAPI):
+ * UIProcess/API/C/WKPage.cpp:
+ (WKPageTerminate):
+ (WKPageSetPageNavigationClient):
+ * UIProcess/API/C/WKPageNavigationClient.h:
+ * UIProcess/API/C/WKProcessTerminationReason.h: Renamed from Source/WebKit2/UIProcess/API/C/WKProcessCrashReason.h.
+ * UIProcess/API/Cocoa/WKWebView.mm:
+ (-[WKWebView _killWebContentProcessAndResetState]):
+ * UIProcess/Cocoa/NavigationState.h:
+ * UIProcess/Cocoa/NavigationState.mm:
+ (WebKit::NavigationState::NavigationClient::processDidTerminate):
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::processDidTerminate):
+ * UIProcess/WebPageProxy.h:
+ * UIProcess/WebProcessProxy.cpp:
+ (WebKit::WebProcessProxy::didClose):
+ (WebKit::WebProcessProxy::requestTermination):
+ (WebKit::WebProcessProxy::logDiagnosticMessageForResourceLimitTermination):
+ (WebKit::WebProcessProxy::didExceedActiveMemoryLimit):
+ (WebKit::WebProcessProxy::didExceedInactiveMemoryLimit):
+ (WebKit::WebProcessProxy::didExceedBackgroundCPULimit):
+ * UIProcess/WebProcessProxy.h:
+ * WebKit2.xcodeproj/project.pbxproj:
+
2017-05-05 Derek Schuff <dschuff@chromium.org>
Mac cmake buildfix after r216037
diff --git a/Source/WebKit2/Shared/ProcessCrashReason.h b/Source/WebKit2/Shared/ProcessTerminationReason.h
similarity index 90%
rename from Source/WebKit2/Shared/ProcessCrashReason.h
rename to Source/WebKit2/Shared/ProcessTerminationReason.h
index 3417f70..6a1f492 100644
--- a/Source/WebKit2/Shared/ProcessCrashReason.h
+++ b/Source/WebKit2/Shared/ProcessTerminationReason.h
@@ -27,11 +27,11 @@
namespace WebKit {
-enum class ProcessCrashReason {
- TerminationDueToMemoryUsage,
- TerminationDueToCPUUsage,
- TerminationRequestedByClient,
- Other
+enum class ProcessTerminationReason {
+ ExceededMemoryLimit,
+ ExceededCPULimit,
+ RequestedByClient,
+ Crash
};
}
diff --git a/Source/WebKit2/UIProcess/API/APINavigationClient.h b/Source/WebKit2/UIProcess/API/APINavigationClient.h
index 8a3e1a7..e459487 100644
--- a/Source/WebKit2/UIProcess/API/APINavigationClient.h
+++ b/Source/WebKit2/UIProcess/API/APINavigationClient.h
@@ -27,7 +27,7 @@
#include "APIData.h"
#include "PluginModuleInfo.h"
-#include "ProcessCrashReason.h"
+#include "ProcessTerminationReason.h"
#include "SameDocumentNavigationType.h"
#include "WebEvent.h"
#include "WebFramePolicyListenerProxy.h"
@@ -83,7 +83,7 @@
virtual void didReceiveAuthenticationChallenge(WebKit::WebPageProxy&, WebKit::AuthenticationChallengeProxy*) { }
// FIXME: These function should not be part of this client.
- virtual void processDidCrash(WebKit::WebPageProxy&, WebKit::ProcessCrashReason) { }
+ virtual void processDidTerminate(WebKit::WebPageProxy&, WebKit::ProcessTerminationReason) { }
virtual void processDidBecomeResponsive(WebKit::WebPageProxy&) { }
virtual void processDidBecomeUnresponsive(WebKit::WebPageProxy&) { }
diff --git a/Source/WebKit2/UIProcess/API/C/WKAPICast.h b/Source/WebKit2/UIProcess/API/C/WKAPICast.h
index d94a9e9..1b66ab366 100644
--- a/Source/WebKit2/UIProcess/API/C/WKAPICast.h
+++ b/Source/WebKit2/UIProcess/API/C/WKAPICast.h
@@ -32,7 +32,7 @@
#include "HTTPCookieAcceptPolicy.h"
#include "InjectedBundleHitTestResultMediaType.h"
#include "PluginModuleInfo.h"
-#include "ProcessCrashReason.h"
+#include "ProcessTerminationReason.h"
#include "ResourceCachesToClear.h"
#include "WKBundleHitTestResult.h"
#include "WKContext.h"
@@ -41,7 +41,7 @@
#include "WKPage.h"
#include "WKPreferencesRef.h"
#include "WKPreferencesRefPrivate.h"
-#include "WKProcessCrashReason.h"
+#include "WKProcessTerminationReason.h"
#include "WKProtectionSpaceTypes.h"
#include "WKResourceCacheManager.h"
#include "WKSharedAPICast.h"
@@ -235,20 +235,20 @@
return kWKCacheModelDocumentViewer;
}
-inline WKProcessCrashReason toAPI(ProcessCrashReason reason)
+inline WKProcessTerminationReason toAPI(ProcessTerminationReason reason)
{
switch (reason) {
- case ProcessCrashReason::TerminationDueToMemoryUsage:
- return kWKProcessCrashReasonTerminationDueToMemoryUsage;
- case ProcessCrashReason::TerminationDueToCPUUsage:
- return kWKProcessCrashReasonTerminationDueToCPUUsage;
- case ProcessCrashReason::TerminationRequestedByClient:
- return kWKProcessCrashReasonTerminationRequestedByClient;
- case ProcessCrashReason::Other:
- return kWKProcessCrashReasonOther;
+ case ProcessTerminationReason::ExceededMemoryLimit:
+ return kWKProcessTerminationReasonExceededMemoryLimit;
+ case ProcessTerminationReason::ExceededCPULimit:
+ return kWKProcessTerminationReasonExceededCPULimit;
+ case ProcessTerminationReason::RequestedByClient:
+ return kWKProcessTerminationReasonRequestedByClient;
+ case ProcessTerminationReason::Crash:
+ return kWKProcessTerminationReasonCrash;
}
- return kWKProcessCrashReasonOther;
+ return kWKProcessTerminationReasonCrash;
}
inline FontSmoothingLevel toFontSmoothingLevel(WKFontSmoothingLevel wkLevel)
diff --git a/Source/WebKit2/UIProcess/API/C/WKPage.cpp b/Source/WebKit2/UIProcess/API/C/WKPage.cpp
index a880d3f..06222aa 100644
--- a/Source/WebKit2/UIProcess/API/C/WKPage.cpp
+++ b/Source/WebKit2/UIProcess/API/C/WKPage.cpp
@@ -422,7 +422,7 @@
void WKPageTerminate(WKPageRef pageRef)
{
- toImpl(pageRef)->terminateProcess();
+ toImpl(pageRef)->process().requestTermination(ProcessTerminationReason::RequestedByClient);
}
WKStringRef WKPageGetSessionHistoryURLValueType()
@@ -2400,13 +2400,15 @@
m_client.didReceiveAuthenticationChallenge(toAPI(&page), toAPI(authenticationChallenge), m_client.base.clientInfo);
}
- void processDidCrash(WebPageProxy& page, WebKit::ProcessCrashReason reason) override
+ void processDidTerminate(WebPageProxy& page, WebKit::ProcessTerminationReason reason) override
{
- if (m_client.webProcessDidCrash)
- m_client.webProcessDidCrash(toAPI(&page), m_client.base.clientInfo);
+ if (m_client.webProcessDidTerminate) {
+ m_client.webProcessDidTerminate(toAPI(&page), toAPI(reason), m_client.base.clientInfo);
+ return;
+ }
- if (m_client.webProcessDidCrashWithReason)
- m_client.webProcessDidCrashWithReason(toAPI(&page), toAPI(reason), m_client.base.clientInfo);
+ if (m_client.webProcessDidCrash && reason != WebKit::ProcessTerminationReason::RequestedByClient)
+ m_client.webProcessDidCrash(toAPI(&page), m_client.base.clientInfo);
}
RefPtr<API::Data> webCryptoMasterKey(WebPageProxy& page) override
diff --git a/Source/WebKit2/UIProcess/API/C/WKPageNavigationClient.h b/Source/WebKit2/UIProcess/API/C/WKPageNavigationClient.h
index d71d4eb..ed1548f 100644
--- a/Source/WebKit2/UIProcess/API/C/WKPageNavigationClient.h
+++ b/Source/WebKit2/UIProcess/API/C/WKPageNavigationClient.h
@@ -30,7 +30,7 @@
#include <WebKit/WKPageLoadTypes.h>
#include <WebKit/WKPageRenderingProgressEvents.h>
#include <WebKit/WKPluginLoadPolicy.h>
-#include <WebKit/WKProcessCrashReason.h>
+#include <WebKit/WKProcessTerminationReason.h>
#ifdef __cplusplus
extern "C" {
@@ -66,7 +66,7 @@
typedef void (*WKPageNavigationWebProcessDidCrashCallback)(WKPageRef page, const void* clientInfo);
-typedef void (*WKPageNavigationWebProcessDidCrashWithReasonCallback)(WKPageRef page, WKProcessCrashReason reason, const void* clientInfo);
+typedef void (*WKPageNavigationWebProcessDidTerminateCallback)(WKPageRef page, WKProcessTerminationReason reason, const void* clientInfo);
typedef WKDataRef (*WKPageNavigationCopyWebCryptoMasterKeyCallback)(WKPageRef page, const void* clientInfo);
@@ -140,7 +140,7 @@
WKPageNavigationDidRemoveNavigationGestureSnapshot didRemoveNavigationGestureSnapshot;
// Version 1.
- WKPageNavigationWebProcessDidCrashWithReasonCallback webProcessDidCrashWithReason;
+ WKPageNavigationWebProcessDidTerminateCallback webProcessDidTerminate;
} WKPageNavigationClientV1;
#ifdef __cplusplus
diff --git a/Source/WebKit2/UIProcess/API/C/WKProcessCrashReason.h b/Source/WebKit2/UIProcess/API/C/WKProcessTerminationReason.h
similarity index 85%
rename from Source/WebKit2/UIProcess/API/C/WKProcessCrashReason.h
rename to Source/WebKit2/UIProcess/API/C/WKProcessTerminationReason.h
index b476203..620e85a 100644
--- a/Source/WebKit2/UIProcess/API/C/WKProcessCrashReason.h
+++ b/Source/WebKit2/UIProcess/API/C/WKProcessTerminationReason.h
@@ -32,12 +32,12 @@
#endif
enum {
- kWKProcessCrashReasonTerminationDueToMemoryUsage = 0,
- kWKProcessCrashReasonTerminationDueToCPUUsage,
- kWKProcessCrashReasonTerminationRequestedByClient,
- kWKProcessCrashReasonOther,
+ kWKProcessTerminationReasonExceededMemoryLimit = 0,
+ kWKProcessTerminationReasonExceededCPULimit,
+ kWKProcessTerminationReasonRequestedByClient,
+ kWKProcessTerminationReasonCrash,
};
-typedef uint32_t WKProcessCrashReason;
+typedef uint32_t WKProcessTerminationReason;
#ifdef __cplusplus
}
diff --git a/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm b/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm
index 9519246..921e08f 100644
--- a/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm
+++ b/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm
@@ -3772,7 +3772,7 @@
- (void)_killWebContentProcessAndResetState
{
- _page->terminateProcess();
+ _page->process().requestTermination(WebKit::ProcessTerminationReason::RequestedByClient);
}
#if PLATFORM(MAC)
diff --git a/Source/WebKit2/UIProcess/Cocoa/NavigationState.h b/Source/WebKit2/UIProcess/Cocoa/NavigationState.h
index 0ad3478..7a9c3c5 100644
--- a/Source/WebKit2/UIProcess/Cocoa/NavigationState.h
+++ b/Source/WebKit2/UIProcess/Cocoa/NavigationState.h
@@ -32,7 +32,7 @@
#import "APIHistoryClient.h"
#import "APINavigationClient.h"
#import "PageLoadState.h"
-#import "ProcessCrashReason.h"
+#import "ProcessTerminationReason.h"
#import "ProcessThrottler.h"
#import "WeakObjCPtr.h"
#import <wtf/RetainPtr.h>
@@ -103,7 +103,7 @@
bool canAuthenticateAgainstProtectionSpace(WebPageProxy&, WebProtectionSpace*) override;
void didReceiveAuthenticationChallenge(WebPageProxy&, AuthenticationChallengeProxy*) override;
- void processDidCrash(WebPageProxy&, ProcessCrashReason) override;
+ void processDidTerminate(WebPageProxy&, ProcessTerminationReason) override;
void processDidBecomeResponsive(WebPageProxy&) override;
void processDidBecomeUnresponsive(WebPageProxy&) override;
diff --git a/Source/WebKit2/UIProcess/Cocoa/NavigationState.mm b/Source/WebKit2/UIProcess/Cocoa/NavigationState.mm
index a459829..027db50 100644
--- a/Source/WebKit2/UIProcess/Cocoa/NavigationState.mm
+++ b/Source/WebKit2/UIProcess/Cocoa/NavigationState.mm
@@ -710,7 +710,7 @@
[static_cast<id <WKNavigationDelegatePrivate>>(navigationDelegate.get()) _webView:m_navigationState.m_webView didReceiveAuthenticationChallenge:wrapper(*authenticationChallenge)];
}
-void NavigationState::NavigationClient::processDidCrash(WebKit::WebPageProxy& page, WebKit::ProcessCrashReason)
+void NavigationState::NavigationClient::processDidTerminate(WebKit::WebPageProxy& page, WebKit::ProcessTerminationReason)
{
if (!m_navigationState.m_navigationDelegateMethods.webViewWebContentProcessDidTerminate && !m_navigationState.m_navigationDelegateMethods.webViewWebProcessDidCrash)
return;
diff --git a/Source/WebKit2/UIProcess/WebPageProxy.cpp b/Source/WebKit2/UIProcess/WebPageProxy.cpp
index e9bd686..d386f88 100644
--- a/Source/WebKit2/UIProcess/WebPageProxy.cpp
+++ b/Source/WebKit2/UIProcess/WebPageProxy.cpp
@@ -2410,16 +2410,6 @@
m_process->send(Messages::WebPage::SetCustomTextEncodingName(encodingName), m_pageID);
}
-void WebPageProxy::terminateProcess()
-{
- // NOTE: This uses a check of m_isValid rather than calling isValid() since
- // we want this to run even for pages being closed or that already closed.
- if (!m_isValid)
- return;
-
- m_process->requestTermination();
-}
-
SessionState WebPageProxy::sessionState(const std::function<bool (WebBackForwardListItem&)>& filter) const
{
SessionState sessionState;
@@ -5320,7 +5310,7 @@
m_pageLoadState.didChangeProcessIsResponsive();
}
-void WebPageProxy::processDidCrash(ProcessCrashReason reason)
+void WebPageProxy::processDidTerminate(ProcessTerminationReason reason)
{
ASSERT(m_isValid);
@@ -5343,8 +5333,8 @@
navigationState().clearAllNavigations();
if (m_navigationClient)
- m_navigationClient->processDidCrash(*this, reason);
- else
+ m_navigationClient->processDidTerminate(*this, reason);
+ else if (reason != ProcessTerminationReason::RequestedByClient)
m_loaderClient->processDidCrash(*this);
if (m_controlledByAutomation) {
diff --git a/Source/WebKit2/UIProcess/WebPageProxy.h b/Source/WebKit2/UIProcess/WebPageProxy.h
index 4c8ee58..dd93d02 100644
--- a/Source/WebKit2/UIProcess/WebPageProxy.h
+++ b/Source/WebKit2/UIProcess/WebPageProxy.h
@@ -40,7 +40,7 @@
#include "MessageSender.h"
#include "NotificationPermissionRequestManagerProxy.h"
#include "PageLoadState.h"
-#include "ProcessCrashReason.h"
+#include "ProcessTerminationReason.h"
#include "ProcessThrottler.h"
#include "SandboxExtension.h"
#include "ShareableBitmap.h"
@@ -687,8 +687,6 @@
double estimatedProgress() const;
- void terminateProcess();
-
SessionState sessionState(const std::function<bool (WebBackForwardListItem&)>& = nullptr) const;
RefPtr<API::Navigation> restoreFromSessionState(SessionState, bool navigate);
@@ -859,7 +857,7 @@
void processDidBecomeUnresponsive();
void processDidBecomeResponsive();
- void processDidCrash(ProcessCrashReason);
+ void processDidTerminate(ProcessTerminationReason);
void willChangeProcessIsResponsive();
void didChangeProcessIsResponsive();
diff --git a/Source/WebKit2/UIProcess/WebProcessProxy.cpp b/Source/WebKit2/UIProcess/WebProcessProxy.cpp
index 95b74cc..830135c 100644
--- a/Source/WebKit2/UIProcess/WebProcessProxy.cpp
+++ b/Source/WebKit2/UIProcess/WebProcessProxy.cpp
@@ -600,7 +600,7 @@
shutDown();
for (size_t i = 0, size = pages.size(); i < size; ++i)
- pages[i]->processDidCrash(ProcessCrashReason::Other);
+ pages[i]->processDidTerminate(ProcessTerminationReason::Crash);
}
@@ -847,7 +847,7 @@
});
}
-void WebProcessProxy::requestTermination()
+void WebProcessProxy::requestTermination(ProcessTerminationReason reason)
{
if (state() == State::Terminated)
return;
@@ -863,7 +863,7 @@
shutDown();
for (size_t i = 0, size = pages.size(); i < size; ++i)
- pages[i]->processDidCrash(ProcessCrashReason::TerminationRequestedByClient);
+ pages[i]->processDidTerminate(reason);
}
void WebProcessProxy::enableSuddenTermination()
@@ -1127,59 +1127,24 @@
m_backgroundResponsivenessTimer.processTerminated();
}
-static Vector<RefPtr<WebPageProxy>> pagesCopy(WTF::IteratorRange<WebProcessProxy::WebPageProxyMap::const_iterator::Values> pages)
+void WebProcessProxy::logDiagnosticMessageForResourceLimitTermination(const String& limitKey)
{
- Vector<RefPtr<WebPageProxy>> vector;
- for (auto& page : pages)
- vector.append(page);
- return vector;
-}
-
-static String diagnosticLoggingKeyForTerminationReason(TerminationReason reason)
-{
- switch (reason) {
- case TerminationReason::ExceededActiveMemoryLimit:
- return DiagnosticLoggingKeys::exceededActiveMemoryLimitKey();
- case TerminationReason::ExceededInactiveMemoryLimit:
- return DiagnosticLoggingKeys::exceededInactiveMemoryLimitKey();
- case TerminationReason::ExceededBackgroundCPULimit:
- return DiagnosticLoggingKeys::exceededBackgroundCPULimitKey();
- default:
- RELEASE_ASSERT_NOT_REACHED();
- }
-}
-
-static ProcessCrashReason toProcessCrashReason(TerminationReason reason)
-{
- switch (reason) {
- case TerminationReason::ExceededActiveMemoryLimit:
- case TerminationReason::ExceededInactiveMemoryLimit:
- return ProcessCrashReason::TerminationDueToMemoryUsage;
- case TerminationReason::ExceededBackgroundCPULimit:
- return ProcessCrashReason::TerminationDueToCPUUsage;
- }
- return ProcessCrashReason::Other;
-}
-
-void WebProcessProxy::terminateProcessDueToResourceLimits(TerminationReason reason)
-{
- for (auto& page : pagesCopy(pages())) {
- page->logDiagnosticMessage(DiagnosticLoggingKeys::simulatedPageCrashKey(), diagnosticLoggingKeyForTerminationReason(reason), ShouldSample::No);
- page->terminateProcess();
- page->processDidCrash(toProcessCrashReason(reason));
- }
+ if (pageCount())
+ (*pages().begin())->logDiagnosticMessage(DiagnosticLoggingKeys::simulatedPageCrashKey(), limitKey, ShouldSample::No);
}
void WebProcessProxy::didExceedActiveMemoryLimit()
{
RELEASE_LOG_ERROR(PerformanceLogging, "%p - WebProcessProxy::didExceedActiveMemoryLimit() Terminating WebProcess that has exceeded the active memory limit", this);
- terminateProcessDueToResourceLimits(TerminationReason::ExceededActiveMemoryLimit);
+ logDiagnosticMessageForResourceLimitTermination(DiagnosticLoggingKeys::exceededActiveMemoryLimitKey());
+ requestTermination(ProcessTerminationReason::ExceededMemoryLimit);
}
void WebProcessProxy::didExceedInactiveMemoryLimit()
{
RELEASE_LOG_ERROR(PerformanceLogging, "%p - WebProcessProxy::didExceedInactiveMemoryLimit() Terminating WebProcess that has exceeded the inactive memory limit", this);
- terminateProcessDueToResourceLimits(TerminationReason::ExceededInactiveMemoryLimit);
+ logDiagnosticMessageForResourceLimitTermination(DiagnosticLoggingKeys::exceededInactiveMemoryLimitKey());
+ requestTermination(ProcessTerminationReason::ExceededMemoryLimit);
}
void WebProcessProxy::didExceedBackgroundCPULimit()
@@ -1200,7 +1165,8 @@
}
RELEASE_LOG_ERROR(PerformanceLogging, "%p - WebProcessProxy::didExceedBackgroundCPULimit() Terminating background WebProcess that has exceeded the background CPU limit", this);
- terminateProcessDueToResourceLimits(TerminationReason::ExceededBackgroundCPULimit);
+ logDiagnosticMessageForResourceLimitTermination(DiagnosticLoggingKeys::exceededBackgroundCPULimitKey());
+ requestTermination(ProcessTerminationReason::ExceededCPULimit);
}
void WebProcessProxy::updateBackgroundResponsivenessTimer()
diff --git a/Source/WebKit2/UIProcess/WebProcessProxy.h b/Source/WebKit2/UIProcess/WebProcessProxy.h
index 154a6fd..bc78d23 100644
--- a/Source/WebKit2/UIProcess/WebProcessProxy.h
+++ b/Source/WebKit2/UIProcess/WebProcessProxy.h
@@ -55,12 +55,6 @@
namespace WebKit {
-enum class TerminationReason {
- ExceededActiveMemoryLimit,
- ExceededInactiveMemoryLimit,
- ExceededBackgroundCPULimit,
-};
-
class NetworkProcessProxy;
class UserMediaCaptureManagerProxy;
class WebBackForwardListItem;
@@ -142,7 +136,7 @@
void disableSuddenTermination();
bool isSuddenTerminationEnabled() { return !m_numberOfTimesSuddenTerminationWasDisabled; }
- void requestTermination();
+ void requestTermination(ProcessTerminationReason);
RefPtr<API::Object> transformHandlesToObjects(API::Object*);
static RefPtr<API::Object> transformObjectsToHandles(API::Object*);
@@ -247,7 +241,7 @@
bool canTerminateChildProcess();
- void terminateProcessDueToResourceLimits(TerminationReason);
+ void logDiagnosticMessageForResourceLimitTermination(const String& limitKey);
ResponsivenessTimer m_responsivenessTimer;
BackgroundProcessResponsivenessTimer m_backgroundResponsivenessTimer;
diff --git a/Source/WebKit2/WebKit2.xcodeproj/project.pbxproj b/Source/WebKit2/WebKit2.xcodeproj/project.pbxproj
index d53b99a..c4f6186f 100644
--- a/Source/WebKit2/WebKit2.xcodeproj/project.pbxproj
+++ b/Source/WebKit2/WebKit2.xcodeproj/project.pbxproj
@@ -898,8 +898,8 @@
41FAF5F81E3C1021001AE678 /* LibWebRTCResolver.h in Headers */ = {isa = PBXBuildFile; fileRef = 41FAF5F61E3C0B47001AE678 /* LibWebRTCResolver.h */; };
41FAF5F91E3C1025001AE678 /* LibWebRTCResolver.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 41FAF5F71E3C0B47001AE678 /* LibWebRTCResolver.cpp */; };
4450AEC01DC3FAE5009943F2 /* SharedMemoryCocoa.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4450AEBF1DC3FAE5009943F2 /* SharedMemoryCocoa.cpp */; };
- 463FD4801EB9459600A2982C /* WKProcessCrashReason.h in Headers */ = {isa = PBXBuildFile; fileRef = 463FD47F1EB9458400A2982C /* WKProcessCrashReason.h */; settings = {ATTRIBUTES = (Private, ); }; };
- 463FD4821EB94EC000A2982C /* ProcessCrashReason.h in Headers */ = {isa = PBXBuildFile; fileRef = 463FD4811EB94EAD00A2982C /* ProcessCrashReason.h */; };
+ 463FD4801EB9459600A2982C /* WKProcessTerminationReason.h in Headers */ = {isa = PBXBuildFile; fileRef = 463FD47F1EB9458400A2982C /* WKProcessTerminationReason.h */; settings = {ATTRIBUTES = (Private, ); }; };
+ 463FD4821EB94EC000A2982C /* ProcessTerminationReason.h in Headers */ = {isa = PBXBuildFile; fileRef = 463FD4811EB94EAD00A2982C /* ProcessTerminationReason.h */; };
46A2B6081E5676A600C3DEDA /* BackgroundProcessResponsivenessTimer.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 46A2B6061E5675A200C3DEDA /* BackgroundProcessResponsivenessTimer.cpp */; };
46A2B6091E5676A600C3DEDA /* BackgroundProcessResponsivenessTimer.h in Headers */ = {isa = PBXBuildFile; fileRef = 46A2B6071E5675A200C3DEDA /* BackgroundProcessResponsivenessTimer.h */; };
4A3CC18A19B063E700D14AEF /* UserMediaPermissionRequestManagerProxy.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4A410F3919AF7B04002EBAB5 /* UserMediaPermissionRequestManagerProxy.cpp */; };
@@ -3120,8 +3120,8 @@
41FAF5F61E3C0B47001AE678 /* LibWebRTCResolver.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = LibWebRTCResolver.h; path = Network/webrtc/LibWebRTCResolver.h; sourceTree = "<group>"; };
41FAF5F71E3C0B47001AE678 /* LibWebRTCResolver.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = LibWebRTCResolver.cpp; path = Network/webrtc/LibWebRTCResolver.cpp; sourceTree = "<group>"; };
4450AEBF1DC3FAE5009943F2 /* SharedMemoryCocoa.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = SharedMemoryCocoa.cpp; path = cocoa/SharedMemoryCocoa.cpp; sourceTree = "<group>"; };
- 463FD47F1EB9458400A2982C /* WKProcessCrashReason.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WKProcessCrashReason.h; sourceTree = "<group>"; };
- 463FD4811EB94EAD00A2982C /* ProcessCrashReason.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ProcessCrashReason.h; sourceTree = "<group>"; };
+ 463FD47F1EB9458400A2982C /* WKProcessTerminationReason.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WKProcessTerminationReason.h; sourceTree = "<group>"; };
+ 463FD4811EB94EAD00A2982C /* ProcessTerminationReason.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ProcessTerminationReason.h; sourceTree = "<group>"; };
46A2B6061E5675A200C3DEDA /* BackgroundProcessResponsivenessTimer.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = BackgroundProcessResponsivenessTimer.cpp; sourceTree = "<group>"; };
46A2B6071E5675A200C3DEDA /* BackgroundProcessResponsivenessTimer.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = BackgroundProcessResponsivenessTimer.h; sourceTree = "<group>"; };
4A410F3519AF7AC3002EBAB5 /* WKUserMediaPermissionRequest.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = WKUserMediaPermissionRequest.cpp; sourceTree = "<group>"; };
@@ -4932,7 +4932,7 @@
BCC43AB9127B95DC00317F16 /* PlatformPopupMenuData.h */,
E18C92F312DB9E7100CF2AEB /* PrintInfo.cpp */,
E1CC1B8E12D7EADF00625838 /* PrintInfo.h */,
- 463FD4811EB94EAD00A2982C /* ProcessCrashReason.h */,
+ 463FD4811EB94EAD00A2982C /* ProcessTerminationReason.h */,
F6A0C13F13281E6E0070430F /* ResourceCachesToClear.h */,
410482CB1DDD2FB500F006D0 /* RTCNetwork.cpp */,
410482CC1DDD2FB500F006D0 /* RTCNetwork.h */,
@@ -7013,7 +7013,7 @@
BCD597CF112B56AC00EC8C23 /* WKPreferences.cpp */,
BCD597CE112B56AC00EC8C23 /* WKPreferencesRef.h */,
762B7484120BBA2D00819339 /* WKPreferencesRefPrivate.h */,
- 463FD47F1EB9458400A2982C /* WKProcessCrashReason.h */,
+ 463FD47F1EB9458400A2982C /* WKProcessTerminationReason.h */,
512F58F312A88A5400629530 /* WKProtectionSpace.cpp */,
512F58F412A88A5400629530 /* WKProtectionSpace.h */,
518ACAE912AEE6BB00B04B83 /* WKProtectionSpaceTypes.h */,
@@ -8516,7 +8516,7 @@
1AC1338618590C4600F3EC05 /* RemoteObjectRegistryMessages.h in Headers */,
0F594790187B3B3A00437857 /* RemoteScrollingCoordinator.h in Headers */,
0F5947A8187B517600437857 /* RemoteScrollingCoordinatorMessages.h in Headers */,
- 463FD4821EB94EC000A2982C /* ProcessCrashReason.h in Headers */,
+ 463FD4821EB94EC000A2982C /* ProcessTerminationReason.h in Headers */,
0F59479B187B3B6000437857 /* RemoteScrollingCoordinatorProxy.h in Headers */,
0F5947A4187B3B7D00437857 /* RemoteScrollingCoordinatorTransaction.h in Headers */,
0F59479D187B3B6000437857 /* RemoteScrollingTree.h in Headers */,
@@ -8673,7 +8673,7 @@
1C8E2A361277852400BC7BD0 /* WebInspectorMessages.h in Headers */,
1C8E28341275D73800BC7BD0 /* WebInspectorProxy.h in Headers */,
1CA8B946127C882A00576C2B /* WebInspectorProxyMessages.h in Headers */,
- 463FD4801EB9459600A2982C /* WKProcessCrashReason.h in Headers */,
+ 463FD4801EB9459600A2982C /* WKProcessTerminationReason.h in Headers */,
1C891D6619B124FF00BA79DD /* WebInspectorUI.h in Headers */,
1CBBE4A119B66C53006B7D81 /* WebInspectorUIMessages.h in Headers */,
A55BA82B1BA38E61007CD33D /* WebInspectorUtilities.h in Headers */,
diff --git a/Tools/ChangeLog b/Tools/ChangeLog
index 833271f..f891d31 100644
--- a/Tools/ChangeLog
+++ b/Tools/ChangeLog
@@ -1,3 +1,18 @@
+2017-05-05 Chris Dumez <cdumez@apple.com>
+
+ Rename webProcessDidCrashWithReason callback to webProcessDidTerminate and stop calling webProcessDidCrash for client terminations
+ https://bugs.webkit.org/show_bug.cgi?id=171624
+
+ Reviewed by Dan Bernstein.
+
+ Extend API test coverage to cover crashes in addition to terminations requested by the client.
+
+ * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+ * TestWebKitAPI/Tests/WebKit2/ProcessDidTerminate.cpp: Renamed from Tools/TestWebKitAPI/Tests/WebKit2/ProcessDidCrashWithReason.cpp.
+ (TestWebKitAPI::webProcessWasTerminatedByClient):
+ (TestWebKitAPI::webProcessCrashed):
+ (TestWebKitAPI::TEST):
+
2017-05-04 Mark Lam <mark.lam@apple.com>
DRT's setAudioResultCallback() and IDBRequest::setResult() need to acquire the JSLock.
diff --git a/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj b/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
index 40cb4d0..c35abf8 100644
--- a/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
+++ b/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
@@ -128,7 +128,7 @@
3FBD1B4A1D3D66AB00E6D6FA /* FullscreenLayoutConstraints.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 3FBD1B491D39D1DB00E6D6FA /* FullscreenLayoutConstraints.html */; };
448D7E471EA6C55500ECC756 /* EnvironmentUtilitiesTest.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 448D7E451EA6C55500ECC756 /* EnvironmentUtilitiesTest.cpp */; };
46397B951DC2C850009A78AE /* DOMNode.mm in Sources */ = {isa = PBXBuildFile; fileRef = 46397B941DC2C850009A78AE /* DOMNode.mm */; };
- 4647B1261EBA3B850041D7EF /* ProcessDidCrashWithReason.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4647B1251EBA3B730041D7EF /* ProcessDidCrashWithReason.cpp */; };
+ 4647B1261EBA3B850041D7EF /* ProcessDidTerminate.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4647B1251EBA3B730041D7EF /* ProcessDidTerminate.cpp */; };
46C519DA1D355AB200DAA51A /* LocalStorageNullEntries.mm in Sources */ = {isa = PBXBuildFile; fileRef = 46C519D81D355A7300DAA51A /* LocalStorageNullEntries.mm */; };
46C519E61D3563FD00DAA51A /* LocalStorageNullEntries.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 46C519E21D35629600DAA51A /* LocalStorageNullEntries.html */; };
46C519E71D3563FD00DAA51A /* LocalStorageNullEntries.localstorage in Copy Resources */ = {isa = PBXBuildFile; fileRef = 46C519E31D35629600DAA51A /* LocalStorageNullEntries.localstorage */; };
@@ -1067,7 +1067,7 @@
448D7E451EA6C55500ECC756 /* EnvironmentUtilitiesTest.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = EnvironmentUtilitiesTest.cpp; sourceTree = "<group>"; };
44A622C114A0E2B60048515B /* WTFStringUtilities.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WTFStringUtilities.h; sourceTree = "<group>"; };
46397B941DC2C850009A78AE /* DOMNode.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = DOMNode.mm; sourceTree = "<group>"; };
- 4647B1251EBA3B730041D7EF /* ProcessDidCrashWithReason.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ProcessDidCrashWithReason.cpp; sourceTree = "<group>"; };
+ 4647B1251EBA3B730041D7EF /* ProcessDidTerminate.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ProcessDidTerminate.cpp; sourceTree = "<group>"; };
46C519D81D355A7300DAA51A /* LocalStorageNullEntries.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = LocalStorageNullEntries.mm; sourceTree = "<group>"; };
46C519E21D35629600DAA51A /* LocalStorageNullEntries.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = LocalStorageNullEntries.html; sourceTree = "<group>"; };
46C519E31D35629600DAA51A /* LocalStorageNullEntries.localstorage */ = {isa = PBXFileReference; lastKnownFileType = file; path = LocalStorageNullEntries.localstorage; sourceTree = "<group>"; };
@@ -2119,7 +2119,7 @@
0766DD1F1A5AD5200023E3BB /* PendingAPIRequestURL.cpp */,
333B9CE11277F23100FEFCE3 /* PreventEmptyUserAgent.cpp */,
F6FDDDD214241AD4004F1729 /* PrivateBrowsingPushStateNoHistoryCallback.cpp */,
- 4647B1251EBA3B730041D7EF /* ProcessDidCrashWithReason.cpp */,
+ 4647B1251EBA3B730041D7EF /* ProcessDidTerminate.cpp */,
8A3AF93A16C9ED2700D248C1 /* ReloadPageAfterCrash.cpp */,
2DD7D3A9178205D00026E1E3 /* ResizeReversePaginatedWebView.cpp */,
8A2C750D16CED9550024F352 /* ResizeWindowAfterCrash.cpp */,
@@ -2863,7 +2863,7 @@
7CCE7EEE1A411AE600447C4C /* DownloadDecideDestinationCrash.cpp in Sources */,
F4BFA68E1E4AD08000154298 /* DragAndDropPasteboardTests.mm in Sources */,
A155022A1E05020B00A24C57 /* DuplicateCompletionHandlerCalls.mm in Sources */,
- 4647B1261EBA3B850041D7EF /* ProcessDidCrashWithReason.cpp in Sources */,
+ 4647B1261EBA3B850041D7EF /* ProcessDidTerminate.cpp in Sources */,
7CCE7EBE1A411A7E00447C4C /* DynamicDeviceScaleFactor.mm in Sources */,
5C0BF8921DD599B600B00328 /* EarlyKVOCrash.mm in Sources */,
7CCE7EE01A411A9A00447C4C /* EditorCommands.mm in Sources */,
diff --git a/Tools/TestWebKitAPI/Tests/WebKit2/ProcessDidCrashWithReason.cpp b/Tools/TestWebKitAPI/Tests/WebKit2/ProcessDidTerminate.cpp
similarity index 61%
rename from Tools/TestWebKitAPI/Tests/WebKit2/ProcessDidCrashWithReason.cpp
rename to Tools/TestWebKitAPI/Tests/WebKit2/ProcessDidTerminate.cpp
index e7aea5e..bbe46a9 100644
--- a/Tools/TestWebKitAPI/Tests/WebKit2/ProcessDidCrashWithReason.cpp
+++ b/Tools/TestWebKitAPI/Tests/WebKit2/ProcessDidTerminate.cpp
@@ -30,11 +30,14 @@
#include "PlatformUtilities.h"
#include "PlatformWebView.h"
#include "Test.h"
+#include <WebKit/WKPagePrivate.h>
#include <WebKit/WKRetainPtr.h>
+#include <signal.h>
namespace TestWebKitAPI {
static bool loadBeforeCrash = false;
+static bool clientTerminationHandlerCalled = false;
static bool crashHandlerCalled = false;
static void didFinishNavigation(WKPageRef page, WKNavigationRef navigation, WKTypeRef userData, const void* clientInfo)
@@ -47,7 +50,20 @@
EXPECT_TRUE(false);
}
-static void didCrashWithReason(WKPageRef page, WKProcessCrashReason reason, const void* clientInfo)
+static void webProcessWasTerminatedByClient(WKPageRef page, WKProcessTerminationReason reason, const void* clientInfo)
+{
+ // Test if first load actually worked.
+ EXPECT_TRUE(loadBeforeCrash);
+
+ // Should only be called once.
+ EXPECT_FALSE(clientTerminationHandlerCalled);
+
+ EXPECT_EQ(kWKProcessTerminationReasonRequestedByClient, reason);
+
+ clientTerminationHandlerCalled = true;
+}
+
+static void webProcessCrashed(WKPageRef page, WKProcessTerminationReason reason, const void* clientInfo)
{
// Test if first load actually worked.
EXPECT_TRUE(loadBeforeCrash);
@@ -55,12 +71,12 @@
// Should only be called once.
EXPECT_FALSE(crashHandlerCalled);
- EXPECT_EQ(kWKProcessCrashReasonTerminationRequestedByClient, reason);
+ EXPECT_EQ(kWKProcessTerminationReasonCrash, reason);
crashHandlerCalled = true;
}
-TEST(WebKit2, ProcessDidCrashWithReasonRequestedByClient)
+TEST(WebKit2, ProcessDidTerminateRequestedByClient)
{
WKRetainPtr<WKContextRef> context(AdoptWK, WKContextCreate());
PlatformWebView webView(context.get());
@@ -69,16 +85,43 @@
memset(&navigationClient, 0, sizeof(navigationClient));
navigationClient.base.version = 1;
navigationClient.didFinishNavigation = didFinishNavigation;
- navigationClient.webProcessDidCrashWithReason = didCrashWithReason;
+ navigationClient.webProcessDidTerminate = webProcessWasTerminatedByClient;
WKPageSetPageNavigationClient(webView.page(), &navigationClient.base);
+ loadBeforeCrash = false;
WKRetainPtr<WKURLRef> url = adoptWK(WKURLCreateWithUTF8CString("about:blank"));
// Load a blank page and next kills WebProcess.
WKPageLoadURL(webView.page(), url.get());
Util::run(&loadBeforeCrash);
+
WKPageTerminate(webView.page());
+ Util::run(&clientTerminationHandlerCalled);
+}
+
+TEST(WebKit2, ProcessDidTerminateWithReasonCrash)
+{
+ WKRetainPtr<WKContextRef> context(AdoptWK, WKContextCreate());
+ PlatformWebView webView(context.get());
+
+ WKPageNavigationClientV1 navigationClient;
+ memset(&navigationClient, 0, sizeof(navigationClient));
+ navigationClient.base.version = 1;
+ navigationClient.didFinishNavigation = didFinishNavigation;
+ navigationClient.webProcessDidTerminate = webProcessCrashed;
+
+ WKPageSetPageNavigationClient(webView.page(), &navigationClient.base);
+
+ loadBeforeCrash = false;
+ WKRetainPtr<WKURLRef> url = adoptWK(WKURLCreateWithUTF8CString("about:blank"));
+ // Load a blank page and next kills WebProcess.
+ WKPageLoadURL(webView.page(), url.get());
+ Util::run(&loadBeforeCrash);
+
+ // Simulate a crash by killing the WebProcess.
+ kill(WKPageGetProcessIdentifier(webView.page()), SIGKILL);
+
Util::run(&crashHandlerCalled);
}
