Title: [290855] trunk/Source/WebKit
Revision
290855
Author
pan...@apple.com
Date
2022-03-04 17:01:11 -0800 (Fri, 04 Mar 2022)

Log Message

Web Inspector: [Cocoa] Continually opening and closing Web Inspector sometimes crashes
https://bugs.webkit.org/show_bug.cgi?id=237484

Reviewed by Darin Adler.

We need to handle operations on the main queue because `webView:stopURLSchemeTask:` will be called from the main
queue, and we must not be in middle the operation when we get the request to stop said operation, otherwise we
may attempt to call `[urlSchemeTask did*]`, which is not permitted after we have been asked to stop that task.
If we add the operation for the `WKURLSchemeTask` to a different queue it is possible we will have already
started the operation on a background queue while at the same time are receiving a request to stop that same
`WKURLSchemeTask`.

* UIProcess/Inspector/mac/WKInspectorResourceURLSchemeHandler.mm:
(-[WKInspectorResourceURLSchemeHandler webView:startURLSchemeTask:]):
(-[WKInspectorResourceURLSchemeHandler webView:stopURLSchemeTask:]):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (290854 => 290855)


--- trunk/Source/WebKit/ChangeLog	2022-03-05 00:48:00 UTC (rev 290854)
+++ trunk/Source/WebKit/ChangeLog	2022-03-05 01:01:11 UTC (rev 290855)
@@ -1,3 +1,21 @@
+2022-03-04  Patrick Angle  <pan...@apple.com>
+
+        Web Inspector: [Cocoa] Continually opening and closing Web Inspector sometimes crashes
+        https://bugs.webkit.org/show_bug.cgi?id=237484
+
+        Reviewed by Darin Adler.
+
+        We need to handle operations on the main queue because `webView:stopURLSchemeTask:` will be called from the main
+        queue, and we must not be in middle the operation when we get the request to stop said operation, otherwise we
+        may attempt to call `[urlSchemeTask did*]`, which is not permitted after we have been asked to stop that task.
+        If we add the operation for the `WKURLSchemeTask` to a different queue it is possible we will have already
+        started the operation on a background queue while at the same time are receiving a request to stop that same
+        `WKURLSchemeTask`.
+
+        * UIProcess/Inspector/mac/WKInspectorResourceURLSchemeHandler.mm:
+        (-[WKInspectorResourceURLSchemeHandler webView:startURLSchemeTask:]):
+        (-[WKInspectorResourceURLSchemeHandler webView:stopURLSchemeTask:]):
+
 2022-03-04  Myles C. Maxfield  <mmaxfi...@apple.com>
 
         [iOS] Books ASSERTs upon opening a book with a debug build of WebKit

Modified: trunk/Source/WebKit/UIProcess/Inspector/mac/WKInspectorResourceURLSchemeHandler.mm (290854 => 290855)


--- trunk/Source/WebKit/UIProcess/Inspector/mac/WKInspectorResourceURLSchemeHandler.mm	2022-03-05 00:48:00 UTC (rev 290854)
+++ trunk/Source/WebKit/UIProcess/Inspector/mac/WKInspectorResourceURLSchemeHandler.mm	2022-03-05 01:01:11 UTC (rev 290855)
@@ -38,7 +38,6 @@
 @implementation WKInspectorResourceURLSchemeHandler {
     RetainPtr<NSMapTable<id <WKURLSchemeTask>, NSOperation *>> _fileLoadOperations;
     RetainPtr<NSBundle> _cachedBundle;
-    RetainPtr<NSOperationQueue> _operationQueue;
     
     RetainPtr<NSSet<NSString *>> _allowedURLSchemesForCSP;
     RetainPtr<NSSet<NSURL *>> _mainResourceURLsForCSP;
@@ -66,6 +65,7 @@
 
 - (void)webView:(WKWebView *)webView startURLSchemeTask:(id <WKURLSchemeTask>)urlSchemeTask
 {
+    dispatch_assert_queue(dispatch_get_main_queue());
     if (!_cachedBundle) {
         _cachedBundle = [NSBundle bundleWithIdentifier:@"com.apple.WebInspectorUI"];
 
@@ -77,20 +77,8 @@
     if (!_fileLoadOperations)
         _fileLoadOperations = adoptNS([[NSMapTable alloc] initWithKeyOptions:NSPointerFunctionsStrongMemory valueOptions:NSPointerFunctionsStrongMemory capacity:5]);
 
-    if (!_operationQueue) {
-        _operationQueue = adoptNS([[NSOperationQueue alloc] init]);
-        _operationQueue.get().underlyingQueue = dispatch_get_global_queue(QOS_CLASS_USER_INTERACTIVE, 0);
-        _operationQueue.get().qualityOfService = NSOperationQualityOfServiceUserInteractive;
-
-        // The default value (NSOperationQueueDefaultMaxConcurrentOperationCount) results in a large number of threads
-        // that can exceed the soft limit if two Web Inspector instances are being loaded simultaneously.
-        _operationQueue.get().maxConcurrentOperationCount = 4;
-    }
-
     NSBlockOperation *operation = [NSBlockOperation blockOperationWithBlock:^{
-        dispatch_async(dispatch_get_main_queue(), ^{
-            [_fileLoadOperations removeObjectForKey:urlSchemeTask];
-        });
+        [_fileLoadOperations removeObjectForKey:urlSchemeTask];
 
         NSURL *requestURL = urlSchemeTask.request.URL;
         NSURL *fileURLForRequest = [_cachedBundle URLForResource:requestURL.relativePath withExtension:@""];
@@ -134,18 +122,16 @@
     }];
     
     [_fileLoadOperations setObject:operation forKey:urlSchemeTask];
-    [_operationQueue addOperation:operation];
+    [[NSOperationQueue mainQueue] addOperation:operation];
 }
 
 - (void)webView:(WKWebView *)webView stopURLSchemeTask:(id <WKURLSchemeTask>)urlSchemeTask
 {
-    // Ensure that all blocks with pending removals are dispatched before doing a map lookup.
-    dispatch_async(dispatch_get_main_queue(), ^{
-        if (NSOperation *operation = [_fileLoadOperations objectForKey:urlSchemeTask]) {
-            [operation cancel];
-            [_fileLoadOperations removeObjectForKey:urlSchemeTask];
-        }
-    });
+    dispatch_assert_queue(dispatch_get_main_queue());
+    if (NSOperation *operation = [_fileLoadOperations objectForKey:urlSchemeTask]) {
+        [operation cancel];
+        [_fileLoadOperations removeObjectForKey:urlSchemeTask];
+    }
 }
 
 @end
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to