Title: [215171] trunk
Revision
215171
Author
wenson_hs...@apple.com
Date
2017-04-09 23:25:22 -0700 (Sun, 09 Apr 2017)

Log Message

[WK2] Add infrastructure to perform actions after an asynchronous position information request finishes
https://bugs.webkit.org/show_bug.cgi?id=170658
<rdar://problem/31431450>

Reviewed by Tim Horton.

Source/WebCore:

Minor adjustments to fix the build in the newest version of the SDK.

* platform/ios/WebItemProviderPasteboard.mm:
(-[WebItemProviderPasteboard setItemsFromObjectRepresentations:]):
(-[WebItemProviderPasteboard _tryToCreateAndAppendObjectOfClass:toArray:usingProvider:]):

Source/WebKit2:

Introduces doAfterPositionInformationUpdate:forRequest:, which WKContentView can use internally to perform
an action requiring InteractionInformationAtPosition asynchronously. See below for additional details.

New API unit test: DataInteractionTests.UnresponsivePageDoesNotHangUI.

* UIProcess/ios/WKContentViewInteraction.h:
* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView _actionForLongPressFromPositionInformation:]):
(-[WKContentView _actionForLongPress]):
(-[WKContentView doAfterPositionInformationUpdate:forRequest:]):

If current position information satisfies the request, then perform the given block immediately; otherwise, we
save the block and request as a pair in a list of pending position information handlers, and then make a new
request to the web process for position information if the currently outgoing request does not already satisfy
the given request.

(-[WKContentView ensurePositionInformationIsUpToDate:]):

Fire all asynchronous position information handlers that are satisfied by the incoming position information.

(-[WKContentView requestAsynchronousPositionInformationUpdate:]):
(-[WKContentView _currentPositionInformationIsValidForRequest:]):
(-[WKContentView _hasValidOutstandingPositionInformationRequest:]):

Pulled out common logic for managing InteractionInformationRequests into separate helper methods.

(-[WKContentView _invokeAndRemovePendingHandlersValidForCurrentPositionInformation]):
(-[WKContentView gestureRecognizerShouldBegin:]):

Refactored to use ensurePositionInformationIsUpToDate:. There is no behavior change here, but it makes sure that
this particular request will also any valid pending position information handlers.

(-[WKContentView _positionInformationDidChange:]):

Fire all asynchronous position information handlers that are satisfied by the incoming position information.

(-[WKContentView pointIsInDataInteractionContent:]): Deleted.

We should no longer use this method, since it makes a synchronous request to the web process -- see
positionInformationMayStartDataInteraction.

Tools:

Adds a new test verifying that when a web page is unresponsive, the process of preparing for data interaction
does not also cause the UI process to spin. We assume here that the call to prepare must be asynchronous, so it
should complete before the unresponsiveness timeout is triggered.

See WebKit2 ChangeLog for more details.

* TestWebKitAPI/Tests/ios/DataInteractionTests.mm:
(TestWebKitAPI::TEST):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (215170 => 215171)


--- trunk/Source/WebCore/ChangeLog	2017-04-10 05:26:44 UTC (rev 215170)
+++ trunk/Source/WebCore/ChangeLog	2017-04-10 06:25:22 UTC (rev 215171)
@@ -1,3 +1,17 @@
+2017-04-09  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        [WK2] Add infrastructure to perform actions after an asynchronous position information request finishes
+        https://bugs.webkit.org/show_bug.cgi?id=170658
+        <rdar://problem/31431450>
+
+        Reviewed by Tim Horton.
+
+        Minor adjustments to fix the build in the newest version of the SDK.
+
+        * platform/ios/WebItemProviderPasteboard.mm:
+        (-[WebItemProviderPasteboard setItemsFromObjectRepresentations:]):
+        (-[WebItemProviderPasteboard _tryToCreateAndAppendObjectOfClass:toArray:usingProvider:]):
+
 2017-04-09  Yusuke Suzuki  <utatane....@gmail.com>
 
         [WTF] Annotate Seconds' member functions and operators with constexpr

Modified: trunk/Source/WebCore/platform/ios/WebItemProviderPasteboard.mm (215170 => 215171)


--- trunk/Source/WebCore/platform/ios/WebItemProviderPasteboard.mm	2017-04-10 05:26:44 UTC (rev 215170)
+++ trunk/Source/WebCore/platform/ios/WebItemProviderPasteboard.mm	2017-04-10 06:25:22 UTC (rev 215171)
@@ -151,6 +151,8 @@
         if (!data.representingObjects.count && !data.additionalData.count)
             continue;
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
         RetainPtr<UIItemProvider> itemProvider = adoptNS([[getUIItemProviderClass() alloc] init]);
         // First, register all platform objects, prioritizing objects at the beginning of the array.
         for (id representingObject in data.representingObjects) {
@@ -173,6 +175,7 @@
             }];
         }
         [providers addObject:itemProvider.get()];
+#pragma clang diagnostic pop
     }
 
     self.itemProviders = providers;
@@ -229,12 +232,12 @@
 
 - (BOOL)_tryToCreateAndAppendObjectOfClass:(Class)objectClass toArray:(NSMutableArray *)array usingProvider:(UIItemProvider *)provider
 {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
     if (![provider canCreateObjectOfClass:objectClass])
         return NO;
 
     // FIXME: <rdar://problem/30451096> Adopt asynchronous UIItemProvider methods when retrieving data.
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wdeprecated-declarations"
     id object = [provider createObjectOfClass:objectClass error:nil];
 #pragma clang diagnostic pop
     if (object)

Modified: trunk/Source/WebKit2/ChangeLog (215170 => 215171)


--- trunk/Source/WebKit2/ChangeLog	2017-04-10 05:26:44 UTC (rev 215170)
+++ trunk/Source/WebKit2/ChangeLog	2017-04-10 06:25:22 UTC (rev 215171)
@@ -1,3 +1,52 @@
+2017-04-09  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        [WK2] Add infrastructure to perform actions after an asynchronous position information request finishes
+        https://bugs.webkit.org/show_bug.cgi?id=170658
+        <rdar://problem/31431450>
+
+        Reviewed by Tim Horton.
+
+        Introduces doAfterPositionInformationUpdate:forRequest:, which WKContentView can use internally to perform
+        an action requiring InteractionInformationAtPosition asynchronously. See below for additional details.
+
+        New API unit test: DataInteractionTests.UnresponsivePageDoesNotHangUI.
+
+        * UIProcess/ios/WKContentViewInteraction.h:
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView _actionForLongPressFromPositionInformation:]):
+        (-[WKContentView _actionForLongPress]):
+        (-[WKContentView doAfterPositionInformationUpdate:forRequest:]):
+
+        If current position information satisfies the request, then perform the given block immediately; otherwise, we
+        save the block and request as a pair in a list of pending position information handlers, and then make a new
+        request to the web process for position information if the currently outgoing request does not already satisfy
+        the given request.
+
+        (-[WKContentView ensurePositionInformationIsUpToDate:]):
+
+        Fire all asynchronous position information handlers that are satisfied by the incoming position information.
+
+        (-[WKContentView requestAsynchronousPositionInformationUpdate:]):
+        (-[WKContentView _currentPositionInformationIsValidForRequest:]):
+        (-[WKContentView _hasValidOutstandingPositionInformationRequest:]):
+
+        Pulled out common logic for managing InteractionInformationRequests into separate helper methods.
+
+        (-[WKContentView _invokeAndRemovePendingHandlersValidForCurrentPositionInformation]):
+        (-[WKContentView gestureRecognizerShouldBegin:]):
+
+        Refactored to use ensurePositionInformationIsUpToDate:. There is no behavior change here, but it makes sure that
+        this particular request will also any valid pending position information handlers.
+
+        (-[WKContentView _positionInformationDidChange:]):
+
+        Fire all asynchronous position information handlers that are satisfied by the incoming position information.
+
+        (-[WKContentView pointIsInDataInteractionContent:]): Deleted.
+
+        We should no longer use this method, since it makes a synchronous request to the web process -- see
+        positionInformationMayStartDataInteraction.
+
 2017-04-09  Chris Dumez  <cdu...@apple.com>
 
         Drop Timer::startRepeating() overload taking a double

Modified: trunk/Source/WebKit2/UIProcess/ios/WKContentViewInteraction.h (215170 => 215171)


--- trunk/Source/WebKit2/UIProcess/ios/WKContentViewInteraction.h	2017-04-10 05:26:44 UTC (rev 215170)
+++ trunk/Source/WebKit2/UIProcess/ios/WKContentViewInteraction.h	2017-04-10 06:25:22 UTC (rev 215171)
@@ -40,6 +40,7 @@
 #import <UIKit/UIView.h>
 #import <WebCore/Color.h>
 #import <WebCore/FloatQuad.h>
+#import <wtf/BlockPtr.h>
 #import <wtf/Forward.h>
 #import <wtf/Vector.h>
 #import <wtf/text/WTFString.h>
@@ -85,6 +86,8 @@
 typedef void (^UIWKSelectionWithDirectionCompletionHandler)(BOOL selectionEndIsMoving);
 typedef void (^UIWKKeyWebEventCompletionHandler)(::WebEvent *theEvent, BOOL wasHandled);
 
+typedef BlockPtr<void(WebKit::InteractionInformationAtPosition)> InteractionInformationCallback;
+
 namespace WebKit {
 
 struct WKSelectionDrawingInfo {
@@ -171,6 +174,7 @@
     WebKit::WKSelectionDrawingInfo _lastSelectionDrawingInfo;
 
     std::optional<WebKit::InteractionInformationRequest> _outstandingPositionInformationRequest;
+    Vector<std::pair<WebKit::InteractionInformationRequest, InteractionInformationCallback>> _pendingPositionInformationHandlers;
     
     std::unique_ptr<WebKit::InputViewUpdateDeferrer> _inputViewUpdateDeferrer;
 

Modified: trunk/Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm (215170 => 215171)


--- trunk/Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm	2017-04-10 05:26:44 UTC (rev 215170)
+++ trunk/Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm	2017-04-10 06:25:22 UTC (rev 215171)
@@ -1281,33 +1281,51 @@
     [_actionSheetAssistant showDataDetectorsSheet];
 }
 
-- (SEL)_actionForLongPress
+- (SEL)_actionForLongPressFromPositionInformation:(const InteractionInformationAtPosition&)positionInformation
 {
-    if (!_positionInformation.touchCalloutEnabled)
+    if (!positionInformation.touchCalloutEnabled)
         return nil;
 
-    if (_positionInformation.isImage)
+    if (positionInformation.isImage)
         return @selector(_showImageSheet);
 
-    if (_positionInformation.isLink) {
-        NSURL *targetURL = [NSURL URLWithString:_positionInformation.url];
+    if (positionInformation.isLink) {
+        NSURL *targetURL = [NSURL URLWithString:positionInformation.url];
         if ([[getDDDetectionControllerClass() tapAndHoldSchemes] containsObject:targetURL.scheme.lowercaseString])
             return @selector(_showDataDetectorsSheet);
         return @selector(_showLinkSheet);
     }
-    
-    if (_positionInformation.isAttachment)
+    if (positionInformation.isAttachment)
         return @selector(_showAttachmentSheet);
 
     return nil;
 }
 
+- (SEL)_actionForLongPress
+{
+    return [self _actionForLongPressFromPositionInformation:_positionInformation];
+}
+
+- (void)doAfterPositionInformationUpdate:(void (^)(InteractionInformationAtPosition))action forRequest:(InteractionInformationRequest)request
+{
+    if ([self _currentPositionInformationIsValidForRequest:request]) {
+        // If the most recent position information is already valid, invoke the given action block immediately.
+        action(_positionInformation);
+        return;
+    }
+
+    _pendingPositionInformationHandlers.append({ request, action });
+
+    if (![self _hasValidOutstandingPositionInformationRequest:request])
+        [self requestAsynchronousPositionInformationUpdate:request];
+}
+
 - (void)ensurePositionInformationIsUpToDate:(WebKit::InteractionInformationRequest)request
 {
-    if (_hasValidPositionInformation && _positionInformation.request.isValidForRequest(request))
+    if ([self _currentPositionInformationIsValidForRequest:request])
         return;
 
-    if (_outstandingPositionInformationRequest && _outstandingPositionInformationRequest->isValidForRequest(request)) {
+    if ([self _hasValidOutstandingPositionInformationRequest:request]) {
         if (auto* connection = _page->process().connection()) {
             connection->waitForAndDispatchImmediately<Messages::WebPageProxy::DidReceivePositionInformation>(_page->pageID(), Seconds::infinity());
             return;
@@ -1316,11 +1334,12 @@
 
     _page->getPositionInformation(request, _positionInformation);
     _hasValidPositionInformation = YES;
+    [self _invokeAndRemovePendingHandlersValidForCurrentPositionInformation];
 }
 
 - (void)requestAsynchronousPositionInformationUpdate:(WebKit::InteractionInformationRequest)request
 {
-    if (_hasValidPositionInformation && _positionInformation.request.isValidForRequest(request))
+    if ([self _currentPositionInformationIsValidForRequest:request])
         return;
 
     _outstandingPositionInformationRequest = request;
@@ -1328,6 +1347,36 @@
     _page->requestPositionInformation(request);
 }
 
+- (BOOL)_currentPositionInformationIsValidForRequest:(const InteractionInformationRequest&)request
+{
+    return _hasValidPositionInformation && _positionInformation.request.isValidForRequest(request);
+}
+
+- (BOOL)_hasValidOutstandingPositionInformationRequest:(const InteractionInformationRequest&)request
+{
+    return _outstandingPositionInformationRequest && _outstandingPositionInformationRequest->isValidForRequest(request);
+}
+
+- (void)_invokeAndRemovePendingHandlersValidForCurrentPositionInformation
+{
+    ASSERT(_hasValidPositionInformation);
+
+    Vector<size_t> indicesOfHandledRequests;
+    for (size_t index = 0; index < _pendingPositionInformationHandlers.size(); ++index) {
+        auto& requestAndHandler = _pendingPositionInformationHandlers[index];
+        if (![self _currentPositionInformationIsValidForRequest:requestAndHandler.first])
+            continue;
+
+        if (requestAndHandler.second)
+            requestAndHandler.second(_positionInformation);
+
+        indicesOfHandledRequests.append(index);
+    }
+
+    while (indicesOfHandledRequests.size())
+        _pendingPositionInformationHandlers.remove(indicesOfHandledRequests.takeLast());
+}
+
 #if ENABLE(DATA_DETECTION)
 - (NSArray *)_dataDetectionResults
 {
@@ -1363,9 +1412,7 @@
         if (_textSelectionAssistant) {
             // Request information about the position with sync message.
             // If the assisted node is the same, prevent the gesture.
-            InteractionInformationRequest request(roundedIntPoint(point));
-            _page->getPositionInformation(request, _positionInformation);
-            _hasValidPositionInformation = YES;
+            [self ensurePositionInformationIsUpToDate:InteractionInformationRequest(roundedIntPoint(point))];
             if (_positionInformation.nodeAtPositionIsAssistedNode)
                 return NO;
         }
@@ -1457,21 +1504,6 @@
     return _positionInformation.isSelectable;
 }
 
-#if ENABLE(DATA_INTERACTION)
-
-- (BOOL)pointIsInDataInteractionContent:(CGPoint)point
-{
-    InteractionInformationRequest request(roundedIntPoint(point));
-    [self ensurePositionInformationIsUpToDate:request];
-
-    if (_positionInformation.isImage || _positionInformation.isLink || _positionInformation.isAttachment)
-        return YES;
-
-    return _positionInformation.hasSelectionAtPosition;
-}
-
-#endif
-
 - (BOOL)pointIsNearMarkedText:(CGPoint)point
 {
     InteractionInformationRequest request(roundedIntPoint(point));
@@ -1771,6 +1803,7 @@
     _hasValidPositionInformation = YES;
     if (_actionSheetAssistant)
         [_actionSheetAssistant updateSheetPosition];
+    [self _invokeAndRemovePendingHandlersValidForCurrentPositionInformation];
 }
 
 - (void)_willStartScrollingOrZooming

Modified: trunk/Tools/ChangeLog (215170 => 215171)


--- trunk/Tools/ChangeLog	2017-04-10 05:26:44 UTC (rev 215170)
+++ trunk/Tools/ChangeLog	2017-04-10 06:25:22 UTC (rev 215171)
@@ -1,3 +1,20 @@
+2017-04-09  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        [WK2] Add infrastructure to perform actions after an asynchronous position information request finishes
+        https://bugs.webkit.org/show_bug.cgi?id=170658
+        <rdar://problem/31431450>
+
+        Reviewed by Tim Horton.
+
+        Adds a new test verifying that when a web page is unresponsive, the process of preparing for data interaction
+        does not also cause the UI process to spin. We assume here that the call to prepare must be asynchronous, so it
+        should complete before the unresponsiveness timeout is triggered.
+
+        See WebKit2 ChangeLog for more details.
+
+        * TestWebKitAPI/Tests/ios/DataInteractionTests.mm:
+        (TestWebKitAPI::TEST):
+
 2017-04-09  Chris Dumez  <cdu...@apple.com>
 
         Drop Timer::startRepeating() overload taking a double

Modified: trunk/Tools/TestWebKitAPI/Tests/ios/DataInteractionTests.mm (215170 => 215171)


--- trunk/Tools/TestWebKitAPI/Tests/ios/DataInteractionTests.mm	2017-04-10 05:26:44 UTC (rev 215170)
+++ trunk/Tools/TestWebKitAPI/Tests/ios/DataInteractionTests.mm	2017-04-10 06:25:22 UTC (rev 215171)
@@ -362,6 +362,18 @@
     EXPECT_WK_STREQ("PASS", [webView stringByEvaluatingJavaScript:@"target.textContent"].UTF8String);
 }
 
+TEST(DataInteractionTests, UnresponsivePageDoesNotHangUI)
+{
+    RetainPtr<TestWKWebView> webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
+    [webView synchronouslyLoadTestPageNamed:@"simple"];
+    [webView evaluateJavaScript:@"while(1);" completionHandler:nil];
+
+    NSTimeInterval startTime = [NSDate timeIntervalSinceReferenceDate];
+    [webView _simulatePrepareForDataInteractionSession:nil completion:^() { }];
+
+    EXPECT_LT([NSDate timeIntervalSinceReferenceDate] - startTime, 1);
+}
+
 } // namespace TestWebKitAPI
 
 #endif // ENABLE(DATA_INTERACTION)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to