Add a whitelist for chrome.fileSystem.requestFileSystem.

Add a whitelist for chrome.fileSystem.requestFileSystem.

Instead of allowing all component apps, this CL whitelists only those apps
which need access to this API.

TEST=All current tests pass.
BUG=323422
Committed: https://crrev.com/89cdf95aef2c71cc6fb6cf463d0f6c93eab52cbd
Cr-Commit-Position: refs/heads/master@{#321942}

[mtomasz, 2015-03-23 05:55:08]

mtomasz@chromium.org changed reviewers:
+ benwells@chromium.org

[mtomasz, 2015-03-23 05:55:32]

@benwells: PTAL. Thanks.

[benwells, 2015-03-23 06:34:32]

https://codereview.chromium.org/1030533002/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/api/file_system/file_system_api.cc (right):

https://codereview.chromium.org/1030533002/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/file_system/file_system_api.cc:99: const char*
const kRequestFileSystemComponentWhitelist[] = {
Can we use the hashed ids? See
https://code.google.com/p/chromium/codesearch#chromium/src/extensions/browser...
for an example.

[mtomasz, 2015-03-23 06:39:10]

https://codereview.chromium.org/1030533002/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/api/file_system/file_system_api.cc (right):

https://codereview.chromium.org/1030533002/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/file_system/file_system_api.cc:99: const char*
const kRequestFileSystemComponentWhitelist[] = {
On 2015/03/23 06:34:31, benwells wrote:
> Can we use the hashed ids? See
>
https://code.google.com/p/chromium/codesearch#chromium/src/extensions/browser...
> for an example.

Would they be better? We already have constants with ids. Hashing wouldn't hide
these apps, as everyone can see which apps have the access via cs.chromium.org,
or via git grep for requestFileSystem manifest entry.

[benwells, 2015-03-23 09:30:09]

https://codereview.chromium.org/1030533002/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/api/file_system/file_system_api.cc (right):

https://codereview.chromium.org/1030533002/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/file_system/file_system_api.cc:99: const char*
const kRequestFileSystemComponentWhitelist[] = {
On 2015/03/23 06:39:10, mtomasz wrote:
> On 2015/03/23 06:34:31, benwells wrote:
> > Can we use the hashed ids? See
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/extensions/browser...
> > for an example.
> 
> Would they be better? We already have constants with ids. Hashing wouldn't
hide
> these apps, as everyone can see which apps have the access via
http://cs.chromium.org,
> or via git grep for requestFileSystem manifest entry.

We normally use the hashed IDs everywhere for consistency, so we don't need to
think about whether ids are public or not. Are there hashed versions of these
IDs about?

[benwells, 2015-03-23 09:31:01]

https://codereview.chromium.org/1030533002/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/api/file_system/file_system_api.cc (right):

https://codereview.chromium.org/1030533002/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/file_system/file_system_api.cc:99: const char*
const kRequestFileSystemComponentWhitelist[] = {
On 2015/03/23 09:30:09, benwells wrote:
> On 2015/03/23 06:39:10, mtomasz wrote:
> > On 2015/03/23 06:34:31, benwells wrote:
> > > Can we use the hashed ids? See
> > >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/extensions/browser...
> > > for an example.
> > 
> > Would they be better? We already have constants with ids. Hashing wouldn't
> hide
> > these apps, as everyone can see which apps have the access via
> http://cs.chromium.org,
> > or via git grep for requestFileSystem manifest entry.
> 
> We normally use the hashed IDs everywhere for consistency, so we don't need to
> think about whether ids are public or not. Are there hashed versions of these
> IDs about?

Are these all component apps? I guess in that case it is OK.

[mtomasz, 2015-03-23 09:38:51]

https://codereview.chromium.org/1030533002/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/api/file_system/file_system_api.cc (right):

https://codereview.chromium.org/1030533002/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/file_system/file_system_api.cc:99: const char*
const kRequestFileSystemComponentWhitelist[] = {
On 2015/03/23 09:31:01, benwells wrote:
> On 2015/03/23 09:30:09, benwells wrote:
> > On 2015/03/23 06:39:10, mtomasz wrote:
> > > On 2015/03/23 06:34:31, benwells wrote:
> > > > Can we use the hashed ids? See
> > > >
> > >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/extensions/browser...
> > > > for an example.
> > > 
> > > Would they be better? We already have constants with ids. Hashing wouldn't
> > hide
> > > these apps, as everyone can see which apps have the access via
> > http://cs.chromium.org,
> > > or via git grep for requestFileSystem manifest entry.
> > 
> > We normally use the hashed IDs everywhere for consistency, so we don't need
to
> > think about whether ids are public or not. Are there hashed versions of
these
> > IDs about?
> 
> Are these all component apps? I guess in that case it is OK.

Yes, they are all component apps. We don't have hashes for them, but we could
calculate. However, we use kFileManagerAppId all over the Chromium code base, so
keeping it also here is IMHO easier to maintain (grep friendly, less constants).

If you're fine with it, let's keep it as it is for now. I'm happy to convert it
to hashes once we have any external apps added to the list.

[benwells, 2015-03-24 03:04:12]

lgtm

[mtomasz, 2015-03-24 05:01:37]

The CQ bit was checked by mtomasz@chromium.org

[mtomasz, 2015-03-24 05:01:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
benwells@chromium.org
Link to the patchset: https://codereview.chromium.org/1030533002/#ps40001
(title: "Fixed ids.")

[commit-bot: I haz the power, 2015-03-24 05:01:47]

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

[commit-bot: I haz the power, 2015-03-24 06:17:24]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-03-24 06:18:23]

Patchset 3 (id:??) landed as
https://crrev.com/89cdf95aef2c71cc6fb6cf463d0f6c93eab52cbd
Cr-Commit-Position: refs/heads/master@{#321942}