Title: [223140] trunk
Revision
223140
Author
rn...@webkit.org
Date
2017-10-10 13:50:20 -0700 (Tue, 10 Oct 2017)

Log Message

Loading should be disabled while constructing the fragment in WebContentReader::readWebArchive
https://bugs.webkit.org/show_bug.cgi?id=178118

Reviewed by Antti Koivisto.

Source/WebCore:

Disable image loading while constructing the document fragment in WebContentReader::readWebArchive
as we do in createFragmentAndAddResources for RTF/RTFD. This refactoring is needed to start using
blob URL in the pasted document fragment for webkit.org/b/124391.

Also modified WebContentReader::readWebArchive to take a reference to SharedBuffer instead of a pointer.

No new tests since existing tests have been updated to cover this behavior change.

* editing/WebContentReader.h:
* editing/cocoa/WebContentReaderCocoa.mm:
(WebCore::WebContentReader::readWebArchive): Use DeferredLoadingScope to disable the loader and images
while constructing the document fragment.
* platform/Pasteboard.h:
* platform/ios/PasteboardIOS.mm:
(WebCore::readPasteboardWebContentDataForType):
* platform/mac/PasteboardMac.mm:
(WebCore::Pasteboard::read):

LayoutTests:

Updated the existing tests to wait for images to load in each step explicitly instead of relying on
them being loaded synchronously or that it's loaded within 100-200ms.

* editing/pasteboard/4641033.html:
* editing/pasteboard/4947130.html:
* editing/pasteboard/4989774.html:
* editing/pasteboard/drag-selected-image-to-contenteditable.html:
* editing/selection/drag-to-contenteditable-iframe.html:
* platform/ios/TestExpectations: Skipped editing/selection/drag-to-contenteditable-iframe.html on iOS
since we don't support testing drag & drop on iOS. Also added [ Skip ] to other entires there.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (223139 => 223140)


--- trunk/LayoutTests/ChangeLog	2017-10-10 20:36:48 UTC (rev 223139)
+++ trunk/LayoutTests/ChangeLog	2017-10-10 20:50:20 UTC (rev 223140)
@@ -1,3 +1,21 @@
+2017-10-10  Ryosuke Niwa  <rn...@webkit.org>
+
+        Loading should be disabled while constructing the fragment in WebContentReader::readWebArchive
+        https://bugs.webkit.org/show_bug.cgi?id=178118
+
+        Reviewed by Antti Koivisto.
+
+        Updated the existing tests to wait for images to load in each step explicitly instead of relying on
+        them being loaded synchronously or that it's loaded within 100-200ms.
+
+        * editing/pasteboard/4641033.html:
+        * editing/pasteboard/4947130.html:
+        * editing/pasteboard/4989774.html:
+        * editing/pasteboard/drag-selected-image-to-contenteditable.html:
+        * editing/selection/drag-to-contenteditable-iframe.html:
+        * platform/ios/TestExpectations: Skipped editing/selection/drag-to-contenteditable-iframe.html on iOS
+        since we don't support testing drag & drop on iOS. Also added [ Skip ] to other entires there.
+
 2017-10-10  Chris Dumez  <cdu...@apple.com>
 
         Entries API should recognize path starting with 2 slashes as valid absolute path

Modified: trunk/LayoutTests/editing/pasteboard/4641033.html (223139 => 223140)


--- trunk/LayoutTests/editing/pasteboard/4641033.html	2017-10-10 20:36:48 UTC (rev 223139)
+++ trunk/LayoutTests/editing/pasteboard/4641033.html	2017-10-10 20:50:20 UTC (rev 223140)
@@ -12,21 +12,27 @@
 function runTest() {
     var start = document.getElementById("start");
     var end = document.getElementById("end");
-    
+
     var s = window.getSelection();
     s.setBaseAndExtent(start, 0, end, end.childNodes.length);
-    
+
     document.execCommand("Copy");
-    
+
     s.setPosition(document.getElementById("paste"), 0);
     document.execCommand("Paste");
-    
+
+    const pastedImageElement = document.querySelector('#paste img');
+    if (pastedImageElement.complete)
+        finish();
+    else
+        pastedImageElement._onload_ = finish;
+}
+function finish() {
     if (window.testRunner)
-        window.testRunner.notifyDone();    
-    
+        window.testRunner.notifyDone();
 }
 if (window.testRunner)
     window.testRunner.waitUntilDone();
