Merge StorageService{OutOfProcess,Sandbox}

These features were originally separated with the intent to experiment
and independently compare unsandboxed vs sandboxed vs in-process
storage.

Since we're no interested in pursuing an unsandboxed service to fulfill
short-term needs, this drops the StorageServiceSandbox feature flag and
change StorageServiceOutOfProcess to imply sandboxing by default.

Bug: 1052045
Change-Id: I0f63f44adddeac5994b6fb3c3cd2e5a62ef9fe92
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2341401
Commit-Queue: Ken Rockot <rockot@google.com>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Alex Gough <ajgo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798943}

content/browser/service_sandbox_type.h

@@ -10,7 +10,6 @@
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/service_process_host.h"
 #include "content/public/common/content_client.h"
-#include "content/public/common/content_features.h"
 #include "sandbox/policy/sandbox_type.h"
 
 // This file maps service classes to sandbox types.  Services which
@@ -81,27 +80,4 @@ content::GetServiceSandboxType<video_capture::mojom::VideoCaptureService>() {
   return sandbox::policy::SandboxType::kVideoCapture;
 }
 
-// storage::mojom::StorageService
-// This service is being moved out of process and will eventually be a utility.
-#if !defined(OS_ANDROID)
-namespace storage {
-namespace mojom {
-class StorageService;
-}
-}  // namespace storage
-
-template <>
-inline sandbox::policy::SandboxType
-content::GetServiceSandboxType<storage::mojom::StorageService>() {
-  const bool should_sandbox =
-      base::FeatureList::IsEnabled(features::kStorageServiceSandbox);
-  const base::FilePath sandboxed_data_dir =
-      GetContentClient()->browser()->GetSandboxedStorageServiceDataDirectory();
-  const bool is_sandboxed = should_sandbox && !sandboxed_data_dir.empty();
-
-  return is_sandboxed ? sandbox::policy::SandboxType::kUtility
-                      : sandbox::policy::SandboxType::kNoSandbox;
-}
-#endif
-
 #endif  // CONTENT_BROWSER_SERVICE_SANDBOX_TYPE_H_

content/browser/storage_partition_impl.cc

