[Attachment Support] Implement SPI for clients to update a given attachment's data
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 1 Dec 2017 23:36:41 +0000 (23:36 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 1 Dec 2017 23:36:41 +0000 (23:36 +0000)
https://bugs.webkit.org/show_bug.cgi?id=180184
<rdar://problem/35355731>

Reviewed by Tim Horton.

Source/WebCore:

Add native API support for Mail to update the data (and optionally, the name and type) of a given attachment
element. See per-method comments below for more detail.

Test: WKAttachmentTests.ChangeAttachmentDataAndFileInformation
      WKAttachmentTests.ChangeAttachmentDataUpdatesWithInPlaceDisplay

* editing/Editor.cpp:
(WebCore::Editor::insertAttachment):
* html/HTMLAttachmentElement.cpp:
(WebCore::HTMLAttachmentElement::setFile):
(WebCore::HTMLAttachmentElement::invalidateShadowRootChildrenIfNecessary):

Pull out logic to hide and reset shadow DOM state into a separate helper, and additionally hide both the image
and video child elements if they exist. This prevents us from getting into a state where both image and video
elements may appear side-by-side when changing data from an image to a video or vice versa.

(WebCore::HTMLAttachmentElement::updateFileWithData):

Add a new helper to update the backing File of an attachment element from data, optionally updating the filename
and content type as well.

(WebCore::HTMLAttachmentElement::populateShadowRootIfNecessary):
* html/HTMLAttachmentElement.h:

Source/WebKit:

Add plumbing to the web process for setting the attachment data (and optionally, the content type and/or file
name) of a given attachment. See WebCore ChangeLog for more detail. Changes covered by new API tests.

* UIProcess/API/APIAttachment.cpp:
(API::Attachment::setDataAndContentType):
* UIProcess/API/APIAttachment.h:
* UIProcess/API/Cocoa/_WKAttachment.h:

Add nullability annotations around _WKAttachment SPI methods.

* UIProcess/API/Cocoa/_WKAttachment.mm:
(-[_WKAttachment setData:newContentType:newFilename:completion:]):
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::setAttachmentDataAndContentType):
* UIProcess/WebPageProxy.h:
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::setAttachmentDataAndContentType):
* WebProcess/WebPage/WebPage.h:
* WebProcess/WebPage/WebPage.messages.in:

Tools:

Adds two new API tests to exercise the attachment data update flow.

* TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:
(TestWebKitAPI::ObserveAttachmentUpdatesForScope::ObserveAttachmentUpdatesForScope):
(-[_WKAttachment synchronouslySetDisplayOptions:error:]):
(-[_WKAttachment synchronouslyRequestData:]):
(-[_WKAttachment synchronouslySetData:newContentType:newFilename:error:]):
(TestWebKitAPI::TEST):

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

16 files changed:
Source/WebCore/ChangeLog
Source/WebCore/editing/Editor.cpp
Source/WebCore/html/HTMLAttachmentElement.cpp
Source/WebCore/html/HTMLAttachmentElement.h
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/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 b01d6fc..cbaaeea 100644 (file)
@@ -1,3 +1,35 @@
+2017-12-01  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [Attachment Support] Implement SPI for clients to update a given attachment's data
+        https://bugs.webkit.org/show_bug.cgi?id=180184
+        <rdar://problem/35355731>
+
+        Reviewed by Tim Horton.
+
+        Add native API support for Mail to update the data (and optionally, the name and type) of a given attachment
+        element. See per-method comments below for more detail.
+
+        Test: WKAttachmentTests.ChangeAttachmentDataAndFileInformation
+              WKAttachmentTests.ChangeAttachmentDataUpdatesWithInPlaceDisplay
+
+        * editing/Editor.cpp:
+        (WebCore::Editor::insertAttachment):
+        * html/HTMLAttachmentElement.cpp:
+        (WebCore::HTMLAttachmentElement::setFile):
+        (WebCore::HTMLAttachmentElement::invalidateShadowRootChildrenIfNecessary):
+
+        Pull out logic to hide and reset shadow DOM state into a separate helper, and additionally hide both the image
+        and video child elements if they exist. This prevents us from getting into a state where both image and video
+        elements may appear side-by-side when changing data from an image to a video or vice versa.
+
+        (WebCore::HTMLAttachmentElement::updateFileWithData):
+
+        Add a new helper to update the backing File of an attachment element from data, optionally updating the filename
+        and content type as well.
+
+        (WebCore::HTMLAttachmentElement::populateShadowRootIfNecessary):
+        * html/HTMLAttachmentElement.h:
+
 2017-12-01  Chris Dumez  <cdumez@apple.com>
 
         Get rid of microtask in ServiceWorkerContainer::jobResolvedWithRegistration()
index bcc6fdc..152f46d 100644 (file)
@@ -3802,7 +3802,7 @@ void Editor::insertAttachment(const String& identifier, const AttachmentDisplayO
 {
     if (!contentType)
         contentType = File::contentTypeForFile(filename);
-    insertAttachmentFromFile(identifier, options, filename, *contentType, File::create(Blob::create(data, *contentType), filename));
+    insertAttachmentFromFile(identifier, options, filename, *contentType, File::create(Blob::create(WTFMove(data), *contentType), filename));
 }
 
 void Editor::insertAttachmentFromFile(const String& identifier, const AttachmentDisplayOptions& options, const String& filename, const String& contentType, Ref<File>&& file)
index 54f0320..1a03379 100644 (file)
@@ -43,6 +43,7 @@
 #include "RenderBlockFlow.h"
 #include "ShadowRoot.h"
 #include "SharedBuffer.h"
+#include <pal/FileSizeFormatter.h>
 
 namespace WebCore {
 
@@ -120,11 +121,20 @@ void HTMLAttachmentElement::setFile(RefPtr<File>&& file)
     if (auto* renderAttachment = attachmentRenderer())
         renderAttachment->invalidate();
 
-    if (auto image = innerImage())
+    invalidateShadowRootChildrenIfNecessary();
+    populateShadowRootIfNecessary();
+}
+
+void HTMLAttachmentElement::invalidateShadowRootChildrenIfNecessary()
+{
+    if (auto image = innerImage()) {
         image->setAttributeWithoutSynchronization(srcAttr, emptyString());
-    if (auto video = innerVideo())
+        image->setInlineStyleProperty(CSSPropertyDisplay, CSSValueNone, true);
+    }
+    if (auto video = innerVideo()) {
         video->setAttributeWithoutSynchronization(srcAttr, emptyString());
-    populateShadowRootIfNecessary();
+        video->setInlineStyleProperty(CSSPropertyDisplay, CSSValueNone, true);
+    }
 }
 
 RenderAttachment* HTMLAttachmentElement::attachmentRenderer() const
@@ -208,6 +218,18 @@ void HTMLAttachmentElement::updateDisplayMode(AttachmentDisplayMode mode)
     invalidateStyleAndRenderersForSubtree();
 }
 
+void HTMLAttachmentElement::updateFileWithData(Ref<SharedBuffer>&& data, std::optional<String>&& newContentType, std::optional<String>&& newFilename)
+{
+    auto filename = newFilename ? *newFilename : attachmentTitle();
+    auto contentType = newContentType ? *newContentType : File::contentTypeForFile(filename);
+    auto file = File::create(Blob::create(WTFMove(data), contentType), filename);
+
+    setAttributeWithoutSynchronization(titleAttr, filename);
+    setAttributeWithoutSynchronization(subtitleAttr, fileSizeDescription(file->size()));
+    setAttributeWithoutSynchronization(typeAttr, contentType);
+    setFile(WTFMove(file));
+}
+
 Ref<HTMLImageElement> HTMLAttachmentElement::ensureInnerImage()
 {
     if (auto image = innerImage())
@@ -250,14 +272,17 @@ void HTMLAttachmentElement::populateShadowRootIfNecessary()
 
     if (MIMETypeRegistry::isSupportedImageMIMEType(mimeType) || MIMETypeRegistry::isPDFMIMEType(mimeType)) {
         auto image = ensureInnerImage();
-        if (image->attributeWithoutSynchronization(srcAttr).isEmpty())
+        if (image->attributeWithoutSynchronization(srcAttr).isEmpty()) {
             image->setAttributeWithoutSynchronization(srcAttr, DOMURL::createObjectURL(document(), *m_file));
+            image->setInlineStyleProperty(CSSPropertyDisplay, CSSValueInline, true);
+        }
 
     } else if (MIMETypeRegistry::isSupportedMediaMIMEType(mimeType)) {
         auto video = ensureInnerVideo();
         if (video->attributeWithoutSynchronization(srcAttr).isEmpty()) {
             video->setAttributeWithoutSynchronization(srcAttr, DOMURL::createObjectURL(document(), *m_file));
             video->setAttributeWithoutSynchronization(controlsAttr, emptyString());
+            video->setInlineStyleProperty(CSSPropertyDisplay, CSSValueInline, true);
         }
     }
 }
index 5e53d65..c8f35d5 100644 (file)
@@ -51,6 +51,7 @@ public:
     void setUniqueIdentifier(const String&);
 
     WEBCORE_EXPORT void updateDisplayMode(AttachmentDisplayMode);
+    WEBCORE_EXPORT void updateFileWithData(Ref<SharedBuffer>&& data, std::optional<String>&& newContentType = std::nullopt, std::optional<String>&& newFilename = std::nullopt);
 
     InsertedIntoAncestorResult insertedIntoAncestor(InsertionType, ContainerNode&) final;
     void removedFromAncestor(RemovalType, ContainerNode&) final;
@@ -75,6 +76,7 @@ private:
     RefPtr<HTMLVideoElement> innerVideo() const;
 
     void populateShadowRootIfNecessary();
