[Attachment Support] Implement SPI for clients to request data for a given attachment
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 13 Nov 2017 15:30:39 +0000 (15:30 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 13 Nov 2017 15:30:39 +0000 (15:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=179586
<rdar://problem/35355720>

Reviewed by Darin Adler.

Source/WebCore:

Adds support in WebCore for fetching data for a given attachment element. See per-method comments below for
more detail.

Test coverage by augmenting existing API tests in WKAttachmentTests, and adding 3 new tests.

* dom/Document.cpp:
(WebCore::Document::didInsertAttachmentElement):
(WebCore::Document::didRemoveAttachmentElement):
(WebCore::Document::attachmentForIdentifier const):

Fetches an attachment element matching the given identifier. Using the identifier => attachment element map here
allows us to avoid the cost of walking the DOM in search for HTMLAttachmentElements every time data is
requested.

* dom/Document.h:

Add a map of attachment identifier => HTMLAttachmentElement in Document. This map is updated when attachment
elements are connected to or disconnected from the document. Additionally, delegate attachment insertion and
removal out to the Editor if possible.

* editing/Editor.cpp:
(WebCore::Editor::insertAttachmentFromFile):
* editing/mac/WebContentReaderMac.mm:
(WebCore::WebContentReader::readFilenames):
* editing/markup.cpp:
(WebCore::createFragmentFromMarkup):

Tweak existing logic that transfers file-backed File objects when deserializing HTMLAttachmentElements from
markup to handle the case where the attachment element is not file-backed. In this case, we construct a new
File object using the deserializing constructor. To do this, we use the attachment element's blob URL to make
sure that the new File references an existing blob matching that URL.

* html/HTMLAttachmentElement.cpp:
(WebCore::AttachmentDataReader::create):
(WebCore::AttachmentDataReader::AttachmentDataReader):

Introduce AttachmentDataReader, a helper class local to HTMLAttachmentElement that is responsible for loading
an attachment element's file and invoking a given callback when loading has succeeded or failed.
Each AttachmentDataReader is retained exclusively by its HTMLAttachmentElement, through the
HTMLAttachmentElement's vector of unique_ptrs to AttachmentDataReaders.

(WebCore::HTMLAttachmentElement::~HTMLAttachmentElement):
(WebCore::HTMLAttachmentElement::blobURL const):

Add a convenience getter for the blob URL of the attachment's backing File object.

(WebCore::HTMLAttachmentElement::setFile):

When setting an attachment element's file, set the blob URL attribute as well to the blob URL. Also, tweak this
to take a RefPtr<File>&& instead of a raw File*.

(WebCore::HTMLAttachmentElement::insertedIntoAncestor):
(WebCore::HTMLAttachmentElement::removedFromAncestor):

Instead of delegating attachment insertion and removal to the Editor, just call out to the Document, which will
now call out to the Editor.

(WebCore::HTMLAttachmentElement::attachmentPath const):

Add a convenience getter for the attachment path attribute.

(WebCore::HTMLAttachmentElement::requestData):
(WebCore::HTMLAttachmentElement::destroyReader):

Called when a AttachmentDataReader has completed (either with success or failure), and is ready to be removed
from the attachment element's list of active data readers.

(WebCore::AttachmentDataReader::~AttachmentDataReader):
(WebCore::AttachmentDataReader::didFinishLoading):
(WebCore::AttachmentDataReader::didFail):
(WebCore::AttachmentDataReader::invokeCallbackAndFinishReading):

When the reader is done loading, or has failed, or is about to be destroyed, fire the callback with loaded data
(if any) and cancel the FileReaderLoader.

* html/HTMLAttachmentElement.h:
* html/HTMLAttributeNames.in:

Adds a new "webkitattachmentbloburl" attribute that keeps track of an attachment element's file's blob URL. This
is used to ensure that information about an attachment element's file is not lost between certain editing
operations (for instance, deleting a line break) that involve serializing and then deserializing markup into
DocumentFragments to then insert.

Source/WebKit:

Adds support in WebKit for fetching data for a given attachment element. See WebCore/ChangeLog for more details.
Most of the changes here are boilerplate plumbing of -requestAttachmentData through the client layers.

Test coverage by augmenting existing API tests in WKAttachmentTests, and adding 3 new tests.

* UIProcess/API/APIAttachment.cpp:
(API::Attachment::requestData):
* UIProcess/API/APIAttachment.h:
* UIProcess/API/Cocoa/_WKAttachment.h:
* UIProcess/API/Cocoa/_WKAttachment.mm:
(-[_WKAttachment requestData:]):
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::sharedBufferCallback):

Add a new IPC callback helper type, SharedBufferCallback. This is similar to the existing DataCallback, but
instead of deserializing to an API::Data, we convert to a SharedBuffer instead. Additionally,
SharedBufferCallback is able to draw a distinction between null data and empty data. This allows -requestData:
to distinguish between cases where (for instance) the data for a given attachment is an empty blob, and when
the attachment doesn't exist at all.

(WebKit::WebPageProxy::dataCallback):
(WebKit::WebPageProxy::insertAttachment):
(WebKit::WebPage::invokeSharedBufferCallback):
(WebKit::WebPageProxy::requestAttachmentData):
* UIProcess/WebPageProxy.h:
* UIProcess/WebPageProxy.messages.in:
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::requestAttachmentData):
* WebProcess/WebPage/WebPage.h:
* WebProcess/WebPage/WebPage.messages.in:

Tools:

Augments existing API tests in WKAttachmentTests to additionally check that -requestData: yields the correct
result when performing various editing operations. Also adds a new API test that cuts and pastes an attachment
inserted using WKWebView attachment SPI, and expects that the data of the attachment can still be fetched using
the _WKAttachment SPI, as well as another test that inserts an empty NSData and expects that requestData: also
yields an empty NSData result.

* TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:
(-[NSData shortDescription]):
(-[_WKAttachment synchronouslyRequestData:]):
(-[_WKAttachment expectRequestedDataToBe:]):
(TestWebKitAPI::TEST):

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

