Make MountPointProvider pluggable from outside webkit/fileapi

Make MountPointProvider pluggable from outside webkit/fileapi

Currently we have some layering leakages under webkit/ because
we have several FS modules (e.g. media modules) that
should not be known by WebKit/FileAPI (or content) layer.

Also some module implementors are forced to add extra glue interfaces
just to across layering boundaries from webkit.

This patch changes to:
- Add CanHandleType method to each MountPointProvider
- Make FileSystemContext's ctor take additional MountPointProviders and
  MountPoints, so that FS module implementors can implement their own
  provider outside webkit/ and plug it into FileAPI layer.

This is the 1st cut of a couple of planned changes.

Brief change plan (internal): https://docs.google.com/a/google.com/document/d/1XEtX0OO_RIA_c0KuKvd9yqK5l3XtsTulwXs3c-mAQ1o/view

BUG=175936
TEST=content_unittests:.*File.*
TEST=content_unittests:.*MountPointProvider.*

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

[kinuko, 2013-04-17 09:43:23]

Can you take the first look?  (I'll write-up an internal change plan / design
doc shortly)

[kinuko, 2013-04-17 10:49:49]

On 2013/04/17 09:43:23, kinuko wrote:
> Can you take the first look?  (I'll write-up an internal change plan / design
> doc shortly)

A brief plan:
https://docs.google.com/a/google.com/document/d/1XEtX0OO_RIA_c0KuKvd9yqK5l3Xt...

[tzik, 2013-04-17 12:06:03]

https://codereview.chromium.org/14096022/diff/1/webkit/chromeos/fileapi/cros_...
File webkit/chromeos/fileapi/cros_mount_point_provider_unittest.cc (right):

https://codereview.chromium.org/14096022/diff/1/webkit/chromeos/fileapi/cros_...
webkit/chromeos/fileapi/cros_mount_point_provider_unittest.cc:361:
TEST(CrosMountPointProvider, CrosMountPonitProviderKeepsMountPointsAlive) {
s/Ponit/Point/

https://codereview.chromium.org/14096022/diff/1/webkit/fileapi/file_system_co...
File webkit/fileapi/file_system_context.cc (right):

https://codereview.chromium.org/14096022/diff/1/webkit/fileapi/file_system_co...
webkit/fileapi/file_system_context.cc:155: return
provider_map_.find(type)->second;
could you reuse the result of find(type)?

https://codereview.chromium.org/14096022/diff/1/webkit/fileapi/file_system_co...
webkit/fileapi/file_system_context.cc:391: static const FileSystemType
mount_types[] = {
could you move this to the top of the file?

https://codereview.chromium.org/14096022/diff/1/webkit/fileapi/file_system_co...
webkit/fileapi/file_system_context.cc:400:
provider_map_.insert(std::make_pair(mount_types[j], provider));
could you add DCHECK the .second of the result?

https://codereview.chromium.org/14096022/diff/1/webkit/fileapi/file_system_co...
webkit/fileapi/file_system_context.cc:403: for (int t =
kFileSystemInternalTypeEnumStart;
kFileSystemInternalTypeEnumStart + 1?

https://codereview.chromium.org/14096022/diff/1/webkit/fileapi/file_system_co...
webkit/fileapi/file_system_context.cc:404: t < kFileSystemInternalTypeEnumEnd;
++t) {
indent?

[kinuko, 2013-04-17 14:27:29]

Updated the patch. I also reverted some CrOS-specific changes (probably we'd
better do that in a separate patch).  Hopefully this change should look simpler.
 PTAL thanks!

https://codereview.chromium.org/14096022/diff/1/webkit/chromeos/fileapi/cros_...
File webkit/chromeos/fileapi/cros_mount_point_provider_unittest.cc (right):

https://codereview.chromium.org/14096022/diff/1/webkit/chromeos/fileapi/cros_...
webkit/chromeos/fileapi/cros_mount_point_provider_unittest.cc:361:
TEST(CrosMountPointProvider, CrosMountPonitProviderKeepsMountPointsAlive) {
On 2013/04/17 12:06:04, tzik wrote:
> s/Ponit/Point/

Removed these CrOS changes from this patch.

https://codereview.chromium.org/14096022/diff/1/webkit/fileapi/file_system_co...
File webkit/fileapi/file_system_context.cc (right):

https://codereview.chromium.org/14096022/diff/1/webkit/fileapi/file_system_co...
webkit/fileapi/file_system_context.cc:155: return
provider_map_.find(type)->second;
On 2013/04/17 12:06:04, tzik wrote:
> could you reuse the result of find(type)?

Done.

https://codereview.chromium.org/14096022/diff/1/webkit/fileapi/file_system_co...
webkit/fileapi/file_system_context.cc:391: static const FileSystemType
mount_types[] = {
On 2013/04/17 12:06:04, tzik wrote:
> could you move this to the top of the file?

It's only referred in this method-- I just dropped 'static' here.

https://codereview.chromium.org/14096022/diff/1/webkit/fileapi/file_system_co...
webkit/fileapi/file_system_context.cc:400:
provider_map_.insert(std::make_pair(mount_types[j], provider));
On 2013/04/17 12:06:04, tzik wrote:
> could you add DCHECK the .second of the result?

Done.

https://codereview.chromium.org/14096022/diff/1/webkit/fileapi/file_system_co...
webkit/fileapi/file_system_context.cc:403: for (int t =
kFileSystemInternalTypeEnumStart;
On 2013/04/17 12:06:04, tzik wrote:
> kFileSystemInternalTypeEnumStart + 1?

Done.

https://codereview.chromium.org/14096022/diff/1/webkit/fileapi/file_system_co...
webkit/fileapi/file_system_context.cc:404: t < kFileSystemInternalTypeEnumEnd;
++t) {
On 2013/04/17 12:06:04, tzik wrote:
> indent?

Done.

https://codereview.chromium.org/14096022/diff/25002/webkit/fileapi/file_syste...
File webkit/fileapi/file_system_context.h (right):

https://codereview.chromium.org/14096022/diff/25002/webkit/fileapi/file_syste...
webkit/fileapi/file_system_context.h:297: 
(will remove this empty line)

https://codereview.chromium.org/14096022/diff/25002/webkit/fileapi/file_syste...
File webkit/fileapi/file_system_context_unittest.cc (right):

https://codereview.chromium.org/14096022/diff/25002/webkit/fileapi/file_syste...
webkit/fileapi/file_system_context_unittest.cc:66: make_scoped_refptr(new
quota::MockSpecialStoragePolicy()),
Will revert this change back to storage_policy_

[kinuko, 2013-04-17 14:53:17]

Avi: can you take a look at 2-line changes in content/content_tests.gypi? 
Thanks!

[Avi (use Gerrit), 2013-04-17 15:01:57]

lgtm stampity stamp

[tzik, 2013-04-18 00:39:00]

lgtm

[kinuko, 2013-04-18 00:45:49]

Kinaba-san and/or Satoru: can you take a look at the changes in:
chrome/browser/chromeos/drive/drive_file_system_util_unittest.cc
webkit/chromeos/fileapi/cros_mount_point_provider.*

Not in this patch, but we plan to make all non-regular mount point providers
(including CrOS one) pluggable from chrome/ layer, which will probably allow you
to completely get rid of webkit/chromeos/fileapi glue code.

[kinaba, 2013-04-18 01:08:16]

lgtm

[kinuko, 2013-04-18 01:46:43]

Committed patchset #3 manually as r194747 (presubmit successful).