Title: [259613] trunk/Source/WebCore
- Revision
- 259613
- Author
- wenson_hs...@apple.com
- Date
- 2020-04-06 17:00:04 -0700 (Mon, 06 Apr 2020)
Log Message
REGRESSION: 4 TestWebKitAPI.DragAndDropTests.DataTransferSetData tests failing on iOS
https://bugs.webkit.org/show_bug.cgi?id=209685
<rdar://problem/60987461>
Reviewed by Megan Gardner.
After updating a WebKit open source test runner to iOS 13.4, 4 pasteboard-related API tests began to fail in
release builds on that particular bot. Logging statements added in r259465, r259518, r259534, and r259541
strongly suggest that this is due to an IPC dispatch race when clearing the platform pasteboard before writing
custom pasteboard data. On iOS, the former is dispatched asynchronously, while the latter is dispatched as sync
IPC. This means that if the UI process happens to be waiting for a sync IPC response from the web process, it
will end up handling the incoming IPC messages out of order by immediately dispatching sync IPC (in this case,
writing custom pasteboard data) before dispatching the async IPC (clearing data). This causes the custom
pasteboard data to be cleared on the platform pasteboard immediately after it is written.
To fix this, we limit clearing pasteboard data to when we would've otherwise avoided writing any custom
pasteboard data, and additionally make it so that writing custom pasteboard data always clears out any pre-
existing content on the pasteboard (obviating the need for a separate message to clear the pasteboard). Note
that writing custom pasteboard data always clears the existing pasteboard on macOS and iOS -- on macOS, we use
`-declareTypes:owner:`; on iOS, we use `-setItemProviders:`; in the case of macCatalyst, we `-setItems:`.
* dom/DataTransfer.cpp:
(WebCore::DataTransfer::commitToPasteboard):
Push the call to clear the pasteboard down from the call sites of `commitToPasteboard` into `commitToPasteboard`
itself; then, only explicitly clear the pasteboard in the case where we aren't writing custom pasteboard data
(i.e. either custom pasteboard data is disabled, or there is no data to write),
(WebCore::DataTransfer::moveDragState): See above.
* editing/Editor.cpp:
(WebCore::dispatchClipboardEvent): See above.
* platform/ios/WebItemProviderPasteboard.mm:
(-[WebItemProviderPasteboard stageRegistrationLists:]):
Remove always-on logging added in r259541 to help diagnose the test failures.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (259612 => 259613)
--- trunk/Source/WebCore/ChangeLog 2020-04-06 23:39:40 UTC (rev 259612)
+++ trunk/Source/WebCore/ChangeLog 2020-04-07 00:00:04 UTC (rev 259613)
@@ -1,3 +1,41 @@
+2020-04-06 Wenson Hsieh <wenson_hs...@apple.com>
+
+ REGRESSION: 4 TestWebKitAPI.DragAndDropTests.DataTransferSetData tests failing on iOS
+ https://bugs.webkit.org/show_bug.cgi?id=209685
+ <rdar://problem/60987461>
+
+ Reviewed by Megan Gardner.
+
+ After updating a WebKit open source test runner to iOS 13.4, 4 pasteboard-related API tests began to fail in
+ release builds on that particular bot. Logging statements added in r259465, r259518, r259534, and r259541
+ strongly suggest that this is due to an IPC dispatch race when clearing the platform pasteboard before writing
+ custom pasteboard data. On iOS, the former is dispatched asynchronously, while the latter is dispatched as sync
+ IPC. This means that if the UI process happens to be waiting for a sync IPC response from the web process, it
+ will end up handling the incoming IPC messages out of order by immediately dispatching sync IPC (in this case,
+ writing custom pasteboard data) before dispatching the async IPC (clearing data). This causes the custom
+ pasteboard data to be cleared on the platform pasteboard immediately after it is written.
+
+ To fix this, we limit clearing pasteboard data to when we would've otherwise avoided writing any custom
+ pasteboard data, and additionally make it so that writing custom pasteboard data always clears out any pre-
+ existing content on the pasteboard (obviating the need for a separate message to clear the pasteboard). Note
+ that writing custom pasteboard data always clears the existing pasteboard on macOS and iOS -- on macOS, we use
+ `-declareTypes:owner:`; on iOS, we use `-setItemProviders:`; in the case of macCatalyst, we `-setItems:`.
+
+ * dom/DataTransfer.cpp:
+ (WebCore::DataTransfer::commitToPasteboard):
+
+ Push the call to clear the pasteboard down from the call sites of `commitToPasteboard` into `commitToPasteboard`
+ itself; then, only explicitly clear the pasteboard in the case where we aren't writing custom pasteboard data
+ (i.e. either custom pasteboard data is disabled, or there is no data to write),
+
+ (WebCore::DataTransfer::moveDragState): See above.
+ * editing/Editor.cpp:
+ (WebCore::dispatchClipboardEvent): See above.
+ * platform/ios/WebItemProviderPasteboard.mm:
+ (-[WebItemProviderPasteboard stageRegistrationLists:]):
+
+ Remove always-on logging added in r259541 to help diagnose the test failures.
+
2020-04-06 Zalan Bujtas <za...@apple.com>
Delete line boxes when moving text renderers between block flows
Modified: trunk/Source/WebCore/dom/DataTransfer.cpp (259612 => 259613)
--- trunk/Source/WebCore/dom/DataTransfer.cpp 2020-04-06 23:39:40 UTC (rev 259612)
+++ trunk/Source/WebCore/dom/DataTransfer.cpp 2020-04-07 00:00:04 UTC (rev 259613)
@@ -425,8 +425,14 @@
{
ASSERT(is<StaticPasteboard>(*m_pasteboard) && !is<StaticPasteboard>(nativePasteboard));
PasteboardCustomData customData = downcast<StaticPasteboard>(*m_pasteboard).takeCustomData();
- if (!customData.hasData())
+ if (!customData.hasData()) {
+ // We clear the platform pasteboard here to ensure that the pasteboard doesn't contain any data
+ // that may have been written before starting the drag or copying, and after ending the last
+ // drag session or paste. After pushing the static pasteboard's contents to the platform, the
+ // pasteboard should only contain data that was in the static pasteboard.
+ nativePasteboard.clear();
return;
+ }
if (RuntimeEnabledFeatures::sharedFeatures().customPasteboardDataEnabled()) {
customData.setOrigin(m_originIdentifier);
@@ -434,6 +440,7 @@
return;
}
+ nativePasteboard.clear();
customData.forEachPlatformString([&] (auto& type, auto& string) {
nativePasteboard.writeString(type, string);
});
@@ -702,11 +709,6 @@
void DataTransfer::moveDragState(Ref<DataTransfer>&& other)
{
RELEASE_ASSERT(is<StaticPasteboard>(other->pasteboard()));
- // We clear the platform pasteboard here to ensure that the pasteboard doesn't contain any data
- // that may have been written before starting the drag, and after ending the last drag session.
- // After pushing the static pasteboard's contents to the platform, the pasteboard should only
- // contain data that was in the static pasteboard.
- m_pasteboard->clear();
other->commitToPasteboard(*m_pasteboard);
m_dropEffect = other->m_dropEffect;
Modified: trunk/Source/WebCore/editing/Editor.cpp (259612 => 259613)
--- trunk/Source/WebCore/editing/Editor.cpp 2020-04-06 23:39:40 UTC (rev 259612)
+++ trunk/Source/WebCore/editing/Editor.cpp 2020-04-07 00:00:04 UTC (rev 259613)
@@ -427,11 +427,8 @@
target->dispatchEvent(event);
bool noDefaultProcessing = event->defaultPrevented();
- if (noDefaultProcessing && (kind == ClipboardEventKind::Copy || kind == ClipboardEventKind::Cut)) {
- auto pasteboard = Pasteboard::createForCopyAndPaste();
- pasteboard->clear();
- dataTransfer->commitToPasteboard(*pasteboard);
- }
+ if (noDefaultProcessing && (kind == ClipboardEventKind::Copy || kind == ClipboardEventKind::Cut))
+ dataTransfer->commitToPasteboard(*Pasteboard::createForCopyAndPaste());
dataTransfer->makeInvalidForSecurity();
Modified: trunk/Source/WebCore/platform/ios/WebItemProviderPasteboard.mm (259612 => 259613)
--- trunk/Source/WebCore/platform/ios/WebItemProviderPasteboard.mm 2020-04-06 23:39:40 UTC (rev 259612)
+++ trunk/Source/WebCore/platform/ios/WebItemProviderPasteboard.mm 2020-04-07 00:00:04 UTC (rev 259613)
@@ -856,8 +856,6 @@
- (void)stageRegistrationLists:(NSArray<WebItemProviderRegistrationInfoList *> *)lists
{
ASSERT(lists.count);
- NSLog(@"%s - %@", __PRETTY_FUNCTION__, lists);
- WTFReportBacktrace();
_stagedRegistrationInfoLists = lists;
}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
