Extensions: Store disable reasons in Sync

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 906 matching lines @@
     const base::DictionaryValue* dict =
         profile()->GetPrefs()->GetDictionary("extensions.settings");
     if (!dict) {
       ADD_FAILURE();
       return 0;
     }
     return dict->size();
   }
 
   void UninstallExtension(const std::string& id, bool use_helper) {
+    UninstallExtension(id, use_helper, Extension::ENABLED);
+  }
+
+  void UninstallExtension(const std::string& id, bool use_helper,
+                          Extension::State expected_state) {
     // Verify that the extension is installed.
     base::FilePath extension_path = extensions_install_dir().AppendASCII(id);
     EXPECT_TRUE(base::PathExists(extension_path));
     size_t pref_key_count = GetPrefKeyCount();
     EXPECT_GT(pref_key_count, 0u);
-    ValidateIntegerPref(id, "state", Extension::ENABLED);
+    ValidateIntegerPref(id, "state", expected_state);
 
     // Uninstall it.
     if (use_helper) {
       EXPECT_TRUE(ExtensionService::UninstallExtensionHelper(
           service(), id, extensions::UNINSTALL_REASON_FOR_TESTING));
     } else {
       EXPECT_TRUE(service()->UninstallExtension(
           id,
           extensions::UNINSTALL_REASON_FOR_TESTING,
           base::Bind(&base::DoNothing),
@@ skipping 4959 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));
-  ExtensionSyncData disable_good_crx(*extension, false, false, false,
-                                     ExtensionSyncData::BOOLEAN_UNSET);
 
   // 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()));
   ASSERT_FALSE(service()->IsExtensionEnabled(good0));
 }
@@ skipping 23 matching lines @@
   // Disable extension 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));
 
   // Now sync data comes in that says to disable good0. This should be
   // ignored.
-  ExtensionSyncData disable_good_crx(*extension, false, false, false,
-                                     ExtensionSyncData::BOOLEAN_FALSE);
+  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()));
 
   // The extension was enabled locally before the sync data arrived, so it
@@ skipping 536 matching lines @@
     EXPECT_TRUE(service()->IsExtensionEnabled(good_crx));
     EXPECT_TRUE(extensions::util::HasSetAllowedScriptingOnAllUrls(
         good_crx, profile()));
     EXPECT_EQ(kDefaultAllowedScripting,
               extensions::util::AllowedScriptingOnAllUrls(good_crx, profile()));
   }
 
   EXPECT_FALSE(service()->pending_extension_manager()->IsIdPending(good_crx));
 }
 
