Changeset 278254 in webkit


Ignore:
Timestamp:
May 30, 2021, 12:18:56 PM (4 years ago)
Author:
Wenson Hsieh
Message:

[iOS] UI process crashes when deallocating WKWebView in a script message handler during an active touch event
​https://bugs.webkit.org/show_bug.cgi?id=226426
rdar://75425319

Reviewed by Darin Adler.

Source/WebKit:

It's possible for the UI process to crash upon being notified that asynchronous, active touch events have been
handled in the web process via the async IPC replay block in WebPageProxy::handlePreventableTouchEvent(). This
happens if the client posts a message from the web process to UI process while handling an active touch event
and deallocates the WKWebView currently handling the touch event in the script message handler.

This is because the async replay block inside WebPageProxy::handlePreventableTouchEvent() strongly captures a
reference to the WebPageProxy, thus keeping it alive; however, the WebPageProxy's weak pointer to the page
client is nulled out, which causes WebPageProxy::pageClient() to crash with a null dereference.

To fix this, we weakly capture WebPageProxy instead and return early if it has already been destroyed by the
time we process the completion handler, and also add a null check for m_pageClient before attempting to call
into it. Note that it's unnecessary to call into doneDeferringTouch(Start|End) to unblock any deferred
gestures here, because the process of destroying the content view will call -cleanUpInteraction and remove all
deferring gestures from the view, regardless of whether they're still in Possible state.

Test: TouchEventTests.DestroyWebViewWhileHandlingTouchEnd

  • UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::handlePreventableTouchEvent):

Tools:

Add a new API test that exercises the crash.

  • TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
  • TestWebKitAPI/Tests/ios/TouchEventTests.mm: Added.

(-[TouchEventScriptMessageHandler userContentController:didReceiveScriptMessage:]):
(-[WKWebView touchEventGestureRecognizer]):
(TestWebKitAPI::updateSimulatedTouchEvent):
(TestWebKitAPI::simulatedTouchEvent):
(TestWebKitAPI::TEST):

  • TestWebKitAPI/Tests/ios/active-touch-events.html: Added.
  • TestWebKitAPI/cocoa/TestWKWebView.h:
  • TestWebKitAPI/cocoa/TestWKWebView.mm:

(-[WKWebView textInputContentView]):
(-[TestWKWebView textInputContentView]): Deleted.

  • TestWebKitAPI/ios/UIKitSPI.h:
