Bug 32912 - Implement File and Blob interfaces as defined in File API spec.
: Implement File and Blob interfaces as defined in File API spec.
Status: RESOLVED FIXED
: WebKit
WebCore JavaScript
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
:
:
: 32993
  Show dependency treegraph
 
Reported: 2009-12-23 16:59 PST by
Modified: 2010-01-20 15:55 PST (History)


Attachments
Proposed Patch (37.97 KB, patch)
2009-12-23 17:21 PST, Jian Li
abarth: review-
jianli: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
Proposed Patch (44.25 KB, patch)
2009-12-23 18:22 PST, Jian Li
jianli: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
Proposed Patch (44.25 KB, patch)
2009-12-28 18:04 PST, Jian Li
dimich: review-
jianli: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
Proposed Patch (43.78 KB, patch)
2010-01-14 15:51 PST, Jian Li
jianli: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
Proposed Patch (43.65 KB, patch)
2010-01-14 17:26 PST, Jian Li
jianli: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
Proposed Patch (39.08 KB, patch)
2010-01-19 10:51 PST, Jian Li
dimich: review-
jianli: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
Proposed Patch (41.01 KB, patch)
2010-01-19 16:05 PST, Jian Li
dimich: review+
jianli: commit‑queue-
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2009-12-23 16:59:47 PST
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.
------- Comment #1 From 2009-12-23 17:21:01 PST -------
Created an attachment (id=45459) [details]
Proposed Patch
------- Comment #2 From 2009-12-23 17:22:30 PST -------
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
------- Comment #3 From 2009-12-23 17:31:57 PST -------
Attachment 45459 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/142502
------- Comment #4 From 2009-12-23 18:00:44 PST -------
(From update of attachment 45459 [details])
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.
------- Comment #5 From 2009-12-23 18:22:12 PST -------
Created an attachment (id=45460) [details]
Proposed Patch

Fixed style problems and added V8 binding changes (I previously planned to put them in a separate patch).
------- Comment #6 From 2009-12-28 17:46:40 PST -------
File API is not part of HTML5.
------- Comment #7 From 2009-12-28 17:50:48 PST -------
(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.
------- Comment #8 From 2009-12-28 18:04:32 PST -------
Created an attachment (id=45582) [details]
Proposed Patch

Update the description in ChangeLog per the bug title change.
------- Comment #9 From 2010-01-08 15:03:05 PST -------
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/public-webapps@w3.org/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.
------- Comment #10 From 2010-01-11 23:08:49 PST -------
<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.
------- Comment #11 From 2010-01-12 12:32:35 PST -------
(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.
------- Comment #12 From 2010-01-14 15:51:49 PST -------
Created an attachment (id=46616) [details]
Proposed Patch

Removed the caching for file length.
------- Comment #13 From 2010-01-14 17:26:08 PST -------
Created an attachment (id=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.
------- Comment #14 From 2010-01-14 20:46:55 PST -------
Attachment 46624 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/187848
------- Comment #15 From 2010-01-19 10:51:27 PST -------
Created an attachment (id=46927) [details]
Proposed Patch

Removed those properties, that are not implemented yet, from IDL files, as suggested by dimich.
------- Comment #16 From 2010-01-19 12:16:20 PST -------
(From update of attachment 46927 [details])
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?
------- Comment #17 From 2010-01-19 16:05:15 PST -------
Created an attachment (id=46957) [details]
Proposed Patch

All fixed. Thanks.
------- Comment #18 From 2010-01-19 16:42:39 PST -------
(From update of attachment 46957 [details])
r=me
------- Comment #19 From 2010-01-20 15:55:06 PST -------
Committed as http://trac.webkit.org/changeset/53574.