Title: [277235] trunk
Revision
277235
Author
commit-qu...@webkit.org
Date
2021-05-08 18:17:13 -0700 (Sat, 08 May 2021)

Log Message

REGRESSION (r276797?): [ macOS/iOS ] TestWebKitAPI.URLSchemeHandler.Exceptions is flakey crashing
https://bugs.webkit.org/show_bug.cgi?id=225373

Patch by Alex Christensen <achristen...@webkit.org> on 2021-05-08
Reviewed by Sam Weinig.

Source/WebKit:

This patch fixes three problems related to the lifetime of WKURLSchemeTasks:

1. There was an unneeded abstraction API::URLSchemeTask which wrapped a WebURLSchemeTask, which could have a different lifetime than its owner.
   This is especially bad since at least one was used on multiple threads.
2. We weren't explicitly keeping a strong reference to the task given to the API client in startURLSchemeTask: and stopURLSchemeTask: which could
   cause all our internal maps to release their references to the task after the first call to didFailWithError and there was a test that did multiple
   calls to didFailWithError and verifies it throws an NSException the second time.
3. We were keeping a HashSet of raw WebURLSchemeHandler pointers, then using each of them without keeping it alive.

This is covered by at least the URLSchemeHandler.Exceptions API test which would crash in many exciting places before this but doesn't crash after this.

* Sources.txt:
* UIProcess/API/APIURLSchemeTask.cpp: Removed.
* UIProcess/API/APIURLSchemeTask.h: Removed.
* UIProcess/API/C/WKTestingSupport.cpp:
(WKGetAPIURLSchemeTaskInstanceCount): Deleted.
* UIProcess/API/C/WKTestingSupport.h:
* UIProcess/API/Cocoa/WKURLSchemeTask.mm:
(-[WKURLSchemeTaskImpl init]):
(-[WKURLSchemeTaskImpl dealloc]):
(-[WKURLSchemeTaskImpl request]):
(-[WKURLSchemeTaskImpl _requestOnlyIfCached]):
(-[WKURLSchemeTaskImpl _willPerformRedirection:newRequest:completionHandler:]):
(-[WKURLSchemeTaskImpl didReceiveResponse:]):
(-[WKURLSchemeTaskImpl didReceiveData:]):
(-[WKURLSchemeTaskImpl didFinish]):
(-[WKURLSchemeTaskImpl didFailWithError:]):
(-[WKURLSchemeTaskImpl _didPerformRedirection:newRequest:]):
(-[WKURLSchemeTaskImpl _frame]):
* UIProcess/API/Cocoa/WKURLSchemeTaskInternal.h:
* UIProcess/Cocoa/WebURLSchemeHandlerCocoa.h:
* UIProcess/Cocoa/WebURLSchemeHandlerCocoa.mm:
(WebKit::WebURLSchemeHandlerCocoa::platformStartTask):
(WebKit::WebURLSchemeHandlerCocoa::platformStopTask):
(WebKit::WebURLSchemeHandlerCocoa::platformTaskCompleted): Deleted.
* UIProcess/Inspector/socket/RemoteInspectorProtocolHandler.h:
* UIProcess/Inspector/win/InspectorResourceURLSchemeHandler.h:
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::stopAllURLSchemeTasks):
* UIProcess/WebURLSchemeHandler.cpp:
(WebKit::WebURLSchemeHandler::taskCompleted):
* UIProcess/WebURLSchemeHandler.h:
* UIProcess/WebURLSchemeTask.h:
* WebKit.xcodeproj/project.pbxproj:

Tools:

* TestWebKitAPI/Tests/WebKitCocoa/WKURLSchemeHandler-leaks.mm:
(runUntilTasksInFlight):

Modified Paths

Removed Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (277234 => 277235)


--- trunk/Source/WebKit/ChangeLog	2021-05-09 00:59:28 UTC (rev 277234)
+++ trunk/Source/WebKit/ChangeLog	2021-05-09 01:17:13 UTC (rev 277235)
@@ -1,3 +1,55 @@
+2021-05-08  Alex Christensen  <achristen...@webkit.org>
+
+        REGRESSION (r276797?): [ macOS/iOS ] TestWebKitAPI.URLSchemeHandler.Exceptions is flakey crashing
+        https://bugs.webkit.org/show_bug.cgi?id=225373
+
+        Reviewed by Sam Weinig.
+
+        This patch fixes three problems related to the lifetime of WKURLSchemeTasks:
+
+        1. There was an unneeded abstraction API::URLSchemeTask which wrapped a WebURLSchemeTask, which could have a different lifetime than its owner.
+           This is especially bad since at least one was used on multiple threads.
+        2. We weren't explicitly keeping a strong reference to the task given to the API client in startURLSchemeTask: and stopURLSchemeTask: which could
+           cause all our internal maps to release their references to the task after the first call to didFailWithError and there was a test that did multiple
+           calls to didFailWithError and verifies it throws an NSException the second time.
+        3. We were keeping a HashSet of raw WebURLSchemeHandler pointers, then using each of them without keeping it alive.
+
+        This is covered by at least the URLSchemeHandler.Exceptions API test which would crash in many exciting places before this but doesn't crash after this.
+
+        * Sources.txt:
+        * UIProcess/API/APIURLSchemeTask.cpp: Removed.
+        * UIProcess/API/APIURLSchemeTask.h: Removed.
+        * UIProcess/API/C/WKTestingSupport.cpp:
+        (WKGetAPIURLSchemeTaskInstanceCount): Deleted.
+        * UIProcess/API/C/WKTestingSupport.h:
+        * UIProcess/API/Cocoa/WKURLSchemeTask.mm:
+        (-[WKURLSchemeTaskImpl init]):
+        (-[WKURLSchemeTaskImpl dealloc]):
+        (-[WKURLSchemeTaskImpl request]):
+        (-[WKURLSchemeTaskImpl _requestOnlyIfCached]):
+        (-[WKURLSchemeTaskImpl _willPerformRedirection:newRequest:completionHandler:]):
+        (-[WKURLSchemeTaskImpl didReceiveResponse:]):
+        (-[WKURLSchemeTaskImpl didReceiveData:]):
+        (-[WKURLSchemeTaskImpl didFinish]):
+        (-[WKURLSchemeTaskImpl didFailWithError:]):
+        (-[WKURLSchemeTaskImpl _didPerformRedirection:newRequest:]):
+        (-[WKURLSchemeTaskImpl _frame]):
+        * UIProcess/API/Cocoa/WKURLSchemeTaskInternal.h:
+        * UIProcess/Cocoa/WebURLSchemeHandlerCocoa.h:
+        * UIProcess/Cocoa/WebURLSchemeHandlerCocoa.mm:
+        (WebKit::WebURLSchemeHandlerCocoa::platformStartTask):
+        (WebKit::WebURLSchemeHandlerCocoa::platformStopTask):
+        (WebKit::WebURLSchemeHandlerCocoa::platformTaskCompleted): Deleted.
+        * UIProcess/Inspector/socket/RemoteInspectorProtocolHandler.h:
+        * UIProcess/Inspector/win/InspectorResourceURLSchemeHandler.h:
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::stopAllURLSchemeTasks):
+        * UIProcess/WebURLSchemeHandler.cpp:
+        (WebKit::WebURLSchemeHandler::taskCompleted):
+        * UIProcess/WebURLSchemeHandler.h:
+        * UIProcess/WebURLSchemeTask.h:
+        * WebKit.xcodeproj/project.pbxproj:
+
 2021-05-08  Carlos Garcia Campos  <cgar...@igalia.com>
 
         [SOUP] Use the new libsoup network metrics API

Modified: trunk/Source/WebKit/Sources.txt (277234 => 277235)


