When dragging a selection, clearing the selection in dragstart should not crash the...
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Jul 2017 20:32:41 +0000 (20:32 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Jul 2017 20:32:41 +0000 (20:32 +0000)
https://bugs.webkit.org/show_bug.cgi?id=174142
<rdar://problem/33067501>

Reviewed by Tim Horton.

Source/WebCore:

Currenly, if the page clears the current selection after dragging starts on selected content, the web process
will crash while attempting to write pasteboard data for a nonexistent selection. This patch adds a trivial
check for this case, bailing if no DHTML dragging data was specified by the page during a selection drag and the
selection has been cleared.

Also removes some unused code for estimating the bounds of the current selection. On iOS, dragging was actually
crashing earlier, in this codepath. However, this information isn't even used anymore, since the drag anchor
point is no longer necessary on iOS.

Test: DataInteractionTests.DoNotCrashWhenSelectionIsClearedInDragStart

* page/DragController.cpp:
(WebCore::DragController::startDrag):

Tools:

Adds a unit test checking that the web process does not crash when the selection is cleared while a selection
drag is starting up.

* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/Tests/WebKit2Cocoa/dragstart-clear-selection.html: Added.
* TestWebKitAPI/Tests/ios/DataInteractionTests.mm:
(TestWebKitAPI::TEST):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@219153 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/page/DragController.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
Tools/TestWebKitAPI/Tests/WebKit2Cocoa/dragstart-clear-selection.html [new file with mode: 0644]
Tools/TestWebKitAPI/Tests/ios/DataInteractionTests.mm

index 54de44ee739afbdcd5224e9a6390508754f0337b..7e9833bcbda3ae0b5f49852073db6e165bb9ae61 100644 (file)
@@ -1,3 +1,25 @@
+2017-07-05  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        When dragging a selection, clearing the selection in dragstart should not crash the web process
+        https://bugs.webkit.org/show_bug.cgi?id=174142
+        <rdar://problem/33067501>
+
+        Reviewed by Tim Horton.
+
+        Currenly, if the page clears the current selection after dragging starts on selected content, the web process
+        will crash while attempting to write pasteboard data for a nonexistent selection. This patch adds a trivial
+        check for this case, bailing if no DHTML dragging data was specified by the page during a selection drag and the
+        selection has been cleared.
+
+        Also removes some unused code for estimating the bounds of the current selection. On iOS, dragging was actually
+        crashing earlier, in this codepath. However, this information isn't even used anymore, since the drag anchor
+        point is no longer necessary on iOS.
+
+        Test: DataInteractionTests.DoNotCrashWhenSelectionIsClearedInDragStart
+
+        * page/DragController.cpp:
+        (WebCore::DragController::startDrag):
+
 2017-07-05  Simon Fraser  <simon.fraser@apple.com>
 
         Try to fix iOS 10.3 public SDK builds.
index a85e7ae2bb61ea6a0ddd6ebbd2eac82b7c30c8d4..d73d384c6133656e7815c97491bbe38038dd5403 100644 (file)
@@ -924,18 +924,16 @@ bool DragController::startDrag(Frame& src, const DragState& state, DragOperation
         PasteboardWriterData pasteboardWriterData;
 
         if (!dataTransfer.pasteboard().hasData()) {
+            if (src.selection().selection().isNone()) {
+                // The page may have cleared out the selection in the dragstart handler, in which case we should bail
+                // out of the drag, since there is no content to write to the pasteboard.
+                return false;
+            }
+
             // FIXME: This entire block is almost identical to the code in Editor::copy, and the code should be shared.
             RefPtr<Range> selectionRange = src.selection().toNormalizedRange();
             ASSERT(selectionRange);
 
-#if ENABLE(DATA_INTERACTION)
-            Vector<SelectionRect> selectionRects;
-            selectionRange->collectSelectionRects(selectionRects);
-            for (auto selectionRect : selectionRects)
-                dragImageBounds.unite(selectionRect.rect());
-            dragImageBounds.inflate(SelectionDragImagePadding);
-#endif
-
             src.editor().willWriteSelectionToPasteboard(selectionRange.get());
 
             if (enclosingTextFormControl(src.selection().selection().start())) {
index 45aab8d51b36e25e8bf51e5121ecbab0c461dfab..a96bb14cd5bdf2f46a1ca820b32f83c8a40e1b55 100644 (file)
@@ -1,3 +1,19 @@
+2017-07-05  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        When dragging a selection, clearing the selection in dragstart should not crash the web process
+        https://bugs.webkit.org/show_bug.cgi?id=174142
+        <rdar://problem/33067501>
+
+        Reviewed by Tim Horton.
+
+        Adds a unit test checking that the web process does not crash when the selection is cleared while a selection
+        drag is starting up.
+
+        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+        * TestWebKitAPI/Tests/WebKit2Cocoa/dragstart-clear-selection.html: Added.
+        * TestWebKitAPI/Tests/ios/DataInteractionTests.mm:
+        (TestWebKitAPI::TEST):
+
 2017-07-05  Daniel Bates  <dabates@apple.com>
 
         Do not pass API::FrameInfo for source frame or clear out page of target frame on
index 47644a400621fb8bdaab40123fcf0bc39b261fcb..ef23767dd923f741c78c5f8599c5d8e6e74f2f6c 100644 (file)
                F4C2AB221DD6D95E00E06D5B /* enormous-video-with-sound.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F4C2AB211DD6D94100E06D5B /* enormous-video-with-sound.html */; };
                F4D4F3B61E4E2BCB00BB2767 /* DataInteractionSimulator.mm in Sources */ = {isa = PBXBuildFile; fileRef = F4D4F3B41E4E2BCB00BB2767 /* DataInteractionSimulator.mm */; };
                F4D4F3B91E4E36E400BB2767 /* DataInteractionTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = F4D4F3B71E4E36E400BB2767 /* DataInteractionTests.mm */; };
+               F4D5E4E81F0C5D38008C1A49 /* dragstart-clear-selection.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F4D5E4E71F0C5D27008C1A49 /* dragstart-clear-selection.html */; };
                F4D7BCD81EA5789800C421D3 /* PositionInformationTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = F4D7BCD61EA574DD00C421D3 /* PositionInformationTests.mm */; };
                F4DEF6ED1E9B4DB60048EF61 /* image-in-link-and-input.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F4DEF6EC1E9B4D950048EF61 /* image-in-link-and-input.html */; };
                F4F137921D9B683E002BEC57 /* large-video-test-now-playing.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F4F137911D9B6832002BEC57 /* large-video-test-now-playing.html */; };
                        dstPath = TestWebKitAPI.resources;
                        dstSubfolderSpec = 7;
                        files = (
+                               F4D5E4E81F0C5D38008C1A49 /* dragstart-clear-selection.html in Copy Resources */,
                                F4A32EC41F05F3850047C544 /* dragstart-change-selection-offscreen.html in Copy Resources */,
                                F4A32ECB1F0643370047C544 /* contenteditable-in-iframe.html in Copy Resources */,
                                F469FB241F01804B00401539 /* contenteditable-and-target.html in Copy Resources */,
                F4D4F3B41E4E2BCB00BB2767 /* DataInteractionSimulator.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = DataInteractionSimulator.mm; sourceTree = "<group>"; };
                F4D4F3B51E4E2BCB00BB2767 /* DataInteractionSimulator.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DataInteractionSimulator.h; sourceTree = "<group>"; };
                F4D4F3B71E4E36E400BB2767 /* DataInteractionTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = DataInteractionTests.mm; sourceTree = "<group>"; };
+               F4D5E4E71F0C5D27008C1A49 /* dragstart-clear-selection.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "dragstart-clear-selection.html"; sourceTree = "<group>"; };
                F4D7BCD61EA574DD00C421D3 /* PositionInformationTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = PositionInformationTests.mm; sourceTree = "<group>"; };
                F4DEF6EC1E9B4D950048EF61 /* image-in-link-and-input.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "image-in-link-and-input.html"; sourceTree = "<group>"; };
                F4F137911D9B6832002BEC57 /* large-video-test-now-playing.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "large-video-test-now-playing.html"; sourceTree = "<group>"; };
                                F41AB99C1EF4692C0083FA08 /* contenteditable-and-textarea.html */,
                                F4A32ECA1F0642F40047C544 /* contenteditable-in-iframe.html */,
                                F41AB99E1EF4692C0083FA08 /* div-and-large-image.html */,
+                               F4D5E4E71F0C5D27008C1A49 /* dragstart-clear-selection.html */,
                                F4A32EC31F05F3780047C544 /* dragstart-change-selection-offscreen.html */,
                                F41AB99B1EF4692C0083FA08 /* file-uploading.html */,
                                F41AB9991EF4692C0083FA08 /* image-and-contenteditable.html */,
diff --git a/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/dragstart-clear-selection.html b/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/dragstart-clear-selection.html
new file mode 100644 (file)
index 0000000..a765682
--- /dev/null
@@ -0,0 +1,18 @@
+<meta name="viewport" content="width=device-width, initial-scale=1">
+<code><p id="paragraph" style="font-size: 200px;">START DRAGGING</p></code>
+<script>
+function select(node) {
+    let range = document.createRange(node);
+    range.setStartBefore(node);
+    range.setEndAfter(node);
+    getSelection().removeAllRanges();
+    getSelection().addRange(range);
+}
+
+select(paragraph.childNodes[0]);
+document.body.addEventListener("dragstart", () => {
+    getSelection().removeAllRanges();
+    paragraph.textContent = "PASS";
+    paragraph.style.color = "green";
+});
+</script>
index 37984f98c3f8e1c160ced0a0d4a2a2b2e50186aa..c18372cd59cda42c0222d6b234675419cf9d7081 100644 (file)
@@ -1006,6 +1006,17 @@ TEST(DataInteractionTests, CancelledLiftDoesNotCauseSubsequentDragsToFail)
     checkStringArraysAreEqual(@[@"dragstart", @"dragend"], [outputText componentsSeparatedByString:@" "]);
 }
 
+TEST(DataInteractionTests, DoNotCrashWhenSelectionIsClearedInDragStart)
+{
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
+    [webView synchronouslyLoadTestPageNamed:@"dragstart-clear-selection"];
+
+    auto simulator = adoptNS([[DataInteractionSimulator alloc] initWithWebView:webView.get()]);
+    [simulator runFrom:CGPointMake(100, 100) to:CGPointMake(100, 100)];
+
+    EXPECT_WK_STREQ("PASS", [webView stringByEvaluatingJavaScript:@"paragraph.textContent"]);
+}
+
 TEST(DataInteractionTests, CustomActionSheetPopover)
 {
     RetainPtr<TestWKWebView> webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);