22 files changed:
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/editing/Editor.cpp
Source/WebCore/editing/mac/WebContentReaderMac.mm
Source/WebCore/editing/markup.cpp
Source/WebCore/html/HTMLAttachmentElement.cpp
Source/WebCore/html/HTMLAttachmentElement.h
Source/WebCore/html/HTMLAttributeNames.in
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/APIAttachment.cpp
Source/WebKit/UIProcess/API/APIAttachment.h
Source/WebKit/UIProcess/API/Cocoa/_WKAttachment.h
Source/WebKit/UIProcess/API/Cocoa/_WKAttachment.mm
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/UIProcess/WebPageProxy.h
Source/WebKit/UIProcess/WebPageProxy.messages.in
Source/WebKit/WebProcess/WebPage/WebPage.cpp
Source/WebKit/WebProcess/WebPage/WebPage.h
Source/WebKit/WebProcess/WebPage/WebPage.messages.in
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm

index cdde386..34612f0 100644 (file)
@@ -1,3 +1,94 @@
+2017-11-13  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [Attachment Support] Implement SPI for clients to request data for a given attachment
+        https://bugs.webkit.org/show_bug.cgi?id=179586
+        <rdar://problem/35355720>
+
+        Reviewed by Darin Adler.
+
+        Adds support in WebCore for fetching data for a given attachment element. See per-method comments below for
+        more detail.
+
+        Test coverage by augmenting existing API tests in WKAttachmentTests, and adding 3 new tests.
+
+        * dom/Document.cpp:
+        (WebCore::Document::didInsertAttachmentElement):
+        (WebCore::Document::didRemoveAttachmentElement):
+        (WebCore::Document::attachmentForIdentifier const):
+
+        Fetches an attachment element matching the given identifier. Using the identifier => attachment element map here
+        allows us to avoid the cost of walking the DOM in search for HTMLAttachmentElements every time data is
+        requested.
+
+        * dom/Document.h:
+
+        Add a map of attachment identifier => HTMLAttachmentElement in Document. This map is updated when attachment
+        elements are connected to or disconnected from the document. Additionally, delegate attachment insertion and
+        removal out to the Editor if possible.
+
+        * editing/Editor.cpp:
+        (WebCore::Editor::insertAttachmentFromFile):
+        * editing/mac/WebContentReaderMac.mm:
+        (WebCore::WebContentReader::readFilenames):
+        * editing/markup.cpp:
+        (WebCore::createFragmentFromMarkup):
+
+        Tweak existing logic that transfers file-backed File objects when deserializing HTMLAttachmentElements from
+        markup to handle the case where the attachment element is not file-backed. In this case, we construct a new
+        File object using the deserializing constructor. To do this, we use the attachment element's blob URL to make
+        sure that the new File references an existing blob matching that URL.
+
+        * html/HTMLAttachmentElement.cpp:
+        (WebCore::AttachmentDataReader::create):
+        (WebCore::AttachmentDataReader::AttachmentDataReader):
+
+        Introduce AttachmentDataReader, a helper class local to HTMLAttachmentElement that is responsible for loading
+        an attachment element's file and invoking a given callback when loading has succeeded or failed.
+        Each AttachmentDataReader is retained exclusively by its HTMLAttachmentElement, through the
+        HTMLAttachmentElement's vector of unique_ptrs to AttachmentDataReaders.
+
+        (WebCore::HTMLAttachmentElement::~HTMLAttachmentElement):
+        (WebCore::HTMLAttachmentElement::blobURL const):
+
+        Add a convenience getter for the blob URL of the attachment's backing File object.
+
+        (WebCore::HTMLAttachmentElement::setFile):
+
+        When setting an attachment element's file, set the blob URL attribute as well to the blob URL. Also, tweak this
+        to take a RefPtr<File>&& instead of a raw File*.
+
+        (WebCore::HTMLAttachmentElement::insertedIntoAncestor):
+        (WebCore::HTMLAttachmentElement::removedFromAncestor):
+
+        Instead of delegating attachment insertion and removal to the Editor, just call out to the Document, which will
+        now call out to the Editor.
+
+        (WebCore::HTMLAttachmentElement::attachmentPath const):
+
+        Add a convenience getter for the attachment path attribute.
+
+        (WebCore::HTMLAttachmentElement::requestData):
+        (WebCore::HTMLAttachmentElement::destroyReader):
+
+        Called when a AttachmentDataReader has completed (either with success or failure), and is ready to be removed
+        from the attachment element's list of active data readers.
+
+        (WebCore::AttachmentDataReader::~AttachmentDataReader):
+        (WebCore::AttachmentDataReader::didFinishLoading):
+        (WebCore::AttachmentDataReader::didFail):
+        (WebCore::AttachmentDataReader::invokeCallbackAndFinishReading):
+
+        When the reader is done loading, or has failed, or is about to be destroyed, fire the callback with loaded data
+        (if any) and cancel the FileReaderLoader.
+
+        * html/HTMLAttachmentElement.h:
+        * html/HTMLAttributeNames.in:
+
+        Adds a new "webkitattachmentbloburl" attribute that keeps track of an attachment element's file's blob URL. This
+        is used to ensure that information about an attachment element's file is not lost between certain editing
+        operations (for instance, deleting a line break) that involve serializing and then deserializing markup into
+        DocumentFragments to then insert.
+
 2017-11-13  Zan Dobersek  <zdobersek@igalia.com>
 
         [Cairo] Remove GraphicsContext::mustUseShadowBlur()
index 6ee1e97..0b989fd 100644 (file)
@@ -74,6 +74,7 @@
 #include "GenericCachedHTMLCollection.h"
 #include "HTMLAllCollection.h"
 #include "HTMLAnchorElement.h"
+#include "HTMLAttachmentElement.h"
 #include "HTMLBaseElement.h"
 #include "HTMLBodyElement.h"
 #include "HTMLCanvasElement.h"
@@ -7462,6 +7463,39 @@ DocumentTimeline& Document::timeline()
     return *m_timeline;
 }
 
