Bug 32912 – Implement File and Blob interfaces as defined in File API spec.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Jian Li]

This is the first step to implement HTML 5 File API as defined in http://www.w3.org/TR/FileAPI. We need to add Blob interface and change File interface to derive from it.

The implementation of File.urn and Blob.slice will not be in the scope of this bug. Separate bugs will be created.

[Jian Li]

Created attachment 45459 [details]
Proposed Patch

[WebKit Review Bot]

Attachment 45459 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/html/File.h:35:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
WebCore/html/Blob.cpp:72:  One line control clauses should not use braces.  [whitespace/braces] [4]
WebCore/html/Blob.h:41:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 3

[WebKit Review Bot]

Attachment 45459 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/142502

[Adam Barth]

Comment on attachment 45459 [details]
Proposed Patch

Looks like you need to teach the V8 code generator about long long to get this to build.  Also, there are style nits and tabs in the patch.

[Jian Li]

Created attachment 45460 [details]
Proposed Patch

Fixed style problems and added V8 binding changes (I previously planned to put them in a separate patch).

[Maciej Stachowiak]

File API is not part of HTML5.

[Jian Li]

(In reply to comment #6)
> File API is not part of HTML5.

Sorry for the confusion. Should I just use "File API" to refer to the spec? Thanks.

[Jian Li]

Created attachment 45582 [details]
Proposed Patch

Update the description in ChangeLog per the bug title change.

[Dmitry Titov]

The change looks ok but  you 'cache' the length in the File at the moment of its creation, which will cause behavior change from current one -  if the file changes on the disk before XHR.send(file) is called. I don't know if this is good or bad.

I've sent mail to public-webapps trying to clarify this: http://www.mail-archive.com/[e-mail]/msg07198.html

File API spec does not tell much about behavior when files on disk change. My first reaction would be to mark any Blob obtained from slice() method as 'invalid' and fail operations with it once file on disk changes - while keeping the 'original' Blob which is also a File 'mutating', following the state of the file on the disk. This would keep current behavior of XHR.send(file) which is also consistent with regular form submission.

[Darin Fisher (:fishd, Google)]

<input type=file> does not require the referenced file to remain unmodified until it is submitted, and since Blob can just be a pointer to the file referenced by an INPUT element, I think it should similarly not care if the file is modified.  We should probably just treat Blob as a path to data.  This implies not caching file length.

[Dmitry Titov]

(In reply to comment #10)
> <input type=file> does not require the referenced file to remain unmodified
> until it is submitted, and since Blob can just be a pointer to the file
> referenced by an INPUT element, I think it should similarly not care if the
> file is modified.  We should probably just treat Blob as a path to data.  This
> implies not caching file length.

Agree. Marking it r- to remove the caching.

[Jian Li]

Created attachment 46616 [details]
Proposed Patch

Removed the caching for file length.

[Jian Li]

Created attachment 46624 [details]
Proposed Patch

Produced a new patch that resolved the conflict with other landed patch so that the commit queue can check this patch.

[WebKit Review Bot]

Attachment 46624 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/187848

[Jian Li]

Created attachment 46927 [details]
Proposed Patch

Removed those properties, that are not implemented yet, from IDL files, as suggested by dimich.

[Dmitry Titov]

Comment on attachment 46927 [details]
Proposed Patch

Almost there. Would be nice to get it building on trybots though since it's mostly a change to build files...

> diff --git a/WebCore/bindings/objc/PublicDOMInterfaces.h b/WebCore/bindings/objc/PublicDOMInterfaces.h

> +@property(readonly, copy) NSString *type;
> +@property(readonly, copy) NSString *urn;

These are from a future patch. Should be removed.

> diff --git a/WebCore/html/Blob.cpp b/WebCore/html/Blob.cpp

> + * Copyright (C) 2009 Google Inc. All rights reserved.

2009 -> 2010?

> +unsigned long long Blob::size() const
> +{
> +    if (m_length != -1)
> +        return static_cast<unsigned long long>(m_length);

Since this patch does not depend on Blob holding m_length and m_start, lets not add them in this patch. They will be added as part of slice() I think.

> diff --git a/WebCore/html/Blob.h b/WebCore/html/Blob.h

> + * Copyright (C) 2009 Google Inc. All rights reserved.

2010

> diff --git a/WebCore/html/Blob.idl b/WebCore/html/Blob.idl

> + * Copyright (C) 2009 Google Inc. All rights reserved.

2010

> +File::~File()

Does it need an empty destructor?

[Jian Li]

Created attachment 46957 [details]
Proposed Patch

All fixed. Thanks.

[Dmitry Titov]

Comment on attachment 46957 [details]
Proposed Patch

r=me

[Jian Li]

Committed as http://trac.webkit.org/changeset/53574.