Rename webProcessDidCrashWithReason callback to webProcessDidTerminate and stop calli...
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 5 May 2017 18:11:46 +0000 (18:11 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 5 May 2017 18:11:46 +0000 (18:11 +0000)
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: https://svn.webkit.org/repository/webkit/trunk@216249 268f45cc-cd09-0410-ab3c-d52691b4dbfc

18 files changed:
Source/WebKit2/ChangeLog
Source/WebKit2/Shared/ProcessTerminationReason.h [moved from Source/WebKit2/Shared/ProcessCrashReason.h with 90% similarity]
Source/WebKit2/UIProcess/API/APINavigationClient.h
Source/WebKit2/UIProcess/API/C/WKAPICast.h
Source/WebKit2/UIProcess/API/C/WKPage.cpp
Source/WebKit2/UIProcess/API/C/WKPageNavigationClient.h
Source/WebKit2/UIProcess/API/C/WKProcessTerminationReason.h [moved from Source/WebKit2/UIProcess/API/C/WKProcessCrashReason.h with 86% similarity]
Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm
Source/WebKit2/UIProcess/Cocoa/NavigationState.h
Source/WebKit2/UIProcess/Cocoa/NavigationState.mm
Source/WebKit2/UIProcess/WebPageProxy.cpp
Source/WebKit2/UIProcess/WebPageProxy.h
Source/WebKit2/UIProcess/WebProcessProxy.cpp
Source/WebKit2/UIProcess/WebProcessProxy.h
Source/WebKit2/WebKit2.xcodeproj/project.pbxproj
Tools/ChangeLog
Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
Tools/TestWebKitAPI/Tests/WebKit2/ProcessDidTerminate.cpp [moved from Tools/TestWebKitAPI/Tests/WebKit2/ProcessDidCrashWithReason.cpp with 61% similarity]

index e51b620..c503ac9 100644 (file)
@@ -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
 
 namespace WebKit {
 
-enum class ProcessCrashReason {
-    TerminationDueToMemoryUsage,
-    TerminationDueToCPUUsage,
-    TerminationRequestedByClient,
-    Other
+enum class ProcessTerminationReason {
+    ExceededMemoryLimit,
+    ExceededCPULimit,
+    RequestedByClient,
+    Crash
 };
 
 }
index 8a3e1a7..e459487 100644 (file)
@@ -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 @@ public:
     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&) { }
 
index d94a9e9..1b66ab3 100644 (file)
@@ -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 @@ inline WKCacheModel toAPI(CacheModel cacheModel)
     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)
index a880d3f..06222aa 100644 (file)
@@ -422,7 +422,7 @@ void WKPageSetCustomTextEncodingName(WKPageRef pageRef, WKStringRef encodingName
 
 void WKPageTerminate(WKPageRef pageRef)
 {
-    toImpl(pageRef)->terminateProcess();
+    toImpl(pageRef)->process().requestTermination(ProcessTerminationReason::RequestedByClient);
 }
 
 WKStringRef WKPageGetSessionHistoryURLValueType()
@@ -2400,13 +2400,15 @@ void WKPageSetPageNavigationClient(WKPageRef pageRef, const WKPageNavigationClie
             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
index d71d4eb..ed1548f 100644 (file)
@@ -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 (*WKPageNavigationDidReceiveAuthenticationChallengeCallback)(WKPage
 
 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 @@ typedef struct WKPageNavigationClientV1 {
     WKPageNavigationDidRemoveNavigationGestureSnapshot didRemoveNavigationGestureSnapshot;
 
     // Version 1.
-    WKPageNavigationWebProcessDidCrashWithReasonCallback webProcessDidCrashWithReason;
+    WKPageNavigationWebProcessDidTerminateCallback webProcessDidTerminate;
 } WKPageNavigationClientV1;
 
 #ifdef __cplusplus
@@ -32,12 +32,12 @@ extern "C" {
 #endif
 
 enum {
-    kWKProcessCrashReasonTerminationDueToMemoryUsage = 0,
-    kWKProcessCrashReasonTerminationDueToCPUUsage,
-    kWKProcessCrashReasonTerminationRequestedByClient,
-    kWKProcessCrashReasonOther,
+    kWKProcessTerminationReasonExceededMemoryLimit = 0,
+    kWKProcessTerminationReasonExceededCPULimit,
+    kWKProcessTerminationReasonRequestedByClient,
+    kWKProcessTerminationReasonCrash,
 };
-typedef uint32_t WKProcessCrashReason;
+typedef uint32_t WKProcessTerminationReason;
 
 #ifdef __cplusplus
 }
index 9519246..921e08f 100644 (file)
@@ -3772,7 +3772,7 @@ WEBCORE_COMMAND(yankAndSelect)
 
 - (void)_killWebContentProcessAndResetState
 {
-    _page->terminateProcess();
+    _page->process().requestTermination(WebKit::ProcessTerminationReason::RequestedByClient);
 }
 
 #if PLATFORM(MAC)
index 0ad3478..7a9c3c5 100644 (file)
@@ -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 @@ private:
 
         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;
 
index a459829..027db50 100644 (file)
@@ -710,7 +710,7 @@ void NavigationState::NavigationClient::didReceiveAuthenticationChallenge(WebPag
     [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;
index e9bd686..d386f88 100644 (file)
@@ -2410,16 +2410,6 @@ void WebPageProxy::setCustomTextEncodingName(const String& encodingName)
     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 @@ void WebPageProxy::didChangeProcessIsResponsive()
     m_pageLoadState.didChangeProcessIsResponsive();
 }
 
-void WebPageProxy::processDidCrash(ProcessCrashReason reason)
+void WebPageProxy::processDidTerminate(ProcessTerminationReason reason)
 {
     ASSERT(m_isValid);
 
@@ -5343,8 +5333,8 @@ void WebPageProxy::processDidCrash(ProcessCrashReason reason)
     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) {
index 4c8ee58..dd93d02 100644 (file)
@@ -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 @@ public:
 
     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 @@ public:
 
     void processDidBecomeUnresponsive();
     void processDidBecomeResponsive();
-    void processDidCrash(ProcessCrashReason);
+    void processDidTerminate(ProcessTerminationReason);
     void willChangeProcessIsResponsive();
     void didChangeProcessIsResponsive();
 
index 95b74cc..830135c 100644 (file)
@@ -600,7 +600,7 @@ void WebProcessProxy::didClose(IPC::Connection&)
     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::deleteWebsiteDataForOrigins(SessionID sessionID, OptionSet
     });
 }
 
-void WebProcessProxy::requestTermination()
+void WebProcessProxy::requestTermination(ProcessTerminationReason reason)
 {
     if (state() == State::Terminated)
         return;
@@ -863,7 +863,7 @@ void WebProcessProxy::requestTermination()
     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 @@ void WebProcessProxy::processTerminated()
     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 @@ void WebProcessProxy::didExceedBackgroundCPULimit()
     }
 
     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()
index 154a6fd..bc78d23 100644 (file)
@@ -55,12 +55,6 @@ struct PluginInfo;
 
 namespace WebKit {
 
-enum class TerminationReason {
-    ExceededActiveMemoryLimit,
-    ExceededInactiveMemoryLimit,
-    ExceededBackgroundCPULimit,
-};
-
 class NetworkProcessProxy;
 class UserMediaCaptureManagerProxy;
 class WebBackForwardListItem;
@@ -142,7 +136,7 @@ public:
     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 @@ private:
 
     bool canTerminateChildProcess();
 
-    void terminateProcessDueToResourceLimits(TerminationReason);
+    void logDiagnosticMessageForResourceLimitTermination(const String& limitKey);
 
     ResponsivenessTimer m_responsivenessTimer;
     BackgroundProcessResponsivenessTimer m_backgroundResponsivenessTimer;
index d53b99a..c4f6186 100644 (file)
                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 */; };
                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>"; };
                                BCC43AB9127B95DC00317F16 /* PlatformPopupMenuData.h */,
                                E18C92F312DB9E7100CF2AEB /* PrintInfo.cpp */,
                                E1CC1B8E12D7EADF00625838 /* PrintInfo.h */,
-                               463FD4811EB94EAD00A2982C /* ProcessCrashReason.h */,
+                               463FD4811EB94EAD00A2982C /* ProcessTerminationReason.h */,
                                F6A0C13F13281E6E0070430F /* ResourceCachesToClear.h */,
                                410482CB1DDD2FB500F006D0 /* RTCNetwork.cpp */,
                                410482CC1DDD2FB500F006D0 /* RTCNetwork.h */,
                                BCD597CF112B56AC00EC8C23 /* WKPreferences.cpp */,
                                BCD597CE112B56AC00EC8C23 /* WKPreferencesRef.h */,
                                762B7484120BBA2D00819339 /* WKPreferencesRefPrivate.h */,
-                               463FD47F1EB9458400A2982C /* WKProcessCrashReason.h */,
+                               463FD47F1EB9458400A2982C /* WKProcessTerminationReason.h */,
                                512F58F312A88A5400629530 /* WKProtectionSpace.cpp */,
                                512F58F412A88A5400629530 /* WKProtectionSpace.h */,
                                518ACAE912AEE6BB00B04B83 /* WKProtectionSpaceTypes.h */,
                                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 */,
                                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 */,
index 833271f..f891d31 100644 (file)
@@ -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.
index 40cb4d0..c35abf8 100644 (file)
                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 */; };
                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>"; };
                                0766DD1F1A5AD5200023E3BB /* PendingAPIRequestURL.cpp */,
                                333B9CE11277F23100FEFCE3 /* PreventEmptyUserAgent.cpp */,
                                F6FDDDD214241AD4004F1729 /* PrivateBrowsingPushStateNoHistoryCallback.cpp */,
-                               4647B1251EBA3B730041D7EF /* ProcessDidCrashWithReason.cpp */,
+                               4647B1251EBA3B730041D7EF /* ProcessDidTerminate.cpp */,
                                8A3AF93A16C9ED2700D248C1 /* ReloadPageAfterCrash.cpp */,
                                2DD7D3A9178205D00026E1E3 /* ResizeReversePaginatedWebView.cpp */,
                                8A2C750D16CED9550024F352 /* ResizeWindowAfterCrash.cpp */,
                                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 */,
 #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 @@ static void didFinishNavigation(WKPageRef page, WKNavigationRef navigation, WKTy
     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 @@ static void didCrashWithReason(WKPageRef page, WKProcessCrashReason reason, cons
     // 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 @@ TEST(WebKit2, ProcessDidCrashWithReasonRequestedByClient)
     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);
 }