+#if ENABLE(ATTACHMENT_ELEMENT)
+
+void Document::didInsertAttachmentElement(HTMLAttachmentElement& attachment)
+{
+    auto identifier = attachment.uniqueIdentifier();
+    if (!identifier)
+        return;
+
+    m_attachmentIdentifierToElementMap.set(identifier, attachment);
+
+    if (frame())
+        frame()->editor().didInsertAttachmentElement(attachment);
+}
+
+void Document::didRemoveAttachmentElement(HTMLAttachmentElement& attachment)
+{
+    auto identifier = attachment.uniqueIdentifier();
+    if (!identifier)
+        return;
+
+    m_attachmentIdentifierToElementMap.remove(identifier);
+
+    if (frame())
+        frame()->editor().didRemoveAttachmentElement(attachment);
+}
+
+RefPtr<HTMLAttachmentElement> Document::attachmentForIdentifier(const String& identifier) const
+{
+    return m_attachmentIdentifierToElementMap.get(identifier);
+}
+
+#endif // ENABLE(ATTACHMENT_ELEMENT)
+
 static MessageSource messageSourceForWTFLogChannel(const WTFLogChannel& channel)
 {
     static const NeverDestroyed<String> mediaChannel = MAKE_STATIC_STRING_IMPL("media");
index d817a96..24b0874 100644 (file)
@@ -216,6 +216,10 @@ class TextAutoSizing;
 class MediaSession;
 #endif
 
+#if ENABLE(ATTACHMENT_ELEMENT)
+class HTMLAttachmentElement;
+#endif
+
 namespace Style {
 class Scope;
 };
@@ -1373,6 +1377,12 @@ public:
 
     DocumentTimeline& timeline();
         
+#if ENABLE(ATTACHMENT_ELEMENT)
+    void didInsertAttachmentElement(HTMLAttachmentElement&);
+    void didRemoveAttachmentElement(HTMLAttachmentElement&);
+    WEBCORE_EXPORT RefPtr<HTMLAttachmentElement> attachmentForIdentifier(const String& identifier) const;
+#endif
+
 protected:
     enum ConstructionFlags { Synthesized = 1, NonRenderedPlaceholder = 1 << 1 };
     Document(Frame*, const URL&, unsigned = DefaultDocumentClass, unsigned constructionFlags = 0);
@@ -1721,6 +1731,10 @@ private:
     RefPtr<IDBClient::IDBConnectionProxy> m_idbConnectionProxy;
 #endif
 
+#if ENABLE(ATTACHMENT_ELEMENT)
+    HashMap<String, Ref<HTMLAttachmentElement>> m_attachmentIdentifierToElementMap;
+#endif
+
     Timer m_didAssociateFormControlsTimer;
     Timer m_cookieCacheExpiryTimer;
 
index 09660a3..ba06f79 100644 (file)
@@ -3802,7 +3802,7 @@ void Editor::insertAttachmentFromFile(const String& identifier, const String& fi
     attachment->setAttribute(HTMLNames::subtitleAttr, fileSizeDescription(file->size()));
     attachment->setAttribute(HTMLNames::typeAttr, contentType);
     attachment->setUniqueIdentifier(identifier);
-    attachment->setFile(file.ptr());
+    attachment->setFile(WTFMove(file));
 
     auto fragmentToInsert = document().createDocumentFragment();
     fragmentToInsert->appendChild(attachment.get());
index d3d7b7d..a798a19 100644 (file)
@@ -65,7 +65,7 @@ bool WebContentReader::readFilenames(const Vector<String>& paths)
 #if ENABLE(ATTACHMENT_ELEMENT)
         if (RuntimeEnabledFeatures::sharedFeatures().attachmentElementEnabled()) {
             auto attachment = HTMLAttachmentElement::create(HTMLNames::attachmentTag, document);
-            attachment->setFile(File::create([[NSURL fileURLWithPath:text] path]).ptr());
+            attachment->setFile(File::create([[NSURL fileURLWithPath:text] path]));
             fragment->appendChild(attachment);
             continue;
         }
index 6b04965..3cf40ed 100644 (file)
@@ -763,7 +763,11 @@ Ref<DocumentFragment> createFragmentFromMarkup(Document& document, const String&
         attachments.append(attachment);
 
     for (auto& attachment : attachments) {
-        attachment->setFile(File::create(attachment->attributeWithoutSynchronization(webkitattachmentpathAttr)).ptr());
+        auto attachmentPath = attachment->attachmentPath();
+        if (attachmentPath.isEmpty())
+            attachment->setFile(File::deserialize({ }, attachment->blobURL(), attachment->attachmentType(), attachment->attachmentTitle()));
+        else
+            attachment->setFile(File::create(attachmentPath));
         attachment->removeAttribute(webkitattachmentpathAttr);
     }
 #endif
index d136b48..38d7dfe 100644 (file)
 
 #include "Editor.h"
 #include "File.h"
+#include "FileReaderLoader.h"
+#include "FileReaderLoaderClient.h"
 #include "Frame.h"
 #include "HTMLNames.h"
 #include "RenderAttachment.h"
+#include "SharedBuffer.h"
 
 namespace WebCore {
 
 using namespace HTMLNames;
 
+class AttachmentDataReader : public FileReaderLoaderClient {
+public:
+    static std::unique_ptr<AttachmentDataReader> create(HTMLAttachmentElement& attachment, Function<void(RefPtr<SharedBuffer>&&)>&& callback)
+    {
+        return std::make_unique<AttachmentDataReader>(attachment, WTFMove(callback));
+    }
+
+    AttachmentDataReader(HTMLAttachmentElement& attachment, Function<void(RefPtr<SharedBuffer>&&)>&& callback)
+        : m_attachment(attachment)
+        , m_callback(std::make_unique<Function<void(RefPtr<SharedBuffer>&&)>>(WTFMove(callback)))
+        , m_loader(std::make_unique<FileReaderLoader>(FileReaderLoader::ReadType::ReadAsArrayBuffer, this))
+    {
+        m_loader->start(&attachment.document(), *attachment.file());
+    }
+
+    ~AttachmentDataReader();
+
+private:
+    void didStartLoading() final { }
+    void didReceiveData() final { }
+    void didFinishLoading() final;
+    void didFail(int error) final;
+
+    void invokeCallbackAndFinishReading(RefPtr<SharedBuffer>&&);
+
+    HTMLAttachmentElement& m_attachment;
+    std::unique_ptr<Function<void(RefPtr<SharedBuffer>&&)>> m_callback;
+    std::unique_ptr<FileReaderLoader> m_loader;
+};
+
 HTMLAttachmentElement::HTMLAttachmentElement(const QualifiedName& tagName, Document& document)
     : HTMLElement(tagName, document)
 {
@@ -61,9 +94,16 @@ File* HTMLAttachmentElement::file() const
     return m_file.get();
 }
 
-void HTMLAttachmentElement::setFile(File* file)
+URL HTMLAttachmentElement::blobURL() const
+{
+    return { { }, attributeWithoutSynchronization(HTMLNames::webkitattachmentbloburlAttr).string() };
+}
+
+void HTMLAttachmentElement::setFile(RefPtr<File>&& file)
 {
-    m_file = file;
+    m_file = WTFMove(file);
+
+    setAttributeWithoutSynchronization(HTMLNames::webkitattachmentbloburlAttr, m_file ? m_file->url() : emptyString());
 
     if (auto* renderer = this->renderer())
         renderer->invalidate();
@@ -72,20 +112,16 @@ void HTMLAttachmentElement::setFile(File* file)
 Node::InsertedIntoAncestorResult HTMLAttachmentElement::insertedIntoAncestor(InsertionType type, ContainerNode& ancestor)
 {
     auto result = HTMLElement::insertedIntoAncestor(type, ancestor);
-    if (auto* frame = document().frame()) {
-        if (type.connectedToDocument)
-            frame->editor().didInsertAttachmentElement(*this);
-    }
+    if (type.connectedToDocument)
+        document().didInsertAttachmentElement(*this);
     return result;
 }
 
 void HTMLAttachmentElement::removedFromAncestor(RemovalType type, ContainerNode& ancestor)
 {
     HTMLElement::removedFromAncestor(type, ancestor);
-    if (auto* frame = document().frame()) {
-        if (type.disconnectedFromDocument)
-            frame->editor().didRemoveAttachmentElement(*this);
-    }
+    if (type.disconnectedFromDocument)
+        document().didRemoveAttachmentElement(*this);
 }
 
 void HTMLAttachmentElement::parseAttribute(const QualifiedName& name, const AtomicString& value)
@@ -121,6 +157,57 @@ String HTMLAttachmentElement::attachmentType() const
     return attributeWithoutSynchronization(typeAttr);
 }
 
+String HTMLAttachmentElement::attachmentPath() const
+{
+    return attributeWithoutSynchronization(webkitattachmentpathAttr);
+}
+
+void HTMLAttachmentElement::requestData(Function<void(RefPtr<SharedBuffer>&&)>&& callback)
+{
+    if (m_file)
+        m_attachmentReaders.append(AttachmentDataReader::create(*this, WTFMove(callback)));
+    else
+        callback(nullptr);
+}
+
+void HTMLAttachmentElement::destroyReader(AttachmentDataReader& finishedReader)
+{
+    m_attachmentReaders.removeFirstMatching([&] (const std::unique_ptr<AttachmentDataReader>& reader) -> bool {
+        return reader.get() == &finishedReader;
+    });
+}
+
+AttachmentDataReader::~AttachmentDataReader()
+{
+    invokeCallbackAndFinishReading(nullptr);
+}
+
+void AttachmentDataReader::didFinishLoading()
+{
+    if (auto arrayBuffer = m_loader->arrayBufferResult())
+        invokeCallbackAndFinishReading(SharedBuffer::create(reinterpret_cast<uint8_t*>(arrayBuffer->data()), arrayBuffer->byteLength()));
+    else
+        invokeCallbackAndFinishReading(nullptr);
+    m_attachment.destroyReader(*this);
+}
+
+void AttachmentDataReader::didFail(int)
+{
+    invokeCallbackAndFinishReading(nullptr);
+    m_attachment.destroyReader(*this);
+}
+
+void AttachmentDataReader::invokeCallbackAndFinishReading(RefPtr<SharedBuffer>&& data)
+{
+    auto callback = WTFMove(m_callback);
+    if (!callback)
+        return;
+
+    m_loader->cancel();
+    m_loader = nullptr;
+    (*callback)(WTFMove(data));
+}
+
 } // namespace WebCore
 
 #endif // ENABLE(ATTACHMENT_ELEMENT)
index bd9c564..6746d90 100644 (file)
 
 namespace WebCore {
 
+class AttachmentDataReader;
 class File;
 class RenderAttachment;
+class SharedBuffer;
 
 class HTMLAttachmentElement final : public HTMLElement {
 public:
     static Ref<HTMLAttachmentElement> create(const QualifiedName&, Document&);
 
+    WEBCORE_EXPORT URL blobURL() const;
     WEBCORE_EXPORT File* file() const;
-    void setFile(File*);
+    void setFile(RefPtr<File>&&);
 
     WEBCORE_EXPORT String uniqueIdentifier() const;
     void setUniqueIdentifier(const String&);
@@ -49,9 +52,13 @@ public:
 
     WEBCORE_EXPORT String attachmentTitle() const;
     String attachmentType() const;
+    String attachmentPath() const;
 
     RenderAttachment* renderer() const;
 
+    WEBCORE_EXPORT void requestData(Function<void(RefPtr<SharedBuffer>&&)>&& callback);
+    void destroyReader(AttachmentDataReader&);
+
 private:
     HTMLAttachmentElement(const QualifiedName&, Document&);
     virtual ~HTMLAttachmentElement();
@@ -69,6 +76,7 @@ private:
     void parseAttribute(const QualifiedName&, const AtomicString&) final;
     
     RefPtr<File> m_file;
+    Vector<std::unique_ptr<AttachmentDataReader>> m_attachmentReaders;
 };
 
 } // namespace WebCore
index b6f0433..7a1b5e1 100644 (file)
@@ -383,6 +383,7 @@ vspace
 webkitallowfullscreen
 webkitattachmentid
 webkitattachmentpath
+webkitattachmentbloburl
 webkitdirectory
 width
 wrap
index 7d8b715..2477ce7 100644 (file)
@@ -1,3 +1,42 @@
+2017-11-13  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [Attachment Support] Implement SPI for clients to request data for a given attachment
+        https://bugs.webkit.org/show_bug.cgi?id=179586
+        <rdar://problem/35355720>
+
+        Reviewed by Darin Adler.
+
+        Adds support in WebKit for fetching data for a given attachment element. See WebCore/ChangeLog for more details.
+        Most of the changes here are boilerplate plumbing of -requestAttachmentData through the client layers.
+
+        Test coverage by augmenting existing API tests in WKAttachmentTests, and adding 3 new tests.
+
+        * UIProcess/API/APIAttachment.cpp:
+        (API::Attachment::requestData):
+        * UIProcess/API/APIAttachment.h:
+        * UIProcess/API/Cocoa/_WKAttachment.h:
+        * UIProcess/API/Cocoa/_WKAttachment.mm:
+        (-[_WKAttachment requestData:]):
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::sharedBufferCallback):
+
+        Add a new IPC callback helper type, SharedBufferCallback. This is similar to the existing DataCallback, but
+        instead of deserializing to an API::Data, we convert to a SharedBuffer instead. Additionally,
+        SharedBufferCallback is able to draw a distinction between null data and empty data. This allows -requestData:
+        to distinguish between cases where (for instance) the data for a given attachment is an empty blob, and when
+        the attachment doesn't exist at all.
+
+        (WebKit::WebPageProxy::dataCallback):
+        (WebKit::WebPageProxy::insertAttachment):
+        (WebKit::WebPage::invokeSharedBufferCallback):
+        (WebKit::WebPageProxy::requestAttachmentData):
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/WebPageProxy.messages.in:
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::requestAttachmentData):
+        * WebProcess/WebPage/WebPage.h:
+        * WebProcess/WebPage/WebPage.messages.in:
+
 2017-11-12  Darin Adler  <darin@apple.com>
 
         More is<> and downcast<>, less static_cast<>
index 91ec0b7..1d49b07 100644 (file)
@@ -26,6 +26,8 @@
 #include "config.h"
 #include "APIAttachment.h"
 
+#include <WebCore/SharedBuffer.h>
+#include <wtf/BlockPtr.h>
 #include <wtf/text/WTFString.h>
 
 namespace API {
@@ -45,4 +47,12 @@ Attachment::~Attachment()
 {
 }
 
+void Attachment::requestData(Function<void(RefPtr<WebCore::SharedBuffer>, WebKit::CallbackBase::Error)>&& callback)
+{
+    if (m_webPage)
+        m_webPage->requestAttachmentData(m_identifier, WTFMove(callback));
+    else
+        callback(nullptr, WebKit::CallbackBase::Error::OwnerWasInvalidated);
+}
+
 }
index 245590e..1ea007d 100644 (file)
 #include "APIObject.h"
 #include <WebKit/WKBase.h>
 #include <WebKit/WebPageProxy.h>
+#include <wtf/RefPtr.h>
 #include <wtf/WeakPtr.h>
 #include <wtf/text/WTFString.h>
 
+namespace WebCore {
+class SharedBuffer;
+}
+
 namespace WebKit {
 class WebPageProxy;
 }
@@ -43,6 +48,7 @@ public:
     virtual ~Attachment();
 
     const WTF::String& identifier() const { return m_identifier; }
+    void requestData(Function<void(RefPtr<WebCore::SharedBuffer>, WebKit::CallbackBase::Error)>&&);
 
 private:
     explicit Attachment(const WTF::String& identifier, WebKit::WebPageProxy&);
index 9403239..1cd4574 100644 (file)
@@ -43,6 +43,7 @@ WK_CLASS_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA))
 
 WK_CLASS_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA))
 @interface _WKAttachment : NSObject