--- trunk/Source/WebKit/Sources.txt	2021-05-09 00:59:28 UTC (rev 277234)
+++ trunk/Source/WebKit/Sources.txt	2021-05-09 01:17:13 UTC (rev 277235)
@@ -372,7 +372,6 @@
 UIProcess/API/APIProcessPoolConfiguration.cpp
 UIProcess/API/APIOpenPanelParameters.cpp
 UIProcess/API/APISessionState.cpp
-UIProcess/API/APIURLSchemeTask.cpp
 UIProcess/API/APIUserScript.cpp
 UIProcess/API/APIUserStyleSheet.cpp
 UIProcess/API/APIWebAuthenticationAssertionResponse.cpp

Deleted: trunk/Source/WebKit/UIProcess/API/APIURLSchemeTask.cpp (277234 => 277235)


--- trunk/Source/WebKit/UIProcess/API/APIURLSchemeTask.cpp	2021-05-09 00:59:28 UTC (rev 277234)
+++ trunk/Source/WebKit/UIProcess/API/APIURLSchemeTask.cpp	2021-05-09 01:17:13 UTC (rev 277235)
@@ -1,44 +0,0 @@
-/*
- * Copyright (C) 2017 Apple Inc. All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
- * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
- * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
- * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
- * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
- * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
- * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
- * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
- * THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#include "config.h"
-#include "APIURLSchemeTask.h"
-
-#include "WebURLSchemeHandler.h"
-#include "WebURLSchemeTask.h"
-
-namespace API {
-
-Ref<URLSchemeTask> URLSchemeTask::create(WebKit::WebURLSchemeTask& webURLSchemeTask)
-{
-    return adoptRef(*new URLSchemeTask(webURLSchemeTask));
-}
-
-URLSchemeTask::URLSchemeTask(WebKit::WebURLSchemeTask& webURLSchemeTask)
-    : m_webURLSchemeTask(webURLSchemeTask)
-{
-}
-
-}

Deleted: trunk/Source/WebKit/UIProcess/API/APIURLSchemeTask.h (277234 => 277235)


--- trunk/Source/WebKit/UIProcess/API/APIURLSchemeTask.h	2021-05-09 00:59:28 UTC (rev 277234)
+++ trunk/Source/WebKit/UIProcess/API/APIURLSchemeTask.h	2021-05-09 01:17:13 UTC (rev 277235)
@@ -1,49 +0,0 @@
-/*
- * Copyright (C) 2017 Apple Inc. All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
- * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
- * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
- * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
- * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
- * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
- * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
- * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
- * THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#pragma once
-
-#include "APIObject.h"
-#include <wtf/InstanceCounted.h>
-
-namespace WebKit {
-class WebURLSchemeTask;
-}
-
-namespace API {
-
-class URLSchemeTask final : public ObjectImpl<Object::Type::URLSchemeTask>, public InstanceCounted<URLSchemeTask> {
-public:
-    static Ref<URLSchemeTask> create(WebKit::WebURLSchemeTask&);
-
-    WebKit::WebURLSchemeTask& task() { return m_webURLSchemeTask.get(); }
-
-private:
-    URLSchemeTask(WebKit::WebURLSchemeTask&);
-
-    Ref<WebKit::WebURLSchemeTask> m_webURLSchemeTask;
-};
-
-}

Modified: trunk/Source/WebKit/UIProcess/API/C/WKTestingSupport.cpp (277234 => 277235)


--- trunk/Source/WebKit/UIProcess/API/C/WKTestingSupport.cpp	2021-05-09 00:59:28 UTC (rev 277234)
+++ trunk/Source/WebKit/UIProcess/API/C/WKTestingSupport.cpp	2021-05-09 01:17:13 UTC (rev 277235)
@@ -26,14 +26,8 @@
 #include "config.h"
 #include "WKTestingSupport.h"
 
-#include "APIURLSchemeTask.h"
 #include "WebURLSchemeTask.h"
 
-size_t WKGetAPIURLSchemeTaskInstanceCount()
-{
-    return API::URLSchemeTask::instanceCount();
-}
-
 size_t WKGetWebURLSchemeTaskInstanceCount()
 {
     return WebKit::WebURLSchemeTask::instanceCount();

Modified: trunk/Source/WebKit/UIProcess/API/C/WKTestingSupport.h (277234 => 277235)


--- trunk/Source/WebKit/UIProcess/API/C/WKTestingSupport.h	2021-05-09 00:59:28 UTC (rev 277234)
+++ trunk/Source/WebKit/UIProcess/API/C/WKTestingSupport.h	2021-05-09 01:17:13 UTC (rev 277235)
@@ -31,7 +31,6 @@
 extern "C" {
 #endif
 
-WK_EXPORT size_t WKGetAPIURLSchemeTaskInstanceCount();
 WK_EXPORT size_t WKGetWebURLSchemeTaskInstanceCount();
 
 #ifdef __cplusplus

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKURLSchemeTask.mm (277234 => 277235)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKURLSchemeTask.mm	2021-05-09 00:59:28 UTC (rev 277234)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKURLSchemeTask.mm	2021-05-09 01:17:13 UTC (rev 277235)
@@ -75,28 +75,33 @@
 
 @implementation WKURLSchemeTaskImpl
 
+- (instancetype)init
+{
+    RELEASE_ASSERT_NOT_REACHED();
+}
+
 - (void)dealloc
 {
     if (WebCoreObjCScheduleDeallocateOnMainRunLoop(WKURLSchemeTaskImpl.class, self))
         return;
-    _urlSchemeTask->API::URLSchemeTask::~URLSchemeTask();
+    _urlSchemeTask->WebURLSchemeTask::~WebURLSchemeTask();
     [super dealloc];
 }
 
 - (NSURLRequest *)request
 {
-    return _urlSchemeTask->task().nsRequest();
+    return _urlSchemeTask->nsRequest();
 }
 
 - (BOOL)_requestOnlyIfCached
 {
-    return _urlSchemeTask->task().nsRequest().cachePolicy == NSURLRequestReturnCacheDataDontLoad;
+    return _urlSchemeTask->nsRequest().cachePolicy == NSURLRequestReturnCacheDataDontLoad;
 }
 
 - (void)_willPerformRedirection:(NSURLResponse *)response newRequest:(NSURLRequest *)request completionHandler:(void (^)(NSURLRequest *))completionHandler
 {
     auto function = [protectedSelf = retainPtr(self), self, protectedResponse = retainPtr(response), response, protectedRequest = retainPtr(request), request, handler = makeBlockPtr(completionHandler)] () mutable {
-        return _urlSchemeTask->task().willPerformRedirection(response, request, [handler = WTFMove(handler)] (WebCore::ResourceRequest&& actualNewRequest) {
+        return _urlSchemeTask->willPerformRedirection(response, request, [handler = WTFMove(handler)] (WebCore::ResourceRequest&& actualNewRequest) {
             handler.get()(actualNewRequest.nsURLRequest(WebCore::HTTPBodyUpdatePolicy::UpdateHTTPBody));
         });
     };
@@ -108,7 +113,7 @@
 - (void)didReceiveResponse:(NSURLResponse *)response
 {
     auto function = [protectedSelf = retainPtr(self), self, protectedResponse = retainPtr(response), response] {
-        return _urlSchemeTask->task().didReceiveResponse(response);
+        return _urlSchemeTask->didReceiveResponse(response);
     };
 
     auto result = getExceptionTypeFromMainRunLoop(WTFMove(function));
@@ -118,7 +123,7 @@
 - (void)didReceiveData:(NSData *)data
 {
     auto function = [protectedSelf = retainPtr(self), self, protectedData = retainPtr(data), data] () mutable {
-        return _urlSchemeTask->task().didReceiveData(WebCore::SharedBuffer::create(data));
+        return _urlSchemeTask->didReceiveData(WebCore::SharedBuffer::create(data));
     };
 
     auto result = getExceptionTypeFromMainRunLoop(WTFMove(function));
@@ -128,7 +133,7 @@
 - (void)didFinish
 {
     auto function = [protectedSelf = retainPtr(self), self] {
-        return _urlSchemeTask->task().didComplete({ });
+        return _urlSchemeTask->didComplete({ });
     };
 
     auto result = getExceptionTypeFromMainRunLoop(WTFMove(function));
@@ -138,7 +143,7 @@
 - (void)didFailWithError:(NSError *)error
 {
     auto function = [protectedSelf = retainPtr(self), self, protectedError = retainPtr(error), error] {
-        return _urlSchemeTask->task().didComplete(error);
+        return _urlSchemeTask->didComplete(error);
     };
 
     auto result = getExceptionTypeFromMainRunLoop(WTFMove(function));
@@ -148,7 +153,7 @@
 - (void)_didPerformRedirection:(NSURLResponse *)response newRequest:(NSURLRequest *)request
 {
     auto function = [protectedSelf = retainPtr(self), self, protectedResponse = retainPtr(response), response, protectedRequest = retainPtr(request), request] {
-        return _urlSchemeTask->task().didPerformRedirection(response, request);
+        return _urlSchemeTask->didPerformRedirection(response, request);
     };
 
     auto result = getExceptionTypeFromMainRunLoop(WTFMove(function));
@@ -157,7 +162,7 @@
 
 - (WKFrameInfo *)_frame
 {
-    return wrapper(_urlSchemeTask->task().frameInfo());
+    return wrapper(_urlSchemeTask->frameInfo());
 }
 
 #pragma mark WKObject protocol implementation

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKURLSchemeTaskInternal.h (277234 => 277235)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKURLSchemeTaskInternal.h	2021-05-09 00:59:28 UTC (rev 277234)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKURLSchemeTaskInternal.h	2021-05-09 01:17:13 UTC (rev 277235)
@@ -25,8 +25,8 @@
 
 #import "WKURLSchemeTaskPrivate.h"
 
-#import "APIURLSchemeTask.h"
 #import "WKObject.h"
+#import "WebURLSchemeTask.h"
 
 @interface WKURLSchemeTaskImpl : NSObject <WKURLSchemeTaskPrivate>
 @end
@@ -33,7 +33,7 @@
 
 namespace WebKit {
 
-template<> struct WrapperTraits<API::URLSchemeTask> {
+template<> struct WrapperTraits<WebURLSchemeTask> {
     using WrapperClass = WKURLSchemeTaskImpl;
 };
 
@@ -41,6 +41,6 @@
 
 @interface WKURLSchemeTaskImpl () <WKObject> {
 @package
-    API::ObjectStorage<API::URLSchemeTask> _urlSchemeTask;
+    API::ObjectStorage<WebKit::WebURLSchemeTask> _urlSchemeTask;
 }
 @end

Modified: trunk/Source/WebKit/UIProcess/Cocoa/WebURLSchemeHandlerCocoa.h (277234 => 277235)


--- trunk/Source/WebKit/UIProcess/Cocoa/WebURLSchemeHandlerCocoa.h	2021-05-09 00:59:28 UTC (rev 277234)
+++ trunk/Source/WebKit/UIProcess/Cocoa/WebURLSchemeHandlerCocoa.h	2021-05-09 01:17:13 UTC (rev 277235)
@@ -30,12 +30,10 @@
 
 @protocol WKURLSchemeHandler;
 
-namespace API {
-class URLSchemeTask;
-}
-
 namespace WebKit {
 
+class WebURLSchemeTask;
+
 class WebURLSchemeHandlerCocoa : public WebURLSchemeHandler {
 public:
     static Ref<WebURLSchemeHandlerCocoa> create(id <WKURLSchemeHandler>);
@@ -47,10 +45,8 @@
 
     void platformStartTask(WebPageProxy&, WebURLSchemeTask&) final;
     void platformStopTask(WebPageProxy&, WebURLSchemeTask&) final;
-    void platformTaskCompleted(WebURLSchemeTask&) final;
 
     RetainPtr<id <WKURLSchemeHandler>> m_apiHandler;
-    HashMap<uint64_t, Ref<API::URLSchemeTask>> m_apiTasks;
 
 }; // class WebURLSchemeHandler
 

Modified: trunk/Source/WebKit/UIProcess/Cocoa/WebURLSchemeHandlerCocoa.mm (277234 => 277235)


--- trunk/Source/WebKit/UIProcess/Cocoa/WebURLSchemeHandlerCocoa.mm	2021-05-09 00:59:28 UTC (rev 277234)
+++ trunk/Source/WebKit/UIProcess/Cocoa/WebURLSchemeHandlerCocoa.mm	2021-05-09 01:17:13 UTC (rev 277235)
@@ -26,7 +26,6 @@
 #import "config.h"
 #import "WebURLSchemeHandlerCocoa.h"
 
-#import "APIURLSchemeTask.h"
 #import "WKFoundation.h"
 #import "WKURLSchemeHandler.h"
 #import "WKURLSchemeTaskInternal.h"
@@ -48,30 +47,18 @@
 
 void WebURLSchemeHandlerCocoa::platformStartTask(WebPageProxy& page, WebURLSchemeTask& task)
 {
-    auto result = m_apiTasks.add(task.identifier(), API::URLSchemeTask::create(task));
-    ASSERT(result.isNewEntry);
-
+    auto strongTask = retainPtr(wrapper(task));
     if (auto webView = page.cocoaView())
-        [m_apiHandler.get() webView:webView.get() startURLSchemeTask:wrapper(result.iterator->value.get())];
+        [m_apiHandler.get() webView:webView.get() startURLSchemeTask:strongTask.get()];
 }
 
 void WebURLSchemeHandlerCocoa::platformStopTask(WebPageProxy& page, WebURLSchemeTask& task)
 {
-    auto iterator = m_apiTasks.find(task.identifier());
-    if (iterator == m_apiTasks.end())
-        return;
-
+    auto strongTask = retainPtr(wrapper(task));
     if (auto webView = page.cocoaView())
-        [m_apiHandler.get() webView:webView.get() stopURLSchemeTask:wrapper(iterator->value.get())];
+        [m_apiHandler.get() webView:webView.get() stopURLSchemeTask:strongTask.get()];
     else
         task.suppressTaskStoppedExceptions();
-
-    m_apiTasks.remove(iterator);
 }
 
-void WebURLSchemeHandlerCocoa::platformTaskCompleted(WebURLSchemeTask& task)
-{
-    m_apiTasks.remove(task.identifier());
-}
-
 } // namespace WebKit

Modified: trunk/Source/WebKit/UIProcess/Inspector/socket/RemoteInspectorProtocolHandler.h (277234 => 277235)


--- trunk/Source/WebKit/UIProcess/Inspector/socket/RemoteInspectorProtocolHandler.h	2021-05-09 00:59:28 UTC (rev 277234)
+++ trunk/Source/WebKit/UIProcess/Inspector/socket/RemoteInspectorProtocolHandler.h	2021-05-09 01:17:13 UTC (rev 277235)
@@ -55,7 +55,6 @@
     // WebURLSchemeHandler
     void platformStartTask(WebPageProxy&, WebURLSchemeTask&) final;
     void platformStopTask(WebPageProxy&, WebURLSchemeTask&) final { }
-    void platformTaskCompleted(WebURLSchemeTask&) final { }
 
     void updateTargetList();
 

Modified: trunk/Source/WebKit/UIProcess/Inspector/win/InspectorResourceURLSchemeHandler.h (277234 => 277235)


--- trunk/Source/WebKit/UIProcess/Inspector/win/InspectorResourceURLSchemeHandler.h	2021-05-09 00:59:28 UTC (rev 277234)
+++ trunk/Source/WebKit/UIProcess/Inspector/win/InspectorResourceURLSchemeHandler.h	2021-05-09 01:17:13 UTC (rev 277235)
@@ -41,7 +41,6 @@
     // WebURLSchemeHandler
     void platformStartTask(WebPageProxy&, WebURLSchemeTask&) final;
     void platformStopTask(WebPageProxy&, WebURLSchemeTask&) final { }
-    void platformTaskCompleted(WebURLSchemeTask&) final { }
 };
 
 } // namespace WebKit

Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (277234 => 277235)


--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2021-05-09 00:59:28 UTC (rev 277234)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2021-05-09 01:17:13 UTC (rev 277235)
@@ -7550,11 +7550,7 @@
 
 void WebPageProxy::stopAllURLSchemeTasks(WebProcessProxy* process)
 {
-    HashSet<WebURLSchemeHandler*> handlers;
-    for (auto& handler : m_urlSchemeHandlersByScheme.values())
-        handlers.add(handler.ptr());
-
-    for (auto* handler : handlers)
+    for (auto& handler : copyToVectorOf<Ref<WebURLSchemeHandler>>(m_urlSchemeHandlersByScheme.values()))
         handler->stopAllTasksForPage(*this, process);
 }
 

Modified: trunk/Source/WebKit/UIProcess/WebURLSchemeHandler.h (277234 => 277235)


--- trunk/Source/WebKit/UIProcess/WebURLSchemeHandler.h	2021-05-09 00:59:28 UTC (rev 277234)
+++ trunk/Source/WebKit/UIProcess/WebURLSchemeHandler.h	2021-05-09 01:17:13 UTC (rev 277235)
@@ -61,7 +61,7 @@
 private:
     virtual void platformStartTask(WebPageProxy&, WebURLSchemeTask&) = 0;
     virtual void platformStopTask(WebPageProxy&, WebURLSchemeTask&) = 0;
-    virtual void platformTaskCompleted(WebURLSchemeTask&) = 0;
+    virtual void platformTaskCompleted(WebURLSchemeTask&) { };
 
     void removeTaskFromPageMap(WebPageProxyIdentifier, uint64_t taskID);
     WebProcessProxy* processForTaskIdentifier(uint64_t) const;

Modified: trunk/Source/WebKit/UIProcess/WebURLSchemeTask.h (277234 => 277235)


--- trunk/Source/WebKit/UIProcess/WebURLSchemeTask.h	2021-05-09 00:59:28 UTC (rev 277234)
+++ trunk/Source/WebKit/UIProcess/WebURLSchemeTask.h	2021-05-09 01:17:13 UTC (rev 277235)
@@ -25,6 +25,7 @@
 
 #pragma once
 
+#include "APIObject.h"
 #include "WebProcessProxy.h"
 #include <WebCore/ResourceRequest.h>
 #include <WebCore/ResourceResponse.h>
@@ -52,7 +53,7 @@
 
 using SyncLoadCompletionHandler = CompletionHandler<void(const WebCore::ResourceResponse&, const WebCore::ResourceError&, const Vector<char>&)>;
 
-class WebURLSchemeTask : public ThreadSafeRefCounted<WebURLSchemeTask>, public InstanceCounted<WebURLSchemeTask> {
+class WebURLSchemeTask : public API::ObjectImpl<API::Object::Type::URLSchemeTask>, public InstanceCounted<WebURLSchemeTask> {
     WTF_MAKE_NONCOPYABLE(WebURLSchemeTask);
 public:
     static Ref<WebURLSchemeTask> create(WebURLSchemeHandler&, WebPageProxy&, WebProcessProxy&, WebCore::PageIdentifier, URLSchemeTaskParameters&&, SyncLoadCompletionHandler&&);

Modified: trunk/Source/WebKit/WebKit.xcodeproj/project.pbxproj (277234 => 277235)


--- trunk/Source/WebKit/WebKit.xcodeproj/project.pbxproj	2021-05-09 00:59:28 UTC (rev 277234)
+++ trunk/Source/WebKit/WebKit.xcodeproj/project.pbxproj	2021-05-09 01:17:13 UTC (rev 277235)
@@ -4147,8 +4147,6 @@
 		51D124311E6DE521002B2820 /* WebURLSchemeHandlerCocoa.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WebURLSchemeHandlerCocoa.h; sourceTree = "<group>"; };
 		51D124321E6DE521002B2820 /* WebURLSchemeHandlerCocoa.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WebURLSchemeHandlerCocoa.mm; sourceTree = "<group>"; };
 		51D124371E6DFD2A002B2820 /* WKURLSchemeTaskInternal.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WKURLSchemeTaskInternal.h; sourceTree = "<group>"; };
-		51D124381E6DFDB9002B2820 /* APIURLSchemeTask.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = APIURLSchemeTask.cpp; sourceTree = "<group>"; };
-		51D124391E6DFDB9002B2820 /* APIURLSchemeTask.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = APIURLSchemeTask.h; sourceTree = "<group>"; };
 		51D124821E734AC8002B2820 /* APIHTTPCookieStore.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = APIHTTPCookieStore.cpp; sourceTree = "<group>"; };
 		51D124831E734AC8002B2820 /* APIHTTPCookieStore.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = APIHTTPCookieStore.h; sourceTree = "<group>"; };
 		51D124841E734AE3002B2820 /* WKHTTPCookieStore.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WKHTTPCookieStore.h; sourceTree = "<group>"; };
@@ -10195,8 +10193,6 @@
 				1AFDE65F1954E9B100C48FFA /* APISessionState.cpp */,
 				1AFDE6601954E9B100C48FFA /* APISessionState.h */,
 				1A4D664718A2D91A00D82E21 /* APIUIClient.h */,
