Attachment 347436 Details for Bug 188715 – Depends on 188496

[patch] Depends on 188496

bug-188715-20180818001401.patch (text/plain), 34.78 KB, created by Wenson Hsieh on 2018-08-18 00:14:01 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2018-08-18 00:14:01 PDT

Size: 34.78 KB

patch

obsolete

>Subversion Revision: 234984
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 0c4055c4d8ea6f3e5b36f2a830f0fe1011f52ef0..9620af2a65eb5b2217faaa1e4a787bb99126eb95 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,23 @@
>+2018-08-17  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [Attachment Support] Remove _WKAttachments and notify the UI client upon mainframe navigation
>+        https://bugs.webkit.org/show_bug.cgi?id=188715
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Rename didInsertAttachment to didInsertAttachmentWithIdentifier. See WebKit ChangeLog for more detail.
>+
>+        Tests:  WKAttachmentTests.InvalidateAttachmentsAfterMainFrameNavigation
>+                WKAttachmentTests.InvalidateAttachmentsAfterWebProcessTermination
>+
>+        * editing/Editor.cpp:
>+        (WebCore::Editor::notifyClientOfAttachmentUpdates):
>+        * page/EditorClient.h:
>+        (WebCore::EditorClient::didInsertAttachmentWithIdentifier):
>+        (WebCore::EditorClient::didRemoveAttachmentWithIdentifier):
>+        (WebCore::EditorClient::didInsertAttachment): Deleted.
>+        (WebCore::EditorClient::didRemoveAttachment): Deleted.
>+
> 2018-08-17  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         [Cocoa] Augment _WKAttachment SPI to handle NSFileWrappers in addition to NSData
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index d02c8b04a28286e3c467be4edb9d15c227640543..3a2bdd637a06004e096bda4262782f3b66fa2aab 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,94 @@
>+2018-08-17  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [Attachment Support] Remove _WKAttachments and notify the UI client upon mainframe navigation
>+        https://bugs.webkit.org/show_bug.cgi?id=188715
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Adds logic for invalidating Attachment objects upon mainframe navigation, or upon web content process
>+        termination. An invalid Attachment's wrapper may still be retained by a UI client; however, calls to -info and
>+        other SPI methods will either return nil or immediately invoke their completion handlers with a nil result and
>+        an NSError. See changes below for more detail.
>+
>+        This patch also takes some extra measures to avoid sending redundant or unexpected removal updates to the UI
>+        client upon invalidating all Attachments.
>+
>+        See Tools changes for new API tests.
>+
>+        * UIProcess/API/APIAttachment.cpp:
>+        (API::Attachment::invalidate):
>+        * UIProcess/API/APIAttachment.h:
>+        * UIProcess/API/Cocoa/WKWebView.mm:
>+        (-[WKWebView _didInsertAttachment:withSource:]):
>+        (-[WKWebView _didRemoveAttachment:]):
>+
>+        Refactor these methods to take references to the API::Attachment that is being inserted or removed, rather than
>+        take attachment identifiers. This allows us to keep logic for setting the InsertionState of Attachment objects
>+        in WebPageProxy, rather than in platform code.
>+
>+        * UIProcess/API/Cocoa/WKWebViewInternal.h:
>+        * UIProcess/API/Cocoa/_WKAttachment.h:
>+        * UIProcess/API/Cocoa/_WKAttachment.mm:
>+        (-[_WKAttachment info]):
>+
>+        If the attachment object is invalid, return nil.
>+
>+        (-[_WKAttachment setDisplayOptions:completion:]):
>+        (-[_WKAttachment setFileWrapper:contentType:completion:]):
>+
>+        If the attachment object is invalid, immediately invoke the completion block with an error.
>+
>+        (-[_WKAttachment isConnected]):
>+
>+        Add a new readonly property for a client to easily determine whether a _WKAttachment is connected to the
>+        document, without having to maintain the current set of connected attachment objects by observing insertion and
>+        removal callbacks.
>+
>+        (): Deleted.
>+        * UIProcess/Cocoa/PageClientImplCocoa.h:
>+        * UIProcess/Cocoa/PageClientImplCocoa.mm:
>+        (WebKit::PageClientImplCocoa::didInsertAttachment):
>+        (WebKit::PageClientImplCocoa::didRemoveAttachment):
>+
>+        Make these take API::Attachment&s rather than identifier strings.
>+
>+        * UIProcess/PageClient.h:
>+        (WebKit::PageClient::didInsertAttachment):
>+        (WebKit::PageClient::didRemoveAttachment):
>+
>+        Also make these take API::Attachment&s rather than identifier strings.
>+
>+        * UIProcess/WebPageProxy.cpp:
>+        (WebKit::WebPageProxy::didCommitLoadForFrame):
>+        (WebKit::WebPageProxy::resetStateAfterProcessExited):
>+        (WebKit::WebPageProxy::invalidateAllAttachments):
>+
>+        Introduce a helper function that invalidates all Attachments and notifies the UI client if needed. This is
>+        invoked upon mainframe navigation, and when the web content process terminates.
>+
>+        (WebKit::WebPageProxy::platformRegisterAttachment):
>+        (WebKit::WebPageProxy::didInsertAttachmentWithIdentifier):
>+        (WebKit::WebPageProxy::didRemoveAttachmentWithIdentifier):
>+        (WebKit::WebPageProxy::didInsertAttachment):
>+        (WebKit::WebPageProxy::didRemoveAttachment):
>+
>+        Keep track of whether we've notified the UI client that an Attachment has been inserted into the document. This
>+        allows us to send removal updates to the UI client only for Attachments that are currently in the document, from
>+        the perspective of the UI client, and allows us to avoid sending extra removal updates in
>+        invalidateAllAttachments for Attachments that either have already been removed, or Attachments which we haven't
>+        inserted yet.
>+
>+        * UIProcess/WebPageProxy.h:
>+        * UIProcess/WebPageProxy.messages.in:
>+        * WebProcess/WebCoreSupport/WebEditorClient.cpp:
>+        (WebKit::WebEditorClient::didInsertAttachmentWithIdentifier):
>+        (WebKit::WebEditorClient::didRemoveAttachmentWithIdentifier):
>+        (WebKit::WebEditorClient::didInsertAttachment): Deleted.
>+        (WebKit::WebEditorClient::didRemoveAttachment): Deleted.
>+        * WebProcess/WebCoreSupport/WebEditorClient.h:
>+
>+        Rename did{Insert|Remove}Attachment to did{Insert|Remove}AttachmentWithIdentifier.
>+
> 2018-08-17  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         [Cocoa] Augment _WKAttachment SPI to handle NSFileWrappers in addition to NSData
>diff --git a/Source/WebCore/editing/Editor.cpp b/Source/WebCore/editing/Editor.cpp
>index ca096a2fc770fdadb7d687b6ad8994942a2ad211..5d9d3515e480266d47de69b752e89efcfaf65299 100644
>--- a/Source/WebCore/editing/Editor.cpp
>+++ b/Source/WebCore/editing/Editor.cpp
>@@ -3863,7 +3863,7 @@ void Editor::notifyClientOfAttachmentUpdates()
>         return;
> 
>     for (auto& identifier : removedAttachmentIdentifiers)
>-        client()->didRemoveAttachment(identifier);
>+        client()->didRemoveAttachmentWithIdentifier(identifier);
> 
>     auto* document = m_frame.document();
>     if (!document)
>@@ -3871,7 +3871,7 @@ void Editor::notifyClientOfAttachmentUpdates()
> 
>     for (auto& identifier : insertedAttachmentIdentifiers) {
>         if (auto attachment = document->attachmentForIdentifier(identifier))
>-            client()->didInsertAttachment(identifier, attachment->attributeWithoutSynchronization(HTMLNames::srcAttr));
>+            client()->didInsertAttachmentWithIdentifier(identifier, attachment->attributeWithoutSynchronization(HTMLNames::srcAttr));
>         else
>             ASSERT_NOT_REACHED();
>     }
>diff --git a/Source/WebCore/page/EditorClient.h b/Source/WebCore/page/EditorClient.h
>index 8ffd7360c6aa7eb2b2c7ca90c1f4b1396bfd312b..e9b1585f0dd29065837d55ef5cb22487ee586683 100644
>--- a/Source/WebCore/page/EditorClient.h
>+++ b/Source/WebCore/page/EditorClient.h
>@@ -76,8 +76,8 @@ public:
>     virtual void registerAttachmentIdentifier(const String& /* identifier */, const String& /* contentType */, const String& /* preferredFileName */, Ref<SharedBuffer>&&) { }
>     virtual void registerAttachmentIdentifier(const String& /* identifier */, const String& /* contentType */, const String& /* filePath */) { }
>     virtual void cloneAttachmentData(const String& /* fromIdentifier */, const String& /* toIdentifier */) { }
>-    virtual void didInsertAttachment(const String& /* identifier */, const String& /* source */) { }
>-    virtual void didRemoveAttachment(const String&) { }
>+    virtual void didInsertAttachmentWithIdentifier(const String& /* identifier */, const String& /* source */) { }
>+    virtual void didRemoveAttachmentWithIdentifier(const String&) { }
>     virtual bool supportsClientSideAttachmentData() const { return false; }
> #endif
> 
>diff --git a/Source/WebKit/UIProcess/API/APIAttachment.cpp b/Source/WebKit/UIProcess/API/APIAttachment.cpp
>index afa8e368b4c77fae1cac9333ca2e62045900cd33..05213fc87af391d786ff51e969a09d4be37201c8 100644
>--- a/Source/WebKit/UIProcess/API/APIAttachment.cpp
>+++ b/Source/WebKit/UIProcess/API/APIAttachment.cpp
>@@ -69,4 +69,15 @@ void Attachment::updateAttributes(uint64_t fileSize, const WTF::String& newConte
>         callback(WebKit::CallbackBase::Error::OwnerWasInvalidated);
> }
> 
>+void Attachment::invalidate()
>+{
>+    m_identifier = { };
>+    m_filePath = { };
>+    m_contentType = { };
>+    m_webPage.clear();
>+#if PLATFORM(COCOA)
>+    m_fileWrapper.clear();
>+#endif
>+}
>+
> }
>diff --git a/Source/WebKit/UIProcess/API/APIAttachment.h b/Source/WebKit/UIProcess/API/APIAttachment.h
>index e7ea9cdf905517a2f8193af627cac4cb9e85f9d4..b178de37a716bc65ff9d27336f3e471739168e03 100644
>--- a/Source/WebKit/UIProcess/API/APIAttachment.h
>+++ b/Source/WebKit/UIProcess/API/APIAttachment.h
>@@ -50,10 +50,15 @@ public:
>     static Ref<Attachment> create(const WTF::String& identifier, WebKit::WebPageProxy&);
>     virtual ~Attachment();
> 
>+    enum class InsertionState : uint8 { NotInserted, Inserted };
>+
>     const WTF::String& identifier() const { return m_identifier; }
>     void setDisplayOptions(WebCore::AttachmentDisplayOptions, Function<void(WebKit::CallbackBase::Error)>&&);
>     void updateAttributes(uint64_t fileSize, const WTF::String& newContentType, const WTF::String& newFilename, Function<void(WebKit::CallbackBase::Error)>&&);
> 
>+    void invalidate();
>+    bool isValid() const { return !!m_webPage; }
>+
> #if PLATFORM(COCOA)
>     NSFileWrapper *fileWrapper() const { return m_fileWrapper.get(); }
>     void setFileWrapper(NSFileWrapper *fileWrapper) { m_fileWrapper = fileWrapper; }
>@@ -65,6 +70,9 @@ public:
>     const WTF::String& contentType() const { return m_contentType; }
>     void setContentType(const WTF::String& contentType) { m_contentType = contentType; }
> 
>+    InsertionState insertionState() const { return m_insertionState; }
>+    void setInsertionState(InsertionState state) { m_insertionState = state; }
>+
> private:
>     explicit Attachment(const WTF::String& identifier, WebKit::WebPageProxy&);
> 
>@@ -75,6 +83,7 @@ private:
>     WTF::String m_filePath;
>     WTF::String m_contentType;
>     WeakPtr<WebKit::WebPageProxy> m_webPage;
>+    InsertionState m_insertionState { InsertionState::NotInserted };
> };
> 
> } // namespace API
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>index aee75bd922fcdb999fcff2a98a8cc710545f6037..23041c374924110444f1a93d120cc33d679efc7f 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>@@ -1241,28 +1241,18 @@ static NSDictionary *dictionaryRepresentationForEditorState(const WebKit::Editor
> 
> #if ENABLE(ATTACHMENT_ELEMENT)
> 
>-- (void)_didInsertAttachment:(NSString *)identifier withSource:(NSString *)source
>+- (void)_didInsertAttachment:(API::Attachment&)attachment withSource:(NSString *)source
> {
>     id <WKUIDelegatePrivate> uiDelegate = (id <WKUIDelegatePrivate>)self.UIDelegate;
>-    if (![uiDelegate respondsToSelector:@selector(_webView:didInsertAttachment:withSource:)])
>-        return;
>-
>-    if (auto attachment = _page->attachmentForIdentifier(identifier))
>-        [uiDelegate _webView:self didInsertAttachment:wrapper(*attachment) withSource:source];
>-    else
>-        ASSERT_NOT_REACHED();
>+    if ([uiDelegate respondsToSelector:@selector(_webView:didInsertAttachment:withSource:)])
>+        [uiDelegate _webView:self didInsertAttachment:wrapper(attachment) withSource:source];
> }
> 
>-- (void)_didRemoveAttachment:(NSString *)identifier
>+- (void)_didRemoveAttachment:(API::Attachment&)attachment
> {
>     id <WKUIDelegatePrivate> uiDelegate = (id <WKUIDelegatePrivate>)self.UIDelegate;
>-    if (![uiDelegate respondsToSelector:@selector(_webView:didRemoveAttachment:)])
>-        return;
>-
>-    if (auto attachment = _page->attachmentForIdentifier(identifier))
>-        [uiDelegate _webView:self didRemoveAttachment:wrapper(*attachment)];
>-    else
>-        ASSERT_NOT_REACHED();
>+    if ([uiDelegate respondsToSelector:@selector(_webView:didRemoveAttachment:)])
>+        [uiDelegate _webView:self didRemoveAttachment:wrapper(attachment)];
> }
> 
> #endif // ENABLE(ATTACHMENT_ELEMENT)
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h b/Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h
>index 3a0541c410417cb0cb8dddde69eee2244cf9da1c..4f3abbcce58f84f3597c1e3ce3d7428349a0e9d4 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h
>@@ -53,6 +53,10 @@
> 
> typedef const struct OpaqueWKPage* WKPageRef;
> 
>+namespace API {
>+class Attachment;
>+}
>+
> namespace WebKit {
> class ViewSnapshot;
> class WebPageProxy;
>@@ -157,8 +161,8 @@ struct PrintInfo;
> #endif
> 
> #if ENABLE(ATTACHMENT_ELEMENT)
>-- (void)_didRemoveAttachment:(NSString *)identifier;
>-- (void)_didInsertAttachment:(NSString *)identifier withSource:(NSString *)source;
>+- (void)_didRemoveAttachment:(API::Attachment&)attachment;
>+- (void)_didInsertAttachment:(API::Attachment&)attachment withSource:(NSString *)source;
> #endif
> 
> - (WKPageRef)_pageForTesting;
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/_WKAttachment.h b/Source/WebKit/UIProcess/API/Cocoa/_WKAttachment.h
>index c737c2e535faf690d1e23bd7503053ca329998e7..12555f02ac5af4b18662a47d766251ca8523dd3d 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/_WKAttachment.h
>+++ b/Source/WebKit/UIProcess/API/Cocoa/_WKAttachment.h
>@@ -57,8 +57,9 @@ WK_CLASS_AVAILABLE(macosx(10.13.4), ios(11.3))
> - (void)setDisplayOptions:(_WKAttachmentDisplayOptions *)options completion:(void(^ _Nullable)(NSError * _Nullable))completionHandler;
> - (void)setFileWrapper:(NSFileWrapper *)fileWrapper contentType:(nullable NSString *)contentType completion:(void(^ _Nullable)(NSError * _Nullable))completionHandler WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
> 
>-@property (nonatomic, readonly) _WKAttachmentInfo *info WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
>+@property (nonatomic, readonly, nullable) _WKAttachmentInfo *info WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
> @property (nonatomic, readonly) NSString *uniqueIdentifier;
>+@property (nonatomic, readonly, getter=isConnected) BOOL connected WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
> 
> // Deprecated SPI.
> - (void)requestInfo:(void(^)(_WKAttachmentInfo * _Nullable, NSError * _Nullable))completionHandler WK_API_DEPRECATED_WITH_REPLACEMENT("-info", macosx(WK_MAC_TBA, WK_MAC_TBA), ios(WK_IOS_TBA, WK_IOS_TBA));
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/_WKAttachment.mm b/Source/WebKit/UIProcess/API/Cocoa/_WKAttachment.mm
>index e5c9e7b4ef923c72d3f00fdafe3a57c90c8c7bf0..d5df097f65d3327af5f7a6dd7864b9a8507a0a0e 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/_WKAttachment.mm
>+++ b/Source/WebKit/UIProcess/API/Cocoa/_WKAttachment.mm
>@@ -41,6 +41,9 @@
> 
> using namespace WebKit;
> 
>+static const NSInteger UnspecifiedAttachmentErrorCode = 1;
>+static const NSInteger InvalidAttachmentErrorCode = 2;
>+
> @implementation _WKAttachmentDisplayOptions : NSObject
> 
> - (WebCore::AttachmentDisplayOptions)coreDisplayOptions
>@@ -142,6 +145,9 @@ static BOOL isDeclaredOrDynamicTypeIdentifier(NSString *type)
> 
> - (_WKAttachmentInfo *)info
> {
>+    if (!_attachment->isValid())
>+        return nil;
>+
>     return [[[_WKAttachmentInfo alloc] initWithFileWrapper:_attachment->fileWrapper() filePath:_attachment->filePath() contentType:_attachment->contentType()] autorelease];
> }
> 
>@@ -152,6 +158,11 @@ static BOOL isDeclaredOrDynamicTypeIdentifier(NSString *type)
> 
> - (void)setDisplayOptions:(_WKAttachmentDisplayOptions *)options completion:(void(^)(NSError *))completionHandler
> {
>+    if (!_attachment->isValid()) {
>+        completionHandler([NSError errorWithDomain:WKErrorDomain code:InvalidAttachmentErrorCode userInfo:nil]);
>+        return;
>+    }
>+
>     auto coreOptions = options ? options.coreDisplayOptions : WebCore::AttachmentDisplayOptions { };
>     _attachment->setDisplayOptions(coreOptions, [capturedBlock = makeBlockPtr(completionHandler)] (CallbackBase::Error error) {
>         if (!capturedBlock)
>@@ -160,12 +171,17 @@ static BOOL isDeclaredOrDynamicTypeIdentifier(NSString *type)
>         if (error == CallbackBase::Error::None)
>             capturedBlock(nil);
>         else
>-            capturedBlock([NSError errorWithDomain:WKErrorDomain code:1 userInfo:nil]);
>+            capturedBlock([NSError errorWithDomain:WKErrorDomain code:UnspecifiedAttachmentErrorCode userInfo:nil]);
>     });
> }
> 
> - (void)setFileWrapper:(NSFileWrapper *)fileWrapper contentType:(NSString *)contentType completion:(void (^)(NSError *))completionHandler
> {
>+    if (!_attachment->isValid()) {
>+        completionHandler([NSError errorWithDomain:WKErrorDomain code:InvalidAttachmentErrorCode userInfo:nil]);
>+        return;
>+    }
>+
>     auto fileSize = [[[fileWrapper fileAttributes] objectForKey:NSFileSize] unsignedLongLongValue];
>     if (!fileSize && fileWrapper.regularFile)
>         fileSize = fileWrapper.regularFileContents.length;
>@@ -178,7 +194,7 @@ static BOOL isDeclaredOrDynamicTypeIdentifier(NSString *type)
>         if (error == CallbackBase::Error::None)
>             capturedBlock(nil);
>         else
>-            capturedBlock([NSError errorWithDomain:WKErrorDomain code:1 userInfo:nil]);
>+            capturedBlock([NSError errorWithDomain:WKErrorDomain code:UnspecifiedAttachmentErrorCode userInfo:nil]);
>     });
> }
> 
>@@ -200,6 +216,11 @@ static BOOL isDeclaredOrDynamicTypeIdentifier(NSString *type)
>     return [NSString stringWithFormat:@"<%@ %p id='%@'>", [self class], self, self.uniqueIdentifier];
> }
> 
>+- (BOOL)isConnected
>+{
>+    return _attachment->insertionState() == API::Attachment::InsertionState::Inserted;
>+}
>+
> @end
> 
> #endif // WK_API_ENABLED
>diff --git a/Source/WebKit/UIProcess/Cocoa/PageClientImplCocoa.h b/Source/WebKit/UIProcess/Cocoa/PageClientImplCocoa.h
>index b821c54d957ebe4f87e522571e31a165b1230534..808d9e920f93121943c00b986f2b4d2a31580001 100644
>--- a/Source/WebKit/UIProcess/Cocoa/PageClientImplCocoa.h
>+++ b/Source/WebKit/UIProcess/Cocoa/PageClientImplCocoa.h
>@@ -29,6 +29,10 @@
> 
> @class WKWebView;
> 
>+namespace API {
>+class Attachment;
>+}
>+
> namespace WebKit {
> 
> class PageClientImplCocoa : public PageClient {
>@@ -39,8 +43,8 @@ public:
>     void isPlayingAudioDidChange() final;
> 
> #if ENABLE(ATTACHMENT_ELEMENT)
>-    void didInsertAttachment(const String& identifier, const String& source) final;
>-    void didRemoveAttachment(const String& identifier) final;
>+    void didInsertAttachment(API::Attachment&, const String& source) final;
>+    void didRemoveAttachment(API::Attachment&) final;
> #endif
> 
> protected:
>diff --git a/Source/WebKit/UIProcess/Cocoa/PageClientImplCocoa.mm b/Source/WebKit/UIProcess/Cocoa/PageClientImplCocoa.mm
>index d6e0e4297101b645489f908d80e58b88cb01f63c..5e65492dafc1524c353481d8a57e08c1c6ac8d2f 100644
>--- a/Source/WebKit/UIProcess/Cocoa/PageClientImplCocoa.mm
>+++ b/Source/WebKit/UIProcess/Cocoa/PageClientImplCocoa.mm
>@@ -46,20 +46,20 @@ void PageClientImplCocoa::isPlayingAudioDidChange()
> 
> #if ENABLE(ATTACHMENT_ELEMENT)
> 
>-void PageClientImplCocoa::didInsertAttachment(const String& identifier, const String& source)
>+void PageClientImplCocoa::didInsertAttachment(API::Attachment& attachment, const String& source)
> {
> #if WK_API_ENABLED
>-    [m_webView _didInsertAttachment:identifier withSource:source];
>+    [m_webView _didInsertAttachment:attachment withSource:source];
> #else
>     UNUSED_PARAM(identifier);
>     UNUSED_PARAM(source);
> #endif
> }
> 
>-void PageClientImplCocoa::didRemoveAttachment(const String& identifier)
>+void PageClientImplCocoa::didRemoveAttachment(API::Attachment& attachment)
> {
> #if WK_API_ENABLED
>-    [m_webView _didRemoveAttachment:identifier];
>+    [m_webView _didRemoveAttachment:attachment];
> #else
>     UNUSED_PARAM(identifier);
> #endif
>diff --git a/Source/WebKit/UIProcess/PageClient.h b/Source/WebKit/UIProcess/PageClient.h
>index e2c691e1cdc540c42cf8d51507828bac5740153f..2683fc8ddb6f1917e9a77e3e2525ec0a425451e5 100644
>--- a/Source/WebKit/UIProcess/PageClient.h
>+++ b/Source/WebKit/UIProcess/PageClient.h
>@@ -52,6 +52,7 @@ OBJC_CLASS NSTextAlternatives;
> #endif
> 
> namespace API {
>+class Attachment;
> class HitTestResult;
> class Object;
> class OpenPanelParameters;
>@@ -437,8 +438,8 @@ public:
> #endif
> 
> #if ENABLE(ATTACHMENT_ELEMENT)
>-    virtual void didInsertAttachment(const String& identifier, const String& source) { }
>-    virtual void didRemoveAttachment(const String& identifier) { }
>+    virtual void didInsertAttachment(API::Attachment&, const String& source) { }
>+    virtual void didRemoveAttachment(API::Attachment&) { }
> #endif
> };
> 
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
>index 965e6f20ed7f2e7edd0bb1a9ce48bd71fb7ce166..cf57eb4f5a21565c8b6eb87aa6b9d551ee67a423 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
>@@ -3673,6 +3673,11 @@ void WebPageProxy::didCommitLoadForFrame(uint64_t frameID, uint64_t navigationID
>             m_navigationClient->didCommitNavigation(*this, navigation.get(), m_process->transformHandlesToObjects(userData.object()).get());
>     } else
>         m_loaderClient->didCommitLoadForFrame(*this, *frame, navigation.get(), m_process->transformHandlesToObjects(userData.object()).get());
>+
>+#if ENABLE(ATTACHMENT_ELEMENT)
>+    if (frame->isMainFrame())
>+        invalidateAllAttachments();
>+#endif
> }
> 
> void WebPageProxy::didFinishDocumentLoadForFrame(uint64_t frameID, uint64_t navigationID, const UserData& userData)
>@@ -6176,9 +6181,7 @@ void WebPageProxy::resetStateAfterProcessExited(ProcessTerminationReason termina
> #endif
> 
> #if ENABLE(ATTACHMENT_ELEMENT)
>-    for (auto identifier : m_attachmentIdentifierToAttachmentMap.keys())
>-        m_pageClient.didRemoveAttachment(identifier);
>-    m_attachmentIdentifierToAttachmentMap.clear();
>+    invalidateAllAttachments();
> #endif
> 
>     if (terminationReason != ProcessTerminationReason::NavigationSwap) {
>@@ -7813,9 +7816,19 @@ void WebPageProxy::cloneAttachmentData(const String& fromIdentifier, const Strin
>     platformCloneAttachment(existingAttachment.releaseNonNull(), WTFMove(newAttachment));
> }
> 
>+void WebPageProxy::invalidateAllAttachments()
>+{
>+    for (auto& attachment : m_attachmentIdentifierToAttachmentMap.values()) {
>+        if (attachment->insertionState() == API::Attachment::InsertionState::Inserted)
>+            didRemoveAttachment(attachment.get());
>+        attachment->invalidate();
>+    }
>+    m_attachmentIdentifierToAttachmentMap.clear();
>+}
>+
> #if !PLATFORM(COCOA)
> 
>-void WebPageProxy::platformRegisterAttachment(Ref<API::Attachment>&&, const String& preferredFileName, const IPC::DataReference&)
>+void WebPageProxy::platformRegisterAttachment(Ref<API::Attachment>&&, const String&, const IPC::DataReference&)
> {
> }
> 
>@@ -7829,16 +7842,28 @@ void WebPageProxy::platformCloneAttachment(Ref<API::Attachment>&&, Ref<API::Atta
> 
> #endif
> 
>-void WebPageProxy::didInsertAttachment(const String& identifier, const String& source)
>+void WebPageProxy::didInsertAttachmentWithIdentifier(const String& identifier, const String& source)
>+{
>+    auto attachment = ensureAttachment(identifier);
>+    didInsertAttachment(attachment.get(), source);
>+}
>+
>+void WebPageProxy::didRemoveAttachmentWithIdentifier(const String& identifier)
>+{
>+    if (auto attachment = attachmentForIdentifier(identifier))
>+        didRemoveAttachment(*attachment);
>+}
>+
>+void WebPageProxy::didInsertAttachment(API::Attachment& attachment, const String& source)
> {
>-    ensureAttachment(identifier);
>-    m_pageClient.didInsertAttachment(identifier, source);
>+    m_pageClient.didInsertAttachment(attachment, source);
>+    attachment.setInsertionState(API::Attachment::InsertionState::Inserted);
> }
> 
>-void WebPageProxy::didRemoveAttachment(const String& identifier)
>+void WebPageProxy::didRemoveAttachment(API::Attachment& attachment)
> {
>-    ASSERT(attachmentForIdentifier(identifier));
>-    m_pageClient.didRemoveAttachment(identifier);
>+    m_pageClient.didRemoveAttachment(attachment);
>+    attachment.setInsertionState(API::Attachment::InsertionState::NotInserted);
> }
> 
> Ref<API::Attachment> WebPageProxy::ensureAttachment(const String& identifier)
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.h b/Source/WebKit/UIProcess/WebPageProxy.h
>index 4e8be03a28dda672b8ef5d5771b0e5998be267b2..186903018220b0e4662bf77a4ca0c1bf451b82a1 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.h
>+++ b/Source/WebKit/UIProcess/WebPageProxy.h
>@@ -1810,9 +1810,12 @@ private:
>     void platformRegisterAttachment(Ref<API::Attachment>&&, const String& filePath);
>     void platformCloneAttachment(Ref<API::Attachment>&& fromAttachment, Ref<API::Attachment>&& toAttachment);
> 
>-    void didInsertAttachment(const String& identifier, const String& source);
>-    void didRemoveAttachment(const String& identifier);
>+    void didInsertAttachmentWithIdentifier(const String& identifier, const String& source);
>+    void didRemoveAttachmentWithIdentifier(const String& identifier);
>+    void didInsertAttachment(API::Attachment&, const String& source);
>+    void didRemoveAttachment(API::Attachment&);
>     Ref<API::Attachment> ensureAttachment(const String& identifier);
>+    void invalidateAllAttachments();
> #endif
> 
> #if PLATFORM(MAC) && ENABLE(WEBPROCESS_WINDOWSERVER_BLOCKING)
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.messages.in b/Source/WebKit/UIProcess/WebPageProxy.messages.in
>index 881f22c607ea55785a47ec5cfc9fe97a003a19fd..491b651b98a2fb7e81db40a461a64346b921db47 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.messages.in
>+++ b/Source/WebKit/UIProcess/WebPageProxy.messages.in
>@@ -528,8 +528,8 @@ messages -> WebPageProxy {
>     RegisterAttachmentIdentifierFromData(String identifier, String contentType, String preferredFileName, IPC::DataReference data)
>     RegisterAttachmentIdentifierFromFilePath(String identifier, String contentType, String filePath)
>     CloneAttachmentData(String fromIdentifier, String toIdentifier)
>-    DidInsertAttachment(String identifier, String source)
>-    DidRemoveAttachment(String identifier)
>+    DidInsertAttachmentWithIdentifier(String identifier, String source)
>+    DidRemoveAttachmentWithIdentifier(String identifier)
> #endif
> 
> #if PLATFORM(MAC) && ENABLE(WEBPROCESS_WINDOWSERVER_BLOCKING)
>diff --git a/Source/WebKit/WebProcess/WebCoreSupport/WebEditorClient.cpp b/Source/WebKit/WebProcess/WebCoreSupport/WebEditorClient.cpp
>index ce7d889255384f83eeb05d71483bb4cdc7a44722..f3af7ddcb89a907b4b23a7d842aa31b1dfcdfdf9 100644
>--- a/Source/WebKit/WebProcess/WebCoreSupport/WebEditorClient.cpp
>+++ b/Source/WebKit/WebProcess/WebCoreSupport/WebEditorClient.cpp
>@@ -175,14 +175,14 @@ void WebEditorClient::cloneAttachmentData(const String& fromIdentifier, const St
>     m_page->send(Messages::WebPageProxy::CloneAttachmentData(fromIdentifier, toIdentifier));
> }
> 
>-void WebEditorClient::didInsertAttachment(const String& identifier, const String& source)
>+void WebEditorClient::didInsertAttachmentWithIdentifier(const String& identifier, const String& source)
> {
>-    m_page->send(Messages::WebPageProxy::DidInsertAttachment(identifier, source));
>+    m_page->send(Messages::WebPageProxy::DidInsertAttachmentWithIdentifier(identifier, source));
> }
> 
>-void WebEditorClient::didRemoveAttachment(const String& identifier)
>+void WebEditorClient::didRemoveAttachmentWithIdentifier(const String& identifier)
> {
>-    m_page->send(Messages::WebPageProxy::DidRemoveAttachment(identifier));
>+    m_page->send(Messages::WebPageProxy::DidRemoveAttachmentWithIdentifier(identifier));
> }
> 
> #endif
>diff --git a/Source/WebKit/WebProcess/WebCoreSupport/WebEditorClient.h b/Source/WebKit/WebProcess/WebCoreSupport/WebEditorClient.h
>index fdb70ebd6070107a3d82b285a5463f8d74b02b7d..fcdb96cd16a5710ed47734ca8fce837bcd501aab 100644
>--- a/Source/WebKit/WebProcess/WebCoreSupport/WebEditorClient.h
>+++ b/Source/WebKit/WebProcess/WebCoreSupport/WebEditorClient.h
>@@ -63,8 +63,8 @@ private:
>     void registerAttachmentIdentifier(const String& identifier, const String& contentType, const String& preferredFileName, Ref<WebCore::SharedBuffer>&&) final;
>     void registerAttachmentIdentifier(const String& identifier, const String& contentType, const String& filePath) final;
>     void cloneAttachmentData(const String& fromIdentifier, const String& toIdentifier) final;
>-    void didInsertAttachment(const String& identifier, const String& source) final;
>-    void didRemoveAttachment(const String& identifier) final;
>+    void didInsertAttachmentWithIdentifier(const String& identifier, const String& source) final;
>+    void didRemoveAttachmentWithIdentifier(const String& identifier) final;
>     bool supportsClientSideAttachmentData() const final { return true; }
> #endif
> 
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 923597e2c26555299be5454cf549e4dd161ac39f..bc1b97e98da559625d2904f929967ae463f55040 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,17 @@
>+2018-08-17  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [Attachment Support] Remove _WKAttachments and notify the UI client upon mainframe navigation
>+        https://bugs.webkit.org/show_bug.cgi?id=188715
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Adds API tests to exercises cases where (1) the UI client is notified of attachment removal upon mainframe
>+        navigation, and (2) the UI client is notified of attachment removal upon web content process termination.
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:
>+        (TestWebKitAPI::ObserveAttachmentUpdatesForScope::expectAttachmentUpdates):
>+        (TestWebKitAPI::TEST):
>+
> 2018-08-17  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         [Cocoa] Augment _WKAttachment SPI to handle NSFileWrappers in addition to NSData
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm
>index 20902d22276b344d3fa54afdd990441d70a79fe0..4b154bd016bf145a9ab89fba1dbcbc9781aeb807 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm
>@@ -27,6 +27,7 @@
> 
> #import "DragAndDropSimulator.h"
> #import "PlatformUtilities.h"
>+#import "TestNavigationDelegate.h"
> #import "TestWKWebView.h"
> #import "WKWebViewConfigurationExtras.h"
> #import <WebKit/WKPreferencesRefPrivate.h>
>@@ -114,12 +115,12 @@ public:
> 
>     void expectAttachmentUpdates(NSArray<_WKAttachment *> *removed, NSArray<_WKAttachment *> *inserted)
>     {
>-        BOOL removedAttachmentsMatch = [observer().removed isEqual:removed];
>+        BOOL removedAttachmentsMatch = [[NSSet setWithArray:observer().removed] isEqual:[NSSet setWithArray:removed]];
>         if (!removedAttachmentsMatch)
>             NSLog(@"Expected removed attachments: %@ to match %@.", observer().removed, removed);
>         EXPECT_TRUE(removedAttachmentsMatch);
> 
>-        BOOL insertedAttachmentsMatch = [observer().inserted isEqual:inserted];
>+        BOOL insertedAttachmentsMatch = [[NSSet setWithArray:observer().inserted] isEqual:[NSSet setWithArray:inserted]];
>         if (!insertedAttachmentsMatch)
>             NSLog(@"Expected inserted attachments: %@ to match %@.", observer().inserted, inserted);
>         EXPECT_TRUE(insertedAttachmentsMatch);
>@@ -1012,6 +1013,70 @@ TEST(WKAttachmentTests, InsertAttachmentUsingFileWrapperWithFilePath)
>     [attachment expectRequestedDataToBe:testPDFData()];
> }
> 
>+TEST(WKAttachmentTests, InvalidateAttachmentsAfterMainFrameNavigation)
>+{
>+    auto webView = webViewForTestingAttachments();
>+    RetainPtr<_WKAttachment> pdfAttachment;
>+    RetainPtr<_WKAttachment> htmlAttachment;
>+    {
>+        ObserveAttachmentUpdatesForScope insertionObserver(webView.get());
>+        pdfAttachment = [webView synchronouslyInsertAttachmentWithFilename:@"attachment.pdf" contentType:@"application/pdf" data:testPDFData()];
>+        htmlAttachment = [webView synchronouslyInsertAttachmentWithFilename:@"index.html" contentType:@"text/html" data:testHTMLData()];
>+        insertionObserver.expectAttachmentUpdates(@[ ], @[ pdfAttachment.get(), htmlAttachment.get() ]);
>+        EXPECT_TRUE([pdfAttachment isConnected]);
>+        EXPECT_TRUE([htmlAttachment isConnected]);
>+    }
>+
>+    ObserveAttachmentUpdatesForScope removalObserver(webView.get());
>+    [webView synchronouslyLoadTestPageNamed:@"simple"];
>+    removalObserver.expectAttachmentUpdates(@[ pdfAttachment.get(), htmlAttachment.get() ], @[ ]);
>+    EXPECT_FALSE([pdfAttachment isConnected]);
>+    EXPECT_FALSE([htmlAttachment isConnected]);
>+    [pdfAttachment expectRequestedDataToBe:nil];
>+    [htmlAttachment expectRequestedDataToBe:nil];
>+}
>+
>+TEST(WKAttachmentTests, InvalidateAttachmentsAfterWebProcessTermination)
>+{
>+    auto webView = webViewForTestingAttachments();
>+    RetainPtr<_WKAttachment> pdfAttachment;
>+    RetainPtr<_WKAttachment> htmlAttachment;
>+    {
>+        ObserveAttachmentUpdatesForScope insertionObserver(webView.get());
>+        pdfAttachment = [webView synchronouslyInsertAttachmentWithFilename:@"attachment.pdf" contentType:@"application/pdf" data:testPDFData()];
>+        htmlAttachment = [webView synchronouslyInsertAttachmentWithFilename:@"index.html" contentType:@"text/html" data:testHTMLData()];
>+        insertionObserver.expectAttachmentUpdates(@[ ], @[ pdfAttachment.get(), htmlAttachment.get() ]);
>+        EXPECT_TRUE([pdfAttachment isConnected]);
>+        EXPECT_TRUE([htmlAttachment isConnected]);
>+    }
>+    {
>+        ObserveAttachmentUpdatesForScope removalObserver(webView.get());
>+        [webView stringByEvaluatingJavaScript:@"getSelection().collapseToEnd()"];
>+        [webView _synchronouslyExecuteEditCommand:@"DeleteBackward" argument:nil];
>+        removalObserver.expectAttachmentUpdates(@[ htmlAttachment.get() ], @[ ]);
>+        EXPECT_TRUE([pdfAttachment isConnected]);
>+        EXPECT_FALSE([htmlAttachment isConnected]);
>+        [htmlAttachment expectRequestedDataToBe:testHTMLData()];
>+    }
>+
>+    __block bool webProcessTerminated = false;
>+    auto navigationDelegate = adoptNS([[TestNavigationDelegate alloc] init]);
>+    [webView setNavigationDelegate:navigationDelegate.get()];
>+    [navigationDelegate setWebContentProcessDidTerminate:^(WKWebView *) {
>+        webProcessTerminated = true;
>+    }];
>+
>+    ObserveAttachmentUpdatesForScope observer(webView.get());
>+    [webView _killWebContentProcess];
>+    TestWebKitAPI::Util::run(&webProcessTerminated);
>+
>+    observer.expectAttachmentUpdates(@[ pdfAttachment.get() ], @[ ]);
>+    EXPECT_FALSE([pdfAttachment isConnected]);
>+    EXPECT_FALSE([htmlAttachment isConnected]);
>+    [pdfAttachment expectRequestedDataToBe:nil];
>+    [htmlAttachment expectRequestedDataToBe:nil];
>+}
>+
> #pragma mark - Platform-specific tests
> 
> #if PLATFORM(MAC)

Actions: View | Formatted Diff | Diff

Attachments on bug 188715: 347436 | 347466 | 347709 | 347729 | 347758