Update the behavior of multiple reads for FileReader

• Component: WebCore JavaScript
• Status: RESOLVED FIXED

[Jian Li]

We need to update the behavior of multiple reads for FileReader per the latest File API spec: http://dev.w3.org/2006/webapi/FileAPI/#MultipleReads

  The FileReader interface makes available four asynchronous read methods -- readAsArrayBuffer, readAsBinaryString, readAsText, and readAsDataURL, which read files into memory. If multiple concurrent read methods are called on the same FileReader object, user agents MUST throw an OperationNotAllowedException with the NOT_ALLOWED_ERR status code on any of the read methods that occur when readyState = LOADING.

[Jian Li]

Created attachment 95523 [details]
Proposed Patch

[David Levin]

Comment on attachment 95523 [details]
Proposed Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=95523&action=review

Looks good. Just a few things to consider before checking in.

The one that concerns me the most is the wording for the error message operationNotAllowedExceptionDescriptions.

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:-23070
> -			developmentRegion = English;

Please fix this (and consider upgrading xcode).

> Source/WebCore/bindings/js/JSDOMBinding.cpp:36
>  #if ENABLE(BLOB) || ENABLE(FILE_SYSTEM)

fwiw, these ifdef sections should come after all other includes (but I know you are just adding to an existing section).

> Source/WebCore/dom/ExceptionCode.cpp:218
> +    "It was not allowed to call multiple concurrent read methods on the same object when the ready state is LOADING."

How about:
   "A read method was called while another read was in progress."
or
   "A read method was called while the object was in the LOADING state due to a previous read call."

> Source/WebCore/fileapi/FileReader.cpp:140
> +    m_loader = adoptPtr(new FileReaderLoader(m_readType, this));

The constructor for FileReaderLoader should be private and have a static create method but of course this is just a move of code.

> Source/WebCore/fileapi/FileReader.h:37
> +#include "ExceptionCode.h"

Out of order. (Should be after EventTarget)

[Jian Li]

Committed as https://trac.webkit.org/changeset/87961.