Changeset 260382 in webkit

Timestamp:

Apr 20, 2020 11:43:45 AM (4 years ago)

Author:

Chris Dumez

Message:

Sending beacons when Fetch KeepAlive feature is disabled crashes the WebProcess
​https://bugs.webkit.org/show_bug.cgi?id=210753
<rdar://problem/61896221>

Reviewed by Geoffrey Garen.

Source/WebCore:

Test: http/wpt/beacon/beacon-legacy-code-path.html

• testing/InternalSettings.cpp:

(WebCore::InternalSettings::Backup::Backup):
(WebCore::InternalSettings::Backup::restoreTo):
(WebCore::InternalSettings::setFetchAPIKeepAliveEnabled):

• testing/InternalSettings.h:
• testing/InternalSettings.idl:

Add internal settings to disable Fetch Keep Alive for layout testing.

Source/WebKit:

• WebProcess/Network/WebLoaderStrategy.cpp:

(WebKit::WebLoaderStrategy::startPingLoad):
Make sure NetworkResourceLoadParameters's webPageProxyID / webPageID / webFrameID are properly
initialized before sending the IPC or IPC decoding will fail.

LayoutTests:

Add layout test coverage.

• http/wpt/beacon/beacon-legacy-code-path-expected.txt: Added.
• http/wpt/beacon/beacon-legacy-code-path.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/http/wpt/beacon/beacon-legacy-code-path-expected.txt (added)
• LayoutTests/http/wpt/beacon/beacon-legacy-code-path.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/testing/InternalSettings.cpp (modified) (3 diffs)
• Source/WebCore/testing/InternalSettings.h (modified) (2 diffs)
• Source/WebCore/testing/InternalSettings.idl (modified) (1 diff)
• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2020-04-20  Chris Dumez  <cdumez@apple.com>
+
+        Sending beacons when Fetch KeepAlive feature is disabled crashes the WebProcess
+        https://bugs.webkit.org/show_bug.cgi?id=210753
+        <rdar://problem/61896221>
+
+        Reviewed by Geoffrey Garen.
+
+        Add layout test coverage.
+
+        * http/wpt/beacon/beacon-legacy-code-path-expected.txt: Added.
+        * http/wpt/beacon/beacon-legacy-code-path.html: Added.
+
 2020-04-20  Youenn Fablet  <youenn@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-04-20  Chris Dumez  <cdumez@apple.com>
+
+        Sending beacons when Fetch KeepAlive feature is disabled crashes the WebProcess
+        https://bugs.webkit.org/show_bug.cgi?id=210753
+        <rdar://problem/61896221>
+
+        Reviewed by Geoffrey Garen.
+
+        Test: http/wpt/beacon/beacon-legacy-code-path.html
+
+        * testing/InternalSettings.cpp:
+        (WebCore::InternalSettings::Backup::Backup):
+        (WebCore::InternalSettings::Backup::restoreTo):
+        (WebCore::InternalSettings::setFetchAPIKeepAliveEnabled):
+        * testing/InternalSettings.h:
+        * testing/InternalSettings.idl:
+        Add internal settings to disable Fetch Keep Alive for layout testing.
+
 2020-04-20  Youenn Fablet  <youenn@apple.com>
 

trunk/Source/WebCore/testing/InternalSettings.cpp

@@ -120 +120 @@
     , m_setScreenCaptureEnabled(RuntimeEnabledFeatures::sharedFeatures().screenCaptureEnabled())
 #endif
+    , m_fetchAPIKeepAliveAPIEnabled(RuntimeEnabledFeatures::sharedFeatures().fetchAPIKeepAliveEnabled())
     , m_shouldMockBoldSystemFontForAccessibility(RenderTheme::singleton().shouldMockBoldSystemFontForAccessibility())
 #if USE(AUDIO_SESSION)
@@ -223 +224 @@
     RuntimeEnabledFeatures::sharedFeatures().setScreenCaptureEnabled(m_setScreenCaptureEnabled);
 #endif
+    RuntimeEnabledFeatures::sharedFeatures().setFetchAPIKeepAliveEnabled(m_fetchAPIKeepAliveAPIEnabled);
     RuntimeEnabledFeatures::sharedFeatures().setCustomPasteboardDataEnabled(m_customPasteboardDataEnabled);
 
