Changeset 242551 in webkit


Ignore:
Timestamp:
Mar 6, 2019 9:38:37 AM (5 years ago)
Author:
Wenson Hsieh
Message:

[iOS] Frequent 1 second IPC deadlocks when showing a paste callout
​https://bugs.webkit.org/show_bug.cgi?id=195354
<rdar://problem/48624675>

Reviewed by Tim Horton.

Source/WebKit:

When triggering programmatic paste, we frequently hit deadlocks due to sync IPC going from the UI process to the
web process and vice versa. What happens in this scenario is that prior to triggering programmatic paste, the
page may try to move focus to a different element (e.g. a hidden editable area) before calling execCommand.
This causes us to send an ElementDidFocus message to the UI process, followed by RequestDOMPasteAccess.

However, upon receiving ElementDidFocus, we reload input views and (in the process) UIKit requests the
autocorrection context, which we implement in WebKit using a sync message to the web process due to
<rdar://problem/16207002> and its blocking bug <rdar://problem/48383001>. This means we'll end up in a state
where both the UI process and web process are blocked on each other waiting for a sync IPC response, and the UI
process is hung for a second until the IPC message times out.

Ideally, we should fix this by addressing <rdar://problem/16207002>. However, this requires potentially large
changes in UIKit (<rdar://problem/48383001>); for the time being, work around this deadlock by refactoring
synchronous autocorrection context requests such that they can be resolved by an out-of-band IPC response
(HandleAutocorrectionContext). Then prior to requesting DOM paste access, preemptively send a
HandleAutocorrectionContext message to the UI process to unblock any pending synchronous autocorrection context
requests.

  • UIProcess/PageClient.h:
  • UIProcess/WebPageProxy.h:
  • UIProcess/WebPageProxy.messages.in:
  • UIProcess/ios/PageClientImplIOS.h:
  • UIProcess/ios/PageClientImplIOS.mm:

(WebKit::PageClientImpl::handleAutocorrectionContext):

  • UIProcess/ios/WKContentViewInteraction.h:

Make it possible for WKContentView to remember its current pending autocorrection context completion handler.
This is invoked and cleared out in -_invokePendingAutocorrectionContextHandler:.

  • UIProcess/ios/WKContentViewInteraction.mm:

(-[WKContentView _invokePendingAutocorrectionContextHandler:]):
(-[WKContentView requestAutocorrectionContextWithCompletionHandler:]):
(-[WKContentView _handleAutocorrectionContext:]):

Handle an autocorrection context response. This is invoked when the web process either handles an autocorrection
context message, or when it preemptively sends an autocorrection context before requesting DOM paste access.

(+[WKAutocorrectionContext emptyAutocorrectionContext]):

Add a helper to create an empty autocorrection context.

  • UIProcess/ios/WebPageProxyIOS.mm:

(WebKit::WebPageProxy::requestAutocorrectionContext):
(WebKit::WebPageProxy::handleAutocorrectionContext):
(WebKit::WebPageProxy::autocorrectionContextSync): Deleted.

Removed this sync version, since we now only have requestAutocorrectionContext.

  • WebProcess/WebPage/WebPage.cpp:

(WebKit::WebPage::requestDOMPasteAccess):

Send WebPageProxy::HandleAutocorrectionContext, and add a FIXME referencing <rdar://problem/16207002>.

  • WebProcess/WebPage/WebPage.h:
  • WebProcess/WebPage/WebPage.messages.in:

Split AutocorrectionContextSync into RequestAutocorrectionContext and HandleAutocorrectionContext; additionally,
remove the existing RequestAutocorrectionContext message.

  • WebProcess/WebPage/ios/WebPageIOS.mm:

(WebKit::WebPage::requestAutocorrectionContext):
(WebKit::WebPage::autocorrectionContextSync): Deleted.

LayoutTests:

Most of these tests currently encounter and rely on the 1 second IPC timeout to finish. To test this fix, force
ignoreSynchronousMessagingTimeouts=true to make them fail if the processes encounter a deadlock.

  • editing/pasteboard/ios/dom-paste-confirmation.html:
  • editing/pasteboard/ios/dom-paste-consecutive-confirmations.html:
  • editing/pasteboard/ios/dom-paste-rejection.html:
  • editing/pasteboard/ios/dom-paste-requires-user-gesture.html:
  • editing/pasteboard/ios/dom-paste-same-origin.html:
Location:
trunk
Files:
19 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r242550 r242551  
     12019-03-06  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [iOS] Frequent 1 second IPC deadlocks when showing a paste callout
     4        https://bugs.webkit.org/show_bug.cgi?id=195354
     5        <rdar://problem/48624675>
     6
     7        Reviewed by Tim Horton.
     8
     9        Most of these tests currently encounter and rely on the 1 second IPC timeout to finish. To test this fix, force
     10        `ignoreSynchronousMessagingTimeouts=true` to make them fail if the processes encounter a deadlock.
     11
     12        * editing/pasteboard/ios/dom-paste-confirmation.html:
     13        * editing/pasteboard/ios/dom-paste-consecutive-confirmations.html:
     14        * editing/pasteboard/ios/dom-paste-rejection.html:
     15        * editing/pasteboard/ios/dom-paste-requires-user-gesture.html:
     16        * editing/pasteboard/ios/dom-paste-same-origin.html:
     17
    1182019-03-06  Javier Fernandez  <jfernandez@igalia.com>
    219

  • trunk/LayoutTests/editing/pasteboard/ios/dom-paste-confirmation.html

    r242317 r242551  
    1 <!DOCTYPE html> <!-- webkit-test-runner [ domPasteAllowed=false useFlexibleViewport=true ] -->
     1<!DOCTYPE html> <!-- webkit-test-runner [ domPasteAllowed=false useFlexibleViewport=true ignoreSynchronousMessagingTimeouts=true ] -->
    22<html>
    33<meta name="viewport" content="width=device-width, initial-scale=1">

  • trunk/LayoutTests/editing/pasteboard/ios/dom-paste-consecutive-confirmations.html

    r242317 r242551  
    1 <!DOCTYPE html> <!-- webkit-test-runner [ domPasteAllowed=false useFlexibleViewport=true ] -->
     1<!DOCTYPE html> <!-- webkit-test-runner [ domPasteAllowed=false useFlexibleViewport=true ignoreSynchronousMessagingTimeouts=true ] -->
    22<html>
    33<meta name="viewport" content="width=device-width, initial-scale=1">

  • trunk/LayoutTests/editing/pasteboard/ios/dom-paste-rejection.html

    r242317 r242551  
    1 <!DOCTYPE html> <!-- webkit-test-runner [ domPasteAllowed=false useFlexibleViewport=true ] -->
     1<!DOCTYPE html> <!-- webkit-test-runner [ domPasteAllowed=false useFlexibleViewport=true ignoreSynchronousMessagingTimeouts=true ] -->
    22<html>
    33<meta name="viewport" content="width=device-width, initial-scale=1">

  • trunk/LayoutTests/editing/pasteboard/ios/dom-paste-requires-user-gesture.html

    r242317 r242551  
    1 <!DOCTYPE html> <!-- webkit-test-runner [ domPasteAllowed=false useFlexibleViewport=true ] -->
     1<!DOCTYPE html> <!-- webkit-test-runner [ domPasteAllowed=false useFlexibleViewport=true ignoreSynchronousMessagingTimeouts=true ] -->
    22<html>
    33<meta name="viewport" content="width=device-width, initial-scale=1">

  • trunk/LayoutTests/editing/pasteboard/ios/dom-paste-same-origin.html

    r242317 r242551  
    1 <!DOCTYPE html> <!-- webkit-test-runner [ domPasteAllowed=false useFlexibleViewport=true ] -->
     1<!DOCTYPE html> <!-- webkit-test-runner [ domPasteAllowed=false useFlexibleViewport=true ignoreSynchronousMessagingTimeouts=true ] -->
    22<html>
    33<meta name="viewport" content="width=device-width, initial-scale=1">

  • trunk/Source/WebKit/ChangeLog

    r242534 r242551  
     12019-03-06  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [iOS] Frequent 1 second IPC deadlocks when showing a paste callout
     4        https://bugs.webkit.org/show_bug.cgi?id=195354
     5        <rdar://problem/48624675>
     6
     7        Reviewed by Tim Horton.
     8
     9        When triggering programmatic paste, we frequently hit deadlocks due to sync IPC going from the UI process to the
     10        web process and vice versa. What happens in this scenario is that prior to triggering programmatic paste, the
     11        page may try to move focus to a different element (e.g. a hidden editable area) before calling `execCommand`.
     12        This causes us to send an ElementDidFocus message to the UI process, followed by RequestDOMPasteAccess.
     13
     14        However, upon receiving ElementDidFocus, we reload input views and (in the process) UIKit requests the
     15        autocorrection context, which we implement in WebKit using a sync message to the web process due to
     16        <rdar://problem/16207002> and its blocking bug <rdar://problem/48383001>. This means we'll end up in a state
     17        where both the UI process and web process are blocked on each other waiting for a sync IPC response, and the UI
     18        process is hung for a second until the IPC message times out.
     19
     20        Ideally, we should fix this by addressing <rdar://problem/16207002>. However, this requires potentially large
     21        changes in UIKit (<rdar://problem/48383001>); for the time being, work around this deadlock by refactoring
     22        synchronous autocorrection context requests such that they can be resolved by an out-of-band IPC response
     23        (HandleAutocorrectionContext). Then prior to requesting DOM paste access, preemptively send a
     24        HandleAutocorrectionContext message to the UI process to unblock any pending synchronous autocorrection context
     25        requests.
     26
     27        * UIProcess/PageClient.h:
     28        * UIProcess/WebPageProxy.h:
     29        * UIProcess/WebPageProxy.messages.in:
     30        * UIProcess/ios/PageClientImplIOS.h:
     31        * UIProcess/ios/PageClientImplIOS.mm:
     32        (WebKit::PageClientImpl::handleAutocorrectionContext):
     33        * UIProcess/ios/WKContentViewInteraction.h:
     34
     35        Make it possible for WKContentView to remember its current pending autocorrection context completion handler.
     36        This is invoked and cleared out in `-_invokePendingAutocorrectionContextHandler:`.
     37
     38        * UIProcess/ios/WKContentViewInteraction.mm:
     39        (-[WKContentView _invokePendingAutocorrectionContextHandler:]):
     40        (-[WKContentView requestAutocorrectionContextWithCompletionHandler:]):
     41        (-[WKContentView _handleAutocorrectionContext:]):
     42
     43        Handle an autocorrection context response. This is invoked when the web process either handles an autocorrection
     44        context message, or when it preemptively sends an autocorrection context before requesting DOM paste access.
     45
     46        (+[WKAutocorrectionContext emptyAutocorrectionContext]):
     47
     48        Add a helper to create an empty autocorrection context.
     49
     50        * UIProcess/ios/WebPageProxyIOS.mm:
     51        (WebKit::WebPageProxy::requestAutocorrectionContext):
     52        (WebKit::WebPageProxy::handleAutocorrectionContext):
     53        (WebKit::WebPageProxy::autocorrectionContextSync): Deleted.
     54
     55        Removed this sync version, since we now only have requestAutocorrectionContext.
     56
     57        * WebProcess/WebPage/WebPage.cpp:
     58        (WebKit::WebPage::requestDOMPasteAccess):
     59
     60        Send WebPageProxy::HandleAutocorrectionContext, and add a FIXME referencing <rdar://problem/16207002>.
     61
     62        * WebProcess/WebPage/WebPage.h:
     63        * WebProcess/WebPage/WebPage.messages.in:
     64
     65        Split AutocorrectionContextSync into RequestAutocorrectionContext and HandleAutocorrectionContext; additionally,
     66        remove the existing RequestAutocorrectionContext message.
     67
     68        * WebProcess/WebPage/ios/WebPageIOS.mm:
     69        (WebKit::WebPage::requestAutocorrectionContext):
     70        (WebKit::WebPage::autocorrectionContextSync): Deleted.
     71
    1722019-03-06  Rob Buis  <rbuis@igalia.com>
    273

  • trunk/Source/WebKit/UIProcess/PageClient.h

    r242339 r242551  
    131131struct FocusedElementInformation;
    132132struct InteractionInformationAtPosition;
     133struct WebAutocorrectionContext;
    133134struct WebHitTestResultData;
    134135
    … …  
    395396    virtual void enableInspectorNodeSearch() = 0;
    396397    virtual void disableInspectorNodeSearch() = 0;
     398
     399    virtual void handleAutocorrectionContext(const WebAutocorrectionContext&) = 0;
    397400#endif
    398401

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

    r242400 r242551  
    651651    void applyAutocorrection(const String& correction, const String& originalText, WTF::Function<void (const String&, CallbackBase::Error)>&&);
    652652    bool applyAutocorrection(const String& correction, const String& originalText);
    653     void requestAutocorrectionContext(Function<void(const WebAutocorrectionContext&, CallbackBase::Error)>&&);
    654     WebAutocorrectionContext autocorrectionContextSync();
     653    void requestAutocorrectionContext();
     654    void handleAutocorrectionContext(const WebAutocorrectionContext&);
    655655    void requestDictationContext(WTF::Function<void (const String&, const String&, const String&, CallbackBase::Error)>&&);
    656656    void replaceDictatedText(const String& oldText, const String& newText);

  • trunk/Source/WebKit/UIProcess/WebPageProxy.messages.in

    r242317 r242551  
    418418
    419419    UpdateStringForFind(String findString)
     420    HandleAutocorrectionContext(struct WebKit::WebAutocorrectionContext context)
    420421#endif
    421422

  • trunk/Source/WebKit/UIProcess/ios/PageClientImplIOS.h

    r242317 r242551  
    237237#endif
    238238
     239    void handleAutocorrectionContext(const WebAutocorrectionContext&) final;
     240
    239241    void didFinishProcessingAllPendingMouseEvents() final { }
    240242

  • trunk/Source/WebKit/UIProcess/ios/PageClientImplIOS.mm

    r242317 r242551  
    858858#endif
    859859
     860void PageClientImpl::handleAutocorrectionContext(const WebAutocorrectionContext& context)
     861{
     862    [m_contentView _handleAutocorrectionContext:context];
     863}
     864
    860865} // namespace WebKit
    861866

  • trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h

    r242317 r242551  
    8686class WebOpenPanelResultListenerProxy;
    8787class WebPageProxy;
     88struct WebAutocorrectionContext;
    8889}
    8990
    … …  
    319320    BOOL _hasSetUpInteractions;
    320321    CompletionHandler<void(WebCore::DOMPasteAccessResponse)> _domPasteRequestHandler;
     322    BlockPtr<void(UIWKAutocorrectionContext *)> _pendingAutocorrectionContextHandler;
    321323
    322324#if ENABLE(DATA_INTERACTION)
    … …  
    466468#endif
    467469
     470- (void)_handleAutocorrectionContext:(const WebKit::WebAutocorrectionContext&)context;
     471
    468472@end
    469473

  • trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm

    r242400 r242551  
    277277
    278278@interface WKAutocorrectionContext : UIWKAutocorrectionContext
     279+ (WKAutocorrectionContext *)emptyAutocorrectionContext;
    279280+ (WKAutocorrectionContext *)autocorrectionContextWithContext:(const WebKit::WebAutocorrectionContext&)context;
    280281@end
    … …  
    35803581}
    35813582
     3583- (void)_invokePendingAutocorrectionContextHandler:(WKAutocorrectionContext *)context
     3584{
     3585    if (auto handler = WTFMove(_pendingAutocorrectionContextHandler))
     3586        handler(context);
     3587}
     3588
    35823589- (void)requestAutocorrectionContextWithCompletionHandler:(void (^)(UIWKAutocorrectionContext *autocorrectionContext))completionHandler
    35833590{
    … …  
    35893596#if USE(UIKIT_KEYBOARD_ADDITIONS)
    35903597    if ([self _disableAutomaticKeyboardUI]) {
    3591         completionHandler([WKAutocorrectionContext autocorrectionContextWithContext:(WebKit::WebAutocorrectionContext { })]);
     3598        completionHandler(WKAutocorrectionContext.emptyAutocorrectionContext);
    35923599        return;
    35933600    }
    … …  
    35973604    const bool useSyncRequest = true;
    35983605
     3606    [self _invokePendingAutocorrectionContextHandler:WKAutocorrectionContext.emptyAutocorrectionContext];
     3607
     3608    _pendingAutocorrectionContextHandler = completionHandler;
     3609    _page->requestAutocorrectionContext();
     3610
    35993611    if (useSyncRequest) {
    3600         completionHandler([WKAutocorrectionContext autocorrectionContextWithContext:_page->autocorrectionContextSync()]);
    3601         return;
    3602     }
    3603 
    3604     _page->requestAutocorrectionContext([protectedSelf = retainPtr(self), completion = makeBlockPtr(completionHandler)] (auto& context, auto) {
    3605         completion([WKAutocorrectionContext autocorrectionContextWithContext:context]);
    3606     });
     3612        _page->process().connection()->waitForAndDispatchImmediately<Messages::WebPageProxy::HandleAutocorrectionContext>(_page->pageID(), 1_s, IPC::WaitForOption::InterruptWaitingIfSyncMessageArrives);
     3613        [self _invokePendingAutocorrectionContextHandler:WKAutocorrectionContext.emptyAutocorrectionContext];
     3614        return;
     3615    }
     3616}
     3617
     3618- (void)_handleAutocorrectionContext:(const WebKit::WebAutocorrectionContext&)context
     3619{
     3620    [self _invokePendingAutocorrectionContextHandler:[WKAutocorrectionContext autocorrectionContextWithContext:context]];
    36073621}
    36083622
    … …  
    74117425@implementation WKAutocorrectionContext
    74127426
     7427+ (WKAutocorrectionContext *)emptyAutocorrectionContext
     7428{
     7429    return [self autocorrectionContextWithContext:WebKit::WebAutocorrectionContext { }];
     7430}
     7431
    74137432+ (WKAutocorrectionContext *)autocorrectionContextWithContext:(const WebKit::WebAutocorrectionContext&)webContext
    74147433{

  • trunk/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm

    r242400 r242551  
    564564}
    565565
    566 void WebPageProxy::requestAutocorrectionContext(Function<void(const WebAutocorrectionContext&, CallbackBase::Error)>&& callback)
    567 {
    568     if (!isValid()) {
    569         callback({ }, CallbackBase::Error::OwnerWasInvalidated);
    570         return;
    571     }
    572 
    573     auto callbackID = m_callbacks.put(WTFMove(callback), m_process->throttler().backgroundActivityToken());
    574     m_process->send(Messages::WebPage::RequestAutocorrectionContext(callbackID), m_pageID);
    575 }
    576 
    577 WebAutocorrectionContext WebPageProxy::autocorrectionContextSync()
    578 {
    579     WebAutocorrectionContext context;
    580     m_process->sendSync(Messages::WebPage::AutocorrectionContextSync(), Messages::WebPage::AutocorrectionContextSync::Reply(context), m_pageID);
    581     return context;
     566void WebPageProxy::requestAutocorrectionContext()
     567{
     568    m_process->send(Messages::WebPage::RequestAutocorrectionContext(), m_pageID);
     569}
     570
     571void WebPageProxy::handleAutocorrectionContext(const WebAutocorrectionContext& context)
     572{
     573    pageClient().handleAutocorrectionContext(context);
    582574}
    583575

  • trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp

    r242468 r242551  
    269269#include "InteractionInformationRequest.h"
    270270#include "RemoteLayerTreeDrawingArea.h"
     271#include "WebAutocorrectionContext.h"
    271272#include <CoreGraphics/CoreGraphics.h>
    272273#include <WebCore/Icon.h>
    … …  
    63746375{
    63756376    auto response = WebCore::DOMPasteAccessResponse::DeniedForGesture;
     6377#if PLATFORM(IOS_FAMILY)
     6378    // FIXME: Computing and sending an autocorrection context is a workaround for the fact that autocorrection context
     6379    // requests on iOS are currently synchronous in the web process. This allows us to immediately fulfill pending
     6380    // autocorrection context requests in the UI process on iOS before handling the DOM paste request. This workaround
     6381    // should be removed once <rdar://problem/16207002> is resolved.
     6382    send(Messages::WebPageProxy::HandleAutocorrectionContext(autocorrectionContext()));
     6383#endif
    63766384    sendSyncWithDelayedReply(Messages::WebPageProxy::RequestDOMPasteAccess(rectForElementAtInteractionLocation(), originIdentifier), Messages::WebPageProxy::RequestDOMPasteAccess::Reply(response));
    63776385    return response;

  • trunk/Source/WebKit/WebProcess/WebPage/WebPage.h

    r242468 r242551  
    642642    void applyAutocorrection(const String& correction, const String& originalText, CallbackID);
    643643    void syncApplyAutocorrection(const String& correction, const String& originalText, CompletionHandler<void(bool)>&&);
    644     void requestAutocorrectionContext(CallbackID);
    645     void autocorrectionContextSync(CompletionHandler<void(WebAutocorrectionContext&&)>&&);
     644    void requestAutocorrectionContext();
    646645    void getPositionInformation(const InteractionInformationRequest&, CompletionHandler<void(InteractionInformationAtPosition&&)>&&);
    647646    void requestPositionInformation(const InteractionInformationRequest&);

  • trunk/Source/WebKit/WebProcess/WebPage/WebPage.messages.in

    r242400 r242551  
    7979    ApplyAutocorrection(String correction, String originalText, WebKit::CallbackID callbackID)
    8080    SyncApplyAutocorrection(String correction, String originalText) -> (bool autocorrectionApplied) Delayed
    81     RequestAutocorrectionContext(WebKit::CallbackID callbackID)
    82     AutocorrectionContextSync() -> (struct WebKit::WebAutocorrectionContext context) Delayed
     81    RequestAutocorrectionContext()
    8382    RequestEvasionRectsAboveSelection() -> (Vector<WebCore::FloatRect> rects) Async
    8483    GetPositionInformation(struct WebKit::InteractionInformationRequest request) -> (struct WebKit::InteractionInformationAtPosition information) Delayed

  • trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

    r242467 r242551  
    21582158}
    21592159
    2160 void WebPage::requestAutocorrectionContext(CallbackID callbackID)
    2161 {
    2162     send(Messages::WebPageProxy::AutocorrectionContextCallback(autocorrectionContext(), callbackID));
    2163 }
    2164 
    2165 void WebPage::autocorrectionContextSync(CompletionHandler<void(WebAutocorrectionContext&&)>&& completionHandler)
    2166 {
    2167     completionHandler(autocorrectionContext());
     2160void WebPage::requestAutocorrectionContext()
     2161{
     2162    send(Messages::WebPageProxy::HandleAutocorrectionContext(autocorrectionContext()));
    21682163}
    21692164
Note: See TracChangeset for help on using the changeset viewer.