Support FileSystem URL in File object

Support FileSystem URL in File

BUG=110119
TEST=existing tests (more will be added when we add more implementation)


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=153651

[darin (slow to review), 2012-08-10 16:50:24]

http://codereview.chromium.org/10828252/diff/4002/net/base/upload_data.h
File net/base/upload_data.h (right):

http://codereview.chromium.org/10828252/diff/4002/net/base/upload_data.h#newc...
net/base/upload_data.h:56: TYPE_FILE_FILESYSTEM,
Can we avoid this?  It seems unfortunate to make the network stack have to know
about filesystem URLs.  The network stack formerly didn't know anything about
them, and I don't think it should start knowing about them now.

Is this a side-effect of sending net::UploadData over IPC?  Maybe we should be
using a different data structure for IPC serialization of upload data.

[kinuko, 2012-08-13 02:32:10]

On 2012/08/10 16:50:24, darin wrote:
> http://codereview.chromium.org/10828252/diff/4002/net/base/upload_data.h
> File net/base/upload_data.h (right):
> 
>
http://codereview.chromium.org/10828252/diff/4002/net/base/upload_data.h#newc...
> net/base/upload_data.h:56: TYPE_FILE_FILESYSTEM,
> Can we avoid this?  It seems unfortunate to make the network stack have to
know
> about filesystem URLs.  The network stack formerly didn't know anything about
> them, and I don't think it should start knowing about them now.
> 
> Is this a side-effect of sending net::UploadData over IPC?  Maybe we should be
> using a different data structure for IPC serialization of upload data.

I think splitting net::UploadData is in our plan. I'll try removing FileSystem
notion from the net layer in this patch too.

[kinuko, 2012-08-13 16:47:31]

On 2012/08/13 02:32:10, kinuko wrote:
> On 2012/08/10 16:50:24, darin wrote:
> > http://codereview.chromium.org/10828252/diff/4002/net/base/upload_data.h
> > File net/base/upload_data.h (right):
> > 
> >
>
http://codereview.chromium.org/10828252/diff/4002/net/base/upload_data.h#newc...
> > net/base/upload_data.h:56: TYPE_FILE_FILESYSTEM,
> > Can we avoid this?  It seems unfortunate to make the network stack have to
> know
> > about filesystem URLs.  The network stack formerly didn't know anything
about
> > them, and I don't think it should start knowing about them now.
> > 
> > Is this a side-effect of sending net::UploadData over IPC?  Maybe we should
be
> > using a different data structure for IPC serialization of upload data.
> 
> I think splitting net::UploadData is in our plan. I'll try removing FileSystem
> notion from the net layer in this patch too.

Made another cl for splitting upload data (naive approach):
http://codereview.chromium.org/10834289/

[kinuko, 2012-08-24 08:33:48]

After landing http://crbug.com/152553 and http://crbug.com/152987 I think this
patch has become saner now, i.e. only webkit/ and content/ need to know about
FileSystem URL.  This patch is to add FileSystem URL support in File object
(i.e. in Blob and ResourceRequestBody).  Can you take a look?  Thanks!!

[jam, 2012-08-27 17:22:17]

removing myself as a reviewer. darin is a reviewer in content as well.

[darin (slow to review), 2012-08-27 18:15:50]

LGTM

http://codereview.chromium.org/10828252/diff/10001/content/browser/fileapi/fi...
File content/browser/fileapi/fileapi_message_filter.cc (right):

http://codereview.chromium.org/10828252/diff/10001/content/browser/fileapi/fi...
content/browser/fileapi/fileapi_message_filter.cc:526: // TODO(kinuko): We must
check permission in FILESYSTEM_FILE cases too.
FILESYSTEM_FILE -> TYPE_FILE_FILESYSTEM ?

[kinuko, 2012-08-28 05:54:14]

Thanks!

http://codereview.chromium.org/10828252/diff/10001/content/browser/fileapi/fi...
File content/browser/fileapi/fileapi_message_filter.cc (right):

http://codereview.chromium.org/10828252/diff/10001/content/browser/fileapi/fi...
content/browser/fileapi/fileapi_message_filter.cc:526: // TODO(kinuko): We must
check permission in FILESYSTEM_FILE cases too.
On 2012/08/27 18:15:51, darin wrote:
> FILESYSTEM_FILE -> TYPE_FILE_FILESYSTEM ?

Done.

[commit-bot: I haz the power, 2012-08-28 05:54:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kinuko@chromium.org/10828252/20001

[commit-bot: I haz the power, 2012-08-28 10:19:46]

Change committed as 153651