Pass the selected non-native file information from the browser to FileInputType.

Pass the selected non-native file information from the browser to FileInputType.

Previously the blink accepts only native files as a result value of the file
chooser. The information is conveyed to the FileInputType. Then FileInputType
generates the File class instances from the information, and the instances are
used to compose the form request.

Thic CL adds non-native file support to the above code path.

* Adds noo-native file information (file system URL and metadata) to structures
  representing selected file (FileChooserFileInfo, SelectedFileInfo).

* When FileInputType generates the File class instances, let the class refer the
  non-native file information.

* When FileInputType checks the change of selected files, let the class refer
  the non-native file information.

BUG=126902
TEST=None

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=183960

[hirono, 2014-10-16 11:47:44]

Patchset #2 (id:20001) has been deleted

[hirono, 2014-10-16 11:48:12]

Patchset #1 (id:1) has been deleted

[hirono, 2014-10-16 11:52:01]

hirono@chromium.org changed reviewers:
+ tkent@chromium.org

[hirono, 2014-10-16 11:52:01]

PTAL the CL? Thank you!

[tkent, 2014-10-17 00:20:22]

https://codereview.chromium.org/640723006/diff/40001/Source/core/html/forms/F...
File Source/core/html/forms/FileInputType.cpp (right):

https://codereview.chromium.org/640723006/diff/40001/Source/core/html/forms/F...
Source/core/html/forms/FileInputType.cpp:248: for (size_t i = 0; i < size; i++)
{
nit: We prefer |++i|.

https://codereview.chromium.org/640723006/diff/40001/Source/core/html/forms/F...
Source/core/html/forms/FileInputType.cpp:249: if
(files[i].fileSystemURL.isNull()) {
Why do you use isNull() though WebFileChooserCompletionImpl uses isEmpty()?

https://codereview.chromium.org/640723006/diff/40001/Source/core/html/forms/F...
Source/core/html/forms/FileInputType.cpp:255: return fileList;
Please add unit test for FileInputType::createFileList().

https://codereview.chromium.org/640723006/diff/40001/Source/core/html/forms/F...
Source/core/html/forms/FileInputType.cpp:294: if
(files->item(i)->hasBackingFile() != m_fileList->item(i)->hasBackingFile()
We should introduce comparison member function, or operator==, and write a unit
test.

https://codereview.chromium.org/640723006/diff/40001/Source/platform/FileChoo...
File Source/platform/FileChooser.h (right):

https://codereview.chromium.org/640723006/diff/40001/Source/platform/FileChoo...
Source/platform/FileChooser.h:58: const FileMetadata metadata;
Please add a comment that |metadata| is available only for the filesystem case.

https://codereview.chromium.org/640723006/diff/40001/public/web/WebFileChoose...
File public/web/WebFileChooserCompletion.h (right):

https://codereview.chromium.org/640723006/diff/40001/public/web/WebFileChoose...
public/web/WebFileChooserCompletion.h:56: double modificationTime;
Please add a comment about expected origin and resolution of modificationTime.
  e.g. 0 is Unix epoch, unit is msec.

[pwnall-personal, 2014-10-17 08:52:43]

https://codereview.chromium.org/640723006/diff/40001/public/web/WebFileChoose...
File public/web/WebFileChooserCompletion.h (right):

https://codereview.chromium.org/640723006/diff/40001/public/web/WebFileChoose...
public/web/WebFileChooserCompletion.h:56: double modificationTime;
On 2014/10/17 00:20:22, tkent wrote:
> Please add a comment about expected origin and resolution of modificationTime.
>   e.g. 0 is Unix epoch, unit is msec.

I'm not an OWNER, so feel free to ignore this comment if you disagree, but I
think lastModified would be a better name here, because it maps to a concept in
the File API spec, which browser developers are likely to be familiar with.
http://dev.w3.org/2006/webapi/FileAPI/#dfn-lastModified

[hirono, 2014-10-17 09:30:59]

Thanks!

https://codereview.chromium.org/640723006/diff/40001/Source/core/html/forms/F...
File Source/core/html/forms/FileInputType.cpp (right):

https://codereview.chromium.org/640723006/diff/40001/Source/core/html/forms/F...
Source/core/html/forms/FileInputType.cpp:248: for (size_t i = 0; i < size; i++)
{
On 2014/10/17 00:20:22, tkent wrote:
> nit: We prefer |++i|.

Done.

https://codereview.chromium.org/640723006/diff/40001/Source/core/html/forms/F...
Source/core/html/forms/FileInputType.cpp:249: if
(files[i].fileSystemURL.isNull()) {
On 2014/10/17 00:20:22, tkent wrote:
> Why do you use isNull() though WebFileChooserCompletionImpl uses isEmpty()?

It should be isEmpty(). Done.

https://codereview.chromium.org/640723006/diff/40001/Source/core/html/forms/F...
Source/core/html/forms/FileInputType.cpp:255: return fileList;
On 2014/10/17 00:20:22, tkent wrote:
> Please add unit test for FileInputType::createFileList().

Done.

https://codereview.chromium.org/640723006/diff/40001/Source/core/html/forms/F...
Source/core/html/forms/FileInputType.cpp:294: if
(files->item(i)->hasBackingFile() != m_fileList->item(i)->hasBackingFile()
On 2014/10/17 00:20:22, tkent wrote:
> We should introduce comparison member function, or operator==, and write a
unit
> test.

Done.

https://codereview.chromium.org/640723006/diff/40001/Source/platform/FileChoo...
File Source/platform/FileChooser.h (right):

https://codereview.chromium.org/640723006/diff/40001/Source/platform/FileChoo...
Source/platform/FileChooser.h:58: const FileMetadata metadata;
On 2014/10/17 00:20:22, tkent wrote:
> Please add a comment that |metadata| is available only for the filesystem
case.

The path and displayName properties are also available only for native file
case, and fileSystemURL and metadata are available only for non-native file
case. Added comments.

https://codereview.chromium.org/640723006/diff/40001/public/web/WebFileChoose...
File public/web/WebFileChooserCompletion.h (right):

https://codereview.chromium.org/640723006/diff/40001/public/web/WebFileChoose...
public/web/WebFileChooserCompletion.h:56: double modificationTime;
On 2014/10/17 00:20:22, tkent wrote:
> Please add a comment about expected origin and resolution of modificationTime.
>   e.g. 0 is Unix epoch, unit is msec.

Done.

https://codereview.chromium.org/640723006/diff/40001/public/web/WebFileChoose...
public/web/WebFileChooserCompletion.h:56: double modificationTime;
On 2014/10/17 08:52:43, pwnall wrote:
> On 2014/10/17 00:20:22, tkent wrote:
> > Please add a comment about expected origin and resolution of
modificationTime.
> >   e.g. 0 is Unix epoch, unit is msec.
> 
> I'm not an OWNER, so feel free to ignore this comment if you disagree, but I
> think lastModified would be a better name here, because it maps to a concept
in
> the File API spec, which browser developers are likely to be familiar with.
> http://dev.w3.org/2006/webapi/FileAPI/#dfn-lastModified

I named it after FileMetadata's member.
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

It looks the name is referring File System API's metadata name.
http://dev.w3.org/2009/dap/file-system/file-dir-sys.html#widl-Metadata-modifi...

So let me keep the name as it is.

[tkent, 2014-10-20 00:48:48]

lgtm

https://codereview.chromium.org/640723006/diff/80001/Source/core/fileapi/File...
File Source/core/fileapi/File.cpp (right):

https://codereview.chromium.org/640723006/diff/80001/Source/core/fileapi/File...
Source/core/fileapi/File.cpp:304: // Compare backing file.
The comment adds nothing other than what the code does.  Please remove it.

https://codereview.chromium.org/640723006/diff/80001/Source/core/fileapi/File...
Source/core/fileapi/File.cpp:311: // Compare file system URL.
Ditto.

https://codereview.chromium.org/640723006/diff/80001/Source/core/fileapi/File...
Source/core/fileapi/File.cpp:315: // Compare blob UUID.
Ditto.

https://codereview.chromium.org/640723006/diff/80001/public/web/WebFileChoose...
File public/web/WebFileChooserCompletion.h (right):

https://codereview.chromium.org/640723006/diff/80001/public/web/WebFileChoose...
public/web/WebFileChooserCompletion.h:56: // 0 is Unit epoch, unit is sec.
Unit -> Unix?

[hirono, 2014-10-20 03:47:21]

The CQ bit was checked by hirono@chromium.org

[commit-bot: I haz the power, 2014-10-20 03:47:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/640723006/100001

[hirono, 2014-10-20 03:47:37]

The CQ bit was unchecked by hirono@chromium.org

[hirono, 2014-10-20 03:49:19]

The CQ bit was checked by hirono@chromium.org

[commit-bot: I haz the power, 2014-10-20 03:49:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/640723006/100001

[hirono, 2014-10-20 03:50:06]

Thanks!

https://codereview.chromium.org/640723006/diff/80001/Source/core/fileapi/File...
File Source/core/fileapi/File.cpp (right):

https://codereview.chromium.org/640723006/diff/80001/Source/core/fileapi/File...
Source/core/fileapi/File.cpp:304: // Compare backing file.
On 2014/10/20 00:48:48, tkent wrote:
> The comment adds nothing other than what the code does.  Please remove it.

Done.

https://codereview.chromium.org/640723006/diff/80001/Source/core/fileapi/File...
Source/core/fileapi/File.cpp:311: // Compare file system URL.
On 2014/10/20 00:48:48, tkent wrote:
> Ditto.

Done.

https://codereview.chromium.org/640723006/diff/80001/Source/core/fileapi/File...
Source/core/fileapi/File.cpp:315: // Compare blob UUID.
On 2014/10/20 00:48:48, tkent wrote:
> Ditto.

Done.

https://codereview.chromium.org/640723006/diff/80001/public/web/WebFileChoose...
File public/web/WebFileChooserCompletion.h (right):

https://codereview.chromium.org/640723006/diff/80001/public/web/WebFileChoose...
public/web/WebFileChooserCompletion.h:56: // 0 is Unit epoch, unit is sec.
On 2014/10/20 00:48:48, tkent wrote:
> Unit -> Unix?

Done.

[commit-bot: I haz the power, 2014-10-20 04:48:33]

Committed patchset #4 (id:100001) as 183960