Bug 211477

Summary: Introduce wrapper class for FileReaderLoader
Product: WebKit Reporter: Nikos Mouchtaris <nmouchtaris>
Component: New BugsAssignee: Nikos Mouchtaris <nmouchtaris>
Status: RESOLVED INVALID    
Severity: Normal CC: wenson_hsieh
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Nikos Mouchtaris 2020-05-05 16:17:54 PDT
Introduce wrapper class for FileReaderLoader
Comment 1 Nikos Mouchtaris 2020-05-05 16:23:29 PDT
Created attachment 398563 [details]
Patch
Comment 2 Wenson Hsieh 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.