@@ -810 +812 @@
 }
 
+void InternalSettings::setFetchAPIKeepAliveEnabled(bool enabled)
+{
+    RuntimeEnabledFeatures::sharedFeatures().setFetchAPIKeepAliveEnabled(enabled);
+}
+
 ExceptionOr<String> InternalSettings::userInterfaceDirectionPolicy()
 {

trunk/Source/WebCore/testing/InternalSettings.h

@@ -132 +132 @@
     static void setPictureInPictureAPIEnabled(bool);
     static void setScreenCaptureEnabled(bool);
+    static void setFetchAPIKeepAliveEnabled(bool);
 
     static bool webAnimationsCSSIntegrationEnabled();
@@ -221 +222 @@
         bool m_webGL2Enabled;
         bool m_setScreenCaptureEnabled;
+        bool m_fetchAPIKeepAliveAPIEnabled;
         
         bool m_shouldMockBoldSystemFontForAccessibility;

trunk/Source/WebCore/testing/InternalSettings.idl

@@ -98 +98 @@
     void setWebGPUEnabled(boolean enabled);
     void setScreenCaptureEnabled(boolean enabled);
+    void setFetchAPIKeepAliveEnabled(boolean enabled);
 
     [MayThrowException] DOMString userInterfaceDirectionPolicy();

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2020-04-20  Chris Dumez  <cdumez@apple.com>
+
+        Sending beacons when Fetch KeepAlive feature is disabled crashes the WebProcess
+        https://bugs.webkit.org/show_bug.cgi?id=210753
+        <rdar://problem/61896221>
+
+        Reviewed by Geoffrey Garen.
+
+        * WebProcess/Network/WebLoaderStrategy.cpp:
+        (WebKit::WebLoaderStrategy::startPingLoad):
+        Make sure NetworkResourceLoadParameters's webPageProxyID / webPageID / webFrameID are properly
+        initialized before sending the IPC or IPC decoding will fail.
+
 2020-04-20  David Kilzer  <ddkilzer@apple.com>
 

trunk/Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp

@@ -640 +640 @@
 void WebLoaderStrategy::startPingLoad(Frame& frame, ResourceRequest& request, const HTTPHeaderMap& originalRequestHeaders, const FetchOptions& options, ContentSecurityPolicyImposition policyCheck, PingLoadCompletionHandler&& completionHandler)
 {
+    auto* webFrame = WebFrame::fromCoreFrame(frame);
     auto* document = frame.document();
-    if (!document) {
+    if (!document || !webFrame) {
         if (completionHandler)
             completionHandler(internalError(request.url()), { });
@@ -647 +648 @@
     }
 
+    auto* webPage = webFrame->page();
+    if (!webPage) {
+        if (completionHandler)
+            completionHandler(internalError(request.url()), { });
+        return;
+    }
+
     NetworkResourceLoadParameters loadParameters;
     loadParameters.identifier = generateLoadIdentifier();
+    loadParameters.webPageProxyID = webPage->webPageProxyIdentifier();
+    loadParameters.webPageID = webPage->identifier();
+    loadParameters.webFrameID = webFrame->frameID();
     loadParameters.request = request;
     loadParameters.sourceOrigin = &document->securityOrigin();
@@ -667 +678 @@
     addParametersShared(&frame, loadParameters);
     
-    auto* webFrameLoaderClient = toWebFrameLoaderClient(frame.loader().client());
-    auto* webFrame = webFrameLoaderClient ? &webFrameLoaderClient->webFrame() : nullptr;
-    auto* webPage = webFrame ? webFrame->page() : nullptr;
-    if (webPage)
-        loadParameters.isNavigatingToAppBoundDomain = webPage->isNavigatingToAppBoundDomain();
+    loadParameters.isNavigatingToAppBoundDomain = webPage->isNavigatingToAppBoundDomain();
     
 #if ENABLE(CONTENT_EXTENSIONS)
     loadParameters.mainDocumentURL = document->topDocument().url();
     // FIXME: Instead of passing userContentControllerIdentifier, we should just pass webPageId to NetworkProcess.
-    if (webPage)
-        loadParameters.userContentControllerIdentifier = webPage->userContentControllerIdentifier();
+    loadParameters.userContentControllerIdentifier = webPage->userContentControllerIdentifier();
 #endif