[fsp] First part of support for reading files.

[fsp] First part of support for reading files.

This patch adds the read file operation, which is however not bound yet to
fileapi.

In the upcoming patch, support for FileStreamReader will be added. After that,
OpenFile, CloseFile and ReadFile will be bound to async file utils.

TEST=unit_tests: *FileSystemProvider*ReadFile*
BUG=248427
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=270985

[mtomasz, 2014-05-14 08:36:32]

@kinaba: PTAL at C++.
@benwells: PTAL ad IDL.

As before, mostly copy paste. This is part 1 from 3 patches. The next one will
be FileStreamReader, the last one bounding all of it to AsyncFileUtil.

[benwells, 2014-05-15 00:22:26]

https://codereview.chromium.org/287673004/diff/100001/chrome/common/extension...
File chrome/common/extensions/api/file_system_provider.idl (right):

https://codereview.chromium.org/287673004/diff/100001/chrome/common/extension...
chrome/common/extensions/api/file_system_provider.idl:88: callback
FileDataCallback = void(DOMString data, bool hasNext);
How does this work if the data isn't a string?

[mtomasz, 2014-05-15 00:26:24]

https://codereview.chromium.org/287673004/diff/100001/chrome/common/extension...
File chrome/common/extensions/api/file_system_provider.idl (right):

https://codereview.chromium.org/287673004/diff/100001/chrome/common/extension...
chrome/common/extensions/api/file_system_provider.idl:88: callback
FileDataCallback = void(DOMString data, bool hasNext);
On 2014/05/15 00:22:26, benwells wrote:
> How does this work if the data isn't a string?

Any data can be sent as string. File contents are always an array of
bytes/chars. We could use ByteArray here, but it would require making an extra
copy in custom bindings, to convert it to DOMString anyway. Also, DOMString is
very easy to use for developers. WDYT?

[benwells, 2014-05-15 01:02:45]

On 2014/05/15 00:26:24, mtomasz wrote:
>
https://codereview.chromium.org/287673004/diff/100001/chrome/common/extension...
> File chrome/common/extensions/api/file_system_provider.idl (right):
> 
>
https://codereview.chromium.org/287673004/diff/100001/chrome/common/extension...
> chrome/common/extensions/api/file_system_provider.idl:88: callback
> FileDataCallback = void(DOMString data, bool hasNext);
> On 2014/05/15 00:22:26, benwells wrote:
> > How does this work if the data isn't a string?
> 
> Any data can be sent as string. File contents are always an array of
> bytes/chars. We could use ByteArray here, but it would require making an extra
> copy in custom bindings, to convert it to DOMString anyway. Also, DOMString is
> very easy to use for developers. WDYT?

Can you show me how the provider would convert arbitrary data to a DOMString?

[mtomasz, 2014-05-15 01:09:05]

On 2014/05/15 01:02:45, benwells wrote:
> On 2014/05/15 00:26:24, mtomasz wrote:
> >
>
https://codereview.chromium.org/287673004/diff/100001/chrome/common/extension...
> > File chrome/common/extensions/api/file_system_provider.idl (right):
> > 
> >
>
https://codereview.chromium.org/287673004/diff/100001/chrome/common/extension...
> > chrome/common/extensions/api/file_system_provider.idl:88: callback
> > FileDataCallback = void(DOMString data, bool hasNext);
> > On 2014/05/15 00:22:26, benwells wrote:
> > > How does this work if the data isn't a string?
> > 
> > Any data can be sent as string. File contents are always an array of
> > bytes/chars. We could use ByteArray here, but it would require making an
extra
> > copy in custom bindings, to convert it to DOMString anyway. Also, DOMString
is
> > very easy to use for developers. WDYT?
> 
> Can you show me how the provider would convert arbitrary data to a DOMString?

For remote file systems:
XMLHttpRequest -> onreadystatechange -> XMLHttpRequest::responseText

For file stored in local DOMFileSystem:
File -> FileReader -> FileReader::readAsText

For data stored in Blobs:
Blob -> FileReader -> FileReader::readAsText

[benwells, 2014-05-15 01:29:36]

On 2014/05/15 01:09:05, mtomasz wrote:
> On 2014/05/15 01:02:45, benwells wrote:
> > On 2014/05/15 00:26:24, mtomasz wrote:
> > >
> >
>
https://codereview.chromium.org/287673004/diff/100001/chrome/common/extension...
> > > File chrome/common/extensions/api/file_system_provider.idl (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/287673004/diff/100001/chrome/common/extension...
> > > chrome/common/extensions/api/file_system_provider.idl:88: callback
> > > FileDataCallback = void(DOMString data, bool hasNext);
> > > On 2014/05/15 00:22:26, benwells wrote:
> > > > How does this work if the data isn't a string?
> > > 
> > > Any data can be sent as string. File contents are always an array of
> > > bytes/chars. We could use ByteArray here, but it would require making an
> extra
> > > copy in custom bindings, to convert it to DOMString anyway. Also,
DOMString
> is
> > > very easy to use for developers. WDYT?
> > 
> > Can you show me how the provider would convert arbitrary data to a
DOMString?
> 
> For remote file systems:
> XMLHttpRequest -> onreadystatechange -> XMLHttpRequest::responseText
> 
> For file stored in local DOMFileSystem:
> File -> FileReader -> FileReader::readAsText
> 
> For data stored in Blobs:
> Blob -> FileReader -> FileReader::readAsText

Huh, that was obvious.

[benwells, 2014-05-15 01:30:59]

lgtm

https://codereview.chromium.org/287673004/diff/110001/chrome/common/extension...
File chrome/common/extensions/api/file_system_provider.idl (right):

https://codereview.chromium.org/287673004/diff/110001/chrome/common/extension...
chrome/common/extensions/api/file_system_provider.idl:162: // Raised when
contents of a file opened previously with <code>openRequestId
There should be a </code> here I think.

[kinaba, 2014-05-15 08:23:40]

https://codereview.chromium.org/287673004/diff/130001/chrome/browser/chromeos...
File chrome/browser/chromeos/file_system_provider/operations/read_file.cc
(right):

https://codereview.chromium.org/287673004/diff/130001/chrome/browser/chromeos...
chrome/browser/chromeos/file_system_provider/operations/read_file.cc:81:
CopyRequestValueToBuffer(result.Pass(), buffer_, offset_, length_);
|offset_| seems to be used in two different ways:
as an argument of OnReadFileRequested for indicating the offset in the file, and
as an argument of CopyRequestValueToBuffer for indicating the offset in the
|buffer_|.

Do they coincide? If so it looks strange to me because then we need to allocate
a buffer of the same size as the whole file.

In other words, shouldn't the third argument here be always zero?

[mtomasz, 2014-05-16 01:39:39]

https://codereview.chromium.org/287673004/diff/130001/chrome/browser/chromeos...
File chrome/browser/chromeos/file_system_provider/operations/read_file.cc
(right):

https://codereview.chromium.org/287673004/diff/130001/chrome/browser/chromeos...
chrome/browser/chromeos/file_system_provider/operations/read_file.cc:81:
CopyRequestValueToBuffer(result.Pass(), buffer_, offset_, length_);
On 2014/05/15 08:23:40, kinaba wrote:
> |offset_| seems to be used in two different ways:
> as an argument of OnReadFileRequested for indicating the offset in the file,
and
> as an argument of CopyRequestValueToBuffer for indicating the offset in the
> |buffer_|.
> 
> Do they coincide? If so it looks strange to me because then we need to
allocate
> a buffer of the same size as the whole file.
> 
> In other words, shouldn't the third argument here be always zero?

You're right. Good catch. Fixed.

[mtomasz, 2014-05-16 02:39:58]

https://codereview.chromium.org/287673004/diff/110001/chrome/common/extension...
File chrome/common/extensions/api/file_system_provider.idl (right):

https://codereview.chromium.org/287673004/diff/110001/chrome/common/extension...
chrome/common/extensions/api/file_system_provider.idl:162: // Raised when
contents of a file opened previously with <code>openRequestId
On 2014/05/15 01:31:00, benwells wrote:
> There should be a </code> here I think.

Done.

[kinaba, 2014-05-16 04:53:53]

lgtm

[mtomasz, 2014-05-16 05:45:46]

The CQ bit was checked by mtomasz@chromium.org

[commit-bot: I haz the power, 2014-05-16 05:45:57]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mtomasz@chromium.org/287673004/210001

[commit-bot: I haz the power, 2014-05-16 09:52:54]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/18...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)

[commit-bot: I haz the power, 2014-05-16 11:17:26]

Change committed as 270985