Title: [261838] trunk/Source/WebKit
Revision
261838
Author
timothy_hor...@apple.com
Date
2020-05-18 16:31:42 -0700 (Mon, 18 May 2020)

Log Message

Rare crash under -[WKContentView resignFirstResponderForWebView]
https://bugs.webkit.org/show_bug.cgi?id=212050
<rdar://problem/60187111>

Reviewed by Wenson Hsieh.

* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView resignFirstResponderForWebView]):
Reorganize this block so that it always takes a strong reference /before/
null checking and using the pointer. Also, add a _page null check.
This is a speculative fix for an unreproducible low-rate crash.

In theory the existing time-of-check race here should not be a problem,
since WKWebView is supposed to be freed on the main thread, and this code
runs on the main thread, but we have ample evidence of WKWebView
being freed off the main thread in various clients.

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (261837 => 261838)


--- trunk/Source/WebKit/ChangeLog	2020-05-18 23:10:40 UTC (rev 261837)
+++ trunk/Source/WebKit/ChangeLog	2020-05-18 23:31:42 UTC (rev 261838)
@@ -1,3 +1,22 @@
+2020-05-18  Tim Horton  <timothy_hor...@apple.com>
+
+        Rare crash under -[WKContentView resignFirstResponderForWebView]
+        https://bugs.webkit.org/show_bug.cgi?id=212050
+        <rdar://problem/60187111>
+
+        Reviewed by Wenson Hsieh.
+
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView resignFirstResponderForWebView]):
+        Reorganize this block so that it always takes a strong reference /before/
+        null checking and using the pointer. Also, add a _page null check.
+        This is a speculative fix for an unreproducible low-rate crash.
+
+        In theory the existing time-of-check race here should not be a problem,
+        since WKWebView is supposed to be freed on the main thread, and this code
+        runs on the main thread, but we have ample evidence of WKWebView
+        being freed off the main thread in various clients.
+
 2020-05-18  David Kilzer  <ddkil...@apple.com>
 
         Use default initializers in TextIndicatorData

Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (261837 => 261838)


--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2020-05-18 23:10:40 UTC (rev 261837)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2020-05-18 23:31:42 UTC (rev 261838)
@@ -1403,16 +1403,20 @@
 
         if (_keyWebEventHandler) {
             dispatch_async(dispatch_get_main_queue(), [weakHandler = WeakObjCPtr<id>(_keyWebEventHandler.get()), weakSelf = WeakObjCPtr<WKContentView>(self)] {
-                if (!weakSelf || !weakHandler)
-                    return;
-
                 auto strongSelf = weakSelf.get();
-                if ([strongSelf isFirstResponder] || weakHandler.get().get() != strongSelf->_keyWebEventHandler.get())
+                if (!strongSelf || [strongSelf isFirstResponder])
                     return;
-
+                auto strongHandler = weakHandler.get();
+                if (!strongHandler)
+                    return;
+                if (strongSelf->_keyWebEventHandler.get() != strongHandler.get())
+                    return;
                 auto keyEventHandler = std::exchange(strongSelf->_keyWebEventHandler, nil);
-                ASSERT(strongSelf->_page->hasQueuedKeyEvent());
-                keyEventHandler(strongSelf->_page->firstQueuedKeyEvent().nativeEvent(), YES);
+                auto page = strongSelf->_page;
+                if (!page)
+                    return;
+                ASSERT(page->hasQueuedKeyEvent());
+                keyEventHandler(page->firstQueuedKeyEvent().nativeEvent(), YES);
             });
         }
     }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to