Don't load inline data when requesting info for an attachment element backed by a...
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Jan 2018 01:13:28 +0000 (01:13 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Jan 2018 01:13:28 +0000 (01:13 +0000)
https://bugs.webkit.org/show_bug.cgi?id=181550

Source/WebCore:

Reviewed by Tim Horton.

When requesting data for an attachment element that is backed by a file path, we currently trigger a load in the
web process to fetch contents of the attachment data as inline data in the AttachmentInfo. This is unnecessary,
since the file path of the attachment element must have come from the UI process anyways, so it is sufficient to
simply send the file path to the UI process and have the UI process read the contents of the path as a memory-
mapped NSData.

This patch lets HTMLAttachmentElement skip over resource loading codepaths when creating an AttachmentInfo for
the client, and also teaches _WKAttachment to read a AttachmentInfo's filepath as memory-mapped data if a file
path is present, and no inline data was specified.

Covered by existing API tests.

* html/HTMLAttachmentElement.cpp:
(WebCore::HTMLAttachmentElement::requestInfo):

Source/WebKit:

Reviewed by Tim Horton

See WebCore/ChangeLog for more information.

* UIProcess/API/Cocoa/_WKAttachment.mm:
(-[_WKAttachmentInfo initWithInfo:]):
(-[_WKAttachmentInfo fileLoadingError]):
(-[_WKAttachment requestInfo:]):

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

Source/WebCore/ChangeLog
Source/WebCore/html/HTMLAttachmentElement.cpp
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/Cocoa/_WKAttachment.mm

index 9f44cd5..cfb411c 100644 (file)
@@ -1,3 +1,25 @@
+2018-01-11  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Don't load inline data when requesting info for an attachment element backed by a file path
+        https://bugs.webkit.org/show_bug.cgi?id=181550
+
+        Reviewed by Tim Horton.
+
+        When requesting data for an attachment element that is backed by a file path, we currently trigger a load in the
+        web process to fetch contents of the attachment data as inline data in the AttachmentInfo. This is unnecessary,
+        since the file path of the attachment element must have come from the UI process anyways, so it is sufficient to
+        simply send the file path to the UI process and have the UI process read the contents of the path as a memory-
+        mapped NSData.
+
+        This patch lets HTMLAttachmentElement skip over resource loading codepaths when creating an AttachmentInfo for
+        the client, and also teaches _WKAttachment to read a AttachmentInfo's filepath as memory-mapped data if a file
+        path is present, and no inline data was specified.
+
+        Covered by existing API tests.
+
+        * html/HTMLAttachmentElement.cpp:
+        (WebCore::HTMLAttachmentElement::requestInfo):
+
 2018-01-10  Ryosuke Niwa  <rniwa@webkit.org>
 
         Make elements of zero width or height focusable
index 66185f8..1823f33 100644 (file)
@@ -304,8 +304,14 @@ void HTMLAttachmentElement::requestInfo(Function<void(const AttachmentInfo&)>&&
         return;
     }
 
-    m_attachmentReaders.append(AttachmentDataReader::create(*this, [protectedFile = makeRef(*m_file), callback = WTFMove(callback)] (RefPtr<SharedBuffer>&& data) {
-        callback({ protectedFile->type(), protectedFile->name(), protectedFile->path(), WTFMove(data) });
+    AttachmentInfo infoWithoutData { m_file->type(), m_file->name(), m_file->path(), nullptr };
+    if (!m_file->path().isEmpty()) {
+        callback(infoWithoutData);
+        return;
+    }
+
+    m_attachmentReaders.append(AttachmentDataReader::create(*this, [infoWithoutData = WTFMove(infoWithoutData), protectedFile = makeRef(*m_file), callback = WTFMove(callback)] (RefPtr<SharedBuffer>&& data) {
+        callback({ infoWithoutData.contentType, infoWithoutData.name, infoWithoutData.filePath, WTFMove(data) });
     }));
 }
 
index 5ae339f..fdf5531 100644 (file)
@@ -1,3 +1,17 @@
+2018-01-11  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Don't load inline data when requesting info for an attachment element backed by a file path
+        https://bugs.webkit.org/show_bug.cgi?id=181550
+
+        Reviewed by Tim Horton
+
+        See WebCore/ChangeLog for more information.
+
+        * UIProcess/API/Cocoa/_WKAttachment.mm:
+        (-[_WKAttachmentInfo initWithInfo:]):
+        (-[_WKAttachmentInfo fileLoadingError]):
+        (-[_WKAttachment requestInfo:]):
+
 2018-01-11  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r226789 and r226794.
index baeb313..2de500c 100644 (file)
@@ -74,6 +74,7 @@ using namespace WebKit;
     RetainPtr<NSString> _name;
     RetainPtr<NSString> _filePath;
     RetainPtr<NSString> _contentType;
+    RetainPtr<NSError> _fileLoadingError;
 }
 
 - (instancetype)initWithInfo:(const WebCore::AttachmentInfo&)info
@@ -85,6 +86,13 @@ using namespace WebKit;
     _contentType = adoptNS([(NSString *)info.contentType ?: @"" copy]);
     _name = adoptNS([(NSString *)info.name ?: @"" copy]);
     _filePath = adoptNS([(NSString *)info.filePath ?: @"" copy]);
+    _fileLoadingError = nil;
+
+    if (!_data && [_filePath length]) {
+        NSError *error = nil;
+        _data = [NSData dataWithContentsOfFile:_filePath.get() options:NSDataReadingMappedIfSafe error:&error];
+        _fileLoadingError = error;
+    }
 
     return self;
 }
@@ -109,6 +117,11 @@ using namespace WebKit;
     return _contentType.get();
 }
 
+- (NSError *)fileLoadingError
+{
+    return _fileLoadingError.get();
+}
+
 @end
 
 @implementation _WKAttachment
@@ -135,6 +148,11 @@ using namespace WebKit;
         }
 
         auto attachmentInfo = adoptNS([[_WKAttachmentInfo alloc] initWithInfo:info]);
+        if (NSError *fileLoadingError = [attachmentInfo fileLoadingError]) {
+            capturedBlock(nil, [NSError errorWithDomain:WKErrorDomain code:WKErrorUnknown userInfo:@{ NSUnderlyingErrorKey : fileLoadingError }]);
+            return;
+        }
+
         capturedBlock(attachmentInfo.get(), nil);
     });
 }