Grant Files.app access for mount points added after its launch.

Grant Files.app access for mount points added after its launch.

Previous implementation of GrantFullAccessToExtension() granted permissions
only for paths that are mounted at that moment, and required hack for
one dynamically mounted path, namely, Google Drive directory.

It's not what is meant for GrantFullAccessToExtension(). Rather, it
should just simply allow access for the extension any time.
In multi-profile settings, more directories, i.e., Downloads/Drive
directories from other profiles are going to be dynamically added.

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

[hashimoto, 2014-01-28 10:55:13]

lgtm

https://codereview.chromium.org/148233008/diff/20001/chrome/browser/chromeos/...
File chrome/browser/chromeos/extensions/file_manager/private_api_file_system.cc
(right):

https://codereview.chromium.org/148233008/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_manager/private_api_file_system.cc:53:
const std::string& extension_id,
nit: This is not used.

https://codereview.chromium.org/148233008/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_manager/private_api_file_system.cc:61:
if (!backend)
nit: This code seems unnecessary because |backend| is not used and the same
check is performed by
FileBrowserPrivateRequestFileSystemFunction::SetupFileSystemAccessPermissions().

https://codereview.chromium.org/148233008/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_manager/private_api_file_system.cc:65:
// Grant R/W permissions to drive 'folder'. File API layer still
nit: What does 'still' mean?

https://codereview.chromium.org/148233008/diff/20001/chrome/browser/chromeos/...
File chrome/browser/chromeos/fileapi/file_access_permissions.cc (right):

https://codereview.chromium.org/148233008/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/fileapi/file_access_permissions.cc:29: const
std::string& extension_id, const base::FilePath& path) {
nit: How about having "if" or DCHECK to verify path is not empty?

[kinaba, 2014-01-28 23:59:49]

https://codereview.chromium.org/148233008/diff/20001/chrome/browser/chromeos/...
File chrome/browser/chromeos/extensions/file_manager/private_api_file_system.cc
(right):

https://codereview.chromium.org/148233008/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_manager/private_api_file_system.cc:53:
const std::string& extension_id,
On 2014/01/28 10:55:13, hashimoto wrote:
> nit: This is not used.

Done.

https://codereview.chromium.org/148233008/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_manager/private_api_file_system.cc:61:
if (!backend)
On 2014/01/28 10:55:13, hashimoto wrote:
> nit: This code seems unnecessary because |backend| is not used and the same
> check is performed by
>
FileBrowserPrivateRequestFileSystemFunction::SetupFileSystemAccessPermissions().

Done.

https://codereview.chromium.org/148233008/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_manager/private_api_file_system.cc:65:
// Grant R/W permissions to drive 'folder'. File API layer still
On 2014/01/28 10:55:13, hashimoto wrote:
> nit: What does 'still' mean?

Just my guess but it's even though we grant permissions by
backend->GrantFileAccess but still needs something elsewhere...

https://codereview.chromium.org/148233008/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_manager/private_api_file_system.cc:68:
render_view_host->GetProcess()->GetID(), mount_point);
Since now this is the only line executed in this function, I simply inlined it
to the caller.

https://codereview.chromium.org/148233008/diff/20001/chrome/browser/chromeos/...
File chrome/browser/chromeos/fileapi/file_access_permissions.cc (right):

https://codereview.chromium.org/148233008/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/fileapi/file_access_permissions.cc:29: const
std::string& extension_id, const base::FilePath& path) {
On 2014/01/28 10:55:13, hashimoto wrote:
> nit: How about having "if" or DCHECK to verify path is not empty?

Done.

[kinaba, 2014-01-29 00:20:22]

+satorux from c/b/cros OWNERs.

(Or maybe shall I add myself to c/b/cros/fileapi/OWNER?)

[satorux1, 2014-01-29 00:44:14]

LGTM. Please add kinaba@ to  c/b/cros/fileapi/OWNERS

[commit-bot: I haz the power, 2014-01-29 00:51:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kinaba@chromium.org/148233008/120001

[commit-bot: I haz the power, 2014-01-29 15:46:11]

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

[commit-bot: I haz the power, 2014-01-29 23:04:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kinaba@chromium.org/148233008/120001

[commit-bot: I haz the power, 2014-01-30 06:25:15]

Change committed as 247852