Do not grant permissions to extensions that come in via Sync disabled due to a permission increase

chrome/browser/extensions/extension_service_unittest.cc

Index: chrome/browser/extensions/extension_service_unittest.cc
diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc
index 7d8042d0bbf90209d00a08cf26ad6f936620fb1e..2e0a50e85661d464180c91ba951c631921647fd7 100644
--- a/chrome/browser/extensions/extension_service_unittest.cc
+++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -6540,60 +6540,41 @@ TEST_F(ExtensionServiceTest, ProcessSyncDataNewExtension) {
   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);
-    EXPECT_EQ(0, prefs->GetDisableReasons(good_crx));
-    // Permissions should have been granted during installation.
-    scoped_refptr<PermissionSet> permissions(
-        prefs->GetGrantedPermissions(good_crx));
-    EXPECT_FALSE(permissions->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);
-    EXPECT_EQ(Extension::DISABLE_USER_ACTION,
-              prefs->GetDisableReasons(good_crx));
-    // Even if the extension came in disabled, its permissions should have been
+  struct TestCase {
+    const char* name;  // For failure output only.
+    bool sync_enabled;  // The "enabled" flag coming in from Sync.
+    int sync_disable_reasons;  // The disable reason(s) coming in from Sync.
+    // The disable reason(s) that should be set on the installed extension.
+    // This will usually be the same as |sync_disable_reasons|, but see the
+    // "Legacy" case.
+    int expect_disable_reasons;
+    // Whether the extension's permissions should be auto-granted during
+    // installation.
+    bool expect_permissions_granted;
+  } test_cases[] = {
+    // Standard case: Extension comes in enabled; permissions should be granted
+    // during installation.
+    { "Standard", true, 0, 0, true },
+    // If the extension comes in disabled, its permissions should still be
     // granted (the user already approved them on another machine).
-    scoped_refptr<PermissionSet> permissions(
-        prefs->GetGrantedPermissions(good_crx));
-    EXPECT_FALSE(permissions->IsEmpty());
-    ASSERT_FALSE(service()->pending_extension_manager()->IsIdPending(good_crx));
-  }
-  UninstallExtension(good_crx, false, Extension::DISABLED);
-  {
-    ext_specifics->set_enabled(false);
+    { "Disabled", false, Extension::DISABLE_USER_ACTION,
+      Extension::DISABLE_USER_ACTION, true },
     // Legacy case (<M45): No disable reasons come in from Sync (see
     // crbug.com/484214). After installation, the reason should be set to
     // DISABLE_UNKNOWN_FROM_SYNC.
-    ext_specifics->set_disable_reasons(0);
+    { "Legacy", false, 0, Extension::DISABLE_UNKNOWN_FROM_SYNC, true },
+    // If the extension came in disabled due to a permissions increase, then the
+    // user has *not* approved the permissions, and they shouldn't be granted.
+    // crbug.com/484214
+    { "PermissionsIncrease", false, Extension::DISABLE_PERMISSIONS_INCREASE,
+      Extension::DISABLE_PERMISSIONS_INCREASE, false },
+  };
+
+  for (const TestCase& test_case : test_cases) {
+    SCOPED_TRACE(test_case.name);
+
+    ext_specifics->set_enabled(test_case.sync_enabled);
+    ext_specifics->set_disable_reasons(test_case.sync_disable_reasons);
 
     syncer::SyncData sync_data =
         syncer::SyncData::CreateLocalData(good_crx, "Name", specifics);
@@ -6605,15 +6586,20 @@ TEST_F(ExtensionServiceTest, ProcessSyncDataNewExtension) {
     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,
+    UpdateExtension(good_crx, path, test_case.sync_enabled ? ENABLED
+                                                           : DISABLED);
+    EXPECT_EQ(test_case.expect_disable_reasons,
               prefs->GetDisableReasons(good_crx));
     scoped_refptr<PermissionSet> permissions(
         prefs->GetGrantedPermissions(good_crx));
-    EXPECT_FALSE(permissions->IsEmpty());
+    EXPECT_EQ(test_case.expect_permissions_granted, !permissions->IsEmpty());
     ASSERT_FALSE(service()->pending_extension_manager()->IsIdPending(good_crx));
+
+    // Remove the extension again, so we can install it again for the next case.
+    UninstallExtension(good_crx, false,
+                       test_case.sync_enabled ? Extension::ENABLED
+                                              : Extension::DISABLED);
   }
-  UninstallExtension(good_crx, false, Extension::DISABLED);
 }
 
 TEST_F(ExtensionServiceTest, ProcessSyncDataTerminatedExtension) {