-window.setTimeout(runTest, 100);
+window._onload_ = runTest;
 
 </script>

Modified: trunk/LayoutTests/editing/pasteboard/4947130.html (223139 => 223140)


--- trunk/LayoutTests/editing/pasteboard/4947130.html	2017-10-10 20:36:48 UTC (rev 223139)
+++ trunk/LayoutTests/editing/pasteboard/4947130.html	2017-10-10 20:50:20 UTC (rev 223140)
@@ -5,23 +5,19 @@
 
 <script>
 
+if (window.testRunner)
+    testRunner.waitUntilDone();
+
 function runTest() {
     if (!window.testRunner)
         return;
-    
-    window.testRunner.waitUntilDone();  
-    
-    window.setTimeout(step2, 200);
-}
 
-function step2() {
-
     var img = document.getElementById("img");
     var left = img.offsetLeft;
     var top = img.offsetTop;
     var x = left + img.offsetWidth / 2;
     var y = top + img.offsetHeight / 2;
-    
+
     eventSender.mouseMoveTo(x, y);
     eventSender.mouseDown();
     eventSender.leapForward(1300);
@@ -29,8 +25,11 @@
     eventSender.mouseMoveTo(left - 28, y);
     eventSender.mouseUp();
 
-    window.testRunner.notifyDone();
+    if (img.complete)
+        testRunner.notifyDone();
+    else
+        img._onload_ = () => testRunner.notifyDone();
 }
 
-runTest();
+window._onload_ = runTest;
 </script>

Modified: trunk/LayoutTests/editing/pasteboard/4989774.html (223139 => 223140)


--- trunk/LayoutTests/editing/pasteboard/4989774.html	2017-10-10 20:36:48 UTC (rev 223139)
+++ trunk/LayoutTests/editing/pasteboard/4989774.html	2017-10-10 20:50:20 UTC (rev 223140)
@@ -1,7 +1,8 @@
 <body style="font-weight: bold;" id="div" contenteditable="true"><img src=""
 
 <script>