+- (void)requestData:(void(^)(NSData *, NSError *))completionHandler;
 @end
 
 #endif
index d1b146f..d03d943 100644 (file)
 #if WK_API_ENABLED
 
 #import "APIAttachment.h"
+#import "WKErrorPrivate.h"
 #import "_WKAttachmentInternal.h"
+#import <WebCore/SharedBuffer.h>
+#import <wtf/BlockPtr.h>
 
 using namespace WebKit;
 
@@ -58,6 +61,19 @@ using namespace WebKit;
     return [object isKindOfClass:[_WKAttachment class]] && [self.uniqueIdentifier isEqual:[(_WKAttachment *)object uniqueIdentifier]];
 }
 
+- (void)requestData:(void(^)(NSData *, NSError *))completionHandler
+{
+    _attachment->requestData([ capturedBlock = makeBlockPtr(completionHandler) ] (RefPtr<WebCore::SharedBuffer> buffer, CallbackBase::Error error) {
+        if (!capturedBlock)
+            return;
+
+        if (buffer && error == CallbackBase::Error::None)
+            capturedBlock(buffer->createNSData().autorelease(), nil);
+        else
+            capturedBlock(nil, [NSError errorWithDomain:WKErrorDomain code:1 userInfo:nil]);
+    });
+}
+
 - (NSString *)uniqueIdentifier
 {
     return _attachment->identifier();
index 5cc3ff7..924df0e 100644 (file)
 #include <WebCore/PublicSuffix.h>
 #include <WebCore/RenderEmbeddedObject.h>
 #include <WebCore/SerializedCryptoKeyWrap.h>
+#include <WebCore/SharedBuffer.h>
 #include <WebCore/TextCheckerClient.h>
 #include <WebCore/TextIndicator.h>
 #include <WebCore/URL.h>
@@ -5161,14 +5162,29 @@ void WebPageProxy::voidCallback(CallbackID callbackID)
     callback->performCallback();
 }
 
-void WebPageProxy::dataCallback(const IPC::DataReference& dataReference, CallbackID callbackID)
+void WebPageProxy::sharedBufferCallback(const IPC::DataReference& dataReference, bool isNull, CallbackID callbackID)
 {
-    auto callback = m_callbacks.take<DataCallback>(callbackID);
+    auto callback = m_callbacks.take<SharedBufferCallback>(callbackID);
     if (!callback) {
-        // FIXME: Log error or assert.
+        ASSERT_NOT_REACHED();
+        return;
+    }
+
+    if (isNull) {
+        ASSERT(dataReference.isEmpty());
+        callback->performCallbackWithReturnValue(nullptr);
         return;
     }
 
+    callback->performCallbackWithReturnValue(SharedBuffer::create(dataReference.data(), dataReference.size()));
+}
+
+void WebPageProxy::dataCallback(const IPC::DataReference& dataReference, CallbackID callbackID)
+{
+    auto callback = m_callbacks.take<DataCallback>(callbackID);
+    if (!callback)
+        return;
+
     callback->performCallbackWithReturnValue(API::Data::create(dataReference.data(), dataReference.size()).ptr());
 }
 
@@ -7115,10 +7131,26 @@ void WebPageProxy::requestStorageAccess(String&& subFrameHost, String&& topFrame
 
 void WebPageProxy::insertAttachment(const String& identifier, const String& filename, std::optional<String> contentType, SharedBuffer& data, Function<void(CallbackBase::Error)>&& callback)
 {
+    if (!isValid()) {
+        callback(CallbackBase::Error::OwnerWasInvalidated);
+        return;
+    }
+
     auto callbackID = m_callbacks.put(WTFMove(callback), m_process->throttler().backgroundActivityToken());
     m_process->send(Messages::WebPage::InsertAttachment(identifier, filename, contentType, IPC::SharedBufferDataReference { &data }, callbackID), m_pageID);
 }
 
+void WebPageProxy::requestAttachmentData(const String& identifier, Function<void(RefPtr<SharedBuffer>, CallbackBase::Error)>&& callback)
+{
+    if (!isValid()) {
+        callback(nullptr, CallbackBase::Error::OwnerWasInvalidated);
+        return;
+    }
+
+    auto callbackID = m_callbacks.put(WTFMove(callback), m_process->throttler().backgroundActivityToken());
+    m_process->send(Messages::WebPage::RequestAttachmentData(identifier, callbackID), m_pageID);
+}
+
 void WebPageProxy::didInsertAttachment(const String& identifier)
 {
     m_pageClient.didInsertAttachment(identifier);
index 3e68236..5d07cb1 100644 (file)
@@ -236,6 +236,7 @@ typedef GenericCallback<uint64_t> UnsignedCallback;
 typedef GenericCallback<EditingRange> EditingRangeCallback;
 typedef GenericCallback<const String&> StringCallback;
 typedef GenericCallback<API::SerializedScriptValue*, bool, const WebCore::ExceptionDetails&> ScriptValueCallback;
+typedef GenericCallback<RefPtr<WebCore::SharedBuffer>> SharedBufferCallback;
 
 #if PLATFORM(GTK)
 typedef GenericCallback<API::Error*> PrintFinishedCallback;
@@ -1213,6 +1214,7 @@ public:
 
 #if ENABLE(ATTACHMENT_ELEMENT)
     void insertAttachment(const String& identifier, const String& filename, std::optional<String> contentType, WebCore::SharedBuffer& data, Function<void(CallbackBase::Error)>&&);
+    void requestAttachmentData(const String& identifier, Function<void(RefPtr<WebCore::SharedBuffer>, CallbackBase::Error)>&&);
 #endif
 
 private:
@@ -1466,6 +1468,7 @@ private:
 
     void voidCallback(CallbackID);
     void dataCallback(const IPC::DataReference&, CallbackID);
+    void sharedBufferCallback(const IPC::DataReference&, bool isNull, CallbackID);
     void imageCallback(const ShareableBitmap::Handle&, CallbackID);
     void stringCallback(const String&, CallbackID);
     void invalidateStringCallback(CallbackID);
index f1ef5e5..15aff31 100644 (file)
@@ -157,6 +157,7 @@ messages -> WebPageProxy {
     # Callback messages
     VoidCallback(WebKit::CallbackID callbackID)
     DataCallback(IPC::DataReference resultData, WebKit::CallbackID callbackID)
+    SharedBufferCallback(IPC::DataReference resultData, bool isNull, WebKit::CallbackID callbackID);
     ImageCallback(WebKit::ShareableBitmap::Handle bitmapHandle, WebKit::CallbackID callbackID)
     StringCallback(String resultString, WebKit::CallbackID callbackID)
     InvalidateStringCallback(WebKit::CallbackID callbackID)
index 9f44599..aef3739 100644 (file)
 #include <WebCore/ElementIterator.h>
 #include <WebCore/EventHandler.h>
 #include <WebCore/EventNames.h>
+#include <WebCore/File.h>
 #include <WebCore/FocusController.h>
 #include <WebCore/FormState.h>
 #include <WebCore/FrameLoadRequest.h>
 #include <WebCore/FrameLoaderTypes.h>
 #include <WebCore/FrameView.h>
+#include <WebCore/HTMLAttachmentElement.h>
 #include <WebCore/HTMLFormElement.h>
 #include <WebCore/HTMLImageElement.h>
 #include <WebCore/HTMLInputElement.h>
@@ -5765,6 +5767,14 @@ void WebPage::storageAccessResponse(bool wasGranted, uint64_t contextId)
     callback(wasGranted);
 }
 
+void WebPage::invokeSharedBufferCallback(RefPtr<SharedBuffer>&& buffer, CallbackID callbackID)
+{
+    if (buffer)
+        send(Messages::WebPageProxy::SharedBufferCallback(IPC::SharedBufferDataReference { buffer.get() }, false, callbackID));
+    else
+        send(Messages::WebPageProxy::SharedBufferCallback({ }, true, callbackID));
+}
+
 #if ENABLE(ATTACHMENT_ELEMENT)
 
 void WebPage::insertAttachment(const String& identifier, const String& filename, std::optional<String> contentType, const IPC::DataReference& data, CallbackID callbackID)
@@ -5774,6 +5784,29 @@ void WebPage::insertAttachment(const String& identifier, const String& filename,
     send(Messages::WebPageProxy::VoidCallback(callbackID));
 }
 
+void WebPage::requestAttachmentData(const String& identifier, CallbackID callbackID)
+{
+    // FIXME: We don't currently handle attachment data requests for attachment elements in subframes.
+    auto* frame = mainFrame();
+    if (!frame || !frame->document()) {
+        invokeSharedBufferCallback({ }, callbackID);
+        return;
+    }
+
+    auto attachment = frame->document()->attachmentForIdentifier(identifier);
+    if (!attachment) {
+        invokeSharedBufferCallback({ }, callbackID);
+        return;
+    }
+
+    attachment->requestData([callbackID, protectedThis = makeRef(*this), protectedAttachment = WTFMove(attachment)] (RefPtr<SharedBuffer>&& buffer) {
+        if (buffer)
+            protectedThis->invokeSharedBufferCallback(WTFMove(buffer), callbackID);
+        else
+            protectedThis->invokeSharedBufferCallback({ }, callbackID);
+    });
+}
+
 #endif // ENABLE(ATTACHMENT_ELEMENT)
 
 } // namespace WebKit
index 1d84d4d..a2f7e02 100644 (file)
@@ -1001,6 +1001,7 @@ public:
 
 #if ENABLE(ATTACHMENT_ELEMENT)
     void insertAttachment(const String& identifier, const String& filename, std::optional<String> contentType, const IPC::DataReference&, CallbackID);
+    void requestAttachmentData(const String& identifier, CallbackID);
 #endif
 
 private:
@@ -1106,6 +1107,8 @@ private:
     void restoreSession(const Vector<BackForwardListItemState>&);
     void didRemoveBackForwardItem(uint64_t);
 
+    void invokeSharedBufferCallback(RefPtr<WebCore::SharedBuffer>&&, CallbackID);
+
 #if ENABLE(REMOTE_INSPECTOR)
     void setAllowsRemoteInspection(bool);
     void setRemoteInspectionNameOverride(const String&);
index 1b28d1a..6d7b6ed 100644 (file)
@@ -487,5 +487,6 @@ messages -> WebPage LegacyReceiver {
 
 #if ENABLE(ATTACHMENT_ELEMENT)
     InsertAttachment(String identifier, String filename, std::optional<String> contentType, IPC::DataReference data, WebKit::CallbackID callbackID)
+    RequestAttachmentData(String identifier, WebKit::CallbackID callbackID)
 #endif
 }
index afc3b37..20f4919 100644 (file)
@@ -1,3 +1,23 @@
+2017-11-13  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [Attachment Support] Implement SPI for clients to request data for a given attachment
+        https://bugs.webkit.org/show_bug.cgi?id=179586
+        <rdar://problem/35355720>
+
+        Reviewed by Darin Adler.
+
+        Augments existing API tests in WKAttachmentTests to additionally check that -requestData: yields the correct
+        result when performing various editing operations. Also adds a new API test that cuts and pastes an attachment
+        inserted using WKWebView attachment SPI, and expects that the data of the attachment can still be fetched using
+        the _WKAttachment SPI, as well as another test that inserts an empty NSData and expects that requestData: also
+        yields an empty NSData result.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:
+        (-[NSData shortDescription]):
+        (-[_WKAttachment synchronouslyRequestData:]):
+        (-[_WKAttachment expectRequestedDataToBe:]):
+        (TestWebKitAPI::TEST):
+
 2017-11-12  Gabriel Ivascu  <givascu@igalia.com>
 
         [GTK] Add functionality to handle font sizes in points
index f4bfe10..20d12c1 100644 (file)
@@ -177,6 +177,50 @@ static NSData *testImageData()
 
 @end
 
+@implementation NSData (AttachmentTesting)
+
+- (NSString *)shortDescription
+{
+    return [NSString stringWithFormat:@"<%tu bytes>", self.length];
+}
+
+@end
+
+@implementation _WKAttachment (AttachmentTesting)
+
+- (NSData *)synchronouslyRequestData:(NSError **)error
+{
+    __block RetainPtr<NSData> result;
+    __block RetainPtr<NSError> resultError;
+    __block bool done = false;
+    [self requestData:^(NSData *data, NSError *error) {
+        result = retainPtr(data);
+        resultError = retainPtr(error);
+        done = true;
+    }];
+
+    TestWebKitAPI::Util::run(&done);
+
+    if (error)
+        *error = resultError.autorelease();
+
+    return result.autorelease();
+}
+
+- (void)expectRequestedDataToBe:(NSData *)expectedData
+{
+    NSError *dataRequestError = nil;
+    NSData *observedData = [self synchronouslyRequestData:&dataRequestError];
+    BOOL observedDataIsEqualToExpectedData = [observedData isEqualToData:expectedData] || observedData == expectedData;
+    EXPECT_TRUE(observedDataIsEqualToExpectedData);
+    if (!observedDataIsEqualToExpectedData) {
+        NSLog(@"Expected data: %@ but observed: %@ for %@", [expectedData shortDescription], [observedData shortDescription], self);
+        NSLog(@"Observed error: %@ while reading data for %@", dataRequestError, self);
+    }
+}
+
+@end
+
 namespace TestWebKitAPI {
 
 TEST(WKAttachmentTests, AttachmentElementInsertion)
@@ -193,6 +237,9 @@ TEST(WKAttachmentTests, AttachmentElementInsertion)
         EXPECT_WK_STREQ(@"38 bytes", [webView valueOfAttribute:@"subtitle" forQuerySelector:@"attachment"]);
         observer.expectAttachmentUpdates(@[ ], @[ firstAttachment.get() ]);
     }
+
+    [firstAttachment expectRequestedDataToBe:testHTMLData()];
+
     {
         ObserveAttachmentUpdatesForScope scope(webView.get());
         // Since no content type is explicitly specified, compute it from the file extension.
@@ -203,6 +250,9 @@ TEST(WKAttachmentTests, AttachmentElementInsertion)
         EXPECT_WK_STREQ(@"37 KB", [webView valueOfAttribute:@"subtitle" forQuerySelector:@"attachment"]);
         scope.expectAttachmentUpdates(@[ firstAttachment.get() ], @[ secondAttachment.get() ]);
     }
+
+    [firstAttachment expectRequestedDataToBe:nil];
+    [secondAttachment expectRequestedDataToBe:testImageData()];
 }
 
 TEST(WKAttachmentTests, AttachmentUpdatesWhenInsertingAndDeletingNewline)
