Attachment 347709 Details for Bug 188715 – Patch

[patch] Patch

bug-188715-20180821152001.patch (text/plain), 34.88 KB, created by Wenson Hsieh on 2018-08-21 15:20:02 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2018-08-21 15:20:02 PDT

Size: 34.88 KB

patch

obsolete

>Subversion Revision: 235140
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index fa04528f28ce9cb946c572bf5e32c50bac75eaaa..416810eded5982a36de2e2030f57834f1015b141 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,24 @@
>+2018-08-21  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
>+        <rdar://problem/43541790>
>+
>+        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-21  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         [Attachment Support] Augment _WKAttachment SPI to handle NSFileWrappers in addition to NSData
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 70508d3187daa3c7f703fc38c39af554e3fd8fe8..40913a847fdbef584c1dce92b21c20f934c83fef 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,95 @@
>+2018-08-21  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
>+        <rdar://problem/43541790>
>+
>+        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-21  Alex Christensen  <achristensen@webkit.org>
> 
>         Remove unused shouldKeepCurrentBackForwardListItemInList check
>diff --git a/Source/WebCore/editing/Editor.cpp b/Source/WebCore/editing/Editor.cpp
>index 514e25e93fa0ac9785129f18eac59679dd7854ff..07009487af3e662160f41c281d4f0ab1c2be9714 100644
>--- a/Source/WebCore/editing/Editor.cpp
>+++ b/Source/WebCore/editing/Editor.cpp
>@@ -3866,7 +3866,7 @@ void Editor::notifyClientOfAttachmentUpdates()
>         return;
> 
>     for (auto& identifier : removedAttachmentIdentifiers)
>-        client()->didRemoveAttachment(identifier);
>+        client()->didRemoveAttachmentWithIdentifier(identifier);
> 
>     auto* document = m_frame.document();
>     if (!document)
>@@ -3874,7 +3874,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 d15620f0240f2a6041498d42999ba2f52f3789ee..6c2af521ab885b75943b8a40fd3535fadd17e9df 100644
>--- a/Source/WebKit/UIProcess/API/APIAttachment.cpp
>+++ b/Source/WebKit/UIProcess/API/APIAttachment.cpp
>@@ -70,6 +70,17 @@ 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
>+}
>+
> }
> 
> #endif // ENABLE(ATTACHMENT_ELEMENT)
>diff --git a/Source/WebKit/UIProcess/API/APIAttachment.h b/Source/WebKit/UIProcess/API/APIAttachment.h
>index dec48233a664b4b5d020507a6ab22901e6d6dc54..bf35f454ac9d654d11cd17be60fc7319327839bc 100644
>--- a/Source/WebKit/UIProcess/API/APIAttachment.h
>+++ b/Source/WebKit/UIProcess/API/APIAttachment.h
>@@ -52,10 +52,15 @@ public:
>     static Ref<Attachment> create(const WTF::String& identifier, WebKit::WebPageProxy&);
>     virtual ~Attachment();
> 
>+    enum class InsertionState : uint8_t { 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; }
>@@ -67,6 +72,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&);
> 
>@@ -77,6 +85,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 f0b7a6e556150ceaa785da2e0a4b7d983759a35f..3379b481eeae23e1763c8cd736727e126d4a018a 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>@@ -1210,28 +1210,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 f1441b4af559a0dafa57f27c17ea52ae6fe8b3a8..a1f302c066de7cbe605a97e3505cd9e89081bc35 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 696adbe470cc1d7197cec65e759ede2ccae8ced0..f495c714cc4a74bc6fe248a0616d80c4d5e0dd2d 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/_WKAttachment.mm
>+++ b/Source/WebKit/UIProcess/API/Cocoa/_WKAttachment.mm
>@@ -42,6 +42,9 @@
> 
> using namespace WebKit;
> 
>+static const NSInteger UnspecifiedAttachmentErrorCode = 1;
>+static const NSInteger InvalidAttachmentErrorCode = 2;
>+
> @implementation _WKAttachmentDisplayOptions : NSObject
> 
> - (WebCore::AttachmentDisplayOptions)coreDisplayOptions
>@@ -146,6 +149,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];
> }
> 
>@@ -156,6 +162,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)
>@@ -164,12 +175,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[NSFileSize] unsignedLongLongValue];
>     if (!fileSize && fileWrapper.regularFile)
>         fileSize = fileWrapper.regularFileContents.length;
>@@ -187,7 +203,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]);
>     });
> }
> 
>@@ -209,6 +225,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 5a9531b04bb7472572ffaa48df760273f8841ac3..922342f5899c2431e17bb374f57390461f9ddfc9 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
>@@ -3670,6 +3670,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)
>@@ -6174,9 +6179,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) {
>@@ -7818,9 +7821,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&)
> {
> }
> 
>@@ -7834,16 +7847,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);
>+    attachment.setInsertionState(API::Attachment::InsertionState::Inserted);
>+    m_pageClient.didInsertAttachment(attachment, source);
> }
> 
>-void WebPageProxy::didRemoveAttachment(const String& identifier)
>+void WebPageProxy::didRemoveAttachment(API::Attachment& attachment)
> {
>-    ASSERT(attachmentForIdentifier(identifier));
>-    m_pageClient.didRemoveAttachment(identifier);
>+    attachment.setInsertionState(API::Attachment::InsertionState::NotInserted);
>+    m_pageClient.didRemoveAttachment(attachment);
> }
> 
> Ref<API::Attachment> WebPageProxy::ensureAttachment(const String& identifier)
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.h b/Source/WebKit/UIProcess/WebPageProxy.h
>index df42fda724d27e6052ea8358d2d384f4a5138053..40d21f26cc1487bfffa1c9a062c94b0f51aa29c3 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.h
>+++ b/Source/WebKit/UIProcess/WebPageProxy.h
>@@ -1809,9 +1809,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 d2e3d1c6e532d38a6a0b0dcbfe507b81b738899e..97048e9024be53c2394009ef0d0cd519423ff1fa 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.messages.in
>+++ b/Source/WebKit/UIProcess/WebPageProxy.messages.in
>@@ -529,8 +529,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 51d77899a64aaba10c0778d4a684e13349644345..f99803bcae0ded3e72380c8982797a83c43da622 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 6df441f7869a79d3e5f84c3149930317da63f032..ef50e8aa8711bc7ed7003deec3f9a901a576438d 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 57da8bf8d71f6bd4fb63dda9c55eb64b996dab94..40d45800879004d9b6bbe4abe68a7d132acd7a28 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,18 @@
>+2018-08-21  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
>+        <rdar://problem/43541790>
>+
>+        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-21  Alex Christensen  <achristensen@webkit.org>
> 
>         Translate WebKit.LimitTitleSize API test into ObjC
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm
>index 953f5a6d82d7ebcd2e51d5af0b8c7f1e2ee5b957..89eea3922d57966a3e27ac82d50388d097a0f133 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)

Flags:

thorton: review+

Actions: View | Formatted Diff | Diff

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