Stub for Privet file system

Stub for Privet file system

This contains a stub Privet file system, behind a flag.

BUG=332182
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=245612

[Vitaly Buka (NO REVIEWS), 2014-01-07 21:08:49]

lgtm

https://codereview.chromium.org/120533006/diff/1/chrome/browser/local_discove...
File chrome/browser/local_discovery/storage/privet_filesystem_async_util.cc
(right):

https://codereview.chromium.org/120533006/diff/1/chrome/browser/local_discove...
chrome/browser/local_discovery/storage/privet_filesystem_async_util.cc:39:
file_info.size = 20;
missaligned

https://codereview.chromium.org/120533006/diff/1/chrome/browser/local_discove...
chrome/browser/local_discovery/storage/privet_filesystem_async_util.cc:63:
callback.Run(base::PLATFORM_FILE_OK,
should be one line

https://codereview.chromium.org/120533006/diff/1/chrome/browser/local_discove...
File chrome/browser/local_discovery/storage/privet_filesystem_async_util.h
(right):

https://codereview.chromium.org/120533006/diff/1/chrome/browser/local_discove...
chrome/browser/local_discovery/storage/privet_filesystem_async_util.h:1: //
Copyright (c) 2014 The Chromium Authors. All rights reserved.
Do we use (c) again?

https://codereview.chromium.org/120533006/diff/1/chrome/browser/local_discove...
chrome/browser/local_discovery/storage/privet_filesystem_async_util.h:63: const
base::FilePath& src_file_path,
missaligned

https://codereview.chromium.org/120533006/diff/1/chrome/browser/local_discove...
File chrome/browser/local_discovery/storage/privet_filesystem_backend.cc
(right):

https://codereview.chromium.org/120533006/diff/1/chrome/browser/local_discove...
chrome/browser/local_discovery/storage/privet_filesystem_backend.cc:57: //
TODO(noamsml)
it could be usefull to have bug ID to find context easily, in case you decide
not to implement this soon.

https://codereview.chromium.org/120533006/diff/1/chrome/browser/local_discove...
chrome/browser/local_discovery/storage/privet_filesystem_backend.cc:81: //
System mount pints
The line addressed by this comment is not clear.
also it looks redundunt

https://codereview.chromium.org/120533006/diff/1/chrome/browser/resources/fil...
File chrome/browser/resources/file_manager/background/js/volume_manager.js
(right):

https://codereview.chromium.org/120533006/diff/1/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/background/js/volume_manager.js:204:
RootType.DRIVE, RootType.DOWNLOADS, RootType.ARCHIVE, RootType.REMOVABLE,
does not feet one line, can you please make it one constant per line

https://codereview.chromium.org/120533006/diff/1/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/background/js/volume_manager.js:205:
RootType.PRIVET
not sure if content of [] should be intended by 2 or 4 spaces

[Vitaly Buka (NO REVIEWS), 2014-01-07 21:14:06]

On 2014/01/07 21:08:49, Vitaly Buka wrote:
> lgtm
> 
>
https://codereview.chromium.org/120533006/diff/1/chrome/browser/local_discove...
> File chrome/browser/local_discovery/storage/privet_filesystem_async_util.cc
> (right):
> 
>
https://codereview.chromium.org/120533006/diff/1/chrome/browser/local_discove...
> chrome/browser/local_discovery/storage/privet_filesystem_async_util.cc:39:
> file_info.size = 20;
> missaligned
> 
>
https://codereview.chromium.org/120533006/diff/1/chrome/browser/local_discove...
> chrome/browser/local_discovery/storage/privet_filesystem_async_util.cc:63:
> callback.Run(base::PLATFORM_FILE_OK,
> should be one line
> 
>
https://codereview.chromium.org/120533006/diff/1/chrome/browser/local_discove...
> File chrome/browser/local_discovery/storage/privet_filesystem_async_util.h
> (right):
> 
>
https://codereview.chromium.org/120533006/diff/1/chrome/browser/local_discove...
> chrome/browser/local_discovery/storage/privet_filesystem_async_util.h:1: //
> Copyright (c) 2014 The Chromium Authors. All rights reserved.
> Do we use (c) again?
> 
>
https://codereview.chromium.org/120533006/diff/1/chrome/browser/local_discove...
> chrome/browser/local_discovery/storage/privet_filesystem_async_util.h:63:
const
> base::FilePath& src_file_path,
> missaligned
> 
>
https://codereview.chromium.org/120533006/diff/1/chrome/browser/local_discove...
> File chrome/browser/local_discovery/storage/privet_filesystem_backend.cc
> (right):
> 
>
https://codereview.chromium.org/120533006/diff/1/chrome/browser/local_discove...
> chrome/browser/local_discovery/storage/privet_filesystem_backend.cc:57: //
> TODO(noamsml)
> it could be usefull to have bug ID to find context easily, in case you decide
> not to implement this soon.
> 
>
https://codereview.chromium.org/120533006/diff/1/chrome/browser/local_discove...
> chrome/browser/local_discovery/storage/privet_filesystem_backend.cc:81: //
> System mount pints
> The line addressed by this comment is not clear.
> also it looks redundunt
> 
>
https://codereview.chromium.org/120533006/diff/1/chrome/browser/resources/fil...
> File chrome/browser/resources/file_manager/background/js/volume_manager.js
> (right):
> 
>
https://codereview.chromium.org/120533006/diff/1/chrome/browser/resources/fil...
> chrome/browser/resources/file_manager/background/js/volume_manager.js:204:
> RootType.DRIVE, RootType.DOWNLOADS, RootType.ARCHIVE, RootType.REMOVABLE,
> does not feet one line, can you please make it one constant per line
> 
>
https://codereview.chromium.org/120533006/diff/1/chrome/browser/resources/fil...
> chrome/browser/resources/file_manager/background/js/volume_manager.js:205:
> RootType.PRIVET
> not sure if content of [] should be intended by 2 or 4 spaces

bug id?

[Noam Samuel, 2014-01-07 22:10:59]

https://codereview.chromium.org/120533006/diff/1/chrome/browser/local_discove...
File chrome/browser/local_discovery/storage/privet_filesystem_async_util.cc
(right):

https://codereview.chromium.org/120533006/diff/1/chrome/browser/local_discove...
chrome/browser/local_discovery/storage/privet_filesystem_async_util.cc:39:
file_info.size = 20;
On 2014/01/07 21:08:49, Vitaly Buka wrote:
> missaligned

Done.

https://codereview.chromium.org/120533006/diff/1/chrome/browser/local_discove...
chrome/browser/local_discovery/storage/privet_filesystem_async_util.cc:63:
callback.Run(base::PLATFORM_FILE_OK,
On 2014/01/07 21:08:49, Vitaly Buka wrote:
> should be one line

Done.

https://codereview.chromium.org/120533006/diff/1/chrome/browser/local_discove...
File chrome/browser/local_discovery/storage/privet_filesystem_async_util.h
(right):

https://codereview.chromium.org/120533006/diff/1/chrome/browser/local_discove...
chrome/browser/local_discovery/storage/privet_filesystem_async_util.h:1: //
Copyright (c) 2014 The Chromium Authors. All rights reserved.
On 2014/01/07 21:08:49, Vitaly Buka wrote:
> Do we use (c) again?

No.

https://codereview.chromium.org/120533006/diff/1/chrome/browser/local_discove...
chrome/browser/local_discovery/storage/privet_filesystem_async_util.h:63: const
base::FilePath& src_file_path,
On 2014/01/07 21:08:49, Vitaly Buka wrote:
> missaligned

Done.

https://codereview.chromium.org/120533006/diff/1/chrome/browser/local_discove...
File chrome/browser/local_discovery/storage/privet_filesystem_backend.cc
(right):

https://codereview.chromium.org/120533006/diff/1/chrome/browser/local_discove...
chrome/browser/local_discovery/storage/privet_filesystem_backend.cc:57: //
TODO(noamsml)
On 2014/01/07 21:08:49, Vitaly Buka wrote:
> it could be usefull to have bug ID to find context easily, in case you decide
> not to implement this soon.

Done.

https://codereview.chromium.org/120533006/diff/1/chrome/browser/local_discove...
chrome/browser/local_discovery/storage/privet_filesystem_backend.cc:81: //
System mount pints
On 2014/01/07 21:08:49, Vitaly Buka wrote:
> The line addressed by this comment is not clear.
> also it looks redundunt

Removed.

https://codereview.chromium.org/120533006/diff/1/chrome/browser/resources/fil...
File chrome/browser/resources/file_manager/background/js/volume_manager.js
(right):

https://codereview.chromium.org/120533006/diff/1/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/background/js/volume_manager.js:204:
RootType.DRIVE, RootType.DOWNLOADS, RootType.ARCHIVE, RootType.REMOVABLE,
On 2014/01/07 21:08:49, Vitaly Buka wrote:
> does not feet one line, can you please make it one constant per line

Done.

https://codereview.chromium.org/120533006/diff/1/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/background/js/volume_manager.js:205:
RootType.PRIVET
On 2014/01/07 21:08:49, Vitaly Buka wrote:
> not sure if content of [] should be intended by 2 or 4 spaces

Was previously 2 spaces.

[Vitaly Buka (NO REVIEWS), 2014-01-07 22:34:23]

lgtm

[Noam Samuel, 2014-01-07 23:12:00]

On 2014/01/07 22:34:23, Vitaly Buka wrote:
> lgtm

+mtomasz for file_manager and file API

[mtomasz, 2014-01-08 00:08:16]

On 2014/01/07 23:12:00, Noam Samuel wrote:
> On 2014/01/07 22:34:23, Vitaly Buka wrote:
> > lgtm
> 
> +mtomasz for file_manager and file API

Can you provide more background why this is needed? What is privet? Is it
available to everyone? Adding new file systems is kind of deprecated, since a JS
api for providing custom file systems via extensions is coming soon (M-34/M-35).

[mtomasz, 2014-01-08 09:35:23]

+@kinaba and +@kinuko who are familiar with backends.

https://codereview.chromium.org/120533006/diff/90001/chrome/browser/chromeos/...
File chrome/browser/chromeos/file_manager/volume_manager.cc (right):

https://codereview.chromium.org/120533006/diff/90001/chrome/browser/chromeos/...
chrome/browser/chromeos/file_manager/volume_manager.cc:139: VolumeInfo
CreatePrivetVolumeInfo() {
Note, that in late M-34/early M-35 we will have a JS api in read only mode for
this, and this code would have to be rewritten in JS.

https://codereview.chromium.org/120533006/diff/90001/chrome/browser/chromeos/...
File chrome/browser/chromeos/file_manager/volume_manager.h (right):

https://codereview.chromium.org/120533006/diff/90001/chrome/browser/chromeos/...
chrome/browser/chromeos/file_manager/volume_manager.h:47: VOLUME_TYPE_PRIVET
How about renaming to CLOUD_DEVICE? Is privet an internal name?

https://codereview.chromium.org/120533006/diff/90001/chrome/browser/resources...
File chrome/browser/resources/file_manager/background/js/volume_manager.js
(right):

https://codereview.chromium.org/120533006/diff/90001/chrome/browser/resources...
chrome/browser/resources/file_manager/background/js/volume_manager.js:208:
RootType.PRIVET
How about renaming to CLOUD? Privet sounds like a code name and it may be
difficult to understand by other developers.

https://codereview.chromium.org/120533006/diff/90001/chrome/browser/resources...
File chrome/browser/resources/file_manager/common/js/path_util.js (right):

https://codereview.chromium.org/120533006/diff/90001/chrome/browser/resources...
chrome/browser/resources/file_manager/common/js/path_util.js:38: // Root for
privet storage volume.
Move above fake entries.

https://codereview.chromium.org/120533006/diff/90001/chrome/browser/resources...
chrome/browser/resources/file_manager/common/js/path_util.js:56: PRIVET:
'/privet'
ditto

[Noam Samuel, 2014-01-09 01:00:09]

https://codereview.chromium.org/120533006/diff/90001/chrome/browser/chromeos/...
File chrome/browser/chromeos/file_manager/volume_manager.cc (right):

https://codereview.chromium.org/120533006/diff/90001/chrome/browser/chromeos/...
chrome/browser/chromeos/file_manager/volume_manager.cc:139: VolumeInfo
CreatePrivetVolumeInfo() {
On 2014/01/08 09:35:24, mtomasz wrote:
> Note, that in late M-34/early M-35 we will have a JS api in read only mode for
> this, and this code would have to be rewritten in JS.

Agreed. This code should eventually be in JS.

https://codereview.chromium.org/120533006/diff/90001/chrome/browser/chromeos/...
File chrome/browser/chromeos/file_manager/volume_manager.h (right):

https://codereview.chromium.org/120533006/diff/90001/chrome/browser/chromeos/...
chrome/browser/chromeos/file_manager/volume_manager.h:47: VOLUME_TYPE_PRIVET
On 2014/01/08 09:35:24, mtomasz wrote:
> How about renaming to CLOUD_DEVICE? Is privet an internal name?

Done.

https://codereview.chromium.org/120533006/diff/90001/chrome/browser/resources...
File chrome/browser/resources/file_manager/background/js/volume_manager.js
(right):

https://codereview.chromium.org/120533006/diff/90001/chrome/browser/resources...
chrome/browser/resources/file_manager/background/js/volume_manager.js:208:
RootType.PRIVET
On 2014/01/08 09:35:24, mtomasz wrote:
> How about renaming to CLOUD? Privet sounds like a code name and it may be
> difficult to understand by other developers.

Done.

https://codereview.chromium.org/120533006/diff/90001/chrome/browser/resources...
File chrome/browser/resources/file_manager/common/js/path_util.js (right):

https://codereview.chromium.org/120533006/diff/90001/chrome/browser/resources...
chrome/browser/resources/file_manager/common/js/path_util.js:38: // Root for
privet storage volume.
On 2014/01/08 09:35:24, mtomasz wrote:
> Move above fake entries.

Done.

https://codereview.chromium.org/120533006/diff/90001/chrome/browser/resources...
chrome/browser/resources/file_manager/common/js/path_util.js:56: PRIVET:
'/privet'
On 2014/01/08 09:35:24, mtomasz wrote:
> ditto

Done.

[mtomasz, 2014-01-09 01:07:04]

lgtm, but please wait for a review from @kinaba and @kinuko.

https://codereview.chromium.org/120533006/diff/260001/chrome/browser/local_di...
File chrome/browser/local_discovery/storage/privet_filesystem_backend.cc
(right):

https://codereview.chromium.org/120533006/diff/260001/chrome/browser/local_di...
chrome/browser/local_discovery/storage/privet_filesystem_backend.cc:81: //
System mount pints
nit: Coma after the comment.

https://codereview.chromium.org/120533006/diff/260001/chrome/browser/local_di...
chrome/browser/local_discovery/storage/privet_filesystem_backend.cc:121: 
nit: Remove the extra line.

[kinuko, 2014-01-09 04:27:40]

https://codereview.chromium.org/120533006/diff/260001/chrome/browser/local_di...
File chrome/browser/local_discovery/storage/privet_filesystem_async_util.cc
(right):

https://codereview.chromium.org/120533006/diff/260001/chrome/browser/local_di...
chrome/browser/local_discovery/storage/privet_filesystem_async_util.cc:15: const
CreateOrOpenCallback& callback) {
Can we have NOTIMPLEMENTED() / NOTREACHED() and error callback in each of these
unimplemented / should-not-be-called methods? Like:

NOTIMPLEMENTED();
base::PlatformFile invalid_file = base::kInvalidPlatformFileValue;
callback.Run(base::PLATFORM_FILE_ERROR_SECURITY,
             base::PassPlatformFile(&invalid_file),
             base::Closure());

https://codereview.chromium.org/120533006/diff/260001/chrome/browser/local_di...
File chrome/browser/local_discovery/storage/privet_filesystem_backend.cc
(right):

https://codereview.chromium.org/120533006/diff/260001/chrome/browser/local_di...
chrome/browser/local_discovery/storage/privet_filesystem_backend.cc:81: //
System mount pints
On 2014/01/09 01:07:04, mtomasz wrote:
> nit: Coma after the comment.

You probably meant 'period' after comments? :)

Also: pints -> points

https://codereview.chromium.org/120533006/diff/260001/chrome/browser/local_di...
File chrome/browser/local_discovery/storage/privet_filesystem_backend.h (right):

https://codereview.chromium.org/120533006/diff/260001/chrome/browser/local_di...
chrome/browser/local_discovery/storage/privet_filesystem_backend.h:61: //
ExternalFileSystemBackend implementation.
It's a bit sad to have more filesystems that depend on this.. are these methods
hooked into the existing volume code like that of chromeos::FileSystemBackend?

[gene, 2014-01-09 08:13:52]

tiny nits below:

https://codereview.chromium.org/120533006/diff/260001/chrome/browser/chromeos...
File chrome/browser/chromeos/extensions/file_manager/private_api_util.cc
(right):

https://codereview.chromium.org/120533006/diff/260001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/file_manager/private_api_util.cc:166:
file_browser_private::VOLUME_TYPE_CLOUD_DEVICE;
add break; for consistancy

https://codereview.chromium.org/120533006/diff/260001/chrome/browser/local_di...
File chrome/browser/local_discovery/storage/privet_filesystem_async_util.cc
(right):

https://codereview.chromium.org/120533006/diff/260001/chrome/browser/local_di...
chrome/browser/local_discovery/storage/privet_filesystem_async_util.cc:128: 
extra empty line

[Noam Samuel, 2014-01-09 18:59:15]

https://codereview.chromium.org/120533006/diff/260001/chrome/browser/local_di...
File chrome/browser/local_discovery/storage/privet_filesystem_backend.h (right):

https://codereview.chromium.org/120533006/diff/260001/chrome/browser/local_di...
chrome/browser/local_discovery/storage/privet_filesystem_backend.h:61: //
ExternalFileSystemBackend implementation.
On 2014/01/09 04:27:41, kinuko wrote:
> It's a bit sad to have more filesystems that depend on this.. are these
methods
> hooked into the existing volume code like that of chromeos::FileSystemBackend?

Hm, on second look this might not be needed. When I started this stub I assumed
I'd be making the FS with a type of "external", but since that doesn't seem to
be the case, maybe I can remove this interface.

[Noam Samuel, 2014-01-09 20:23:23]

https://codereview.chromium.org/120533006/diff/260001/chrome/browser/chromeos...
File chrome/browser/chromeos/extensions/file_manager/private_api_util.cc
(right):

https://codereview.chromium.org/120533006/diff/260001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/file_manager/private_api_util.cc:166:
file_browser_private::VOLUME_TYPE_CLOUD_DEVICE;
On 2014/01/09 08:13:53, gene wrote:
> add break; for consistancy

Done.

https://codereview.chromium.org/120533006/diff/260001/chrome/browser/local_di...
File chrome/browser/local_discovery/storage/privet_filesystem_async_util.cc
(right):

https://codereview.chromium.org/120533006/diff/260001/chrome/browser/local_di...
chrome/browser/local_discovery/storage/privet_filesystem_async_util.cc:15: const
CreateOrOpenCallback& callback) {
On 2014/01/09 04:27:41, kinuko wrote:
> Can we have NOTIMPLEMENTED() / NOTREACHED() and error callback in each of
these
> unimplemented / should-not-be-called methods? Like:
> 
> NOTIMPLEMENTED();
> base::PlatformFile invalid_file = base::kInvalidPlatformFileValue;
> callback.Run(base::PLATFORM_FILE_ERROR_SECURITY,
>              base::PassPlatformFile(&invalid_file),
>              base::Closure());

Done.

https://codereview.chromium.org/120533006/diff/260001/chrome/browser/local_di...
chrome/browser/local_discovery/storage/privet_filesystem_async_util.cc:128: 
On 2014/01/09 08:13:53, gene wrote:
> extra empty line

Done.

https://codereview.chromium.org/120533006/diff/260001/chrome/browser/local_di...
File chrome/browser/local_discovery/storage/privet_filesystem_backend.cc
(right):

https://codereview.chromium.org/120533006/diff/260001/chrome/browser/local_di...
chrome/browser/local_discovery/storage/privet_filesystem_backend.cc:81: //
System mount pints
On 2014/01/09 04:27:41, kinuko wrote:
> On 2014/01/09 01:07:04, mtomasz wrote:
> > nit: Coma after the comment.
> 
> You probably meant 'period' after comments? :)
> 
> Also: pints -> points

That comment doesn't really make a whole lot of sense; removed it entirely.

https://codereview.chromium.org/120533006/diff/260001/chrome/browser/local_di...
chrome/browser/local_discovery/storage/privet_filesystem_backend.cc:121: 
On 2014/01/09 01:07:04, mtomasz wrote:
> nit: Remove the extra line.

Done.

[Noam Samuel, 2014-01-13 19:08:45]

On 2014/01/09 20:23:23, Noam Samuel wrote:
>
https://codereview.chromium.org/120533006/diff/260001/chrome/browser/chromeos...
> File chrome/browser/chromeos/extensions/file_manager/private_api_util.cc
> (right):
> 
>
https://codereview.chromium.org/120533006/diff/260001/chrome/browser/chromeos...
> chrome/browser/chromeos/extensions/file_manager/private_api_util.cc:166:
> file_browser_private::VOLUME_TYPE_CLOUD_DEVICE;
> On 2014/01/09 08:13:53, gene wrote:
> > add break; for consistancy
> 
> Done.
> 
>
https://codereview.chromium.org/120533006/diff/260001/chrome/browser/local_di...
> File chrome/browser/local_discovery/storage/privet_filesystem_async_util.cc
> (right):
> 
>
https://codereview.chromium.org/120533006/diff/260001/chrome/browser/local_di...
> chrome/browser/local_discovery/storage/privet_filesystem_async_util.cc:15:
const
> CreateOrOpenCallback& callback) {
> On 2014/01/09 04:27:41, kinuko wrote:
> > Can we have NOTIMPLEMENTED() / NOTREACHED() and error callback in each of
> these
> > unimplemented / should-not-be-called methods? Like:
> > 
> > NOTIMPLEMENTED();
> > base::PlatformFile invalid_file = base::kInvalidPlatformFileValue;
> > callback.Run(base::PLATFORM_FILE_ERROR_SECURITY,
> >              base::PassPlatformFile(&invalid_file),
> >              base::Closure());
> 
> Done.
> 
>
https://codereview.chromium.org/120533006/diff/260001/chrome/browser/local_di...
> chrome/browser/local_discovery/storage/privet_filesystem_async_util.cc:128: 
> On 2014/01/09 08:13:53, gene wrote:
> > extra empty line
> 
> Done.
> 
>
https://codereview.chromium.org/120533006/diff/260001/chrome/browser/local_di...
> File chrome/browser/local_discovery/storage/privet_filesystem_backend.cc
> (right):
> 
>
https://codereview.chromium.org/120533006/diff/260001/chrome/browser/local_di...
> chrome/browser/local_discovery/storage/privet_filesystem_backend.cc:81: //
> System mount pints
> On 2014/01/09 04:27:41, kinuko wrote:
> > On 2014/01/09 01:07:04, mtomasz wrote:
> > > nit: Coma after the comment.
> > 
> > You probably meant 'period' after comments? :)
> > 
> > Also: pints -> points
> 
> That comment doesn't really make a whole lot of sense; removed it entirely.
> 
>
https://codereview.chromium.org/120533006/diff/260001/chrome/browser/local_di...
> chrome/browser/local_discovery/storage/privet_filesystem_backend.cc:121: 
> On 2014/01/09 01:07:04, mtomasz wrote:
> > nit: Remove the extra line.
> 
> Done.

Ping?

[Vitaly Buka (NO REVIEWS), 2014-01-13 19:09:37]

lgtm

[kinuko, 2014-01-14 01:47:45]

https://codereview.chromium.org/120533006/diff/600001/chrome/browser/local_di...
File chrome/browser/local_discovery/storage/privet_filesystem_backend.h (right):

https://codereview.chromium.org/120533006/diff/600001/chrome/browser/local_di...
chrome/browser/local_discovery/storage/privet_filesystem_backend.h:64: virtual
std::vector<base::FilePath> GetRootDirectories() const OVERRIDE;
As you noted I don't think you need to implement ExternalFileSystemBackend, or
at least it's not wired into so that these methods won't be called.  Could we
remove this inheritance until we find it really necessary?

[Noam Samuel, 2014-01-14 18:50:15]

https://codereview.chromium.org/120533006/diff/600001/chrome/browser/local_di...
File chrome/browser/local_discovery/storage/privet_filesystem_backend.h (right):

https://codereview.chromium.org/120533006/diff/600001/chrome/browser/local_di...
chrome/browser/local_discovery/storage/privet_filesystem_backend.h:64: virtual
std::vector<base::FilePath> GetRootDirectories() const OVERRIDE;
On 2014/01/14 01:47:45, kinuko wrote:
> As you noted I don't think you need to implement ExternalFileSystemBackend, or
> at least it's not wired into so that these methods won't be called.  Could we
> remove this inheritance until we find it really necessary?

Done.

[kinuko, 2014-01-15 01:13:06]

lgtm

[kinaba, 2014-01-15 07:21:34]

On 2014/01/15 01:13:06, kinuko wrote:
> lgtm

lgtm assuming we switch to use js fileSystemProvieder asap once the new api has
landed.

[commit-bot: I haz the power, 2014-01-16 18:14:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/noamsml@chromium.org/120533006/640001

[commit-bot: I haz the power, 2014-01-16 18:34:30]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[commit-bot: I haz the power, 2014-01-16 22:14:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/noamsml@chromium.org/120533006/640001

[commit-bot: I haz the power, 2014-01-16 22:31:52]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[Noam Samuel, 2014-01-17 00:10:42]

+finnur@chromium.org for file_browser_private.idl

[Noam Samuel, 2014-01-17 00:11:08]

Er, actually +finnur for file_browser_private.idl

[Finnur, 2014-01-17 15:48:20]

OWNERS LGTM, with nit.

https://codereview.chromium.org/120533006/diff/640001/chrome/common/extension...
File chrome/common/extensions/api/file_browser_private.idl (right):

https://codereview.chromium.org/120533006/diff/640001/chrome/common/extension...
chrome/common/extensions/api/file_browser_private.idl:11: enum VolumeType {
drive, downloads, removable, archive, cloud_device };
Nit: Remove spaces added to beginning of line.

[Finnur, 2014-01-17 15:49:32]

On 2014/01/17 15:48:20, Finnur wrote:
> OWNERS LGTM, with nit.
> 
>
https://codereview.chromium.org/120533006/diff/640001/chrome/common/extension...
> File chrome/common/extensions/api/file_browser_private.idl (right):
> 
>
https://codereview.chromium.org/120533006/diff/640001/chrome/common/extension...
> chrome/common/extensions/api/file_browser_private.idl:11: enum VolumeType {
> drive, downloads, removable, archive, cloud_device };
> Nit: Remove spaces added to beginning of line.

Also, sorry it took so long to respond. This didn't make my review queue for
some reason. Must be some kind of filter fail, although I can't pinpoint what
happened...

[Noam Samuel, 2014-01-17 18:35:29]

https://codereview.chromium.org/120533006/diff/640001/chrome/common/extension...
File chrome/common/extensions/api/file_browser_private.idl (right):

https://codereview.chromium.org/120533006/diff/640001/chrome/common/extension...
chrome/common/extensions/api/file_browser_private.idl:11: enum VolumeType {
drive, downloads, removable, archive, cloud_device };
On 2014/01/17 15:48:21, Finnur wrote:
> Nit: Remove spaces added to beginning of line.

Done.

[commit-bot: I haz the power, 2014-01-17 18:35:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/noamsml@chromium.org/120533006/970001

[commit-bot: I haz the power, 2014-01-17 20:55:15]

Change committed as 245612