+TEST_F(ExtensionServiceTest, ProcessSyncDataNewExtension) {
+  InitializeEmptyExtensionService();
+  InitializeExtensionSyncService();
+  syncer::FakeSyncChangeProcessor processor;
+  extension_sync_service()->MergeDataAndStartSyncing(
+      syncer::EXTENSIONS,
+      syncer::SyncDataList(),
+      scoped_ptr<syncer::SyncChangeProcessor>(
+          new syncer::FakeSyncChangeProcessor),
+      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+
+  sync_pb::EntitySpecifics specifics;
+  sync_pb::ExtensionSpecifics* ext_specifics = specifics.mutable_extension();
+  ext_specifics->set_id(good_crx);
+  ext_specifics->set_version(base::Version("1").GetString());
+
+  const base::FilePath path = data_dir().AppendASCII("good.crx");
+  const ExtensionPrefs* prefs = ExtensionPrefs::Get(profile());
+
+  {
+    ext_specifics->set_enabled(true);
+    ext_specifics->set_disable_reasons(0);
+
+    syncer::SyncData sync_data =
+        syncer::SyncData::CreateLocalData(good_crx, "Name", specifics);
+    syncer::SyncChange sync_change(FROM_HERE,
+                                   syncer::SyncChange::ACTION_UPDATE,
+                                   sync_data);
+    syncer::SyncChangeList list(1);
+    list[0] = sync_change;
+    extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
+
+    ASSERT_TRUE(service()->pending_extension_manager()->IsIdPending(good_crx));
+    UpdateExtension(good_crx, path, ENABLED);
+    // Permissions should have been granted during installation.
+    EXPECT_FALSE(prefs->GetGrantedPermissions(good_crx)->IsEmpty());
+    ASSERT_FALSE(service()->pending_extension_manager()->IsIdPending(good_crx));
+  }
+  UninstallExtension(good_crx, false);
+  {
+    ext_specifics->set_enabled(false);
+    ext_specifics->set_disable_reasons(Extension::DISABLE_USER_ACTION);
+
+    syncer::SyncData sync_data =
+        syncer::SyncData::CreateLocalData(good_crx, "Name", specifics);
+    syncer::SyncChange sync_change(FROM_HERE,
+                                   syncer::SyncChange::ACTION_UPDATE,
+                                   sync_data);
+    syncer::SyncChangeList list(1);
+    list[0] = sync_change;
+    extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
+
+    ASSERT_TRUE(service()->pending_extension_manager()->IsIdPending(good_crx));
+    UpdateExtension(good_crx, path, DISABLED);
+    // Even if the extension came in disabled, its permissions should have been
+    // granted (the user already approved them on another machine).
+    EXPECT_FALSE(prefs->GetGrantedPermissions(good_crx)->IsEmpty());
+    ASSERT_FALSE(service()->pending_extension_manager()->IsIdPending(good_crx));
+  }
+  UninstallExtension(good_crx, false, Extension::DISABLED);
+  {
+    ext_specifics->set_enabled(false);
+    // Legacy case: No disable reasons are set.

[not at google - send to devlin, 2015/05/19 14:32:10]

"The disable reason should be set to "DISABLE_UNKNOWN_FROM_SYNC".

[Marc Treib, 2015/05/19 15:30:56]

Done.

+    ext_specifics->set_disable_reasons(0);
+
+    syncer::SyncData sync_data =
+        syncer::SyncData::CreateLocalData(good_crx, "Name", specifics);
+    syncer::SyncChange sync_change(FROM_HERE,
+                                   syncer::SyncChange::ACTION_UPDATE,
+                                   sync_data);
+    syncer::SyncChangeList list(1);
+    list[0] = sync_change;
+    extension_sync_service()->ProcessSyncChanges(FROM_HERE, list);
+
+    ASSERT_TRUE(service()->pending_extension_manager()->IsIdPending(good_crx));
+    UpdateExtension(good_crx, path, DISABLED);
+    EXPECT_EQ(Extension::DISABLE_UNKNOWN_FROM_SYNC,
+              prefs->GetDisableReasons(good_crx));

[not at google - send to devlin, 2015/05/19 14:32:10]

Why is this the only place that GetDisableReasons is checked? I.e. could there
be a similar line up at ~6578 which has
EXPECT_EQ(Extension::DISABLE_USER_ACTION, prefs->GetDisableReasons(good_crx))?

[Marc Treib, 2015/05/19 15:30:56]

Good point; added.

+    EXPECT_FALSE(prefs->GetGrantedPermissions(good_crx)->IsEmpty());
+    ASSERT_FALSE(service()->pending_extension_manager()->IsIdPending(good_crx));
+  }
+  UninstallExtension(good_crx, false, Extension::DISABLED);
+}
+
 TEST_F(ExtensionServiceTest, ProcessSyncDataTerminatedExtension) {
   InitializeExtensionServiceWithUpdater();
   InitializeExtensionSyncService();
   syncer::FakeSyncChangeProcessor processor;
   extension_sync_service()->MergeDataAndStartSyncing(
       syncer::EXTENSIONS,
       syncer::SyncDataList(),
       scoped_ptr<syncer::SyncChangeProcessor>(
           new syncer::FakeSyncChangeProcessor),
       scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
@@ skipping 1276 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());
 }