+    void invalidateShadowRootChildrenIfNecessary();
 
     AttachmentDisplayMode defaultDisplayMode() const
     {
index 30b09fd..c47f250 100644 (file)
@@ -1,3 +1,31 @@
+2017-12-01  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [Attachment Support] Implement SPI for clients to update a given attachment's data
+        https://bugs.webkit.org/show_bug.cgi?id=180184
+        <rdar://problem/35355731>
+
+        Reviewed by Tim Horton.
+
+        Add plumbing to the web process for setting the attachment data (and optionally, the content type and/or file
+        name) of a given attachment. See WebCore ChangeLog for more detail. Changes covered by new API tests.
+
+        * UIProcess/API/APIAttachment.cpp:
+        (API::Attachment::setDataAndContentType):
+        * UIProcess/API/APIAttachment.h:
+        * UIProcess/API/Cocoa/_WKAttachment.h:
+
+        Add nullability annotations around _WKAttachment SPI methods.
+
+        * UIProcess/API/Cocoa/_WKAttachment.mm:
+        (-[_WKAttachment setData:newContentType:newFilename:completion:]):
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::setAttachmentDataAndContentType):
+        * UIProcess/WebPageProxy.h:
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::setAttachmentDataAndContentType):
+        * WebProcess/WebPage/WebPage.h:
+        * WebProcess/WebPage/WebPage.messages.in:
+
 2017-12-01  Brady Eidson  <beidson@apple.com>
 
         Add Internals.terminateServiceWorker, and the ability to restart service workers for postMessage.
index 778ba66..85a8a3e 100644 (file)
@@ -64,4 +64,14 @@ void Attachment::setDisplayOptions(WebCore::AttachmentDisplayOptions options, Fu
         callback(WebKit::CallbackBase::Error::OwnerWasInvalidated);
 }
 
+void Attachment::setDataAndContentType(WebCore::SharedBuffer& data, const WTF::String& newContentType, const WTF::String& newFilename, Function<void(WebKit::CallbackBase::Error)>&& callback)
+{
+    auto optionalNewContentType = newContentType.isNull() ? std::nullopt : std::optional<WTF::String> { newContentType };
+    auto optionalNewFilename = newFilename.isNull() ? std::nullopt : std::optional<WTF::String> { newFilename };
+    if (m_webPage)
+        m_webPage->setAttachmentDataAndContentType(m_identifier, data, WTFMove(optionalNewContentType), WTFMove(optionalNewFilename), WTFMove(callback));
+    else
+        callback(WebKit::CallbackBase::Error::OwnerWasInvalidated);
+}
+
 }
index 9fe75c9..d26f2ef 100644 (file)
@@ -51,6 +51,7 @@ public:
     const WTF::String& identifier() const { return m_identifier; }
     void requestData(Function<void(RefPtr<WebCore::SharedBuffer>, WebKit::CallbackBase::Error)>&&);
     void setDisplayOptions(WebCore::AttachmentDisplayOptions, Function<void(WebKit::CallbackBase::Error)>&&);
+    void setDataAndContentType(WebCore::SharedBuffer&, const WTF::String& newContentType, const WTF::String& newFilename, Function<void(WebKit::CallbackBase::Error)>&&);
 
 private:
     explicit Attachment(const WTF::String& identifier, WebKit::WebPageProxy&);
index f83689d..210ca0e 100644 (file)
@@ -29,6 +29,8 @@
 
 #if WK_API_ENABLED
 
+NS_ASSUME_NONNULL_BEGIN
+
 typedef NS_ENUM(NSInteger, _WKAttachmentDisplayMode) {
     _WKAttachmentDisplayModeAuto = 1,
     _WKAttachmentDisplayModeInPlace,
@@ -42,8 +44,11 @@ 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;
-- (void)setDisplayOptions:(_WKAttachmentDisplayOptions *)options completion:(void(^)(NSError *))completionHandler;
+- (void)requestData:(void(^)(NSData * _Nullable, NSError * _Nullable))completionHandler;
+- (void)setDisplayOptions:(_WKAttachmentDisplayOptions *)options completion:(void(^ _Nullable)(NSError * _Nullable))completionHandler;
+- (void)setData:(NSData *)data newContentType:(nullable NSString *)newContentType newFilename:(nullable NSString *)newFilename completion:(void(^ _Nullable)(NSError * _Nullable))completionHandler;
 @end
 
+NS_ASSUME_NONNULL_END
+
 #endif
index e38edc5..939cc87 100644 (file)
@@ -108,6 +108,20 @@ using namespace WebKit;
     });
 }
 
+- (void)setData:(NSData *)data newContentType:(NSString *)newContentType newFilename:(NSString *)newFilename completion:(void(^)(NSError *))completionHandler
+{
+    auto buffer = WebCore::SharedBuffer::create(data);
+    _attachment->setDataAndContentType(buffer.get(), newContentType, newFilename, [capturedBlock = makeBlockPtr(completionHandler), capturedBuffer = buffer.copyRef()] (CallbackBase::Error error) {
+        if (!capturedBlock)
+            return;
+
+        if (error == CallbackBase::Error::None)
+            capturedBlock(nil);
+        else
+            capturedBlock([NSError errorWithDomain:WKErrorDomain code:1 userInfo:nil]);
+    });
+}
+
 - (NSString *)uniqueIdentifier
 {
     return _attachment->identifier();
index fdb1f4f..94535be 100644 (file)
@@ -7183,6 +7183,17 @@ void WebPageProxy::setAttachmentDisplayOptions(const String& identifier, Attachm
     m_process->send(Messages::WebPage::SetAttachmentDisplayOptions(identifier, options, callbackID), m_pageID);
 }
 
+void WebPageProxy::setAttachmentDataAndContentType(const String& identifier, SharedBuffer& data, std::optional<String>&& newContentType, std::optional<String>&& newFilename, 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::SetAttachmentDataAndContentType(identifier, IPC::SharedBufferDataReference { &data }, WTFMove(newContentType), WTFMove(newFilename), callbackID), m_pageID);
+}
+
 void WebPageProxy::didInsertAttachment(const String& identifier)
 {
     m_pageClient.didInsertAttachment(identifier);
index 0a27524..fb29824 100644 (file)
@@ -1226,6 +1226,7 @@ public:
     void insertAttachment(const String& identifier, const WebCore::AttachmentDisplayOptions&, 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)>&&);
     void setAttachmentDisplayOptions(const String& identifier, WebCore::AttachmentDisplayOptions, Function<void(CallbackBase::Error)>&&);
+    void setAttachmentDataAndContentType(const String& identifier, WebCore::SharedBuffer& data, std::optional<String>&& newContentType, std::optional<String>&& newFilename, Function<void(CallbackBase::Error)>&&);
 #endif
 
 private:
index 13b3a7b..b7ad5d3 100644 (file)
@@ -5799,6 +5799,15 @@ void WebPage::setAttachmentDisplayOptions(const String& identifier, const Attach
     send(Messages::WebPageProxy::VoidCallback(callbackID));
 }
 
+void WebPage::setAttachmentDataAndContentType(const String& identifier, const IPC::DataReference& data, std::optional<String> newContentType, std::optional<String> newFilename, CallbackID callbackID)
+{
+    if (auto attachment = attachmentElementWithIdentifier(identifier)) {
+        attachment->document().updateLayout();
+        attachment->updateFileWithData(SharedBuffer::create(data.data(), data.size()), WTFMove(newContentType), WTFMove(newFilename));
+    }
+    send(Messages::WebPageProxy::VoidCallback(callbackID));
+}
+
 RefPtr<HTMLAttachmentElement> WebPage::attachmentElementWithIdentifier(const String& identifier) const
 {
     // FIXME: Handle attachment elements in subframes too as well.
index 42be0f8..10ba9bb 100644 (file)
@@ -1018,6 +1018,7 @@ public:
     void insertAttachment(const String& identifier, const WebCore::AttachmentDisplayOptions&, const String& filename, std::optional<String> contentType, const IPC::DataReference&, CallbackID);
     void requestAttachmentData(const String& identifier, CallbackID);
     void setAttachmentDisplayOptions(const String& identifier, const WebCore::AttachmentDisplayOptions&, CallbackID);
+    void setAttachmentDataAndContentType(const String& identifier, const IPC::DataReference&, std::optional<String> newContentType, std::optional<String> newFilename, CallbackID);
 #endif
 
 private:
index a0836eb..faf4ccb 100644 (file)
@@ -489,5 +489,6 @@ messages -> WebPage LegacyReceiver {
     InsertAttachment(String identifier, struct WebCore::AttachmentDisplayOptions options, String filename, std::optional<String> contentType, IPC::DataReference data, WebKit::CallbackID callbackID)
     RequestAttachmentData(String identifier, WebKit::CallbackID callbackID)
     SetAttachmentDisplayOptions(String identifier, struct WebCore::AttachmentDisplayOptions options, WebKit::CallbackID callbackID)
+    SetAttachmentDataAndContentType(String identifier, IPC::DataReference data, std::optional<String> newContentType, std::optional<String> newFilename, WebKit::CallbackID callbackID)
 #endif
 }
index 250da60..cbf7e9f 100644 (file)
@@ -1,3 +1,20 @@
+2017-12-01  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [Attachment Support] Implement SPI for clients to update a given attachment's data
+        https://bugs.webkit.org/show_bug.cgi?id=180184
+        <rdar://problem/35355731>
+
+        Reviewed by Tim Horton.
+
+        Adds two new API tests to exercise the attachment data update flow.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:
+        (TestWebKitAPI::ObserveAttachmentUpdatesForScope::ObserveAttachmentUpdatesForScope):
+        (-[_WKAttachment synchronouslySetDisplayOptions:error:]):
+        (-[_WKAttachment synchronouslyRequestData:]):
+        (-[_WKAttachment synchronouslySetData:newContentType:newFilename:error:]):
+        (TestWebKitAPI::TEST):
+
 2017-12-01  Alicia Boya García  <aboya@igalia.com>
 
         [GTK] Add SampleMap.cpp API tests to CMake
index b3db738..93ad2a4 100644 (file)
@@ -82,7 +82,7 @@ public:
     ObserveAttachmentUpdatesForScope(TestWKWebView *webView)
         : m_webView(webView)
     {
-        m_previousDelegate = retainPtr(webView.UIDelegate);
+        m_previousDelegate = webView.UIDelegate;
         m_observer = adoptNS([[AttachmentUpdateObserver alloc] init]);
         webView.UIDelegate = m_observer.get();
     }
@@ -233,7 +233,7 @@ static _WKAttachmentDisplayOptions *displayOptionsWithMode(_WKAttachmentDisplayM
     __block RetainPtr<NSError> resultError;
     __block bool done = false;
     [self setDisplayOptions:options completion:^(NSError *error) {
-        resultError = retainPtr(error);
+        resultError = error;
         done = true;
     }];
 
@@ -249,8 +249,8 @@ static _WKAttachmentDisplayOptions *displayOptionsWithMode(_WKAttachmentDisplayM
     __block RetainPtr<NSError> resultError;
     __block bool done = false;
     [self requestData:^(NSData *data, NSError *error) {
-        result = retainPtr(data);
-        resultError = retainPtr(error);
+        result = data;
+        resultError = error;
         done = true;
     }];
 
@@ -262,6 +262,21 @@ static _WKAttachmentDisplayOptions *displayOptionsWithMode(_WKAttachmentDisplayM
     return result.autorelease();
 }
 
+- (void)synchronouslySetData:(NSData *)data newContentType:(NSString *)newContentType newFilename:(NSString *)newFilename error:(NSError **)error
+{
+    __block RetainPtr<NSError> resultError;
+    __block bool done = false;
+    [self setData:data newContentType:newContentType newFilename:newFilename completion:^(NSError *error) {
+        resultError = error;
+        done = true;
+    }];
+
+    TestWebKitAPI::Util::run(&done);
+
+    if (error)
+        *error = resultError.autorelease();
+}
+
 - (void)expectRequestedDataToBe:(NSData *)expectedData
 {
     NSError *dataRequestError = nil;
@@ -286,7 +301,7 @@ TEST(WKAttachmentTests, AttachmentElementInsertion)
     {
         ObserveAttachmentUpdatesForScope observer(webView.get());
         // Use the given content type for the attachment element's type.
-        firstAttachment = retainPtr([webView synchronouslyInsertAttachmentWithFilename:@"foo" contentType:@"text/html" data:testHTMLData() options:nil]);
+        firstAttachment = [webView synchronouslyInsertAttachmentWithFilename:@"foo" contentType:@"text/html" data:testHTMLData() options:nil];
         EXPECT_WK_STREQ(@"foo", [webView valueOfAttribute:@"title" forQuerySelector:@"attachment"]);
         EXPECT_WK_STREQ(@"text/html", [webView valueOfAttribute:@"type" forQuerySelector:@"attachment"]);
         EXPECT_WK_STREQ(@"38 bytes", [webView valueOfAttribute:@"subtitle" forQuerySelector:@"attachment"]);
@@ -299,7 +314,7 @@ TEST(WKAttachmentTests, AttachmentElementInsertion)
         ObserveAttachmentUpdatesForScope scope(webView.get());
         // Since no content type is explicitly specified, compute it from the file extension.
         [webView _executeEditCommand:@"DeleteBackward" argument:nil completion:nil];
-        secondAttachment = retainPtr([webView synchronouslyInsertAttachmentWithFilename:@"bar.png" contentType:nil data:testImageData() options:nil]);
+        secondAttachment = [webView synchronouslyInsertAttachmentWithFilename:@"bar.png" contentType:nil data:testImageData() options:nil];
         EXPECT_WK_STREQ(@"bar.png", [webView valueOfAttribute:@"title" forQuerySelector:@"attachment"]);
         EXPECT_WK_STREQ(@"image/png", [webView valueOfAttribute:@"type" forQuerySelector:@"attachment"]);
         EXPECT_WK_STREQ(@"37 KB", [webView valueOfAttribute:@"subtitle" forQuerySelector:@"attachment"]);
@@ -316,7 +331,7 @@ TEST(WKAttachmentTests, AttachmentUpdatesWhenInsertingAndDeletingNewline)
     RetainPtr<_WKAttachment> attachment;
     {
         ObserveAttachmentUpdatesForScope observer(webView.get());
-        attachment = retainPtr([webView synchronouslyInsertAttachmentWithFilename:@"foo.txt" contentType:@"text/plain" data:testHTMLData() options:nil]);
+        attachment = [webView synchronouslyInsertAttachmentWithFilename:@"foo.txt" contentType:@"text/plain" data:testHTMLData() options:nil];
         observer.expectAttachmentUpdates(@[ ], @[attachment.get()]);
     }
     [webView expectUpdatesAfterCommand:@"InsertParagraph" withArgument:nil expectedRemovals:@[] expectedInsertions:@[]];
@@ -334,11 +349,11 @@ TEST(WKAttachmentTests, AttachmentUpdatesWhenInsertingAndDeletingNewline)
 TEST(WKAttachmentTests, AttachmentUpdatesWhenUndoingAndRedoing)
 {
     auto webView = webViewForTestingAttachments();
-    RetainPtr<NSData> htmlData = retainPtr(testHTMLData());
+    RetainPtr<NSData> htmlData = testHTMLData();
     RetainPtr<_WKAttachment> attachment;
     {
         ObserveAttachmentUpdatesForScope observer(webView.get());
-        attachment = retainPtr([webView synchronouslyInsertAttachmentWithFilename:@"foo.txt" contentType:@"text/plain" data:testHTMLData() options:nil]);
+        attachment = [webView synchronouslyInsertAttachmentWithFilename:@"foo.txt" contentType:@"text/plain" data:testHTMLData() options:nil];
         observer.expectAttachmentUpdates(@[ ], @[attachment.get()]);
     }
     [webView expectUpdatesAfterCommand:@"Undo" withArgument:nil expectedRemovals:@[attachment.get()] expectedInsertions:@[]];
@@ -364,7 +379,7 @@ TEST(WKAttachmentTests, AttachmentUpdatesWhenChangingFontStyles)
     [webView _synchronouslyExecuteEditCommand:@"InsertText" argument:@"Hello"];
     {
         ObserveAttachmentUpdatesForScope observer(webView.get());
-        attachment = retainPtr([webView synchronouslyInsertAttachmentWithFilename:@"foo.txt" contentType:@"text/plain" data:testHTMLData() options:nil]);
+        attachment = [webView synchronouslyInsertAttachmentWithFilename:@"foo.txt" contentType:@"text/plain" data:testHTMLData() options:nil];
         observer.expectAttachmentUpdates(@[ ], @[attachment.get()]);
     }
     [webView expectUpdatesAfterCommand:@"InsertText" withArgument:@"World" expectedRemovals:@[] expectedInsertions:@[]];
@@ -385,7 +400,7 @@ TEST(WKAttachmentTests, AttachmentUpdatesWhenInsertingLists)
     RetainPtr<_WKAttachment> attachment;
     {
         ObserveAttachmentUpdatesForScope observer(webView.get());
-        attachment = retainPtr([webView synchronouslyInsertAttachmentWithFilename:@"foo.txt" contentType:@"text/plain" data:testHTMLData() options:nil]);
+        attachment = [webView synchronouslyInsertAttachmentWithFilename:@"foo.txt" contentType:@"text/plain" data:testHTMLData() options:nil];
         observer.expectAttachmentUpdates(@[ ], @[attachment.get()]);
     }
     [webView expectUpdatesAfterCommand:@"InsertOrderedList" withArgument:nil expectedRemovals:@[] expectedInsertions:@[]];
@@ -421,7 +436,7 @@ TEST(WKAttachmentTests, AttachmentUpdatesWhenCuttingAndPasting)
     RetainPtr<_WKAttachment> attachment;
     {
         ObserveAttachmentUpdatesForScope observer(webView.get());
-        attachment = retainPtr([webView synchronouslyInsertAttachmentWithFilename:@"foo.txt" contentType:@"text/plain" data:testHTMLData() options:nil]);
+        attachment = [webView synchronouslyInsertAttachmentWithFilename:@"foo.txt" contentType:@"text/plain" data:testHTMLData() options:nil];
         observer.expectAttachmentUpdates(@[], @[attachment.get()]);
     }
     [attachment expectRequestedDataToBe:testHTMLData()];
@@ -446,7 +461,7 @@ TEST(WKAttachmentTests, AttachmentDataForEmptyFile)
     RetainPtr<_WKAttachment> attachment;
     {
         ObserveAttachmentUpdatesForScope observer(webView.get());
-        attachment = retainPtr([webView synchronouslyInsertAttachmentWithFilename:@"empty.txt" contentType:@"text/plain" data:[NSData data] options:nil]);
+        attachment = [webView synchronouslyInsertAttachmentWithFilename:@"empty.txt" contentType:@"text/plain" data:[NSData data] options:nil];
         observer.expectAttachmentUpdates(@[], @[attachment.get()]);
     }
     [attachment expectRequestedDataToBe:[NSData data]];
@@ -465,7 +480,7 @@ TEST(WKAttachmentTests, MultipleSimultaneousAttachmentDataRequests)
     RetainPtr<_WKAttachment> attachment;
     {
         ObserveAttachmentUpdatesForScope observer(webView.get());
-        attachment = retainPtr([webView synchronouslyInsertAttachmentWithFilename:@"foo.txt" contentType:@"text/plain" data:htmlData.get() options:nil]);
+        attachment = [webView synchronouslyInsertAttachmentWithFilename:@"foo.txt" contentType:@"text/plain" data:htmlData.get() options:nil];
         observer.expectAttachmentUpdates(@[], @[attachment.get()]);
     }
     __block RetainPtr<NSData> dataForFirstRequest;
@@ -473,11 +488,11 @@ TEST(WKAttachmentTests, MultipleSimultaneousAttachmentDataRequests)
     __block bool done = false;
     [attachment requestData:^(NSData *data, NSError *error) {
         EXPECT_TRUE(!error);
-        dataForFirstRequest = retainPtr(data);
+        dataForFirstRequest = data;
     }];
     [attachment requestData:^(NSData *data, NSError *error) {
         EXPECT_TRUE(!error);
-        dataForSecondRequest = retainPtr(data);
+        dataForSecondRequest = data;
         done = true;
     }];
 
@@ -494,7 +509,7 @@ TEST(WKAttachmentTests, InPlaceImageAttachmentToggleDisplayMode)
     RetainPtr<_WKAttachment> attachment;
     {
         ObserveAttachmentUpdatesForScope observer(webView.get());
-        attachment = retainPtr([webView synchronouslyInsertAttachmentWithFilename:@"icon.png" contentType:@"image/png" data:imageData.get() options:displayOptionsWithMode(_WKAttachmentDisplayModeAsIcon)]);
+        attachment = [webView synchronouslyInsertAttachmentWithFilename:@"icon.png" contentType:@"image/png" data:imageData.get() options:displayOptionsWithMode(_WKAttachmentDisplayModeAsIcon)];
         [attachment expectRequestedDataToBe:imageData.get()];
         observer.expectAttachmentUpdates(@[], @[attachment.get()]);
     }
@@ -520,7 +535,7 @@ TEST(WKAttachmentTests, InPlaceImageAttachmentParagraphInsertion)
     RetainPtr<_WKAttachment> attachment;
     {
         ObserveAttachmentUpdatesForScope observer(webView.get());
-        attachment = retainPtr([webView synchronouslyInsertAttachmentWithFilename:@"icon.png" contentType:@"image/png" data:imageData.get() options:displayOptionsWithMode(_WKAttachmentDisplayModeInPlace)]);
+        attachment = [webView synchronouslyInsertAttachmentWithFilename:@"icon.png" contentType:@"image/png" data:imageData.get() options:displayOptionsWithMode(_WKAttachmentDisplayModeInPlace)];
         observer.expectAttachmentUpdates(@[], @[attachment.get()]);
     }
     [webView expectUpdatesAfterCommand:@"InsertParagraph" withArgument:nil expectedRemovals:@[] expectedInsertions:@[]];
@@ -544,7 +559,7 @@ TEST(WKAttachmentTests, InPlaceVideoAttachmentInsertionWithinList)
     [webView _synchronouslyExecuteEditCommand:@"InsertOrderedList" argument:nil];
     {
         ObserveAttachmentUpdatesForScope observer(webView.get());
-        attachment = retainPtr([webView synchronouslyInsertAttachmentWithFilename:@"test.mp4" contentType:@"video/mp4" data:videoData.get() options:displayOptionsWithMode(_WKAttachmentDisplayModeInPlace)]);
+        attachment = [webView synchronouslyInsertAttachmentWithFilename:@"test.mp4" contentType:@"video/mp4" data:videoData.get() options:displayOptionsWithMode(_WKAttachmentDisplayModeInPlace)];
         observer.expectAttachmentUpdates(@[], @[attachment.get()]);
     }
     [webView waitForAttachmentElementSizeToBecome:CGSizeMake(320, 240)];
@@ -564,7 +579,7 @@ TEST(WKAttachmentTests, InPlacePDFAttachmentCutAndPaste)
     RetainPtr<_WKAttachment> attachment;
     {
         ObserveAttachmentUpdatesForScope observer(webView.get());
-        attachment = retainPtr([webView synchronouslyInsertAttachmentWithFilename:@"test.pdf" contentType:@"application/pdf" data:pdfData.get() options:displayOptionsWithMode(_WKAttachmentDisplayModeInPlace)]);
+        attachment = [webView synchronouslyInsertAttachmentWithFilename:@"test.pdf" contentType:@"application/pdf" data:pdfData.get() options:displayOptionsWithMode(_WKAttachmentDisplayModeInPlace)];
         observer.expectAttachmentUpdates(@[], @[attachment.get()]);
         [webView waitForAttachmentElementSizeToBecome:CGSizeMake(130, 29)];
     }
@@ -577,6 +592,89 @@ TEST(WKAttachmentTests, InPlacePDFAttachmentCutAndPaste)
     [attachment expectRequestedDataToBe:pdfData.get()];
 }
 
+TEST(WKAttachmentTests, ChangeAttachmentDataAndFileInformation)
+{
+    auto webView = webViewForTestingAttachments();
+    RetainPtr<_WKAttachment> attachment;
+    {
+        RetainPtr<NSData> pdfData = testPDFData();
+        ObserveAttachmentUpdatesForScope observer(webView.get());
+        attachment = [webView synchronouslyInsertAttachmentWithFilename:@"test.pdf" contentType:@"application/pdf" data:pdfData.get() options:displayOptionsWithMode(_WKAttachmentDisplayModeAsIcon)];
+        [attachment expectRequestedDataToBe:pdfData.get()];
+        EXPECT_WK_STREQ(@"test.pdf", [webView valueOfAttribute:@"title" forQuerySelector:@"attachment"]);
+        EXPECT_WK_STREQ(@"application/pdf", [webView valueOfAttribute:@"type" forQuerySelector:@"attachment"]);
+        observer.expectAttachmentUpdates(@[], @[attachment.get()]);
+    }
+    {
+        RetainPtr<NSData> imageData = testImageData();
+        ObserveAttachmentUpdatesForScope observer(webView.get());
+        [attachment synchronouslySetData:imageData.get() newContentType:@"image/png" newFilename:@"icon.png" error:nil];
+        [attachment expectRequestedDataToBe:imageData.get()];
+        EXPECT_WK_STREQ(@"icon.png", [webView valueOfAttribute:@"title" forQuerySelector:@"attachment"]);
+        EXPECT_WK_STREQ(@"image/png", [webView valueOfAttribute:@"type" forQuerySelector:@"attachment"]);
+        observer.expectAttachmentUpdates(@[], @[]);
+    }
+    {
+        RetainPtr<NSData> textData = testHTMLData();
+        ObserveAttachmentUpdatesForScope observer(webView.get());
+        [attachment synchronouslySetDisplayOptions:displayOptionsWithMode(_WKAttachmentDisplayModeAsIcon) error:nil];
+        // The new content type should be inferred from the file name.
+        [attachment synchronouslySetData:textData.get() newContentType:nil newFilename:@"foo.txt" error:nil];
+        [attachment expectRequestedDataToBe:textData.get()];
+        EXPECT_WK_STREQ(@"foo.txt", [webView valueOfAttribute:@"title" forQuerySelector:@"attachment"]);
+        EXPECT_WK_STREQ(@"text/plain", [webView valueOfAttribute:@"type" forQuerySelector:@"attachment"]);
+        observer.expectAttachmentUpdates(@[], @[]);
+    }
+    {
+        RetainPtr<NSData> secondTextData = [@"Hello world" dataUsingEncoding:NSUTF8StringEncoding];
+        ObserveAttachmentUpdatesForScope observer(webView.get());
+        // Both the previous file name and type should be inferred.
+        [attachment synchronouslySetData:secondTextData.get() newContentType:nil newFilename:nil error:nil];
+        [attachment expectRequestedDataToBe:secondTextData.get()];
+        EXPECT_WK_STREQ(@"foo.txt", [webView valueOfAttribute:@"title" forQuerySelector:@"attachment"]);
+        EXPECT_WK_STREQ(@"text/plain", [webView valueOfAttribute:@"type" forQuerySelector:@"attachment"]);
+        observer.expectAttachmentUpdates(@[], @[]);
+    }
+    [webView expectUpdatesAfterCommand:@"DeleteBackward" withArgument:nil expectedRemovals:@[attachment.get()] expectedInsertions:@[]];
+}
+
+TEST(WKAttachmentTests, ChangeAttachmentDataUpdatesWithInPlaceDisplay)
+{
+    auto webView = webViewForTestingAttachments();
+    RetainPtr<_WKAttachment> attachment;
+    {
+        RetainPtr<NSData> pdfData = testPDFData();
+        ObserveAttachmentUpdatesForScope observer(webView.get());
+        attachment = [webView synchronouslyInsertAttachmentWithFilename:@"test.pdf" contentType:@"application/pdf" data:pdfData.get() options:displayOptionsWithMode(_WKAttachmentDisplayModeInPlace)];
+        [webView waitForAttachmentElementSizeToBecome:CGSizeMake(130, 29)];
+        [attachment expectRequestedDataToBe:pdfData.get()];
+        EXPECT_WK_STREQ(@"test.pdf", [webView valueOfAttribute:@"title" forQuerySelector:@"attachment"]);
+        EXPECT_WK_STREQ(@"application/pdf", [webView valueOfAttribute:@"type" forQuerySelector:@"attachment"]);
+        observer.expectAttachmentUpdates(@[], @[attachment.get()]);
+    }
+    {
+        RetainPtr<NSData> videoData = testVideoData();
+        ObserveAttachmentUpdatesForScope observer(webView.get());
+        [attachment synchronouslySetData:videoData.get() newContentType:@"video/mp4" newFilename:@"test.mp4" error:nil];
+        [webView waitForAttachmentElementSizeToBecome:CGSizeMake(320, 240)];
+        [attachment expectRequestedDataToBe:videoData.get()];
+        EXPECT_WK_STREQ(@"test.mp4", [webView valueOfAttribute:@"title" forQuerySelector:@"attachment"]);
+        EXPECT_WK_STREQ(@"video/mp4", [webView valueOfAttribute:@"type" forQuerySelector:@"attachment"]);
+        observer.expectAttachmentUpdates(@[], @[]);
+    }
+    {
+        RetainPtr<NSData> imageData = testImageData();
+        ObserveAttachmentUpdatesForScope observer(webView.get());
+        [attachment synchronouslySetData:imageData.get() newContentType:@"image/png" newFilename:@"icon.png" error:nil];
+        [webView waitForAttachmentElementSizeToBecome:CGSizeMake(215, 174)];
+        [attachment expectRequestedDataToBe:imageData.get()];
+        EXPECT_WK_STREQ(@"icon.png", [webView valueOfAttribute:@"title" forQuerySelector:@"attachment"]);
+        EXPECT_WK_STREQ(@"image/png", [webView valueOfAttribute:@"type" forQuerySelector:@"attachment"]);
+        observer.expectAttachmentUpdates(@[], @[]);
+    }
+    [webView expectUpdatesAfterCommand:@"DeleteBackward" withArgument:nil expectedRemovals:@[attachment.get()] expectedInsertions:@[]];
+}
+
 } // namespace TestWebKitAPI
 
 #endif // WK_API_ENABLED