Separate the syncing of extension settings and app settings into separate data

chrome/browser/extensions/extension_settings_sync_unittest.cc

 // Copyright (c) 2011 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 "testing/gtest/include/gtest/gtest.h"
 
 #include "base/bind.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "base/scoped_temp_dir.h"
 #include "base/task.h"
-#include "chrome/browser/extensions/extension_settings_backend.h"
 #include "chrome/browser/extensions/extension_settings_frontend.h"
 #include "chrome/browser/extensions/extension_settings_storage_cache.h"
 #include "chrome/browser/extensions/extension_settings_sync_util.h"
 #include "chrome/browser/extensions/syncable_extension_settings_storage.h"
+#include "chrome/browser/extensions/extension_settings_test_util.h"
 #include "chrome/browser/sync/api/sync_change_processor.h"
-#include "chrome/test/base/testing_profile.h"
 #include "content/browser/browser_thread.h"
 
 // TODO(kalman): Integration tests for sync.
 
+using namespace extension_settings_test_util;
+
 namespace {
 
 // Gets the pretty-printed JSON for a value.
 static std::string GetJson(const Value& value) {
   std::string json;
   base::JSONWriter::Write(&value, true, &json);
   return json;
 }
 
 // Returns whether two Values are equal.
@@ skipping 77 matching lines @@
       ADD_FAILURE() << matching_changes.size() << " matching changes for " <<
            extension_id << "/" << key << " (out of " << changes_.size() << ")";
     }
     return matching_changes[0];
   }
 
  private:
   ExtensionSettingSyncDataList changes_;
 };
 
-// To be called as a callback from ExtensionSettingsFrontend::RunWithSettings.
-void AssignSettings(
-    ExtensionSettingsBackend** dst, ExtensionSettingsBackend* src) {
+void AssignSettingsService(SyncableService** dst, SyncableService* src) {
   *dst = src;
 }
 
 }  // namespace
 
 class ExtensionSettingsSyncTest : public testing::Test {
  public:
   ExtensionSettingsSyncTest()
       : ui_thread_(BrowserThread::UI, MessageLoop::current()),
-        file_thread_(BrowserThread::FILE, MessageLoop::current()),
-        frontend_(&profile_),
-        backend_(NULL) {
+        file_thread_(BrowserThread::FILE, MessageLoop::current()) {}
+
+  virtual void SetUp() OVERRIDE {
+    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+    profile_.reset(new MockProfile(temp_dir_.path()));
+    frontend_.reset(new ExtensionSettingsFrontend(profile_.get()));
   }
 
-  virtual void SetUp() OVERRIDE {
-    frontend_.RunWithBackend(base::Bind(&AssignSettings, &backend_));
-    MessageLoop::current()->RunAllPending();
-    ASSERT_TRUE(backend_ != NULL);
+  virtual void TearDown() OVERRIDE {
+    frontend_.reset();
+    profile_.reset();
   }
 
  protected:
-  // Creates a new extension storage object and adds a record of the extension
-  // to the extension service.
-  SyncableExtensionSettingsStorage* GetStorage(
-      const std::string& extension_id) {
-    return static_cast<SyncableExtensionSettingsStorage*>(
-        backend_->GetStorage(extension_id));
+  // Adds a record of an extension or app to the extension service, then returns
+  // its storage area.
+  ExtensionSettingsStorage* AddExtensionAndGetStorage(
+      const std::string& id, bool is_app) {
+    profile_->GetMockExtensionService()->AddExtension(id, is_app);
+    return GetStorage(id, frontend_.get());
   }
 
-  // Gets all the sync data from |backend_| as a map from extension id to its
-  // sync data.
-  std::map<std::string, ExtensionSettingSyncDataList> GetAllSyncData() {
-    SyncDataList as_list =
-        backend_->GetAllSyncData(syncable::EXTENSION_SETTINGS);
+  // Gets the SyncableService for the given sync type.
+  SyncableService* GetSyncableService(syncable::ModelType model_type) {
+    SyncableService* settings_service = NULL;
+    frontend_->RunWithSyncableService(
+        model_type, base::Bind(&AssignSettingsService, &settings_service));
+    MessageLoop::current()->RunAllPending();
+    return settings_service;
+  }
+
+  // Gets all the sync data from the SyncableService for a sync type as a map
+  // from extension id to its sync data.
+  std::map<std::string, ExtensionSettingSyncDataList> GetAllSyncData(
+      syncable::ModelType type) {
+    SyncDataList as_list = GetSyncableService(type)->GetAllSyncData(type);
     std::map<std::string, ExtensionSettingSyncDataList> as_map;
     for (SyncDataList::iterator it = as_list.begin();
         it != as_list.end(); ++it) {
       ExtensionSettingSyncData sync_data(*it);
       as_map[sync_data.extension_id()].push_back(sync_data);
     }
     return as_map;
   }
 
   // Need these so that the DCHECKs for running on FILE or UI threads pass.
   MessageLoop message_loop_;
   BrowserThread ui_thread_;
   BrowserThread file_thread_;
 
+  ScopedTempDir temp_dir_;
   MockSyncChangeProcessor sync_;
-  TestingProfile profile_;
-  ExtensionSettingsFrontend frontend_;
-
-  // Get from frontend_->RunWithBackend, so weak reference.
-  ExtensionSettingsBackend* backend_;
+  scoped_ptr<MockProfile> profile_;
+  scoped_ptr<ExtensionSettingsFrontend> frontend_;
 };
 
+// Get a semblance of coverage for both EXTENSION_SETTINGS and APP_SETTINGS
+// sync by roughly alternative which one to test.

[akalin, 2011/10/28 06:04:54]

Consider parametrized tests here

[not at google - send to devlin, 2011/10/31 00:02:23]

Ditto.

+
 TEST_F(ExtensionSettingsSyncTest, NoDataDoesNotInvokeSync) {
-  ASSERT_EQ(0u, GetAllSyncData().size());
+  syncable::ModelType type = syncable::EXTENSION_SETTINGS;
+  bool is_app = false;
+
+  ASSERT_EQ(0u, GetAllSyncData(type).size());
 
   // Have one extension created before sync is set up, the other created after.
-  GetStorage("s1");
-  ASSERT_EQ(0u, GetAllSyncData().size());
+  AddExtensionAndGetStorage("s1", is_app);
+  ASSERT_EQ(0u, GetAllSyncData(type).size());
 
-  backend_->MergeDataAndStartSyncing(
-      syncable::EXTENSION_SETTINGS,
-      SyncDataList(),
-      &sync_);
+  GetSyncableService(type)->MergeDataAndStartSyncing(
+      type, SyncDataList(), &sync_);
 
-  GetStorage("s2");
-  ASSERT_EQ(0u, GetAllSyncData().size());
+  AddExtensionAndGetStorage("s2", is_app);
+  ASSERT_EQ(0u, GetAllSyncData(type).size());
 
-  backend_->StopSyncing(syncable::EXTENSION_SETTINGS);
+  GetSyncableService(type)->StopSyncing(type);
 
   ASSERT_EQ(0u, sync_.changes().size());
-  ASSERT_EQ(0u, GetAllSyncData().size());
+  ASSERT_EQ(0u, GetAllSyncData(type).size());
 }
 
 TEST_F(ExtensionSettingsSyncTest, InSyncDataDoesNotInvokeSync) {
+  syncable::ModelType type = syncable::APP_SETTINGS;
+  bool is_app = true;
+
   StringValue value1("fooValue");
   ListValue value2;
   value2.Append(StringValue::CreateStringValue("barValue"));
 
-  SyncableExtensionSettingsStorage* storage1 = GetStorage("s1");
-  SyncableExtensionSettingsStorage* storage2 = GetStorage("s2");
+  ExtensionSettingsStorage* storage1 = AddExtensionAndGetStorage("s1", is_app);
+  ExtensionSettingsStorage* storage2 = AddExtensionAndGetStorage("s2", is_app);
 
   storage1->Set("foo", value1);
   storage2->Set("bar", value2);
 
   std::map<std::string, ExtensionSettingSyncDataList> all_sync_data =
-      GetAllSyncData();
+      GetAllSyncData(type);
   ASSERT_EQ(2u, all_sync_data.size());
   ASSERT_EQ(1u, all_sync_data["s1"].size());
   ASSERT_PRED_FORMAT2(ValuesEq, &value1, &all_sync_data["s1"][0].value());
   ASSERT_EQ(1u, all_sync_data["s2"].size());
   ASSERT_PRED_FORMAT2(ValuesEq, &value2, &all_sync_data["s2"][0].value());
 
   SyncDataList sync_data;
   sync_data.push_back(extension_settings_sync_util::CreateData(
       "s1", "foo", value1));
   sync_data.push_back(extension_settings_sync_util::CreateData(
       "s2", "bar", value2));
 
-  backend_->MergeDataAndStartSyncing(
-      syncable::EXTENSION_SETTINGS, sync_data, &sync_);
-  backend_->StopSyncing(syncable::EXTENSION_SETTINGS);
+  GetSyncableService(type)->MergeDataAndStartSyncing(type, sync_data, &sync_);
+  GetSyncableService(type)->StopSyncing(type);
 
   // Already in sync, so no changes.
   ASSERT_EQ(0u, sync_.changes().size());
 }
 
 TEST_F(ExtensionSettingsSyncTest, LocalDataWithNoSyncDataIsPushedToSync) {
+  syncable::ModelType type = syncable::EXTENSION_SETTINGS;
+  bool is_app = false;
+
   StringValue value1("fooValue");
   ListValue value2;
   value2.Append(StringValue::CreateStringValue("barValue"));
 
-  SyncableExtensionSettingsStorage* storage1 = GetStorage("s1");
-  SyncableExtensionSettingsStorage* storage2 = GetStorage("s2");
+  ExtensionSettingsStorage* storage1 = AddExtensionAndGetStorage("s1", is_app);
+  ExtensionSettingsStorage* storage2 = AddExtensionAndGetStorage("s2", is_app);
 
   storage1->Set("foo", value1);
   storage2->Set("bar", value2);
 
-  backend_->MergeDataAndStartSyncing(
-      syncable::EXTENSION_SETTINGS, SyncDataList(), &sync_);
+  GetSyncableService(type)->MergeDataAndStartSyncing(
+      type, SyncDataList(), &sync_);
 
   // All settings should have been pushed to sync.
   ASSERT_EQ(2u, sync_.changes().size());
   ExtensionSettingSyncData change = sync_.GetOnlyChange("s1", "foo");
   ASSERT_EQ(SyncChange::ACTION_ADD, change.change_type());
   ASSERT_TRUE(value1.Equals(&change.value()));
   change = sync_.GetOnlyChange("s2", "bar");
   ASSERT_EQ(SyncChange::ACTION_ADD, change.change_type());
   ASSERT_TRUE(value2.Equals(&change.value()));
 
-  backend_->StopSyncing(syncable::EXTENSION_SETTINGS);
+  GetSyncableService(type)->StopSyncing(type);
 }
 
 TEST_F(ExtensionSettingsSyncTest, AnySyncDataOverwritesLocalData) {
+  syncable::ModelType type = syncable::APP_SETTINGS;
+  bool is_app = true;
+
   StringValue value1("fooValue");
   ListValue value2;
   value2.Append(StringValue::CreateStringValue("barValue"));
 
   // Maintain dictionaries mirrored to the expected values of the settings in
   // each storage area.
   DictionaryValue expected1, expected2;
 
   // Pre-populate one of the storage areas.
-  SyncableExtensionSettingsStorage* storage1 = GetStorage("s1");
+  ExtensionSettingsStorage* storage1 = AddExtensionAndGetStorage("s1", is_app);
   storage1->Set("overwriteMe", value1);
 
   SyncDataList sync_data;
   sync_data.push_back(extension_settings_sync_util::CreateData(
       "s1", "foo", value1));
   sync_data.push_back(extension_settings_sync_util::CreateData(
       "s2", "bar", value2));
-  backend_->MergeDataAndStartSyncing(
-      syncable::EXTENSION_SETTINGS, sync_data, &sync_);
+  GetSyncableService(type)->MergeDataAndStartSyncing(type, sync_data, &sync_);
   expected1.Set("foo", value1.DeepCopy());
   expected2.Set("bar", value2.DeepCopy());
 
-  SyncableExtensionSettingsStorage* storage2 = GetStorage("s2");
+  ExtensionSettingsStorage* storage2 = AddExtensionAndGetStorage("s2", is_app);
 
   // All changes should be local, so no sync changes.
   ASSERT_EQ(0u, sync_.changes().size());
 
   // Sync settings should have been pushed to local settings.
   ASSERT_PRED_FORMAT2(SettingsEq, &expected1, storage1->Get());
   ASSERT_PRED_FORMAT2(SettingsEq, &expected2, storage2->Get());
 
-  backend_->StopSyncing(syncable::EXTENSION_SETTINGS);
+  GetSyncableService(type)->StopSyncing(type);
 }
 
 TEST_F(ExtensionSettingsSyncTest, ProcessSyncChanges) {
+  syncable::ModelType type = syncable::EXTENSION_SETTINGS;
+  bool is_app = false;
+
   StringValue value1("fooValue");
   ListValue value2;
   value2.Append(StringValue::CreateStringValue("barValue"));
 
   // Maintain dictionaries mirrored to the expected values of the settings in
   // each storage area.
   DictionaryValue expected1, expected2;
 
   // Make storage1 initialised from local data, storage2 initialised from sync.
-  SyncableExtensionSettingsStorage* storage1 = GetStorage("s1");
-  SyncableExtensionSettingsStorage* storage2 = GetStorage("s2");
+  ExtensionSettingsStorage* storage1 = AddExtensionAndGetStorage("s1", is_app);
+  ExtensionSettingsStorage* storage2 = AddExtensionAndGetStorage("s2", is_app);
 
   storage1->Set("foo", value1);
   expected1.Set("foo", value1.DeepCopy());
 
   SyncDataList sync_data;
   sync_data.push_back(extension_settings_sync_util::CreateData(
       "s2", "bar", value2));
 
-  backend_->MergeDataAndStartSyncing(
-      syncable::EXTENSION_SETTINGS, sync_data, &sync_);
+  GetSyncableService(type)->MergeDataAndStartSyncing(type, sync_data, &sync_);
   expected2.Set("bar", value2.DeepCopy());
 
   // Make sync add some settings.
   SyncChangeList change_list;
   change_list.push_back(extension_settings_sync_util::CreateAdd(
       "s1", "bar", value2));
   change_list.push_back(extension_settings_sync_util::CreateAdd(
       "s2", "foo", value1));
-  backend_->ProcessSyncChanges(FROM_HERE, change_list);
+  GetSyncableService(type)->ProcessSyncChanges(FROM_HERE, change_list);
   expected1.Set("bar", value2.DeepCopy());
   expected2.Set("foo", value1.DeepCopy());
 
   ASSERT_PRED_FORMAT2(SettingsEq, &expected1, storage1->Get());
   ASSERT_PRED_FORMAT2(SettingsEq, &expected2, storage2->Get());
 
   // Make sync update some settings, storage1 the new setting, storage2 the
   // initial setting.
   change_list.clear();
   change_list.push_back(extension_settings_sync_util::CreateUpdate(
       "s1", "bar", value2));
   change_list.push_back(extension_settings_sync_util::CreateUpdate(
       "s2", "bar", value1));
-  backend_->ProcessSyncChanges(FROM_HERE, change_list);
+  GetSyncableService(type)->ProcessSyncChanges(FROM_HERE, change_list);
   expected1.Set("bar", value2.DeepCopy());
   expected2.Set("bar", value1.DeepCopy());
 
   ASSERT_PRED_FORMAT2(SettingsEq, &expected1, storage1->Get());
   ASSERT_PRED_FORMAT2(SettingsEq, &expected2, storage2->Get());
 
   // Make sync remove some settings, storage1 the initial setting, storage2 the
   // new setting.
   change_list.clear();
   change_list.push_back(extension_settings_sync_util::CreateDelete(
       "s1", "foo"));
   change_list.push_back(extension_settings_sync_util::CreateDelete(
       "s2", "foo"));
-  backend_->ProcessSyncChanges(FROM_HERE, change_list);
+  GetSyncableService(type)->ProcessSyncChanges(FROM_HERE, change_list);
   expected1.Remove("foo", NULL);
   expected2.Remove("foo", NULL);
 
   ASSERT_PRED_FORMAT2(SettingsEq, &expected1, storage1->Get());
   ASSERT_PRED_FORMAT2(SettingsEq, &expected2, storage2->Get());
 
-  backend_->StopSyncing(syncable::EXTENSION_SETTINGS);
+  GetSyncableService(type)->StopSyncing(type);
 }
 
 TEST_F(ExtensionSettingsSyncTest, PushToSync) {
+  syncable::ModelType type = syncable::APP_SETTINGS;
+  bool is_app = true;
+
   StringValue value1("fooValue");
   ListValue value2;
   value2.Append(StringValue::CreateStringValue("barValue"));
 
   // Make storage1/2 initialised from local data, storage3/4 initialised from
   // sync.
-  SyncableExtensionSettingsStorage* storage1 = GetStorage("s1");
-  SyncableExtensionSettingsStorage* storage2 = GetStorage("s2");
-  SyncableExtensionSettingsStorage* storage3 = GetStorage("s3");
-  SyncableExtensionSettingsStorage* storage4 = GetStorage("s4");
+  ExtensionSettingsStorage* storage1 = AddExtensionAndGetStorage("s1", is_app);
+  ExtensionSettingsStorage* storage2 = AddExtensionAndGetStorage("s2", is_app);
+  ExtensionSettingsStorage* storage3 = AddExtensionAndGetStorage("s3", is_app);
+  ExtensionSettingsStorage* storage4 = AddExtensionAndGetStorage("s4", is_app);
 
   storage1->Set("foo", value1);
   storage2->Set("foo", value1);
 
   SyncDataList sync_data;
   sync_data.push_back(extension_settings_sync_util::CreateData(
       "s3", "bar", value2));
   sync_data.push_back(extension_settings_sync_util::CreateData(
       "s4", "bar", value2));
 
-  backend_->MergeDataAndStartSyncing(
-      syncable::EXTENSION_SETTINGS, sync_data, &sync_);
+  GetSyncableService(type)->MergeDataAndStartSyncing(type, sync_data, &sync_);
 
   // Add something locally.
   storage1->Set("bar", value2);
   storage2->Set("bar", value2);
   storage3->Set("foo", value1);
   storage4->Set("foo", value1);
 
   ExtensionSettingSyncData change = sync_.GetOnlyChange("s1", "bar");
     ASSERT_EQ(SyncChange::ACTION_ADD, change.change_type());
     ASSERT_TRUE(value2.Equals(&change.value()));
@@ skipping 91 matching lines @@
   ASSERT_EQ(
       SyncChange::ACTION_DELETE,
       sync_.GetOnlyChange("s3", "bar").change_type());
   ASSERT_EQ(
       SyncChange::ACTION_DELETE,
       sync_.GetOnlyChange("s4", "foo").change_type());
   ASSERT_EQ(
       SyncChange::ACTION_DELETE,
       sync_.GetOnlyChange("s4", "bar").change_type());
 
-  backend_->StopSyncing(syncable::EXTENSION_SETTINGS);
+  GetSyncableService(type)->StopSyncing(type);
 }
+
+TEST_F(ExtensionSettingsSyncTest, ExtensionAndAppSettingsSyncSeparately) {
+  StringValue value1("fooValue");
+  ListValue value2;
+  value2.Append(StringValue::CreateStringValue("barValue"));
+
+  // storage1 is an extension, storage2 is an app.
+  ExtensionSettingsStorage* storage1 = AddExtensionAndGetStorage("s1", false);
+  ExtensionSettingsStorage* storage2 = AddExtensionAndGetStorage("s2", true);
+
+  storage1->Set("foo", value1);
+  storage2->Set("bar", value2);
+
+  std::map<std::string, ExtensionSettingSyncDataList> extension_sync_data =
+      GetAllSyncData(syncable::EXTENSION_SETTINGS);
+  ASSERT_EQ(1u, extension_sync_data.size());
+  ASSERT_EQ(1u, extension_sync_data["s1"].size());
+  ASSERT_PRED_FORMAT2(ValuesEq, &value1, &extension_sync_data["s1"][0].value());
+
+  std::map<std::string, ExtensionSettingSyncDataList> app_sync_data =
+      GetAllSyncData(syncable::APP_SETTINGS);
+  ASSERT_EQ(1u, app_sync_data.size());
+  ASSERT_EQ(1u, app_sync_data["s2"].size());
+  ASSERT_PRED_FORMAT2(ValuesEq, &value2, &app_sync_data["s2"][0].value());
+
+  // Stop each separately, there should be no changes either time.
+  SyncDataList sync_data;
+  sync_data.push_back(extension_settings_sync_util::CreateData(
+      "s1", "foo", value1));
+
+  GetSyncableService(syncable::EXTENSION_SETTINGS)->
+      MergeDataAndStartSyncing(syncable::EXTENSION_SETTINGS, sync_data, &sync_);
+  GetSyncableService(syncable::EXTENSION_SETTINGS)->
+      StopSyncing(syncable::EXTENSION_SETTINGS);
+  ASSERT_EQ(0u, sync_.changes().size());
+
+  sync_data.clear();
+  sync_data.push_back(extension_settings_sync_util::CreateData(
+      "s2", "bar", value2));
+
+  GetSyncableService(syncable::APP_SETTINGS)->
+      MergeDataAndStartSyncing(syncable::APP_SETTINGS, sync_data, &sync_);
+  GetSyncableService(syncable::APP_SETTINGS)->
+      StopSyncing(syncable::APP_SETTINGS);
+  ASSERT_EQ(0u, sync_.changes().size());
+}