Use ApiUnitTest for storage API tests (storage_api_unittest and storage_frontend_unittest).

Make these tests and settings_quota_unittest run in extensions_unittests.

Move a test that depends on sync (not available in the extensions module) to settings_sync_unittest.

Depends on previous CL: https://chromiumcodereview.appspot.com/461273003/

BUG=397164

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

Cr-Commit-Position: refs/heads/master@{#292412}

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

@@ -8,15 +8,18 @@
 #include "base/json/json_writer.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
+#include "base/strings/stringprintf.h"
 #include "chrome/browser/extensions/api/storage/settings_sync_util.h"
 #include "chrome/browser/extensions/api/storage/sync_value_store_cache.h"
 #include "chrome/browser/extensions/api/storage/syncable_settings_storage.h"
+#include "chrome/test/base/testing_profile.h"
 #include "content/public/test/test_browser_thread.h"
 #include "extensions/browser/api/storage/leveldb_settings_storage_factory.h"
 #include "extensions/browser/api/storage/settings_storage_factory.h"
 #include "extensions/browser/api/storage/settings_test_util.h"
 #include "extensions/browser/api/storage/storage_frontend.h"
 #include "extensions/browser/extension_system.h"
+#include "extensions/browser/mock_extension_system.h"
 #include "extensions/browser/value_store/testing_value_store.h"
 #include "extensions/common/manifest.h"
 #include "sync/api/sync_change_processor.h"
@@ -205,10 +208,15 @@ class ExtensionSettingsSyncTest : public testing::Test {
 
   virtual void SetUp() OVERRIDE {
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
-    profile_.reset(new util::MockProfile(temp_dir_.path()));
+    profile_.reset(new TestingProfile(temp_dir_.path()));
     storage_factory_->Reset(new LeveldbSettingsStorageFactory());
     frontend_.reset(
         StorageFrontend::CreateForTesting(storage_factory_, profile_.get()));
+
+    ExtensionsBrowserClient::Get()
+        ->GetExtensionSystemFactory()
+        ->SetTestingFactoryAndUse(
+            profile_.get(), &util::MockExtensionSystemWithEventRouter::Build);
   }
 
   virtual void TearDown() OVERRIDE {
@@ -257,7 +265,7 @@ class ExtensionSettingsSyncTest : public testing::Test {
   content::TestBrowserThread file_thread_;
 
   base::ScopedTempDir temp_dir_;
-  scoped_ptr<util::MockProfile> profile_;
+  scoped_ptr<TestingProfile> profile_;
   scoped_ptr<StorageFrontend> frontend_;
   scoped_refptr<util::ScopedSettingsStorageFactory> storage_factory_;
   scoped_ptr<MockSyncChangeProcessor> sync_processor_;
@@ -1423,4 +1431,65 @@ TEST_F(ExtensionSettingsSyncTest, Dots) {
   }
 }
 
+// In other (frontend) tests, we assume that the result of GetStorage
+// is a pointer to the a Storage owned by a Frontend object, but for
+// the unlimitedStorage case, this might not be true. So, write the
+// tests in a "callback" style.  We should really rewrite all tests to
+// be asynchronous in this way.
+
+namespace {
+
+static void UnlimitedSyncStorageTestCallback(ValueStore* sync_storage) {
+  // Sync storage should still run out after ~100K; the unlimitedStorage
+  // permission can't apply to sync.
+  scoped_ptr<base::Value> kilobyte = util::CreateKilobyte();
+  for (int i = 0; i < 100; ++i) {
+    sync_storage->Set(
+        ValueStore::DEFAULTS, base::StringPrintf("%d", i), *kilobyte);
+  }
+
+  EXPECT_TRUE(sync_storage->Set(ValueStore::DEFAULTS, "WillError", *kilobyte)
+                  ->HasError());
+}
+
+static void UnlimitedLocalStorageTestCallback(ValueStore* local_storage) {
+  // Local storage should never run out.
+  scoped_ptr<base::Value> megabyte = util::CreateMegabyte();
+  for (int i = 0; i < 7; ++i) {
+    local_storage->Set(
+        ValueStore::DEFAULTS, base::StringPrintf("%d", i), *megabyte);
+  }
+
+  EXPECT_FALSE(local_storage->Set(ValueStore::DEFAULTS, "WontError", *megabyte)
+                   ->HasError());
+}
+
+}  // namespace
+
+#if defined(OS_WIN)
+// See: http://crbug.com/227296
+#define MAYBE_UnlimitedStorageForLocalButNotSync \
+  DISABLED_UnlimitedStorageForLocalButNotSync
+#else
+#define MAYBE_UnlimitedStorageForLocalButNotSync \
+  UnlimitedStorageForLocalButNotSync
+#endif
+TEST_F(ExtensionSettingsSyncTest, MAYBE_UnlimitedStorageForLocalButNotSync) {
+  const std::string id = "ext";
+  std::set<std::string> permissions;
+  permissions.insert("unlimitedStorage");
+  scoped_refptr<const Extension> extension =
+      util::AddExtensionWithIdAndPermissions(
+          profile_.get(), id, Manifest::TYPE_EXTENSION, permissions);
+
+  frontend_->RunWithStorage(extension,
+                            settings_namespace::SYNC,
+                            base::Bind(&UnlimitedSyncStorageTestCallback));
+  frontend_->RunWithStorage(extension,
+                            settings_namespace::LOCAL,
+                            base::Bind(&UnlimitedLocalStorageTestCallback));
+
+  base::MessageLoop::current()->RunUntilIdle();
+}
+
 }  // namespace extensions

chrome/chrome_tests_unit.gypi

@@ -531,11 +531,6 @@
         '../extensions/browser/api/cast_channel/cast_socket_unittest.cc',
         '../extensions/browser/api/cast_channel/logger_unittest.cc',
         '../extensions/browser/api/power/power_api_unittest.cc',
-        '../extensions/browser/api/storage/settings_quota_unittest.cc',
-        '../extensions/browser/api/storage/settings_test_util.cc',
-        '../extensions/browser/api/storage/settings_test_util.h',
-        '../extensions/browser/api/storage/storage_api_unittest.cc',
-        '../extensions/browser/api/storage/storage_frontend_unittest.cc',
         '../extensions/browser/app_window/app_window_geometry_cache_unittest.cc',
         '../extensions/browser/error_map_unittest.cc',
         '../extensions/browser/extension_error_test_util.cc',

extensions/browser/api/storage/settings_test_util.cc

@@ -16,6 +16,24 @@ namespace extensions {
 
 namespace settings_test_util {
 
+// Creates a kilobyte of data.
+scoped_ptr<base::Value> CreateKilobyte() {
+  std::string kilobyte_string;
+  for (int i = 0; i < 1024; ++i) {
+    kilobyte_string += "a";
+  }
+  return scoped_ptr<base::Value>(new base::StringValue(kilobyte_string));
+}
+
+// Creates a megabyte of data.
+scoped_ptr<base::Value> CreateMegabyte() {
+  base::ListValue* megabyte = new base::ListValue();
+  for (int i = 0; i < 1000; ++i) {
+    megabyte->Append(CreateKilobyte().release());
+  }
+  return scoped_ptr<base::Value>(megabyte);
+}
+
 // Intended as a StorageCallback from GetStorage.
 static void AssignStorage(ValueStore** dst, ValueStore* src) {
   *dst = src;
@@ -36,15 +54,16 @@ ValueStore* GetStorage(scoped_refptr<const Extension> extension,
   return GetStorage(extension, settings_namespace::SYNC, frontend);
 }
 
-scoped_refptr<const Extension> AddExtensionWithId(Profile* profile,
-                                                  const std::string& id,
-                                                  Manifest::Type type) {
+scoped_refptr<const Extension> AddExtensionWithId(
+    content::BrowserContext* context,
+    const std::string& id,
+    Manifest::Type type) {
   return AddExtensionWithIdAndPermissions(
-      profile, id, type, std::set<std::string>());
+      context, id, type, std::set<std::string>());
 }
 
 scoped_refptr<const Extension> AddExtensionWithIdAndPermissions(
-    Profile* profile,
+    content::BrowserContext* context,
     const std::string& id,
     Manifest::Type type,
     const std::set<std::string>& permissions_set) {
@@ -89,7 +108,7 @@ scoped_refptr<const Extension> AddExtensionWithIdAndPermissions(
 
   // Ensure lookups via ExtensionRegistry (and ExtensionService) work even if
   // the test discards the referenced to the returned extension.
-  ExtensionRegistry::Get(profile)->AddEnabled(extension);
+  ExtensionRegistry::Get(context)->AddEnabled(extension);
 
   for (std::set<std::string>::const_iterator it = permissions_set.begin();
       it != permissions_set.end(); ++it) {
@@ -99,32 +118,26 @@ scoped_refptr<const Extension> AddExtensionWithIdAndPermissions(
   return extension;
 }
 
-// MockExtensionSystem
+// MockExtensionSystemWithEventRouter
 
-MockExtensionSystem::MockExtensionSystem(Profile* profile)
-      : TestExtensionSystem(profile) {}
-MockExtensionSystem::~MockExtensionSystem() {}
-
-EventRouter* MockExtensionSystem::event_router() {
-  if (!event_router_.get())
-    event_router_.reset(new EventRouter(profile_, NULL));
-  return event_router_.get();
+MockExtensionSystemWithEventRouter::MockExtensionSystemWithEventRouter(
+    content::BrowserContext* context)
+    : MockExtensionSystem(context) {
 }
 
-KeyedService* BuildMockExtensionSystem(content::BrowserContext* profile) {
-  return new MockExtensionSystem(static_cast<Profile*>(profile));
+MockExtensionSystemWithEventRouter::~MockExtensionSystemWithEventRouter() {
 }
 
-// MockProfile
-
-MockProfile::MockProfile(const base::FilePath& file_path)
-    : TestingProfile(file_path) {
-  ExtensionsBrowserClient::Get()
-      ->GetExtensionSystemFactory()
-      ->SetTestingFactoryAndUse(this, &BuildMockExtensionSystem);
+KeyedService* MockExtensionSystemWithEventRouter::Build(
+    content::BrowserContext* context) {
+  return new MockExtensionSystemWithEventRouter(context);
 }
 
-MockProfile::~MockProfile() {}
+EventRouter* MockExtensionSystemWithEventRouter::event_router() {
+  if (!event_router_.get())
+    event_router_.reset(new EventRouter(browser_context(), NULL));
+  return event_router_.get();
+}
 
 // ScopedSettingsFactory
 

extensions/browser/api/storage/settings_test_util.h

@@ -12,11 +12,11 @@
 #include "base/memory/linked_ptr.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.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/api/storage/settings_storage_factory.h"
 #include "extensions/browser/event_router.h"
+#include "extensions/browser/mock_extension_system.h"
 #include "extensions/common/extension.h"
 
 class ValueStore;
@@ -27,6 +27,12 @@ class StorageFrontend;
 // Utilities for extension settings API tests.
 namespace settings_test_util {
 
+// Creates a kilobyte of data.
+scoped_ptr<base::Value> CreateKilobyte();
+
+// Creates a megabyte of data.
+scoped_ptr<base::Value> CreateMegabyte();
+
 // Synchronously gets the storage area for an extension from |frontend|.
 ValueStore* GetStorage(scoped_refptr<const Extension> extension,
                        settings_namespace::Namespace setting_namespace,
@@ -36,39 +42,36 @@ ValueStore* GetStorage(scoped_refptr<const Extension> extension,
 ValueStore* GetStorage(scoped_refptr<const Extension> extension,
                        StorageFrontend* frontend);
 
-// Creates an extension with |id| and adds it to the registry for |profile|.
-scoped_refptr<const Extension> AddExtensionWithId(Profile* profile,
-                                                  const std::string& id,
-                                                  Manifest::Type type);
+// Creates an extension with |id| and adds it to the registry for |context|.
+scoped_refptr<const Extension> AddExtensionWithId(
+    content::BrowserContext* context,
+    const std::string& id,
+    Manifest::Type type);
 
 // Creates an extension with |id| with a set of |permissions| and adds it to
-// the registry for |profile|.
+// the registry for |context|.
 scoped_refptr<const Extension> AddExtensionWithIdAndPermissions(
-    Profile* profile,
+    content::BrowserContext* context,
     const std::string& id,
     Manifest::Type type,
     const std::set<std::string>& permissions);
 
-// A mock ExtensionSystem to serve an EventRouter.
-class MockExtensionSystem : public TestExtensionSystem {
+// A MockExtensionSystem to serve an EventRouter.
+class MockExtensionSystemWithEventRouter : public MockExtensionSystem {
  public:
-  explicit MockExtensionSystem(Profile* profile);
-  virtual ~MockExtensionSystem();
+  explicit MockExtensionSystemWithEventRouter(content::BrowserContext* context);
+  virtual ~MockExtensionSystemWithEventRouter();
+
+  // Factory method for SetTestingFactoryAndUse.
+  static KeyedService* Build(content::BrowserContext* context);
 
+  // MockExtensionSystem overrides:
   virtual EventRouter* event_router() OVERRIDE;
 
  private:
   scoped_ptr<EventRouter> event_router_;
 
-  DISALLOW_COPY_AND_ASSIGN(MockExtensionSystem);
-};
-
-// A Profile which returns an ExtensionSystem with enough functionality for
-// the tests.
-class MockProfile : public TestingProfile {
- public:
-  explicit MockProfile(const base::FilePath& file_path);
-  virtual ~MockProfile();
+  DISALLOW_COPY_AND_ASSIGN(MockExtensionSystemWithEventRouter);
 };
 
 // SettingsStorageFactory which acts as a wrapper for other factories.

extensions/browser/api/storage/storage_api_unittest.cc

@@ -6,16 +6,19 @@
 #include "base/files/file_path.h"
 #include "base/memory/ref_counted.h"
 #include "base/strings/stringprintf.h"
-#include "chrome/browser/extensions/extension_api_unittest.h"
-#include "chrome/browser/extensions/test_extension_system.h"
+#include "content/public/test/test_browser_context.h"
+#include "extensions/browser/api/extensions_api_client.h"
 #include "extensions/browser/api/storage/leveldb_settings_storage_factory.h"
 #include "extensions/browser/api/storage/settings_storage_quota_enforcer.h"
 #include "extensions/browser/api/storage/settings_test_util.h"
 #include "extensions/browser/api/storage/storage_api.h"
 #include "extensions/browser/api/storage/storage_frontend.h"
+#include "extensions/browser/api_unittest.h"
 #include "extensions/browser/event_router.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_system.h"
+#include "extensions/browser/mock_extension_system.h"
+#include "extensions/browser/test_extensions_browser_client.h"
 #include "extensions/browser/value_store/leveldb_value_store.h"
 #include "extensions/browser/value_store/value_store.h"
 #include "extensions/common/manifest.h"
@@ -36,15 +39,14 @@ KeyedService* CreateStorageFrontendForTesting(
 
 }  // namespace
 
-class StorageApiUnittest : public ExtensionApiUnittest {
+class StorageApiUnittest : public ApiUnitTest {
  public:
+  StorageApiUnittest() {}
+
   virtual void SetUp() OVERRIDE {
-    ExtensionApiUnittest::SetUp();
-    TestExtensionSystem* extension_system =
-        static_cast<TestExtensionSystem*>(ExtensionSystem::Get(profile()));
-    // StorageFrontend requires an EventRouter.
-    extension_system->SetEventRouter(scoped_ptr<EventRouter>(
-        new EventRouter(profile(), ExtensionPrefs::Get(profile()))));
+    ApiUnitTest::SetUp();
+    extensions_browser_client()->set_extension_system_factory(
+        &extension_system_factory_);
   }
 
  protected:
@@ -74,13 +76,18 @@ class StorageApiUnittest : public ExtensionApiUnittest {
     }
     return testing::AssertionSuccess();
   }
+
+  MockExtensionSystemFactory<
+      settings_test_util::MockExtensionSystemWithEventRouter>
+      extension_system_factory_;
+  ExtensionsAPIClient extensions_api_client_;
 };
 
 TEST_F(StorageApiUnittest, RestoreCorruptedStorage) {
   // Ensure a StorageFrontend can be created on demand. The StorageFrontend
   // will be owned by the KeyedService system.
   StorageFrontend::GetFactoryInstance()->SetTestingFactory(
-      profile(), &CreateStorageFrontendForTesting);
+      browser_context(), &CreateStorageFrontendForTesting);
 
   const char kKey[] = "key";
   const char kValue[] = "value";
@@ -97,7 +104,7 @@ TEST_F(StorageApiUnittest, RestoreCorruptedStorage) {
   ValueStore* store =
       settings_test_util::GetStorage(extension_ref(),
                                      settings_namespace::LOCAL,
-                                     StorageFrontend::Get(profile()));
+                                     StorageFrontend::Get(browser_context()));
   ASSERT_TRUE(store);
   SettingsStorageQuotaEnforcer* quota_store =
       static_cast<SettingsStorageQuotaEnforcer*>(store);

extensions/extensions.gyp

@@ -789,6 +789,8 @@
       'sources': [
         'browser/api/dns/mock_host_resolver_creator.cc',
         'browser/api/dns/mock_host_resolver_creator.h',
+        'browser/api/storage/settings_test_util.cc',
+        'browser/api/storage/settings_test_util.h',
         'browser/api_test_utils.cc',
         'browser/api_test_utils.h',
         'browser/extensions_test.cc',
@@ -908,6 +910,9 @@
         'browser/api/api_resource_manager_unittest.cc',
         'browser/api/declarative/deduping_factory_unittest.cc',
         'browser/api/sockets_tcp/sockets_tcp_api_unittest.cc',
+        'browser/api/storage/settings_quota_unittest.cc',
+        'browser/api/storage/storage_api_unittest.cc',
+        'browser/api/storage/storage_frontend_unittest.cc',
         'browser/computed_hashes_unittest.cc',
         'browser/content_hash_tree_unittest.cc',
         'browser/event_listener_map_unittest.cc',