WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED INVALID
211477
Introduce wrapper class for FileReaderLoader
https://bugs.webkit.org/show_bug.cgi?id=211477
Summary
Introduce wrapper class for FileReaderLoader
Nikos Mouchtaris
Reported
2020-05-05 16:17:54 PDT
Introduce wrapper class for FileReaderLoader
Attachments
Patch
(12.53 KB, patch)
2020-05-05 16:23 PDT
,
Nikos Mouchtaris
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Nikos Mouchtaris
Comment 1
2020-05-05 16:23:29 PDT
Created
attachment 398563
[details]
Patch
Wenson Hsieh
Comment 2
2020-05-06 14:51:48 PDT
Comment on
attachment 398563
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=398563&action=review
> Source/WebCore/page/FileReaderLoaderWrapper.cpp:49 > +
Nit - extra newline here.
> Source/WebCore/page/FileReaderLoaderWrapper.cpp:77 > +
Nit - extra newline here.
> Source/WebCore/page/FileReaderLoaderWrapper.cpp:82 > +
Nit - extra newline here.
> Source/WebCore/page/FileReaderLoaderWrapper.cpp:91 > +}
Nit - newline after this closing brace.
> Source/WebCore/page/FileReaderLoaderWrapper.h:43 > + static Ref<FileReaderLoaderWrapper> create(int id, CompletionHandler<void(ExceptionOr<int>)>&& completionHandler) { return adoptRef(*new FileReaderLoaderWrapper(id, WTFMove(completionHandler))); }
Nit - we generally avoid using `id` by itself as a variable name, since it will mean that this header cannot be included in Objective-C++ files (*.mm). Since this is an index into a Vector, perhaps it should also be a `size_t` instead of an `int`?
> Source/WebCore/page/FileReaderLoaderWrapper.h:47 > + RefPtr<SharedBuffer> readBuffer() const { return m_blobData; }
Nit - this should be marked const.
> Source/WebCore/page/FileReaderLoaderWrapper.h:48 > + String fileName() { return m_fileName; }
const here too.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug