Extension syncing: Introduce a NeedsSync pref

chrome/browser/extensions/extension_service_unittest.cc

 // Copyright (c) 2013 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 <algorithm>
 #include <set>
 #include <vector>
 
 #include "base/at_exit.h"
 #include "base/basictypes.h"
@@ skipping 54 matching lines @@
 #include "chrome/browser/policy/profile_policy_connector.h"
 #include "chrome/browser/policy/profile_policy_connector_factory.h"
 #include "chrome/browser/prefs/pref_service_syncable.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/api/plugins/plugins_handler.h"
 #include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
 #include "chrome/common/extensions/manifest_handlers/content_scripts_handler.h"
+#include "chrome/common/extensions/sync_helper.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/scoped_browser_locale.h"
 #include "chrome/test/base/testing_pref_service_syncable.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/crx_file/id_util.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "content/public/browser/dom_storage_context.h"
 #include "content/public/browser/gpu_data_manager.h"
 #include "content/public/browser/indexed_db_context.h"
@@ skipping 5815 matching lines @@
   service()->Init();
   ASSERT_TRUE(service()->is_ready());
 
   ASSERT_EQ(3u, loaded_.size());
 
   // We start enabled.
   const Extension* extension = service()->GetExtensionById(good0, true);
   ASSERT_TRUE(extension);
   ASSERT_TRUE(service()->IsExtensionEnabled(good0));
 
+  // Sync starts up.
+  extension_sync_service()->MergeDataAndStartSyncing(
+      syncer::EXTENSIONS,
+      syncer::SyncDataList(),
+      make_scoped_ptr(new syncer::FakeSyncChangeProcessor),
+      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+
   // Then sync data arrives telling us to disable |good0|.
   ExtensionSyncData disable_good_crx(*extension, false,
                                      Extension::DISABLE_USER_ACTION, false,
                                      false, ExtensionSyncData::BOOLEAN_UNSET);
-  syncer::SyncDataList sync_data;
-  sync_data.push_back(disable_good_crx.GetSyncData());
-  extension_sync_service()->MergeDataAndStartSyncing(
-      syncer::EXTENSIONS,
-      sync_data,
-      scoped_ptr<syncer::SyncChangeProcessor>(
-          new syncer::FakeSyncChangeProcessor),
-      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+  syncer::SyncChange sync_change(FROM_HERE,
+                                 syncer::SyncChange::ACTION_UPDATE,
+                                 disable_good_crx.GetSyncData());
+  syncer::SyncChangeList list(1, sync_change);
+  extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
+
   ASSERT_FALSE(service()->IsExtensionEnabled(good0));

[Marc Treib, 2015/07/16 13:20:42]

Turns out I broke this test: Installing the extension before Sync is up will set
NeedsSync. So the sync change to disable that comes into
MergeDataAndStartSyncing will be ignored, which (I think) is fine, Sync
shouldn't even know about this extension yet.
To fix is simply to send a "proper" sync change after MergeDataAndStartSyncing.

 }
 
-TEST_F(ExtensionServiceTest, DontDisableExtensionWithPendingEnableFromSync) {
-  // Start the extensions service with one external extension already installed.
+TEST_F(ExtensionServiceTest, IgnoreSyncChangesWhenLocalStateIsMoreRecent) {
+  // Start the extension service with three extensions already installed.
   base::FilePath source_install_dir =
       data_dir().AppendASCII("good").AppendASCII("Extensions");
   base::FilePath pref_path =
       source_install_dir.DirName().Append(chrome::kPreferencesFilename);
 
   InitializeInstalledExtensionService(pref_path, source_install_dir);
   InitializeExtensionSyncService();
 
   // The user has enabled sync.
   ProfileSyncService* sync_service =
       ProfileSyncServiceFactory::GetForProfile(profile());
   sync_service->SetSyncSetupCompleted();
 
   service()->Init();
   ASSERT_TRUE(service()->is_ready());
   ASSERT_EQ(3u, loaded_.size());
 
-  const Extension* extension = service()->GetExtensionById(good0, true);
   ASSERT_TRUE(service()->IsExtensionEnabled(good0));
+  ASSERT_TRUE(service()->IsExtensionEnabled(good2));
 
-  // Disable extension before first sync data arrives.
+  // Disable and re-enable good0 before first sync data arrives.
   service()->DisableExtension(good0, Extension::DISABLE_USER_ACTION);
   ASSERT_FALSE(service()->IsExtensionEnabled(good0));
-
-  // Enable extension - this is now the most recent state.
   service()->EnableExtension(good0);
   ASSERT_TRUE(service()->IsExtensionEnabled(good0));
+  // Disable good2 before first sync data arrives (good1 is considered
+  // non-syncable because it has plugin permission).
+  service()->DisableExtension(good2, Extension::DISABLE_USER_ACTION);
+  ASSERT_FALSE(service()->IsExtensionEnabled(good2));
 
-  // Now sync data comes in that says to disable good0. This should be
-  // ignored.
-  ExtensionSyncData disable_good_crx(*extension, false,
-                                     Extension::DISABLE_USER_ACTION, false,
-                                     false, ExtensionSyncData::BOOLEAN_UNSET);
+  const Extension* extension0 = service()->GetExtensionById(good0, true);
+  const Extension* extension2 = service()->GetExtensionById(good2, true);
+  ASSERT_TRUE(extensions::sync_helper::IsSyncable(extension0));
+  ASSERT_TRUE(extensions::sync_helper::IsSyncable(extension2));
+
+  // Now sync data comes in that says to disable good0 and enable good2.
+  ExtensionSyncData disable_good0(*extension0, false,
+                                  Extension::DISABLE_USER_ACTION, false, false,
+                                  ExtensionSyncData::BOOLEAN_UNSET);
+  ExtensionSyncData enable_good2(*extension2, true, Extension::DISABLE_NONE,
+                                 false, false,
+                                 ExtensionSyncData::BOOLEAN_UNSET);
   syncer::SyncDataList sync_data;
-  sync_data.push_back(disable_good_crx.GetSyncData());
+  sync_data.push_back(disable_good0.GetSyncData());
+  sync_data.push_back(enable_good2.GetSyncData());
   extension_sync_service()->MergeDataAndStartSyncing(
       syncer::EXTENSIONS,
       sync_data,
-      scoped_ptr<syncer::SyncChangeProcessor>(
-          new syncer::FakeSyncChangeProcessor),
-      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+      make_scoped_ptr(new syncer::FakeSyncChangeProcessor),
+      make_scoped_ptr(new syncer::SyncErrorFactoryMock));
 
-  // The extension was enabled locally before the sync data arrived, so it
-  // should still be enabled now.
-  ASSERT_TRUE(service()->IsExtensionEnabled(good0));
+  // Both sync changes should be ignored, since the local state was changed
+  // before sync started, and so the local state is considered more recent.
+  EXPECT_TRUE(service()->IsExtensionEnabled(good0));
+  EXPECT_FALSE(service()->IsExtensionEnabled(good2));
 }
 
 TEST_F(ExtensionServiceTest, GetSyncData) {
   InitializeEmptyExtensionService();
   InitializeExtensionSyncService();
   InstallCRX(data_dir().AppendASCII("good.crx"), INSTALL_NEW);
   const Extension* extension = service()->GetInstalledExtension(good_crx);
   ASSERT_TRUE(extension);
 
   extension_sync_service()->MergeDataAndStartSyncing(
@@ skipping 2120 matching lines @@
 
   service()->Observe(chrome::NOTIFICATION_PROFILE_DESTRUCTION_STARTED,
                      content::Source<Profile>(profile()),
                      content::NotificationService::NoDetails());
   EXPECT_EQ(UnloadedExtensionInfo::REASON_PROFILE_SHUTDOWN, unloaded_reason_);
   EXPECT_EQ(0u, registry()->enabled_extensions().size());
   EXPECT_EQ(0u, registry()->disabled_extensions().size());
   EXPECT_EQ(0u, registry()->terminated_extensions().size());
   EXPECT_EQ(0u, registry()->blacklisted_extensions().size());
 }