fileapi: Add CrosMountPointProvier::AddMountPoints()

Adding system mount points in the constructor was rather awkward.
Besides, adding "Downloads" in BrowserContext was also awkward.
These mount points should be added in a common place.

BUG=245587
TEST=Files.app works as before
R=kinuko@chromium.org, sky@chromium.org, tbarzic@chromium.org

Review URL: https://codereview.chromium.org/16082006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@204725 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 42 insertions, 13 deletions

diff --git a/webkit/browser/chromeos/fileapi/cros_mount_point_provider.cc b/webkit/browser/chromeos/fileapi/cros_mount_point_provider.cc
index fad2d58..cc59ac5 100644
--- a/webkit/browser/chromeos/fileapi/cros_mount_point_provider.cc
+++ b/webkit/browser/chromeos/fileapi/cros_mount_point_provider.cc
@@ -8,6 +8,7 @@
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
+#include "base/path_service.h"
 #include "base/stringprintf.h"
 #include "base/synchronization/lock.h"
 #include "base/utf_string_conversions.h"
@@ -59,8 +60,28 @@ CrosMountPointProvider::CrosMountPointProvider(
           new fileapi::IsolatedFileUtil())),
       mount_points_(mount_points),
       system_mount_points_(system_mount_points) {
-  // Add default system mount points.
-  // TODO(satorux): Move this to a more relevant place: crbug.com/245587
+}
+
+CrosMountPointProvider::~CrosMountPointProvider() {
+}
+
+void CrosMountPointProvider::AddSystemMountPoints() {
+  // RegisterFileSystem() is no-op if the mount point with the same name
+  // already exists, hence it's safe to call without checking if a mount
+  // point already exists or not.
+
+  // TODO(satorux): "Downloads" directory should probably be per-profile. For
+  // this to be per-profile, a unique directory path should be chosen per
+  // profile, and the mount point should be added to
+  // mount_points_. crbug.com/247236
+  base::FilePath home_path;
+  if (PathService::Get(base::DIR_HOME, &home_path)) {
+    system_mount_points_->RegisterFileSystem(
+        "Downloads",
+        fileapi::kFileSystemTypeNativeLocal,
+        home_path.AppendASCII("Downloads"));
+  }
+
   system_mount_points_->RegisterFileSystem(
       "archive",
       fileapi::kFileSystemTypeNativeLocal,
@@ -75,9 +96,6 @@ CrosMountPointProvider::CrosMountPointProvider(
       base::FilePath(FILE_PATH_LITERAL("/usr/share/oem")));
 }
 
-CrosMountPointProvider::~CrosMountPointProvider() {
-}
-
 bool CrosMountPointProvider::CanHandleType(fileapi::FileSystemType type) const {
   switch (type) {
     case fileapi::kFileSystemTypeExternal:
diff --git a/webkit/browser/chromeos/fileapi/cros_mount_point_provider.h b/webkit/browser/chromeos/fileapi/cros_mount_point_provider.h
index 11bc961..0df64b5 100644
--- a/webkit/browser/chromeos/fileapi/cros_mount_point_provider.h
+++ b/webkit/browser/chromeos/fileapi/cros_mount_point_provider.h
@@ -35,7 +35,7 @@ class FileAccessPermissions;
 // ExternalFileSystemMountPointProvider. This class is responsible for a
 // number of things, including:
 //
-// - Add default mount points
+// - Add system mount points
 // - Grant/revoke/check file access permissions
 // - Create FileSystemOperation per file system type
 // - Create FileStreamReader/Writer per file system type
@@ -78,6 +78,10 @@ class WEBKIT_STORAGE_EXPORT CrosMountPointProvider
       fileapi::ExternalMountPoints* system_mount_points);
   virtual ~CrosMountPointProvider();
 
+  // Adds system mount points, such as "archive", and "removable". This
+  // function is no-op if these mount points are already present.
+  void AddSystemMountPoints();
+
   // Returns true if CrosMountpointProvider can handle |url|, i.e. its
   // file system type matches with what this provider supports.
   // This could be called on any threads.
@@ -144,9 +148,8 @@ class WEBKIT_STORAGE_EXPORT CrosMountPointProvider
   scoped_ptr<FileAccessPermissions> file_access_permissions_;
   scoped_ptr<fileapi::AsyncFileUtilAdapter> local_file_util_;
 
-  // Mount points specific to the owning context.
-  //
-  // Add/Remove MountPoints will affect only these mount points.
+  // Mount points specific to the owning context (i.e. per-profile mount
+  // points).
   //
   // It is legal to have mount points with the same name as in
   // system_mount_points_. Also, mount point paths may overlap with mount point
diff --git a/webkit/browser/chromeos/fileapi/cros_mount_point_provider_unittest.cc b/webkit/browser/chromeos/fileapi/cros_mount_point_provider_unittest.cc
index a0c85a2..5de8a4b 100644
--- a/webkit/browser/chromeos/fileapi/cros_mount_point_provider_unittest.cc
+++ b/webkit/browser/chromeos/fileapi/cros_mount_point_provider_unittest.cc
@@ -7,6 +7,7 @@
 #include <set>
 
 #include "base/files/file_path.h"
+#include "base/path_service.h"
 #include "chromeos/dbus/cros_disks_client.h"
 #include "googleurl/src/url_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -41,11 +42,16 @@ TEST(CrosMountPointProviderTest, DefaultMountPoints) {
       storage_policy,
       mount_points.get(),
       fileapi::ExternalMountPoints::GetSystemInstance());
+  provider.AddSystemMountPoints();
   std::vector<base::FilePath> root_dirs = provider.GetRootDirectories();
   std::set<base::FilePath> root_dirs_set(root_dirs.begin(), root_dirs.end());
 
-  // By default there should be 3 mount points (in system mount points):
-  EXPECT_EQ(3u, root_dirs.size());
+  // By default there should be 4 mount points (in system mount points):
+  EXPECT_EQ(4u, root_dirs.size());
+  base::FilePath home_path;
+  ASSERT_TRUE(PathService::Get(base::DIR_HOME, &home_path));
+
+  EXPECT_TRUE(root_dirs_set.count(home_path.AppendASCII("Downloads")));
   EXPECT_TRUE(root_dirs_set.count(
       chromeos::CrosDisksClient::GetRemovableDiskMountPoint()));
   EXPECT_TRUE(root_dirs_set.count(
diff --git a/webkit/browser/fileapi/file_system_context.cc b/webkit/browser/fileapi/file_system_context.cc
index 9c7e000..95d5dc5 100644
--- a/webkit/browser/fileapi/file_system_context.cc
+++ b/webkit/browser/fileapi/file_system_context.cc
@@ -92,11 +92,13 @@ FileSystemContext::FileSystemContext(
 #if defined(OS_CHROMEOS)
   // TODO(kinuko): Move this out of webkit/fileapi layer.
   DCHECK(external_mount_points);
-  external_provider_.reset(
+  chromeos::CrosMountPointProvider* cros_mount_provider =
       new chromeos::CrosMountPointProvider(
           special_storage_policy,
           external_mount_points,
-          ExternalMountPoints::GetSystemInstance()));
+          ExternalMountPoints::GetSystemInstance());
+  cros_mount_provider->AddSystemMountPoints();
+  external_provider_.reset(cros_mount_provider);
   RegisterMountPointProvider(external_provider_.get());
 #endif