Reland: Add ExtensionsApiClient interface, use it in the storage API SettingsFrontend

app_shell needs the chrome.storage.local extension API so the core storage implementation is moving to //extensions/browser/api. However, the "managed storage" API implementation needs to remain in Chrome.

* Introduce ExtensionsApiClient in extensions/browser/api
* Introduce ChromeExtensionsApiClient in chrome/browser/extensions/api
* Delegate out the storage ManagedValueStoreCache creation to Chrome
* Move settings_namespace.h/cc to e/b/api/storage so we can use an enum in the client interface definition

Most APIs will probably need to make only one or two client interface calls, so we're starting with a single ExtensionsApiClient interface. It we find individual APIs tend to need a lot of methods we can split it into one client interface per API, but that seems like overkill right now.

Reland of https://codereview.chromium.org/180213008/ with fixes for Android GCMProfileServiceSingleProfileTest.Unregister

BUG=348058
TEST=unit_tests *Settings* and browser_tests ExtensionSettings*
TBR=rdevlin.cronin@chromium.org, rockot@chromium.org, yoz@chromium.org

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

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

apps/shell/browser/shell_extensions_browser_client.cc

@@ -12,6 +12,7 @@
 #include "chrome/common/extensions/api/generated_api.h"
 #include "components/user_prefs/pref_registry_syncable.h"
 #include "components/user_prefs/user_prefs.h"
+#include "extensions/browser/api/extensions_api_client.h"
 #include "extensions/browser/app_sorting.h"
 #include "extensions/browser/extension_prefs.h"
 
@@ -27,10 +28,9 @@ void RegisterPrefs(user_prefs::PrefRegistrySyncable* registry) {
 
 }  // namespace
 
-
 ShellExtensionsBrowserClient::ShellExtensionsBrowserClient(
     BrowserContext* context)
-    : browser_context_(context) {
+    : browser_context_(context), api_client_(new ExtensionsAPIClient) {
   // Set up the preferences service.
   base::PrefServiceFactory factory;
   factory.set_user_prefs(new TestingPrefStore);

apps/shell/browser/shell_extensions_browser_client.h

@@ -12,6 +12,8 @@ class PrefService;
 
 namespace extensions {
 
+class ExtensionsAPIClient;
+
 // An ExtensionsBrowserClient that supports a single content::BrowserContent
 // with no related incognito context.
 class ShellExtensionsBrowserClient : public ExtensionsBrowserClient {
@@ -65,6 +67,9 @@ class ShellExtensionsBrowserClient : public ExtensionsBrowserClient {
   // The single BrowserContext for app_shell. Not owned.
   content::BrowserContext* browser_context_;
 
+  // Support for extension APIs.
+  scoped_ptr<ExtensionsAPIClient> api_client_;
+
   // The PrefService for |browser_context_|.
   scoped_ptr<PrefService> prefs_;
 

chrome/browser/extensions/api/chrome_extensions_api_client.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/extensions/api/chrome_extensions_api_client.h"
+
+#if defined(ENABLE_CONFIGURATION_POLICY)
+#include "chrome/browser/extensions/api/storage/managed_value_store_cache.h"
+#endif
+
+namespace extensions {
+
+ChromeExtensionsAPIClient::ChromeExtensionsAPIClient() {}
+
+ChromeExtensionsAPIClient::~ChromeExtensionsAPIClient() {}
+
+void ChromeExtensionsAPIClient::AddAdditionalValueStoreCaches(
+    content::BrowserContext* context,
+    const scoped_refptr<SettingsStorageFactory>& factory,
+    const scoped_refptr<ObserverListThreadSafe<SettingsObserver> >& observers,
+    std::map<settings_namespace::Namespace, ValueStoreCache*>* caches) {
+#if defined(ENABLE_CONFIGURATION_POLICY)
+  // Add support for chrome.storage.managed.
+  (*caches)[settings_namespace::MANAGED] =
+      new ManagedValueStoreCache(context, factory, observers);
+#endif
+}
+
+}  // namespace extensions

chrome/browser/extensions/api/chrome_extensions_api_client.h

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_EXTENSIONS_API_CHROME_EXTENSIONS_API_CLIENT_H_
+#define CHROME_BROWSER_EXTENSIONS_API_CHROME_EXTENSIONS_API_CLIENT_H_
+
+#include "base/compiler_specific.h"
+#include "extensions/browser/api/extensions_api_client.h"
+
+namespace extensions {
+
+// Extra support for extensions APIs in Chrome.
+class ChromeExtensionsAPIClient : public ExtensionsAPIClient {
+ public:
+  ChromeExtensionsAPIClient();
+  virtual ~ChromeExtensionsAPIClient();
+
+  // ExtensionsApiClient implementation.
+  virtual void AddAdditionalValueStoreCaches(
+      content::BrowserContext* context,
+      const scoped_refptr<SettingsStorageFactory>& factory,
+      const scoped_refptr<ObserverListThreadSafe<SettingsObserver> >& observers,
+      std::map<settings_namespace::Namespace, ValueStoreCache*>* caches)
+      OVERRIDE;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ChromeExtensionsAPIClient);
+};
+
+}  // namespace extensions
+
+#endif  // CHROME_BROWSER_EXTENSIONS_API_CHROME_EXTENSIONS_API_CLIENT_H_

chrome/browser/extensions/api/storage/policy_value_store.cc

@@ -6,10 +6,10 @@
 
 #include "base/logging.h"
 #include "base/values.h"
-#include "chrome/browser/extensions/api/storage/settings_namespace.h"
 #include "components/policy/core/common/policy_map.h"
 #include "components/policy/core/common/policy_types.h"
 #include "content/public/browser/browser_thread.h"
+#include "extensions/browser/api/storage/settings_namespace.h"
 #include "extensions/browser/value_store/value_store_change.h"
 #include "extensions/browser/value_store/value_store_util.h"
 

chrome/browser/extensions/api/storage/settings_apitest.cc

@@ -8,7 +8,6 @@
 #include "base/run_loop.h"
 #include "base/values.h"
 #include "chrome/browser/extensions/api/storage/settings_frontend.h"
-#include "chrome/browser/extensions/api/storage/settings_namespace.h"
 #include "chrome/browser/extensions/api/storage/settings_sync_util.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/extensions/extension_system_factory.h"
@@ -16,6 +15,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "extensions/browser/api/storage/settings_namespace.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/common/value_builder.h"
 #include "sync/api/fake_sync_change_processor.h"

chrome/browser/extensions/api/storage/settings_backend.h

@@ -21,6 +21,7 @@ class ValueStore;
 namespace extensions {
 class SettingsStorageFactory;
 
+// Performs work for the storage API on the FILE thread.
 class SettingsBackend {
  public:
   SettingsBackend(const scoped_refptr<SettingsStorageFactory>& storage_factory,

chrome/browser/extensions/api/storage/settings_frontend.cc

@@ -13,18 +13,14 @@
 #include "base/lazy_instance.h"
 #include "chrome/browser/extensions/api/storage/leveldb_settings_storage_factory.h"
 #include "chrome/browser/extensions/api/storage/sync_or_local_value_store_cache.h"
-#include "chrome/browser/extensions/extension_service.h"
 #include "chrome/common/extensions/api/storage.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
+#include "extensions/browser/api/extensions_api_client.h"
 #include "extensions/browser/event_router.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 
-#if defined(ENABLE_CONFIGURATION_POLICY)
-#include "chrome/browser/extensions/api/storage/managed_value_store_cache.h"
-#endif
-
 using content::BrowserContext;
 using content::BrowserThread;
 
@@ -138,13 +134,10 @@ void SettingsFrontend::Init(
           observers_,
           browser_context_path);
 
-#if defined(ENABLE_CONFIGURATION_POLICY)
-  caches_[settings_namespace::MANAGED] =
-      new ManagedValueStoreCache(
-          browser_context_,
-          factory,
-          observers_);
-#endif
+  // Add any additional caches the embedder supports (for example, a cache
+  // for chrome.storage.managed).
+  ExtensionsAPIClient::Get()->AddAdditionalValueStoreCaches(
+      browser_context_, factory, observers_, &caches_);
 }
 
 SettingsFrontend::~SettingsFrontend() {

chrome/browser/extensions/api/storage/settings_frontend.h

@@ -10,11 +10,11 @@
 
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
-#include "chrome/browser/extensions/api/storage/settings_namespace.h"
 #include "chrome/browser/extensions/api/storage/settings_observer.h"
 #include "chrome/browser/extensions/api/storage/settings_storage_factory.h"
 #include "chrome/browser/extensions/api/storage/settings_storage_quota_enforcer.h"
 #include "chrome/browser/extensions/api/storage/value_store_cache.h"
+#include "extensions/browser/api/storage/settings_namespace.h"
 #include "extensions/browser/browser_context_keyed_api_factory.h"
 #include "sync/api/syncable_service.h"
 

chrome/browser/extensions/api/storage/settings_frontend_unittest.cc

@@ -11,9 +11,9 @@
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/api/storage/leveldb_settings_storage_factory.h"
 #include "chrome/browser/extensions/api/storage/settings_frontend.h"
-#include "chrome/browser/extensions/api/storage/settings_namespace.h"
 #include "chrome/browser/extensions/api/storage/settings_test_util.h"
 #include "content/public/test/test_browser_thread.h"
+#include "extensions/browser/api/storage/settings_namespace.h"
 #include "extensions/browser/value_store/value_store.h"
 #include "testing/gtest/include/gtest/gtest.h"
 

chrome/browser/extensions/api/storage/settings_observer.h

@@ -6,7 +6,7 @@
 #define CHROME_BROWSER_EXTENSIONS_API_STORAGE_SETTINGS_OBSERVER_H_
 
 #include "base/observer_list_threadsafe.h"
-#include "chrome/browser/extensions/api/storage/settings_namespace.h"
+#include "extensions/browser/api/storage/settings_namespace.h"
 
 namespace extensions {
 

chrome/browser/extensions/api/storage/settings_sync_processor.cc

@@ -2,10 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/extensions/api/storage/settings_namespace.h"
 #include "chrome/browser/extensions/api/storage/settings_sync_processor.h"
 #include "chrome/browser/extensions/api/storage/settings_sync_util.h"
 #include "content/public/browser/browser_thread.h"
+#include "extensions/browser/api/storage/settings_namespace.h"
 #include "sync/api/sync_change_processor.h"
 #include "sync/api/sync_data.h"
 #include "sync/protocol/extension_setting_specifics.pb.h"

chrome/browser/extensions/api/storage/settings_test_util.h

@@ -12,12 +12,12 @@
 #include "base/memory/linked_ptr.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
-#include "chrome/browser/extensions/api/storage/settings_namespace.h"
 #include "chrome/browser/extensions/api/storage/settings_storage_factory.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/test_extension_service.h"
 #include "chrome/browser/extensions/test_extension_system.h"
 #include "chrome/test/base/testing_profile.h"
+#include "extensions/browser/api/storage/settings_namespace.h"
 #include "extensions/browser/event_router.h"
 #include "extensions/common/extension.h"
 

chrome/browser/extensions/api/storage/storage_api.h

@@ -7,9 +7,9 @@
 
 #include "base/compiler_specific.h"
 #include "base/memory/ref_counted.h"
-#include "chrome/browser/extensions/api/storage/settings_namespace.h"
 #include "chrome/browser/extensions/api/storage/settings_observer.h"
 #include "chrome/browser/extensions/chrome_extension_function.h"
+#include "extensions/browser/api/storage/settings_namespace.h"
 #include "extensions/browser/value_store/value_store.h"
 
 namespace extensions {

chrome/browser/extensions/api/storage/syncable_settings_storage.cc

@@ -5,10 +5,10 @@
 #include "chrome/browser/extensions/api/storage/syncable_settings_storage.h"
 
 #include "base/strings/stringprintf.h"
-#include "chrome/browser/extensions/api/storage/settings_namespace.h"
 #include "chrome/browser/extensions/api/storage/settings_sync_processor.h"
 #include "chrome/browser/extensions/api/storage/settings_sync_util.h"
 #include "content/public/browser/browser_thread.h"
+#include "extensions/browser/api/storage/settings_namespace.h"
 #include "sync/api/sync_data.h"
 #include "sync/protocol/extension_setting_specifics.pb.h"
 

chrome/browser/extensions/chrome_extensions_browser_client.cc

@@ -9,6 +9,7 @@
 #include "chrome/browser/app_mode/app_mode_utils.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/activity_log/activity_log.h"
+#include "chrome/browser/extensions/api/chrome_extensions_api_client.h"
 #include "chrome/browser/extensions/api/preference/chrome_direct_setting.h"
 #include "chrome/browser/extensions/api/preference/preference_api.h"
 #include "chrome/browser/extensions/api/runtime/runtime_api.h"
@@ -39,7 +40,8 @@
 
 namespace extensions {
 
-ChromeExtensionsBrowserClient::ChromeExtensionsBrowserClient() {
+ChromeExtensionsBrowserClient::ChromeExtensionsBrowserClient()
+    : api_client_(new ChromeExtensionsAPIClient) {
   // Only set if it hasn't already been set (e.g. by a test).
   if (GetCurrentChannel() == GetDefaultChannel())
     SetCurrentChannel(chrome::VersionInfo::GetChannel());

chrome/browser/extensions/chrome_extensions_browser_client.h

@@ -20,6 +20,8 @@ class BrowserContext;
 
 namespace extensions {
 
+class ChromeExtensionsAPIClient;
+
 // Implementation of extensions::BrowserClient for Chrome, which includes
 // knowledge of Profiles, BrowserContexts and incognito.
 //
@@ -78,6 +80,9 @@ class ChromeExtensionsBrowserClient : public ExtensionsBrowserClient {
   // Observer for Chrome-specific notifications.
   ChromeNotificationObserver notification_observer_;
 
+  // Client for API implementations.
+  scoped_ptr<ChromeExtensionsAPIClient> api_client_;
+
   DISALLOW_COPY_AND_ASSIGN(ChromeExtensionsBrowserClient);
 };
 

chrome/chrome_browser_extensions.gypi

@@ -115,6 +115,8 @@
         'browser/extensions/api/api_resource.cc',
         'browser/extensions/api/api_resource.h',
         'browser/extensions/api/api_resource_manager.h',
+        'browser/extensions/api/chrome_extensions_api_client.cc',
+        'browser/extensions/api/chrome_extensions_api_client.h',
         'browser/extensions/api/alarms/alarm_manager.cc',
         'browser/extensions/api/alarms/alarm_manager.h',
         'browser/extensions/api/alarms/alarms_api.cc',
@@ -509,8 +511,6 @@
         'browser/extensions/api/storage/settings_backend.h',
         'browser/extensions/api/storage/settings_frontend.cc',
         'browser/extensions/api/storage/settings_frontend.h',
-        'browser/extensions/api/storage/settings_namespace.cc',
-        'browser/extensions/api/storage/settings_namespace.h',
         'browser/extensions/api/storage/settings_observer.h',
         'browser/extensions/api/storage/settings_storage_factory.h',
         'browser/extensions/api/storage/settings_storage_quota_enforcer.cc',
@@ -974,6 +974,8 @@
             ['exclude', '^browser/extensions/.*_api\.cc$'],
             ['include', '^browser/extensions/api/activity_log_private/activity_log_private_api.cc'],
             ['include', '^browser/extensions/api/activity_log_private/activity_log_private_api.h'],
+            ['include', '^browser/extensions/api/chrome_extensions_api_client.cc'],
+            ['include', '^browser/extensions/api/chrome_extensions_api_client.h'],
             ['include', '^browser/extensions/api/content_settings/content_settings_api_constants.cc'],
             ['include', '^browser/extensions/api/content_settings/content_settings_helpers.cc'],
             ['include', '^browser/extensions/api/content_settings/content_settings_store.cc'],

extensions/browser/api/extensions_api_client.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "extensions/browser/api/extensions_api_client.h"
+
+namespace extensions {
+namespace {
+
+ExtensionsAPIClient* g_instance = NULL;
+
+}  // namespace
+
+ExtensionsAPIClient::ExtensionsAPIClient() { g_instance = this; }
+
+ExtensionsAPIClient::~ExtensionsAPIClient() { g_instance = NULL; }
+
+// static
+ExtensionsAPIClient* ExtensionsAPIClient::Get() { return g_instance; }
+
+void ExtensionsAPIClient::AddAdditionalValueStoreCaches(
+    content::BrowserContext* context,
+    const scoped_refptr<SettingsStorageFactory>& factory,
+    const scoped_refptr<ObserverListThreadSafe<SettingsObserver> >& observers,
+    std::map<settings_namespace::Namespace, ValueStoreCache*>* caches) {}
+
+}  // namespace extensions

extensions/browser/api/extensions_api_client.h

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef EXTENSIONS_BROWSER_API_EXTENSIONS_API_CLIENT_H_
+#define EXTENSIONS_BROWSER_API_EXTENSIONS_API_CLIENT_H_
+
+#include <map>
+
+#include "base/memory/ref_counted.h"
+#include "extensions/browser/api/storage/settings_namespace.h"
+
+template <class T>
+class ObserverListThreadSafe;
+
+namespace content {
+class BrowserContext;
+}
+
+namespace extensions {
+
+class SettingsObserver;
+class SettingsStorageFactory;
+class ValueStoreCache;
+
+// Allows the embedder of the extensions module to customize its support for
+// API features. The embedder must create a single instance in the browser
+// process. Provides a default implementation that does nothing.
+class ExtensionsAPIClient {
+ public:
+  // Construction sets the single instance.
+  ExtensionsAPIClient();
+
+  // Destruction clears the single instance.
+  virtual ~ExtensionsAPIClient();
+
+  // Returns the single instance of |this|.
+  static ExtensionsAPIClient* Get();
+
+  // Storage API support.
+
+  // Add any additional value store caches (e.g. for chrome.storage.managed)
+  // to |caches|. By default adds nothing.
+  virtual void AddAdditionalValueStoreCaches(
+      content::BrowserContext* context,
+      const scoped_refptr<SettingsStorageFactory>& factory,
+      const scoped_refptr<ObserverListThreadSafe<SettingsObserver> >& observers,
+      std::map<settings_namespace::Namespace, ValueStoreCache*>* caches);
+
+  // NOTE: If this interface gains too many methods (perhaps more than 20) it
+  // should be split into one interface per API.
+};
+
+}  // namespace extensions
+
+#endif  // EXTENSIONS_BROWSER_API_EXTENSIONS_API_CLIENT_H_

chrome/browser/extensions/api/storage/settings_namespace.cc → extensions/browser/api/storage/settings_namespace.cc

-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/extensions/api/storage/settings_namespace.h"
+#include "extensions/browser/api/storage/settings_namespace.h"
 
 #include "base/logging.h"
 
@@ -12,16 +12,20 @@ namespace settings_namespace {
 
 namespace {
 const char kLocalNamespace[] = "local";
-const char kSyncNamespace[]  = "sync";
+const char kSyncNamespace[] = "sync";
 const char kManagedNamespace[] = "managed";
 }  // namespace
 
 std::string ToString(Namespace settings_namespace) {
   switch (settings_namespace) {
-    case LOCAL:   return kLocalNamespace;
-    case SYNC:    return kSyncNamespace;
-    case MANAGED: return kManagedNamespace;
-    case INVALID: break;
+    case LOCAL:
+      return kLocalNamespace;
+    case SYNC:
+      return kSyncNamespace;
+    case MANAGED:
+      return kManagedNamespace;
+    case INVALID:
+      break;
   }
   NOTREACHED();
   return std::string();

chrome/browser/extensions/api/storage/settings_namespace.h → extensions/browser/api/storage/settings_namespace.h

-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_EXTENSIONS_API_STORAGE_SETTINGS_NAMESPACE_H_
-#define CHROME_BROWSER_EXTENSIONS_API_STORAGE_SETTINGS_NAMESPACE_H_
+#ifndef EXTENSIONS_BROWSER_API_STORAGE_SETTINGS_NAMESPACE_H_
+#define EXTENSIONS_BROWSER_API_STORAGE_SETTINGS_NAMESPACE_H_
 
 #include <string>
 
@@ -31,4 +31,4 @@ Namespace FromString(const std::string& ns_string);
 
 }  // namespace extensions
 
-#endif  // CHROME_BROWSER_EXTENSIONS_API_STORAGE_SETTINGS_NAMESPACE_H_
+#endif  // EXTENSIONS_BROWSER_API_STORAGE_SETTINGS_NAMESPACE_H_

extensions/extensions.gyp

@@ -170,6 +170,10 @@
         'browser/admin_policy.h',
         'browser/api/async_api_function.cc',
         'browser/api/async_api_function.h',
+        'browser/api/extensions_api_client.cc',
+        'browser/api/extensions_api_client.h',
+        'browser/api/storage/settings_namespace.cc',
+        'browser/api/storage/settings_namespace.h',
         'browser/api_activity_monitor.h',
         'browser/app_sorting.h',
         'browser/blacklist_state.h',