@@ -218,12 +268,18 @@ TEST(WKAttachmentTests, AttachmentUpdatesWhenInsertingAndDeletingNewline)
     [webView expectUpdatesAfterCommand:@"DeleteBackward" withArgument:nil expectedRemovals:@[] expectedInsertions:@[]];
     [webView stringByEvaluatingJavaScript:@"getSelection().collapse(document.body)"];
     [webView expectUpdatesAfterCommand:@"InsertParagraph" withArgument:nil expectedRemovals:@[] expectedInsertions:@[]];
+
+    [webView expectUpdatesAfterCommand:@"DeleteBackward" withArgument:nil expectedRemovals:@[] expectedInsertions:@[]];
+    [attachment expectRequestedDataToBe:testHTMLData()];
+
     [webView expectUpdatesAfterCommand:@"DeleteForward" withArgument:nil expectedRemovals:@[attachment.get()] expectedInsertions:@[]];
+    [attachment expectRequestedDataToBe:nil];
 }
 
 TEST(WKAttachmentTests, AttachmentUpdatesWhenUndoingAndRedoing)
 {
     auto webView = webViewForTestingAttachments();
+    RetainPtr<NSData> htmlData = retainPtr(testHTMLData());
     RetainPtr<_WKAttachment> attachment;
     {
         ObserveAttachmentUpdatesForScope observer(webView.get());
@@ -231,10 +287,19 @@ TEST(WKAttachmentTests, AttachmentUpdatesWhenUndoingAndRedoing)
         observer.expectAttachmentUpdates(@[ ], @[attachment.get()]);
     }
     [webView expectUpdatesAfterCommand:@"Undo" withArgument:nil expectedRemovals:@[attachment.get()] expectedInsertions:@[]];
+    [attachment expectRequestedDataToBe:nil];
+
     [webView expectUpdatesAfterCommand:@"Redo" withArgument:nil expectedRemovals:@[] expectedInsertions:@[attachment.get()]];
+    [attachment expectRequestedDataToBe:htmlData.get()];
+
     [webView expectUpdatesAfterCommand:@"DeleteBackward" withArgument:nil expectedRemovals:@[attachment.get()] expectedInsertions:@[]];
+    [attachment expectRequestedDataToBe:nil];
+
     [webView expectUpdatesAfterCommand:@"Undo" withArgument:nil expectedRemovals:@[] expectedInsertions:@[attachment.get()]];
+    [attachment expectRequestedDataToBe:htmlData.get()];
+
     [webView expectUpdatesAfterCommand:@"Redo" withArgument:nil expectedRemovals:@[attachment.get()] expectedInsertions:@[]];
+    [attachment expectRequestedDataToBe:nil];
 }
 
 TEST(WKAttachmentTests, AttachmentUpdatesWhenChangingFontStyles)
@@ -252,9 +317,11 @@ TEST(WKAttachmentTests, AttachmentUpdatesWhenChangingFontStyles)
     [webView expectUpdatesAfterCommand:@"ToggleBold" withArgument:nil expectedRemovals:@[] expectedInsertions:@[]];
     [webView expectUpdatesAfterCommand:@"ToggleItalic" withArgument:nil expectedRemovals:@[] expectedInsertions:@[]];
     [webView expectUpdatesAfterCommand:@"ToggleUnderline" withArgument:nil expectedRemovals:@[] expectedInsertions:@[]];
+    [attachment expectRequestedDataToBe:testHTMLData()];
 
     // Inserting text should delete the current selection, removing the attachment in the process.
     [webView expectUpdatesAfterCommand:@"InsertText" withArgument:@"foo" expectedRemovals:@[attachment.get()] expectedInsertions:@[]];
+    [attachment expectRequestedDataToBe:nil];
 }
 
 TEST(WKAttachmentTests, AttachmentUpdatesWhenInsertingLists)
@@ -271,6 +338,7 @@ TEST(WKAttachmentTests, AttachmentUpdatesWhenInsertingLists)
     [webView expectUpdatesAfterCommand:@"InsertOrderedList" withArgument:nil expectedRemovals:@[] expectedInsertions:@[]];
     [webView expectUpdatesAfterCommand:@"InsertUnorderedList" withArgument:nil expectedRemovals:@[] expectedInsertions:@[]];
     [webView expectUpdatesAfterCommand:@"InsertUnorderedList" withArgument:nil expectedRemovals:@[] expectedInsertions:@[]];
+    [attachment expectRequestedDataToBe:testHTMLData()];
 }
 
 TEST(WKAttachmentTests, AttachmentUpdatesWhenInsertingRichMarkup)
@@ -289,6 +357,79 @@ TEST(WKAttachmentTests, AttachmentUpdatesWhenInsertingRichMarkup)
         [webView waitForNextPresentationUpdate];
         observer.expectAttachmentUpdates(@[attachment.get()], @[ ]);
     }
+    [attachment expectRequestedDataToBe:nil];
+}
+
+TEST(WKAttachmentTests, AttachmentUpdatesWhenCuttingAndPasting)
+{
+    auto webView = webViewForTestingAttachments();
+    RetainPtr<_WKAttachment> attachment;
+    {
+        ObserveAttachmentUpdatesForScope observer(webView.get());
+        attachment = retainPtr([webView synchronouslyInsertAttachmentWithFilename:@"foo.txt" contentType:@"text/plain" data:testHTMLData() options:nil]);
+        observer.expectAttachmentUpdates(@[], @[attachment.get()]);
+    }
+    [attachment expectRequestedDataToBe:testHTMLData()];
+    [webView _synchronouslyExecuteEditCommand:@"SelectAll" argument:nil];
+    {
+        ObserveAttachmentUpdatesForScope observer(webView.get());
+        [webView _synchronouslyExecuteEditCommand:@"Cut" argument:nil];
+        observer.expectAttachmentUpdates(@[attachment.get()], @[]);
+    }
+    [attachment expectRequestedDataToBe:nil];
+    {
+        ObserveAttachmentUpdatesForScope observer(webView.get());
+        [webView _synchronouslyExecuteEditCommand:@"Paste" argument:nil];
+        observer.expectAttachmentUpdates(@[], @[attachment.get()]);
+    }
+    [attachment expectRequestedDataToBe:testHTMLData()];
+}
+
+TEST(WKAttachmentTests, AttachmentDataForEmptyFile)
+{
+    auto webView = webViewForTestingAttachments();
+    RetainPtr<_WKAttachment> attachment;
+    {
+        ObserveAttachmentUpdatesForScope observer(webView.get());
+        attachment = retainPtr([webView synchronouslyInsertAttachmentWithFilename:@"empty.txt" contentType:@"text/plain" data:[NSData data] options:nil]);
+        observer.expectAttachmentUpdates(@[], @[attachment.get()]);
+    }
+    [attachment expectRequestedDataToBe:[NSData data]];
+    {
+        ObserveAttachmentUpdatesForScope scope(webView.get());
+        [webView _synchronouslyExecuteEditCommand:@"DeleteBackward" argument:nil];
+        scope.expectAttachmentUpdates(@[attachment.get()], @[]);
+    }
+    [attachment expectRequestedDataToBe:nil];
+}
+
+TEST(WKAttachmentTests, MultipleSimultaneousAttachmentDataRequests)
+{
+    auto webView = webViewForTestingAttachments();
+    RetainPtr<NSData> htmlData = testHTMLData();
+    RetainPtr<_WKAttachment> attachment;
+    {
+        ObserveAttachmentUpdatesForScope observer(webView.get());
+        attachment = retainPtr([webView synchronouslyInsertAttachmentWithFilename:@"foo.txt" contentType:@"text/plain" data:htmlData.get() options:nil]);
+        observer.expectAttachmentUpdates(@[], @[attachment.get()]);
+    }
+    __block RetainPtr<NSData> dataForFirstRequest;
+    __block RetainPtr<NSData> dataForSecondRequest;
+    __block bool done = false;
+    [attachment requestData:^(NSData *data, NSError *error) {
+        EXPECT_TRUE(!error);
+        dataForFirstRequest = retainPtr(data);
+    }];
+    [attachment requestData:^(NSData *data, NSError *error) {
+        EXPECT_TRUE(!error);
+        dataForSecondRequest = retainPtr(data);
+        done = true;
+    }];
+
+    Util::run(&done);
+
+    EXPECT_TRUE([dataForFirstRequest isEqualToData:htmlData.get()]);
+    EXPECT_TRUE([dataForSecondRequest isEqualToData:htmlData.get()]);
 }
 
 } // namespace TestWebKitAPI