[Extensions] Fix ExternalInstallManager crash

A crash could occur in ExternalInstallManager if an extension was
unacknowledged (and thus in
ExternalInstallManager::unacknowledged_ids_), had reached the maximum
number of prompts (3), and hadn't had an error shown for it during the
session.

If this happened, the entry would remain in unacknowledged_ids_ and the
extension wouldn't have an entry in shown_ids_, so that if the extension
later became loaded (or another external extension was installed), the
extension would no longer be present in disabled extensions in the
registry. This caused a failed CHECK() in
ExternalInstallManager::UpdateExternalExtensionAlert().

Fix this case by properly erasing the entry from unacknowledged_ids_
when the extension reaches the prompt count.

Add a unittest to cover the case.

Bug: 736292

Change-Id: If7cbf87e3a757e66930c0cfda40ed229a2e37166
Reviewed-on: https://chromium-review.googlesource.com/1012491Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551772}

chrome/browser/extensions/extension_service_unittest.cc

@@ -4475,6 +4475,15 @@ TEST_F(ExtensionServiceTest, ExternalExtensionRemainsDisabledIfIgnored) {
   EXPECT_TRUE(registry()->disabled_extensions().Contains(good_crx));
   EXPECT_EQ(extensions::disable_reason::DISABLE_EXTERNAL_EXTENSION,
             prefs->GetDisableReasons(good_crx));
+
+  // Then re-enabling the extension (or otherwise causing the alert to be
+  // updated again) should work. Regression test for https://crbug.com/736292.
+  {
+    extensions::TestExtensionRegistryObserver registry_observer(registry());
+    service()->EnableExtension(good_crx);
+    registry_observer.WaitForExtensionLoaded();
+    base::RunLoop().RunUntilIdle();
+  }
 }
 
 #if !defined(OS_CHROMEOS)

chrome/browser/extensions/external_install_manager.cc

@@ -139,7 +139,10 @@ void ExternalInstallManager::UpdateExternalExtensionAlert() {
       ExtensionRegistry::Get(browser_context_)->disabled_extensions();
   const ExtensionSet& blocked_extensions =
       ExtensionRegistry::Get(browser_context_)->blocked_extensions();
-  for (const auto& id : unacknowledged_ids_) {
+
+  // The list of ids can be mutated during this loop, so make a copy.
+  const std::set<ExtensionId> ids_copy = unacknowledged_ids_;
+  for (const auto& id : ids_copy) {
     if (base::ContainsKey(errors_, id) || shown_ids_.count(id) > 0)
       continue;
 
@@ -157,6 +160,7 @@ void ExternalInstallManager::UpdateExternalExtensionAlert() {
       // Stop prompting for this extension and record metrics.
       extension_prefs_->AcknowledgeExternalExtension(id);
       LogExternalExtensionEvent(extension, EXTERNAL_EXTENSION_IGNORED);
+      unacknowledged_ids_.erase(id);
       continue;
     }
 
@@ -172,6 +176,7 @@ void ExternalInstallManager::UpdateExternalExtensionAlert() {
 
 void ExternalInstallManager::AcknowledgeExternalExtension(
     const std::string& id) {
+  unacknowledged_ids_.erase(id);
   extension_prefs_->AcknowledgeExternalExtension(id);
   UpdateExternalExtensionAlert();
 }