Location:
trunk
Files:
2 added
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r278253 r278254  
     12021-05-30  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [iOS] UI process crashes when deallocating WKWebView in a script message handler during an active touch event
     4        https://bugs.webkit.org/show_bug.cgi?id=226426
     5        rdar://75425319
     6
     7        Reviewed by Darin Adler.
     8
     9        It's possible for the UI process to crash upon being notified that asynchronous, active touch events have been
     10        handled in the web process via the async IPC replay block in `WebPageProxy::handlePreventableTouchEvent()`. This
     11        happens if the client posts a message from the web process to UI process while handling an active touch event
     12        and deallocates the WKWebView currently handling the touch event in the script message handler.
     13
     14        This is because the async replay block inside `WebPageProxy::handlePreventableTouchEvent()` strongly captures a
     15        reference to the `WebPageProxy`, thus keeping it alive; however, the `WebPageProxy`'s weak pointer to the page
     16        client is nulled out, which causes `WebPageProxy::pageClient()` to crash with a null dereference.
     17
     18        To fix this, we weakly capture `WebPageProxy` instead and return early if it has already been destroyed by the
     19        time we process the completion handler, and also add a null check for `m_pageClient` before attempting to call
     20        into it. Note that it's unnecessary to call into `doneDeferringTouch(Start|End)` to unblock any deferred
     21        gestures here, because the process of destroying the content view will call `-cleanUpInteraction` and remove all
     22        deferring gestures from the view, regardless of whether they're still in Possible state.
     23
     24        Test: TouchEventTests.DestroyWebViewWhileHandlingTouchEnd
     25
     26        * UIProcess/WebPageProxy.cpp:
     27        (WebKit::WebPageProxy::handlePreventableTouchEvent):
     28
    1292021-05-30  Darin Adler  <darin@apple.com>
    230

  • trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

    r278253 r278254  
    31193119            ++m_handlingPreventableTouchEndCount;
    31203120
    3121         sendWithAsyncReply(Messages::EventDispatcher::TouchEvent(m_webPageID, event), [this, protectedThis = makeRef(*this), event] (bool handled) {
     3121        sendWithAsyncReply(Messages::EventDispatcher::TouchEvent(m_webPageID, event), [this, weakThis = makeWeakPtr(*this), event] (bool handled) {
     3122            auto protectedThis = makeRefPtr(weakThis.get());
     3123            if (!protectedThis)
     3124                return;
     3125
    31223126            bool didFinishDeferringTouchStart = false;
    31233127            ASSERT_IMPLIES(event.type() == WebEvent::TouchStart, m_handlingPreventableTouchStartCount);
    … …  
    31353139
    31363140            didReceiveEvent(event.type(), handledOrFailedWithError);
     3141            if (!m_pageClient)
     3142                return;
     3143
    31373144            pageClient().doneWithTouchEvent(event, handledOrFailedWithError);
    31383145

  • trunk/Tools/ChangeLog

    r278253 r278254  
     12021-05-30  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [iOS] UI process crashes when deallocating WKWebView in a script message handler during an active touch event
     4        https://bugs.webkit.org/show_bug.cgi?id=226426
     5        rdar://75425319
     6
     7        Reviewed by Darin Adler.
     8
     9        Add a new API test that exercises the crash.
     10
     11        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
     12        * TestWebKitAPI/Tests/ios/TouchEventTests.mm: Added.
     13        (-[TouchEventScriptMessageHandler userContentController:didReceiveScriptMessage:]):
     14        (-[WKWebView touchEventGestureRecognizer]):
     15        (TestWebKitAPI::updateSimulatedTouchEvent):
     16        (TestWebKitAPI::simulatedTouchEvent):
     17        (TestWebKitAPI::TEST):
     18        * TestWebKitAPI/Tests/ios/active-touch-events.html: Added.
     19        * TestWebKitAPI/cocoa/TestWKWebView.h:
     20        * TestWebKitAPI/cocoa/TestWKWebView.mm:
     21        (-[WKWebView textInputContentView]):
     22        (-[TestWKWebView textInputContentView]): Deleted.
     23        * TestWebKitAPI/ios/UIKitSPI.h:
     24
    1252021-05-30  Darin Adler  <darin@apple.com>
    226

  • trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj

    r278172 r278254  
    12301230                F4A32EC41F05F3850047C544 /* dragstart-change-selection-offscreen.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F4A32EC31F05F3780047C544 /* dragstart-change-selection-offscreen.html */; };
    12311231                F4A32ECB1F0643370047C544 /* contenteditable-in-iframe.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F4A32ECA1F0642F40047C544 /* contenteditable-in-iframe.html */; };
     1232                F4A7CE782662D6E800228685 /* TouchEventTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = F4A7CE772662D6E800228685 /* TouchEventTests.mm */; };
     1233                F4A7CE7A2662D86C00228685 /* active-touch-events.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F4A7CE792662D83E00228685 /* active-touch-events.html */; };
    12321234                F4A9202F1FEE34E900F59590 /* apple-data-url.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F4A9202E1FEE34C800F59590 /* apple-data-url.html */; };
    12331235                F4AB578A1F65165400DB0DA1 /* custom-draggable-div.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F4AB57891F65164B00DB0DA1 /* custom-draggable-div.html */; };
    … …  
    13541356                                55A81800218102210004A39A /* 400x400-green.png in Copy Resources */,
    13551357                                379028B914FAC24C007E6B43 /* acceptsFirstMouse.html in Copy Resources */,
     1358                                F4A7CE7A2662D86C00228685 /* active-touch-events.html in Copy Resources */,
    13561359                                725C3EF322058A5B007C36FC /* AdditionalSupportedImageTypes.html in Copy Resources */,
    13571360                                1C2B81871C8925A000A5529F /* Ahem.ttf in Copy Resources */,
    … …  
    30703073                F4A32EC31F05F3780047C544 /* dragstart-change-selection-offscreen.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "dragstart-change-selection-offscreen.html"; sourceTree = "<group>"; };
    30713074                F4A32ECA1F0642F40047C544 /* contenteditable-in-iframe.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "contenteditable-in-iframe.html"; sourceTree = "<group>"; };
     3075                F4A7CE772662D6E800228685 /* TouchEventTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = TouchEventTests.mm; sourceTree = "<group>"; };
     3076                F4A7CE792662D83E00228685 /* active-touch-events.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "active-touch-events.html"; sourceTree = "<group>"; };
    30723077                F4A9202E1FEE34C800F59590 /* apple-data-url.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "apple-data-url.html"; sourceTree = "<group>"; };
    30733078                F4AB57891F65164B00DB0DA1 /* custom-draggable-div.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "custom-draggable-div.html"; sourceTree = "<group>"; };
    … …  
    38323837                                F494B369263120780060A310 /* TextServicesTests.mm */,
    38333838                                1C24DEEC263001DE00450D07 /* TextStyleFontSize.mm */,
     3839                                F4A7CE772662D6E800228685 /* TouchEventTests.mm */,
    38343840                                F460F6742614DE2F0064F2B6 /* UIFocusTests.mm */,
    38353841                                F46849BD1EEF58E400B937FE /* UIPasteboardTests.mm */,
    … …  
    41854191                        isa = PBXGroup;
    41864192                        children = (
     4193                                F4A7CE792662D83E00228685 /* active-touch-events.html */,
    41874194                                0F16BED72304A1D100B4A167 /* composited.html */,
    41884195                                CEDA12402437C9EA00C28A9E /* editable-region-composited-and-non-composited-overlap.html */,
    … …  
    57625769                                7CCE7EDD1A411A9200447C4C /* TimeRanges.cpp in Sources */,
    57635770                                7C83E0BD1D0A650C00FEBCF3 /* TopContentInset.mm in Sources */,
     5771                                F4A7CE782662D6E800228685 /* TouchEventTests.mm in Sources */,
    57645772                                7CCE7ED31A411A7E00447C4C /* TypingStyleCrash.mm in Sources */,
    57655773                                57152B7821DD4E8D000C37CA /* U2fCommandConstructorTest.cpp in Sources */,

  • trunk/Tools/TestWebKitAPI/cocoa/TestWKWebView.h

    r276351 r278254  
    4949
    5050@interface WKWebView (TestWebKitAPI)
     51#if PLATFORM(IOS_FAMILY)
     52@property (nonatomic, readonly) UIView <UITextInputPrivate, UITextInputInternal, UITextInputMultiDocument, UIWKInteractionViewProtocol, UITextInputTokenizer> *textInputContentView;
     53#endif
    5154@property (nonatomic, readonly) NSString *contentsAsString;
    5255@property (nonatomic, readonly) NSArray<NSString *> *tagsInBody;
    … …  
    113116
    114117@interface TestWKWebView (IOSOnly)
    115 @property (nonatomic, readonly) UIView <UITextInputPrivate, UITextInputInternal, UITextInputMultiDocument, UIWKInteractionViewProtocol, UITextInputTokenizer> *textInputContentView;
    116118@property (nonatomic, readonly) RetainPtr<NSArray> selectionRectsAfterPresentationUpdate;
    117119@property (nonatomic, readonly) CGRect caretViewRectInContentCoordinates;

  • trunk/Tools/TestWebKitAPI/cocoa/TestWKWebView.mm

    r276351 r278254  
    132132}
    133133
     134#if PLATFORM(IOS_FAMILY)
     135
     136- (UIView <UITextInputPrivate, UITextInputMultiDocument> *)textInputContentView
     137{
     138    return (UIView <UITextInputPrivate, UITextInputMultiDocument> *)[self valueForKey:@"_currentContentView"];
     139}
     140
     141#endif // PLATFORM(IOS_FAMILY)
     142
    134143- (NSString *)contentsAsString
    135144{
    … …  
    686695}
    687696
    688 - (UIView <UITextInputPrivate, UITextInputMultiDocument> *)textInputContentView
    689 {
    690     return (UIView <UITextInputPrivate, UITextInputMultiDocument> *)[self valueForKey:@"_currentContentView"];
    691 }
    692 
    693697- (RetainPtr<NSArray>)selectionRectsAfterPresentationUpdate
    694698{

  • trunk/Tools/TestWebKitAPI/ios/UIKitSPI.h

    r276441 r278254  
    4848#import <UIKit/UIWKTextInteractionAssistant.h>
    4949#import <UIKit/UIWebFormAccessory.h>
     50#import <UIKit/UIWebTouchEventsGestureRecognizer.h>
    5051#import <UIKit/_UINavigationInteractiveTransition.h>
    5152
Note: See TracChangeset for help on using the changeset viewer.