- Revision
- 219153
- Author
- wenson_hs...@apple.com
- Date
- 2017-07-05 13:32:41 -0700 (Wed, 05 Jul 2017)
Log Message
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.
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):
Modified Paths
Added Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (219152 => 219153)
--- trunk/Source/WebCore/ChangeLog 2017-07-05 20:27:24 UTC (rev 219152)
+++ trunk/Source/WebCore/ChangeLog 2017-07-05 20:32:41 UTC (rev 219153)
@@ -1,3 +1,25 @@
+2017-07-05 Wenson Hsieh <wenson_hs...@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.fra...@apple.com>
Try to fix iOS 10.3 public SDK builds.
Modified: trunk/Source/WebCore/page/DragController.cpp (219152 => 219153)
--- trunk/Source/WebCore/page/DragController.cpp 2017-07-05 20:27:24 UTC (rev 219152)
+++ trunk/Source/WebCore/page/DragController.cpp 2017-07-05 20:32:41 UTC (rev 219153)
@@ -924,18 +924,16 @@
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())) {
Modified: trunk/Tools/ChangeLog (219152 => 219153)
--- trunk/Tools/ChangeLog 2017-07-05 20:27:24 UTC (rev 219152)
+++ trunk/Tools/ChangeLog 2017-07-05 20:32:41 UTC (rev 219153)
@@ -1,3 +1,19 @@
+2017-07-05 Wenson Hsieh <wenson_hs...@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 <daba...@apple.com>
Do not pass API::FrameInfo for source frame or clear out page of target frame on
Modified: trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (219152 => 219153)
--- trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2017-07-05 20:27:24 UTC (rev 219152)
+++ trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2017-07-05 20:32:41 UTC (rev 219153)
@@ -661,6 +661,7 @@
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 */; };
@@ -737,6 +738,7 @@
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 */,
@@ -1642,6 +1644,7 @@
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>"; };
@@ -2039,6 +2042,7 @@
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 */,
Added: trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/dragstart-clear-selection.html (0 => 219153)
--- trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/dragstart-clear-selection.html (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/dragstart-clear-selection.html 2017-07-05 20:32:41 UTC (rev 219153)
@@ -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>
Modified: trunk/Tools/TestWebKitAPI/Tests/ios/DataInteractionTests.mm (219152 => 219153)
--- trunk/Tools/TestWebKitAPI/Tests/ios/DataInteractionTests.mm 2017-07-05 20:27:24 UTC (rev 219152)
+++ trunk/Tools/TestWebKitAPI/Tests/ios/DataInteractionTests.mm 2017-07-05 20:32:41 UTC (rev 219153)
@@ -1006,6 +1006,17 @@
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)]);