-				51D124381E6DFDB9002B2820 /* APIURLSchemeTask.cpp */,
-				51D124391E6DFDB9002B2820 /* APIURLSchemeTask.h */,
 				7CB365AF1D31DD1E007158CA /* APIUserInitiatedAction.h */,
 				7C89D2A51A6789EA003A5FDE /* APIUserScript.cpp */,
 				7C89D2921A67122F003A5FDE /* APIUserScript.h */,

Modified: trunk/Tools/ChangeLog (277234 => 277235)


--- trunk/Tools/ChangeLog	2021-05-09 00:59:28 UTC (rev 277234)
+++ trunk/Tools/ChangeLog	2021-05-09 01:17:13 UTC (rev 277235)
@@ -1,3 +1,13 @@
+2021-05-08  Alex Christensen  <achristen...@webkit.org>
+
+        REGRESSION (r276797?): [ macOS/iOS ] TestWebKitAPI.URLSchemeHandler.Exceptions is flakey crashing
+        https://bugs.webkit.org/show_bug.cgi?id=225373
+
+        Reviewed by Sam Weinig.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/WKURLSchemeHandler-leaks.mm:
+        (runUntilTasksInFlight):
+
 2021-05-08  Chris Dumez  <cdu...@apple.com>
 
         Port Filesystem::pathByAppendingComponent() & Filesystem:: pathByAppendingComponents() to std::filesystem

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKURLSchemeHandler-leaks.mm (277234 => 277235)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKURLSchemeHandler-leaks.mm	2021-05-09 00:59:28 UTC (rev 277234)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKURLSchemeHandler-leaks.mm	2021-05-09 01:17:13 UTC (rev 277235)
@@ -68,9 +68,7 @@
 static void runUntilTasksInFlight(size_t count)
 {
     while (true) {
-        EXPECT_EQ(WKGetAPIURLSchemeTaskInstanceCount(), WKGetWebURLSchemeTaskInstanceCount());
-
-        if (WKGetAPIURLSchemeTaskInstanceCount() == count)
+        if (WKGetWebURLSchemeTaskInstanceCount() == count)
             return;
 
         TestWebKitAPI::Util::spinRunLoop(10);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to