Title: [222768] trunk
- Revision
- 222768
- Author
- [email protected]
- Date
- 2017-10-02 22:10:29 -0700 (Mon, 02 Oct 2017)
Log Message
REGRESSION(r222595): Intermittent crash while accessing DataTransferItemList
https://bugs.webkit.org/show_bug.cgi?id=177791
<rdar://problem/34781456>
Reviewed by Ryosuke Niwa.
Source/WebCore:
DataTransfer::moveDragState() currently attempts to move the other DataTransfer's DataTransferItemList and
DragImageLoader as members of its own. This is incorrect, since both of these entities hold raw references of
some form to the other DataTransfer, yet they are held as unique_ptrs in the new DataTransfer. To fix this, we
(1) remove the line of code that moves the item list, since item lists will be lazily generated on the new
DataTransfer anyways, and (2) update the DataTransfer pointer on the old DataTransfer's DragImageLoader after
moving it to the new DataTransfer.
Test: editing/pasteboard/drag-end-crash-accessing-item-list.html
* dom/DataTransfer.cpp:
(WebCore::DragImageLoader::moveToDataTransfer):
(WebCore::DataTransfer::moveDragState):
LayoutTests:
Add a new layout test that simulates the crash encountered in this bug by forcing a garbage collection sweep
right before accessing the pasteboard in a "dragend" event handler.
* TestExpectations:
* editing/pasteboard/drag-end-crash-accessing-item-list-expected.txt: Added.
* editing/pasteboard/drag-end-crash-accessing-item-list.html: Added.
* platform/mac-wk1/TestExpectations:
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (222767 => 222768)
--- trunk/LayoutTests/ChangeLog 2017-10-03 04:35:28 UTC (rev 222767)
+++ trunk/LayoutTests/ChangeLog 2017-10-03 05:10:29 UTC (rev 222768)
@@ -1,3 +1,19 @@
+2017-10-02 Wenson Hsieh <[email protected]>
+
+ REGRESSION(r222595): Intermittent crash while accessing DataTransferItemList
+ https://bugs.webkit.org/show_bug.cgi?id=177791
+ <rdar://problem/34781456>
+
+ Reviewed by Ryosuke Niwa.
+
+ Add a new layout test that simulates the crash encountered in this bug by forcing a garbage collection sweep
+ right before accessing the pasteboard in a "dragend" event handler.
+
+ * TestExpectations:
+ * editing/pasteboard/drag-end-crash-accessing-item-list-expected.txt: Added.
+ * editing/pasteboard/drag-end-crash-accessing-item-list.html: Added.
+ * platform/mac-wk1/TestExpectations:
+
2017-10-02 Brent Fulgham <[email protected]>
Merge three Blink test cases
Modified: trunk/LayoutTests/TestExpectations (222767 => 222768)
--- trunk/LayoutTests/TestExpectations 2017-10-03 04:35:28 UTC (rev 222767)
+++ trunk/LayoutTests/TestExpectations 2017-10-03 05:10:29 UTC (rev 222768)
@@ -70,6 +70,7 @@
editing/pasteboard/data-transfer-get-data-on-drop-plain-text.html [ Skip ]
editing/pasteboard/data-transfer-get-data-on-drop-rich-text.html [ Skip ]
editing/pasteboard/data-transfer-get-data-on-drop-url.html [ Skip ]
+editing/pasteboard/drag-end-crash-accessing-item-list.html [ Skip ]
# Only iOS supports QuickLook
quicklook [ Skip ]
Added: trunk/LayoutTests/editing/pasteboard/drag-end-crash-accessing-item-list-expected.txt (0 => 222768)
--- trunk/LayoutTests/editing/pasteboard/drag-end-crash-accessing-item-list-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/pasteboard/drag-end-crash-accessing-item-list-expected.txt 2017-10-03 05:10:29 UTC (rev 222768)
@@ -0,0 +1 @@
+DRAG ME AND LET GO
Added: trunk/LayoutTests/editing/pasteboard/drag-end-crash-accessing-item-list.html (0 => 222768)
--- trunk/LayoutTests/editing/pasteboard/drag-end-crash-accessing-item-list.html (rev 0)
+++ trunk/LayoutTests/editing/pasteboard/drag-end-crash-accessing-item-list.html 2017-10-03 05:10:29 UTC (rev 222768)
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<body>
+ <div draggable="true" style="width: 200px; height: 200px" id="source">DRAG ME AND LET GO</div>
+</body>
+<script>
+function forceGarbageCollection() {
+ if (window.GCController)
+ GCController.collect();
+ else {
+ for (let i = 0; i < 1000; i++)
+ new ArrayBuffer(0x100000);
+ }
+}
+
+source.addEventListener("dragend", event => {
+ forceGarbageCollection();
+ event.dataTransfer.items.clear();
+ if (window.testRunner)
+ testRunner.notifyDone();
+});
+
+source.addEventListener("dragstart", event => {
+ event.dataTransfer.items.add(event.target.id, "text/plain");
+ event.dataTransfer.items.add("Test1", "text/html");
+ event.dataTransfer.items.add("Test2", "text/uri-list");
+});
+
+if (window.testRunner && window.eventSender && window.internals) {
+ internals.settings.setCustomPasteboardDataEnabled(true);
+ testRunner.waitUntilDone();
+ testRunner.dumpAsText();
+ eventSender.mouseMoveTo(100, 100);
+ eventSender.mouseDown();
+ eventSender.leapForward(1000);
+ eventSender.mouseMoveTo(400, 400);
+ eventSender.mouseUp();
+}
+</script>
Modified: trunk/LayoutTests/platform/mac-wk1/TestExpectations (222767 => 222768)
--- trunk/LayoutTests/platform/mac-wk1/TestExpectations 2017-10-03 04:35:28 UTC (rev 222767)
+++ trunk/LayoutTests/platform/mac-wk1/TestExpectations 2017-10-03 05:10:29 UTC (rev 222768)
@@ -11,6 +11,7 @@
editing/pasteboard/data-transfer-get-data-on-drop-plain-text.html [ Pass ]
editing/pasteboard/data-transfer-get-data-on-drop-rich-text.html [ Pass ]
editing/pasteboard/data-transfer-get-data-on-drop-url.html [ Pass ]
+editing/pasteboard/drag-end-crash-accessing-item-list.html [ Pass ]
#//////////////////////////////////////////////////////////////////////////////////////////
# End platform-specific directories.
Modified: trunk/Source/WebCore/ChangeLog (222767 => 222768)
--- trunk/Source/WebCore/ChangeLog 2017-10-03 04:35:28 UTC (rev 222767)
+++ trunk/Source/WebCore/ChangeLog 2017-10-03 05:10:29 UTC (rev 222768)
@@ -1,3 +1,24 @@
+2017-10-02 Wenson Hsieh <[email protected]>
+
+ REGRESSION(r222595): Intermittent crash while accessing DataTransferItemList
+ https://bugs.webkit.org/show_bug.cgi?id=177791
+ <rdar://problem/34781456>
+
+ Reviewed by Ryosuke Niwa.
+
+ DataTransfer::moveDragState() currently attempts to move the other DataTransfer's DataTransferItemList and
+ DragImageLoader as members of its own. This is incorrect, since both of these entities hold raw references of
+ some form to the other DataTransfer, yet they are held as unique_ptrs in the new DataTransfer. To fix this, we
+ (1) remove the line of code that moves the item list, since item lists will be lazily generated on the new
+ DataTransfer anyways, and (2) update the DataTransfer pointer on the old DataTransfer's DragImageLoader after
+ moving it to the new DataTransfer.
+
+ Test: editing/pasteboard/drag-end-crash-accessing-item-list.html
+
+ * dom/DataTransfer.cpp:
+ (WebCore::DragImageLoader::moveToDataTransfer):
+ (WebCore::DataTransfer::moveDragState):
+
2017-10-02 Chris Dumez <[email protected]>
Rename computeSharedStringHash() overload taking a URL to computedVisitedLinkHash()
Modified: trunk/Source/WebCore/dom/DataTransfer.cpp (222767 => 222768)
--- trunk/Source/WebCore/dom/DataTransfer.cpp 2017-10-03 04:35:28 UTC (rev 222767)
+++ trunk/Source/WebCore/dom/DataTransfer.cpp 2017-10-03 05:10:29 UTC (rev 222768)
@@ -51,6 +51,7 @@
explicit DragImageLoader(DataTransfer*);
void startLoading(CachedResourceHandle<CachedImage>&);
void stopLoading(CachedResourceHandle<CachedImage>&);
+ void moveToDataTransfer(DataTransfer&);
private:
void imageChanged(CachedImage*, const IntRect*) override;
@@ -365,6 +366,11 @@
{
}
+void DragImageLoader::moveToDataTransfer(DataTransfer& newDataTransfer)
+{
+ m_dataTransfer = &newDataTransfer;
+}
+
void DragImageLoader::startLoading(CachedResourceHandle<WebCore::CachedImage>& image)
{
// FIXME: Does this really trigger a load? Does it need to?
@@ -508,7 +514,8 @@
m_dragImage = other->m_dragImage;
m_dragImageElement = WTFMove(other->m_dragImageElement);
m_dragImageLoader = WTFMove(other->m_dragImageLoader);
- m_itemList = WTFMove(other->m_itemList);
+ if (m_dragImageLoader)
+ m_dragImageLoader->moveToDataTransfer(*this);
m_fileList = WTFMove(other->m_fileList);
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
