Do not unblocklist a remotely disabled extension.

When the blocklist updates, the blacklist state
map may be empty or does not include the remotely
disabled extension. This means that not even a
"NOT_MALWARE" is sent for the extension. The
extension then gets put into no_longer_blocked
which is not what is expected.

Bug: 1107040
Change-Id: Ia58f0d7b831b54b02843d5c03f9179e84ec11ac5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2305598
Commit-Queue: Bettina Dea <bdea@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790076}

chrome/browser/extensions/extension_service.cc

@@ -2135,22 +2135,27 @@ void ExtensionService::ManageBlocklist(
   std::set<std::string> blocklisted;
   ExtensionIdSet greylist;
   ExtensionIdSet unchanged;
-  for (const auto& it : state_map) {
-    // If it was previously disabled remotely for malware, do not remove from
-    // the blocklisted_extensions set. The disable reason should be removed
-    // first before updating its blocklist state.
+
+  // If it was previously disabled remotely for malware, do not remove from
+  // the blocklisted_extensions set. The disable reason should be removed
+  // first before updating its blocklist state. Thus, we add these extensions
+  // to |unchanged| set.
+  for (const auto& extension_id :
+       registry_->blocklisted_extensions().GetIDs()) {
     if (extension_prefs_->HasDisableReason(
-            it.first, disable_reason::DISABLE_REMOTELY_FOR_MALWARE)) {
-      unchanged.insert(it.first);
-      continue;
+            extension_id, disable_reason::DISABLE_REMOTELY_FOR_MALWARE)) {
+      unchanged.insert(extension_id);
     }
+  }
 
+  for (const auto& it : state_map) {
     switch (it.second) {
       case NOT_BLOCKLISTED:
         break;
 
       case BLOCKLISTED_MALWARE:
-        blocklisted.insert(it.first);
+        if (!base::Contains(unchanged, it.first))
+          blocklisted.insert(it.first);
         break;
 
       case BLOCKLISTED_SECURITY_VULNERABILITY:

chrome/browser/extensions/extension_service.h

@@ -716,6 +716,7 @@ class ExtensionService : public ExtensionServiceInterface,
   FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest,
                            DestroyingProfileClearsExtensions);
   FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest, SetUnsetBlocklistInPrefs);
+  FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest, NoUnsetBlocklistInPrefs);
   FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest,
                            BlocklistedExtensionWillNotInstall);
   FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest,

chrome/browser/extensions/extension_service_unittest.cc

@@ -3398,6 +3398,46 @@ TEST_F(ExtensionServiceTest, SetUnsetBlocklistInPrefs) {
   EXPECT_TRUE(ValidateBooleanPref(good2, kPrefBlocklist, true));
   EXPECT_FALSE(IsPrefExist("invalid_id", kPrefBlocklist));
 }
+
+// Tests that an extension that was disabled through Omaha won't be
+// re-enabled if it's not present in the Safe Browsing blocklist.
+// Regression test for https://crbug.com/1107040.
+TEST_F(ExtensionServiceTest, NoUnsetBlocklistInPrefs) {
+  TestBlocklist test_blocklist;
+  // A profile with 3 extensions installed: good0, good1, and good2.
+  // We really only care about good0 for this test since the other
+  // functionality is already tested in the above test.
+  InitializeGoodInstalledExtensionService();
+  test_blocklist.Attach(service()->blocklist_);
+  service()->Init();
+
+  EXPECT_TRUE(registry()->enabled_extensions().Contains(good0));
+  EXPECT_FALSE(registry()->blocklisted_extensions().Contains(good0));
+
+  base::Value attributes(base::Value::Type::DICTIONARY);
+  attributes.SetKey("_malware", base::Value(true));
+
+  ExtensionPrefs* prefs = ExtensionPrefs::Get(profile());
+  service()->PerformActionBasedOnOmahaAttributes(good0, attributes);
+  EXPECT_EQ(disable_reason::DISABLE_REMOTELY_FOR_MALWARE,
+            prefs->GetDisableReasons(good0));
+  EXPECT_TRUE(IsPrefExist(good0, kPrefBlocklist));
+  EXPECT_FALSE(registry()->enabled_extensions().Contains(good0));
+  EXPECT_TRUE(registry()->blocklisted_extensions().Contains(good0));
+
+  // Un-blocklist all extensions.
+  test_blocklist.Clear(false);
+  content::RunAllTasksUntilIdle();
+
+  // If the extension has a DISABLE_REMOTELY_FOR_MALWARE disable reason,
+  // the extension should still not be enabled even if it's no on the
+  // SB blocklist. This disable reason needs to be removed prior to
+  // unblocklisting/re-enabling.
+  EXPECT_FALSE(registry()->enabled_extensions().Contains(good0));
+  EXPECT_TRUE(registry()->blocklisted_extensions().Contains(good0));
+  EXPECT_TRUE(ValidateBooleanPref(good0, kPrefBlocklist, true));
+  EXPECT_FALSE(IsPrefExist(good1, kPrefBlocklist));
+}
 #endif  // defined(ENABLE_BLOCKLIST_TESTS)
 
 #if defined(ENABLE_BLOCKLIST_TESTS)