-function partTwo() {
+function runTest()
+{
     sel.setBaseAndExtent(body, 0, body, body.childNodes.length);
     document.execCommand("Copy");
     sel.setPosition(body, body.childNodes.length);
@@ -9,7 +10,11 @@
     document.execCommand("Paste");
     document.execCommand("InsertLineBreak");
     document.execCommand("InsertText", false, "This tests for a bug where an images pasted on the same line would appear on different lines.  You should see several pictures above all in the same line/paragraph.");
-    window.testRunner.notifyDone();    
+    Promise.all(Array.from(document.querySelectorAll('img')).map((img) => {
+        if (img.complete)
+            return;
+        return new Promise((resolve) => img._onload_ = resolve);
+    })).then(() => testRunner.notifyDone());
 }
 
 var body = document.body;
@@ -17,7 +22,7 @@
 if (window.testRunner) {
     window.testRunner.waitUntilDone();
     
-    window.setTimeout(partTwo, 200);
+    window._onload_ = runTest;
 } else {
     document.execCommand("SelectAll")
     document.execCommand("InsertText", false, "This tests for a bug where an images pasted on the same line would appear on different lines. To run it manually, Undo, then select the image, Copy, put the caret after the image, then Paste twice. All three images should be in separate paragraphs.");    

Modified: trunk/LayoutTests/editing/pasteboard/drag-selected-image-to-contenteditable.html (223139 => 223140)


--- trunk/LayoutTests/editing/pasteboard/drag-selected-image-to-contenteditable.html	2017-10-10 20:36:48 UTC (rev 223139)
+++ trunk/LayoutTests/editing/pasteboard/drag-selected-image-to-contenteditable.html	2017-10-10 20:50:20 UTC (rev 223140)
@@ -19,14 +19,9 @@
 function runTest() {
     if (window.testRunner)
         testRunner.waitUntilDone();
-    // Let the subframe come into being.
-    window.setTimeout(step2, 100);
     e = document.getElementById("dragme");
     setSelectionCommand(e, 0, e, 1);
-}
 
-function step2() 
-{
     if (!window.testRunner) {
         log("This test uses the eventSender.  To run it manually, drag the selected image into the editable div and drop it.  It should appear inside the editable div.");
         return;
@@ -46,9 +41,14 @@
     
     eventSender.mouseMoveTo(x, y);
     eventSender.mouseUp();
-    
-    testRunner.notifyDone();
+
+    Promise.all(Array.from(document.querySelectorAll('img')).map((img) => {
+        if (img.complete)
+            return;
+        return new Promise((resolve) => img._onload_ = resolve);
+    })).then(() => testRunner.notifyDone());
 }
+
 </script>
 </head>
 

Modified: trunk/LayoutTests/editing/selection/drag-to-contenteditable-iframe.html (223139 => 223140)


--- trunk/LayoutTests/editing/selection/drag-to-contenteditable-iframe.html	2017-10-10 20:36:48 UTC (rev 223139)
+++ trunk/LayoutTests/editing/selection/drag-to-contenteditable-iframe.html	2017-10-10 20:50:20 UTC (rev 223140)
@@ -31,14 +31,18 @@
     eventSender.mouseDown();
     eventSender.leapForward(1000);
     
-    e = document.getElementById("frame");
-    x = e.offsetLeft + e.offsetWidth / 2;
-    y = e.offsetTop + e.offsetHeight / 2;
+    const frame = document.getElementById("frame");
+    x = frame.offsetLeft + frame.offsetWidth / 2;
+    y = frame.offsetTop + frame.offsetHeight / 2;
     
     eventSender.mouseMoveTo(x, y);
     eventSender.mouseUp();
-    
-    testRunner.notifyDone();
+
+    const img = frame.contentDocument.querySelector('img');
+    if (img.complete)
+        testRunner.notifyDone();
+    else
+        img._onload_ = () => testRunner.notifyDone();
 }
 
 window._onload_ = runTest;

Modified: trunk/LayoutTests/platform/ios/TestExpectations (223139 => 223140)


--- trunk/LayoutTests/platform/ios/TestExpectations	2017-10-10 20:36:48 UTC (rev 223139)
+++ trunk/LayoutTests/platform/ios/TestExpectations	2017-10-10 20:50:20 UTC (rev 223140)
@@ -236,60 +236,61 @@
 fast/images/animated-webp-expected.html
 
 # Drag-and-drop is not supported:
-editing/pasteboard/datatransfer-items-drop-plaintext-file.html
-editing/pasteboard/drag-link-with-data-transfer-adds-trusted-link-to-pasteboard.html
-fast/events/bogus-dropEffect-effectAllowed.html
-fast/events/clear-drag-state.html
-fast/events/clear-edit-drag-state.html
-fast/events/content-changed-during-drop.html
-fast/events/crash-on-mutate-during-drop.html
-fast/events/drag-and-drop-link-fast-multiple-times-does-not-crash.html
-fast/events/drag-and-drop-link-containing-block.html
-fast/events/drag-and-drop-autoscroll-inner-frame.html
-fast/events/drag-and-drop-autoscroll.html
-fast/events/drag-and-drop-dataTransfer-types-nocrash.html
-fast/events/drag-and-drop-fire-drag-dragover.html
-fast/events/drag-and-drop-set-drag-data-arguments.html
-fast/events/drag-and-drop-subframe-dataTransfer.html
-fast/events/drag-and-drop.html
-fast/events/drag-customData.html
-fast/events/drag-dataTransferItemList-file-handling.html
-fast/events/drag-dataTransferItemList.html
-fast/events/drag-display-none-element.html
-fast/events/drag-file-crash.html
-fast/events/drag-image-filename.html
-fast/events/drag-in-frames.html
-fast/events/drag-and-drop-link.html
-fast/events/drag-and-drop-link-into-focused-contenteditable.html
-fast/events/drag-outside-window.html
-fast/events/drag-parent-node.html
-fast/events/drag-selects-image.html
-fast/events/drag-to-navigate.html
-fast/events/draggable-div-nodata.html
-fast/events/draggable-div-customdata.html
-fast/events/drop-handler-should-not-stop-navigate.html
-fast/events/drop-with-file-paths.html
-fast/events/dropzone-001.html
-fast/events/dropzone-002.html
-fast/events/dropzone-003.html
-fast/events/dropzone-004.html
-fast/events/dropzone-005.html
-fast/events/file-input-hidden-in-ondrop.html
-fast/events/input-element-display-none-in-dragleave-crash.html
-fast/events/moving-text-should-fire-drop-and-dragend-events-2.html
-fast/events/moving-text-should-fire-drop-and-dragend-events.html
-fast/events/only-valid-drop-targets-receive-file-drop.html
-fast/events/prevent-drag-to-navigate.html
-fast/events/standalone-image-drag-to-editable.html
-fast/files/local-file-drag-security.html
+editing/pasteboard/datatransfer-items-drop-plaintext-file.html [ Skip ]
+editing/pasteboard/drag-link-with-data-transfer-adds-trusted-link-to-pasteboard.html [ Skip ]
+editing/selection/drag-to-contenteditable-iframe.html [ Skip ]
+fast/events/bogus-dropEffect-effectAllowed.html [ Skip ]
+fast/events/clear-drag-state.html [ Skip ]
+fast/events/clear-edit-drag-state.html [ Skip ]
+fast/events/content-changed-during-drop.html [ Skip ]
+fast/events/crash-on-mutate-during-drop.html [ Skip ]
+fast/events/drag-and-drop-link-fast-multiple-times-does-not-crash.html [ Skip ]
+fast/events/drag-and-drop-link-containing-block.html [ Skip ]
+fast/events/drag-and-drop-autoscroll-inner-frame.html [ Skip ]
+fast/events/drag-and-drop-autoscroll.html [ Skip ]
+fast/events/drag-and-drop-dataTransfer-types-nocrash.html [ Skip ]
+fast/events/drag-and-drop-fire-drag-dragover.html [ Skip ]
+fast/events/drag-and-drop-set-drag-data-arguments.html [ Skip ]
+fast/events/drag-and-drop-subframe-dataTransfer.html [ Skip ]
+fast/events/drag-and-drop.html [ Skip ]
+fast/events/drag-customData.html [ Skip ]
+fast/events/drag-dataTransferItemList-file-handling.html [ Skip ]
+fast/events/drag-dataTransferItemList.html [ Skip ]
+fast/events/drag-display-none-element.html [ Skip ]
+fast/events/drag-file-crash.html [ Skip ]
+fast/events/drag-image-filename.html [ Skip ]
+fast/events/drag-in-frames.html [ Skip ]
+fast/events/drag-and-drop-link.html [ Skip ]
+fast/events/drag-and-drop-link-into-focused-contenteditable.html [ Skip ]
+fast/events/drag-outside-window.html [ Skip ]
+fast/events/drag-parent-node.html [ Skip ]
+fast/events/drag-selects-image.html [ Skip ]
+fast/events/drag-to-navigate.html [ Skip ]
+fast/events/draggable-div-nodata.html [ Skip ]
+fast/events/draggable-div-customdata.html [ Skip ]
+fast/events/drop-handler-should-not-stop-navigate.html [ Skip ]
+fast/events/drop-with-file-paths.html [ Skip ]
+fast/events/dropzone-001.html [ Skip ]
+fast/events/dropzone-002.html [ Skip ]
+fast/events/dropzone-003.html [ Skip ]
+fast/events/dropzone-004.html [ Skip ]
+fast/events/dropzone-005.html [ Skip ]
+fast/events/file-input-hidden-in-ondrop.html [ Skip ]
+fast/events/input-element-display-none-in-dragleave-crash.html [ Skip ]
+fast/events/moving-text-should-fire-drop-and-dragend-events-2.html [ Skip ]
+fast/events/moving-text-should-fire-drop-and-dragend-events.html [ Skip ]
+fast/events/only-valid-drop-targets-receive-file-drop.html [ Skip ]
+fast/events/prevent-drag-to-navigate.html [ Skip ]
+fast/events/standalone-image-drag-to-editable.html [ Skip ]
+fast/files/local-file-drag-security.html [ Skip ]
 fast/history/page-cache-createObjectURL.html [ Skip ]
-http/tests/local/drag-over-remote-content.html
-http/tests/local/fileapi/send-dragged-file.html
-http/tests/local/fileapi/send-sliced-dragged-file.html
-http/tests/misc/bubble-drag-events.html
-http/tests/security/drag-drop-local-file.html
-http/tests/security/drag-drop-same-unique-origin.html
-http/tests/security/drag-over-remote-content-iframe.html
+http/tests/local/drag-over-remote-content.html [ Skip ]
+http/tests/local/fileapi/send-dragged-file.html [ Skip ]
+http/tests/local/fileapi/send-sliced-dragged-file.html [ Skip ]
+http/tests/misc/bubble-drag-events.html [ Skip ]
+http/tests/security/drag-drop-local-file.html [ Skip ]
+http/tests/security/drag-drop-same-unique-origin.html [ Skip ]
+http/tests/security/drag-over-remote-content-iframe.html [ Skip ]
 
 # No support for force events
 fast/events/force-click-link-selection-behavior.html [ Skip ]

Modified: trunk/Source/WebCore/ChangeLog (223139 => 223140)


--- trunk/Source/WebCore/ChangeLog	2017-10-10 20:36:48 UTC (rev 223139)
+++ trunk/Source/WebCore/ChangeLog	2017-10-10 20:50:20 UTC (rev 223140)
@@ -1,3 +1,28 @@
+2017-10-10  Ryosuke Niwa  <rn...@webkit.org>
+
+        Loading should be disabled while constructing the fragment in WebContentReader::readWebArchive
+        https://bugs.webkit.org/show_bug.cgi?id=178118
+
+        Reviewed by Antti Koivisto.
+
+        Disable image loading while constructing the document fragment in WebContentReader::readWebArchive
+        as we do in createFragmentAndAddResources for RTF/RTFD. This refactoring is needed to start using
+        blob URL in the pasted document fragment for webkit.org/b/124391.
+
+        Also modified WebContentReader::readWebArchive to take a reference to SharedBuffer instead of a pointer.
+
+        No new tests since existing tests have been updated to cover this behavior change.
+
+        * editing/WebContentReader.h:
+        * editing/cocoa/WebContentReaderCocoa.mm:
+        (WebCore::WebContentReader::readWebArchive): Use DeferredLoadingScope to disable the loader and images
+        while constructing the document fragment.
+        * platform/Pasteboard.h:
+        * platform/ios/PasteboardIOS.mm:
+        (WebCore::readPasteboardWebContentDataForType):
+        * platform/mac/PasteboardMac.mm:
+        (WebCore::Pasteboard::read):
+
 2017-10-10  Antti Koivisto  <an...@apple.com>
 
         Layers should be destroyed by RenderLayerModelObject

Modified: trunk/Source/WebCore/editing/WebContentReader.h (223139 => 223140)


--- trunk/Source/WebCore/editing/WebContentReader.h	2017-10-10 20:36:48 UTC (rev 223139)
+++ trunk/Source/WebCore/editing/WebContentReader.h	2017-10-10 20:50:20 UTC (rev 223140)
@@ -54,7 +54,7 @@
 
 private:
 #if PLATFORM(COCOA)
-    bool readWebArchive(SharedBuffer*) override;
+    bool readWebArchive(SharedBuffer&) override;
     bool readFilenames(const Vector<String>&) override;
     bool readHTML(const String&) override;
     bool readRTFD(SharedBuffer&) override;

Modified: trunk/Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm (223139 => 223140)


--- trunk/Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm	2017-10-10 20:36:48 UTC (rev 223139)
+++ trunk/Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm	2017-10-10 20:50:20 UTC (rev 223140)
@@ -179,18 +179,12 @@
     return WTFMove(fragmentAndResources.fragment);
 }
 
-bool WebContentReader::readWebArchive(SharedBuffer* buffer)
+bool WebContentReader::readWebArchive(SharedBuffer& buffer)
 {
-    if (frame.settings().preferMIMETypeForImages())
+    if (frame.settings().preferMIMETypeForImages() || !frame.document())
         return false;
 
-    if (!frame.document())
-        return false;
-
-    if (!buffer)
-        return false;
-
-    auto archive = LegacyWebArchive::create(URL(), *buffer);
+    auto archive = LegacyWebArchive::create(URL(), buffer);
     if (!archive)
         return false;
 
@@ -202,12 +196,13 @@
     if (!frame.loader().client().canShowMIMETypeAsHTML(type))
         return false;
 
-    // FIXME: The code in createFragmentAndAddResources calls setDefersLoading(true). Don't we need that here?
+    DeferredLoadingScope scope(frame);
+    auto markupString = String::fromUTF8(mainResource->data().data(), mainResource->data().size());
+    addFragment(createFragmentFromMarkup(*frame.document(), markupString, mainResource->url(), DisallowScriptingAndPluginContent));
+
     if (DocumentLoader* loader = frame.loader().documentLoader())
         loader->addAllArchiveResources(*archive);
 
-    auto markupString = String::fromUTF8(mainResource->data().data(), mainResource->data().size());
-    addFragment(createFragmentFromMarkup(*frame.document(), markupString, mainResource->url(), DisallowScriptingAndPluginContent));
     return true;
 }
 

Modified: trunk/Source/WebCore/platform/Pasteboard.h (223139 => 223140)


--- trunk/Source/WebCore/platform/Pasteboard.h	2017-10-10 20:36:48 UTC (rev 223139)
+++ trunk/Source/WebCore/platform/Pasteboard.h	2017-10-10 20:50:20 UTC (rev 223140)
@@ -130,7 +130,7 @@
     virtual ~PasteboardWebContentReader() { }
 
 #if !(PLATFORM(GTK) || PLATFORM(WIN))
-    virtual bool readWebArchive(SharedBuffer*) = 0;
+    virtual bool readWebArchive(SharedBuffer&) = 0;
     virtual bool readFilenames(const Vector<String>&) = 0;
     virtual bool readHTML(const String&) = 0;
     virtual bool readRTFD(SharedBuffer&) = 0;

Modified: trunk/Source/WebCore/platform/ios/PasteboardIOS.mm (223139 => 223140)


--- trunk/Source/WebCore/platform/ios/PasteboardIOS.mm	2017-10-10 20:36:48 UTC (rev 223139)
+++ trunk/Source/WebCore/platform/ios/PasteboardIOS.mm	2017-10-10 20:50:20 UTC (rev 223140)
@@ -165,8 +165,8 @@
 static bool readPasteboardWebContentDataForType(PasteboardWebContentReader& reader, PasteboardStrategy& strategy, NSString *type, int itemIndex, const String& pasteboardName)
 {
     if ([type isEqualToString:WebArchivePboardType]) {
-        RefPtr<SharedBuffer> buffer = strategy.readBufferFromPasteboard(itemIndex, WebArchivePboardType, pasteboardName);
-        return reader.readWebArchive(buffer.get());
+        auto buffer = strategy.readBufferFromPasteboard(itemIndex, WebArchivePboardType, pasteboardName);
+        return buffer && reader.readWebArchive(*buffer);
     }
 
     if ([type isEqualToString:(NSString *)kUTTypeHTML]) {

Modified: trunk/Source/WebCore/platform/mac/PasteboardMac.mm (223139 => 223140)


--- trunk/Source/WebCore/platform/mac/PasteboardMac.mm	2017-10-10 20:36:48 UTC (rev 223139)
+++ trunk/Source/WebCore/platform/mac/PasteboardMac.mm	2017-10-10 20:50:20 UTC (rev 223140)
@@ -333,8 +333,8 @@
     strategy.getTypes(types, m_pasteboardName);
 
     if (types.contains(WebArchivePboardType)) {
-        if (RefPtr<SharedBuffer> buffer = strategy.bufferForType(WebArchivePboardType, m_pasteboardName)) {
-            if (reader.readWebArchive(buffer.get()))
+        if (auto buffer = strategy.bufferForType(WebArchivePboardType, m_pasteboardName)) {
+            if (reader.readWebArchive(*buffer))
                 return;
         }
     }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to