@@ -174,44 +174,36 @@ mojo::Remote<storage::mojom::StorageService>& GetStorageServiceRemote() {
       GetStorageServiceRemoteStorage();
   if (!remote) {
 #if !defined(OS_ANDROID)
-    if (base::FeatureList::IsEnabled(features::kStorageServiceOutOfProcess)) {
-      const bool should_sandbox =
-          base::FeatureList::IsEnabled(features::kStorageServiceSandbox);
+    const bool oop_storage_enabled =
+        base::FeatureList::IsEnabled(features::kStorageServiceOutOfProcess);
+    const bool single_process_mode =
+        base::CommandLine::ForCurrentProcess()->HasSwitch(
+            switches::kSingleProcess);
+    if (oop_storage_enabled && !single_process_mode) {
       const base::FilePath sandboxed_data_dir =
           GetContentClient()
               ->browser()
               ->GetSandboxedStorageServiceDataDirectory();
-      const bool is_sandboxed = should_sandbox && !sandboxed_data_dir.empty();
-      if (should_sandbox && !is_sandboxed) {
-        DLOG(ERROR) << "Running unsandboxed Storage Service instance,because "
-                    << "the current ContentBrowserClient does not specify a "
-                    << "sandboxed data directory.";
-      }
+      DCHECK(!sandboxed_data_dir.empty())
+          << "Cannot run Storage Service out-of-process without a non-default "
+          << "implementation of "
+          << "ContentBrowserClient::GetSandboxedStorageServiceDataDirectory().";
       remote = ServiceProcessHost::Launch<storage::mojom::StorageService>(
           ServiceProcessHost::Options()
               .WithDisplayName("Storage Service")
               .Pass());
       remote.reset_on_disconnect();
 
-      if (is_sandboxed) {
-        // In sandboxed mode, provide the service with an API it can use to
-        // access filesystem contents *only* within the embedder's specified
-        // data directory.
-        const base::FilePath data_dir =
-            GetContentClient()
-                ->browser()
-                ->GetSandboxedStorageServiceDataDirectory();
-        DCHECK(!data_dir.empty())
-            << "Storage Service sandboxing requires a root data directory.";
-        mojo::PendingRemote<storage::mojom::Directory> directory;
-        base::ThreadPool::CreateSequencedTaskRunner(
-            {base::MayBlock(), base::TaskPriority::USER_VISIBLE})
-            ->PostTask(
-                FROM_HERE,
-                base::BindOnce(&BindStorageServiceFilesystemImpl, data_dir,
-                               directory.InitWithNewPipeAndPassReceiver()));
-        remote->SetDataDirectory(data_dir, std::move(directory));
-      }
+      // Provide the service with an API it can use to access filesystem
+      // contents *only* within the embedder's specified data directory.
+      mojo::PendingRemote<storage::mojom::Directory> directory;
+      base::ThreadPool::CreateSequencedTaskRunner(
+          {base::MayBlock(), base::TaskPriority::USER_VISIBLE})
+          ->PostTask(FROM_HERE,
+                     base::BindOnce(
+                         &BindStorageServiceFilesystemImpl, sandboxed_data_dir,
+                         directory.InitWithNewPipeAndPassReceiver()));
+      remote->SetDataDirectory(sandboxed_data_dir, std::move(directory));
     } else
 #endif  // !defined(OS_ANDROID)
     {

content/browser/storage_service_sandbox_browsertest.cc

@@ -29,9 +29,7 @@ class StorageServiceSandboxBrowserTest : public ContentBrowserTest {
   StorageServiceSandboxBrowserTest() {
     // These tests only make sense when the service is running out-of-process
     // with sandboxing enabled.
-    feature_list_.InitWithFeatures({features::kStorageServiceOutOfProcess,
-                                    features::kStorageServiceSandbox},
-                                   {});
+    feature_list_.InitWithFeatures({features::kStorageServiceOutOfProcess}, {});
   }
 
   DOMStorageContextWrapper* dom_storage() {

content/public/common/content_features.cc

@@ -651,11 +651,6 @@ const base::Feature kStoragePressureUI {
 const base::Feature kStorageServiceOutOfProcess{
     "StorageServiceOutOfProcess", base::FEATURE_DISABLED_BY_DEFAULT};
 
-// Enables the Storage Service sandbox whenever out-of-process Storage Service
-// is enabled.
-const base::Feature kStorageServiceSandbox{"StorageServiceSandbox",
-                                           base::FEATURE_DISABLED_BY_DEFAULT};
-
 // Controls whether site isolation should use origins instead of scheme and
 // eTLD+1.
 const base::Feature kStrictOriginIsolation{"StrictOriginIsolation",

content/public/common/content_features.h

@@ -140,7 +140,6 @@ CONTENT_EXPORT extern const base::Feature kSmsReceiver;
 CONTENT_EXPORT extern const base::Feature kSpareRendererForSitePerProcess;
 CONTENT_EXPORT extern const base::Feature kStoragePressureUI;
 CONTENT_EXPORT extern const base::Feature kStorageServiceOutOfProcess;
-CONTENT_EXPORT extern const base::Feature kStorageServiceSandbox;
 CONTENT_EXPORT extern const base::Feature kStrictOriginIsolation;
 CONTENT_EXPORT extern const base::Feature kSubresourceWebBundles;
 CONTENT_EXPORT extern const base::Feature kSyntheticPointerActions;

extensions/shell/browser/shell_content_browser_client.cc

@@ -328,6 +328,11 @@ void ShellContentBrowserClient::OverrideURLLoaderFactoryParams(
       browser_context, origin, is_for_isolated_world, factory_params);
 }
 
+base::FilePath
+ShellContentBrowserClient::GetSandboxedStorageServiceDataDirectory() {
+  return GetBrowserContext()->GetPath();
+}
+
 std::string ShellContentBrowserClient::GetUserAgent() {
   // Must contain a user agent string for version sniffing. For example,
   // pluginless WebRTC Hangouts checks the Chrome version number.

extensions/shell/browser/shell_content_browser_client.h

@@ -108,6 +108,7 @@ class ShellContentBrowserClient : public content::ContentBrowserClient {
       const url::Origin& origin,
       bool is_for_isolated_world,
       network::mojom::URLLoaderFactoryParams* factory_params) override;
+  base::FilePath GetSandboxedStorageServiceDataDirectory() override;
   std::string GetUserAgent() override;
 
  protected: