Title: [218672] trunk
Revision
218672
Author
[email protected]
Date
2017-06-21 18:41:36 -0700 (Wed, 21 Jun 2017)

Log Message

[iOS DnD] [WK2] Cancelling a drag interaction using the ObjC SPI causes subsequent dragging to fail
https://bugs.webkit.org/show_bug.cgi?id=173659
<rdar://problem/32879788>

Reviewed by Tim Horton.

Source/WebKit2:

Currently, a drag session that failed to begin is cleaned up either when the web process responds in
-[WKContentView _didHandleStartDataInteractionRequest:] with started = NO, or if started is YES, in
-dragInteraction:session:didEndWithOperation: instead. However, it is possible that even when the pasteboard has
items and the drag has started in the web process, something in the UI process may then cause the drag to be
cancelled (for instance, an SPI client returning empty arrays for all adjusted item providers, or UIKit not even
calling into -dragInteraction:itemsForBeginningSession: when we invoke their completion block).

In any case, if the drag session is drag-item-less after calling the drag start completion block, clean up and
cancel the drag.

* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView _didHandleStartDataInteractionRequest:]):
(-[WKContentView dragInteraction:itemsForBeginningSession:]):

Tools:

Adds a new test to verify that a lift cancelled within the UI process does not cause subsequent dragging to fail.

* TestWebKitAPI/Tests/WebKit2Cocoa/link-and-target-div.html:

Augment this test page to log dragend and dragstart events on the drag source as well.

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

Modified Paths

Diff

Modified: trunk/Source/WebKit2/ChangeLog (218671 => 218672)


--- trunk/Source/WebKit2/ChangeLog	2017-06-22 01:25:50 UTC (rev 218671)
+++ trunk/Source/WebKit2/ChangeLog	2017-06-22 01:41:36 UTC (rev 218672)
@@ -1,3 +1,25 @@
+2017-06-21  Wenson Hsieh  <[email protected]>
+
+        [iOS DnD] [WK2] Cancelling a drag interaction using the ObjC SPI causes subsequent dragging to fail
+        https://bugs.webkit.org/show_bug.cgi?id=173659
+        <rdar://problem/32879788>
+
+        Reviewed by Tim Horton.
+
+        Currently, a drag session that failed to begin is cleaned up either when the web process responds in
+        -[WKContentView _didHandleStartDataInteractionRequest:] with started = NO, or if started is YES, in
+        -dragInteraction:session:didEndWithOperation: instead. However, it is possible that even when the pasteboard has
+        items and the drag has started in the web process, something in the UI process may then cause the drag to be
+        cancelled (for instance, an SPI client returning empty arrays for all adjusted item providers, or UIKit not even
+        calling into -dragInteraction:itemsForBeginningSession: when we invoke their completion block).
+
+        In any case, if the drag session is drag-item-less after calling the drag start completion block, clean up and
+        cancel the drag.
+
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView _didHandleStartDataInteractionRequest:]):
+        (-[WKContentView dragInteraction:itemsForBeginningSession:]):
+
 2017-06-21  Tim Horton  <[email protected]>
 
         REGRESSION (r218606): 3D Touch action menu for links is missing items

Modified: trunk/Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm (218671 => 218672)


--- trunk/Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm	2017-06-22 01:25:50 UTC (rev 218671)
+++ trunk/Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm	2017-06-22 01:41:36 UTC (rev 218672)
@@ -4232,16 +4232,19 @@
     _dataInteractionState.dragStartCompletionBlock = nil;
     ASSERT(savedCompletionBlock);
 
-    if (!started) {
-        // The web process rejected the drag start request, so don't go through with the drag.
-        // By clearing out the source session state, we force -itemsForDragInteraction:session: to return an empty array, which
-        // causes UIKit to bail before beginning the lift animation.
-        [self cleanUpDragSourceSessionState];
-    }
-
     RELEASE_LOG(DragAndDrop, "Handling drag start request (started: %d, completion block: %p)", started, savedCompletionBlock.get());
     if (savedCompletionBlock)
         savedCompletionBlock();
+
+    if (![_dataInteractionState.dragSession items].count) {
+        CGPoint adjustedOrigin = _dataInteractionState.adjustedOrigin;
+        [self cleanUpDragSourceSessionState];
+        if (started) {
+            // A client of the Objective C SPI or UIKit might have prevented the drag from beginning entirely in the UI process, in which case
+            // we need to balance the `dragstart` event with a `dragend`.
+            _page->dragEnded(roundedIntPoint(adjustedOrigin), roundedIntPoint([self convertPoint:adjustedOrigin toView:self.window]), DragOperationNone);
+        }
+    }
 }
 
 static RetainPtr<UIImage> uiImageForImage(RefPtr<Image> image)
@@ -4551,6 +4554,11 @@
         return @[ ];
     }
 
+    if (_dataInteractionState.sourceAction == DragSourceActionNone) {
+        RELEASE_LOG(DragAndDrop, "Drag session failed: %p (no drag source action)", session);
+        return @[ ];
+    }
+
     WebItemProviderPasteboard *draggingPasteboard = [WebItemProviderPasteboard sharedInstance];
     ASSERT(interaction == _dataInteraction);
     NSUInteger numberOfItems = draggingPasteboard.numberOfItems;

Modified: trunk/Tools/ChangeLog (218671 => 218672)


--- trunk/Tools/ChangeLog	2017-06-22 01:25:50 UTC (rev 218671)
+++ trunk/Tools/ChangeLog	2017-06-22 01:41:36 UTC (rev 218672)
@@ -1,3 +1,21 @@
+2017-06-21  Wenson Hsieh  <[email protected]>
+
+        [iOS DnD] [WK2] Cancelling a drag interaction using the ObjC SPI causes subsequent dragging to fail
+        https://bugs.webkit.org/show_bug.cgi?id=173659
+        <rdar://problem/32879788>
+
+        Reviewed by Tim Horton.
+
+        Adds a new test to verify that a lift cancelled within the UI process does not cause subsequent dragging to fail.
+
+        * TestWebKitAPI/Tests/WebKit2Cocoa/link-and-target-div.html:
+
+        Augment this test page to log dragend and dragstart events on the drag source as well.
+
+        * TestWebKitAPI/Tests/ios/DataInteractionTests.mm:
+        (checkStringArraysAreEqual):
+        (TestWebKitAPI::TEST):
+
 2017-06-21  Antoine Quint  <[email protected]>
 
         Ensure DRT always logs rAF suspension debugging code

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/link-and-target-div.html (218671 => 218672)


--- trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/link-and-target-div.html	2017-06-22 01:25:50 UTC (rev 218671)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/link-and-target-div.html	2017-06-22 01:41:36 UTC (rev 218672)
@@ -21,9 +21,12 @@
     color: green;
 }
 </style>
-<div><a href="" world</a></div>
+<div><a id="source" href="" world</a></div>
 <div id="target"></div>
+<div id="output"></div>
 <script>
+source.addEventListener("dragstart", event => logOutput("dragstart"));
+source.addEventListener("dragend", event => logOutput("dragend"));
 target.addEventListener("dragenter", event => event.preventDefault());
 target.addEventListener("dragover", event => event.preventDefault());
 target.addEventListener("drop", event => {
@@ -30,4 +33,10 @@
     target.innerHTML = "<code>PASS</code>";
     event.preventDefault();
 });
+function logOutput(message)
+{
+    if (output.innerHTML.length)
+        message = ` ${message}`;
+    output.insertAdjacentHTML("beforeend", `<code>${message}</code>`);
+}
 </script>

Modified: trunk/Tools/TestWebKitAPI/Tests/ios/DataInteractionTests.mm (218671 => 218672)


--- trunk/Tools/TestWebKitAPI/Tests/ios/DataInteractionTests.mm	2017-06-22 01:25:50 UTC (rev 218671)
+++ trunk/Tools/TestWebKitAPI/Tests/ios/DataInteractionTests.mm	2017-06-22 01:41:36 UTC (rev 218672)
@@ -120,6 +120,18 @@
     EXPECT_EQ(estimatedSize.height, sourceItemProvider.estimatedDisplayedSize.height);
 }
 
+static void checkStringArraysAreEqual(NSArray<NSString *> *expected, NSArray<NSString *> *observed)
+{
+    EXPECT_EQ(expected.count, observed.count);
+    for (NSUInteger index = 0; index < expected.count; ++index) {
+        NSString *expectedString = [expected objectAtIndex:index];
+        NSString *observedString = [observed objectAtIndex:index];
+        EXPECT_WK_STREQ(expectedString, observedString);
+        if (![expectedString isEqualToString:observedString])
+            NSLog(@"Expected observed string: %@ to match expected string: %@ at index: %tu", observedString, expectedString, index);
+    }
+}
+
 namespace TestWebKitAPI {
 
 TEST(DataInteractionTests, ImageToContentEditable)
@@ -929,6 +941,33 @@
     EXPECT_WK_STREQ("", [webView editorValue].UTF8String);
 }
 
+TEST(DataInteractionTests, CancelledLiftDoesNotCauseSubsequentDragsToFail)
+{
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
+    [webView synchronouslyLoadTestPageNamed:@"link-and-target-div"];
+
+    auto dataInteractionSimulator = adoptNS([[DataInteractionSimulator alloc] initWithWebView:webView.get()]);
+    [dataInteractionSimulator setConvertItemProvidersBlock:^NSArray *(UIItemProvider *, NSArray *, NSDictionary *)
+    {
+        return @[ ];
+    }];
+    [dataInteractionSimulator runFrom:CGPointMake(100, 50) to:CGPointMake(100, 300)];
+    EXPECT_EQ(DataInteractionCancelled, [dataInteractionSimulator phase]);
+    EXPECT_WK_STREQ("", [webView stringByEvaluatingJavaScript:@"target.textContent"]);
+    NSString *outputText = [webView stringByEvaluatingJavaScript:@"output.textContent"];
+    checkStringArraysAreEqual(@[@"dragstart", @"dragend"], [outputText componentsSeparatedByString:@" "]);
+
+    [webView stringByEvaluatingJavaScript:@"output.innerHTML = ''"];
+    [dataInteractionSimulator setConvertItemProvidersBlock:^NSArray *(UIItemProvider *itemProvider, NSArray *, NSDictionary *)
+    {
+        return @[ itemProvider ];
+    }];
+    [dataInteractionSimulator runFrom:CGPointMake(100, 50) to:CGPointMake(100, 300)];
+    EXPECT_WK_STREQ("PASS", [webView stringByEvaluatingJavaScript:@"target.textContent"]);
+    [webView stringByEvaluatingJavaScript:@"output.textContent"];
+    checkStringArraysAreEqual(@[@"dragstart", @"dragend"], [outputText componentsSeparatedByString:@" "]);
+}
+
 TEST(DataInteractionTests, CustomActionSheetPopover)
 {
     RetainPtr<TestWKWebView> webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to