Allow re-enabling of DISABLE_NOT_VERIFIED extensions if appropriate

chrome/browser/extensions/install_verifier.cc

Index: chrome/browser/extensions/install_verifier.cc
diff --git a/chrome/browser/extensions/install_verifier.cc b/chrome/browser/extensions/install_verifier.cc
index 88b8cd62706211ed785c6b72be0da4c1ab6eb3f5..53fb8ec9419d5147b54647c4cbe8dd74fdbb1732 100644
--- a/chrome/browser/extensions/install_verifier.cc
+++ b/chrome/browser/extensions/install_verifier.cc
@@ -82,35 +82,10 @@ void InstallVerifier::Init() {
   } else {
     UMA_HISTOGRAM_BOOLEAN("InstallVerifier.InitNoSignature", true);
   }
+}
 
-  if (!signature_.get() && ShouldFetchSignature()) {
-    // We didn't have any signature but are in fetch mode, so do a request for
-    // a signature if needed.
-    scoped_ptr<ExtensionPrefs::ExtensionsInfo> all_info =
-        prefs_->GetInstalledExtensionsInfo();
-    ExtensionIdSet to_add;
-    if (all_info.get()) {
-      for (ExtensionPrefs::ExtensionsInfo::const_iterator i = all_info->begin();
-           i != all_info->end(); ++i) {
-        const ExtensionInfo& info = **i;
-        const base::DictionaryValue* dictionary = info.extension_manifest.get();
-        if (dictionary && ManifestURL::UpdatesFromGallery(dictionary)) {
-          Manifest manifest(info.extension_location,
-                            scoped_ptr<DictionaryValue>(
-                                dictionary->DeepCopy()));
-          if (manifest.is_extension())
-            to_add.insert(info.extension_id);
-        }
-      }
-    }
-    if (to_add.empty()) {
-      // Write an empty signature so we don't have to redo this at next Init.
-      signature_.reset(new InstallSignature());
-      SaveToPrefs();
-    } else {
-      AddMany(to_add, AddResultCallback());
-    }
-  }
+bool InstallVerifier::NeedsBootstrap() {
+  return signature_.get() == NULL && ShouldFetchSignature();
 }
 
 void InstallVerifier::Add(const std::string& id,
@@ -122,8 +97,11 @@ void InstallVerifier::Add(const std::string& id,
 
 void InstallVerifier::AddMany(const ExtensionIdSet& ids,
                               const AddResultCallback& callback) {
-  if (!ShouldFetchSignature())
+  if (!ShouldFetchSignature()) {
+    if (!callback.is_null())
+      callback.Run(true);
     return;
+  }
 
   if (signature_.get()) {
     ExtensionIdSet not_allowed_yet =
@@ -202,7 +180,7 @@ bool InstallVerifier::MustRemainDisabled(const Extension* extension,
 
   bool verified =
       FromStore(extension) &&
-      IsVerified(extension->id()) &&
+      (signature_.get() == NULL || IsVerified(extension->id())) &&

[Finnur, 2013/12/16 14:21:51]

Wait... it is verified if there's no signature? Where you testing something or
is this actually intended? If so, document why this makes sense...

[asargent_no_longer_on_chrome, 2013/12/16 18:47:10]

It's temporarily allowed - the idea is that if someone gets thrown into
enforcement mode but has never done a check to the server before, we want to
hold off on disabling their extensions until we can successfully contact the
server once. That's what the "NeedsBootstrap" method above is for. 

Anyhow, I'll add a comment. (We may eventually need to make this more strict to
prevent abuse since there is an obvious attack vector here, but in the short
term we want to avoid the 100% false positive rate for existing profiles)

       !ContainsKey(InstallSigner::GetForcedNotFromWebstore(), extension->id());
 
   if (!verified) {
@@ -347,16 +325,8 @@ void InstallVerifier::SignatureCallback(
           provisional_, signature_->ids);
     }
 
-    // See if we were able to sign all of |ids|.
-    ExtensionIdSet not_allowed =
-        base::STLSetDifference<ExtensionIdSet>(operation->ids,
-                                               signature_->ids);
-
-    UMA_HISTOGRAM_COUNTS_100("InstallVerifier.CallbackNotAllowed",
-                             not_allowed.size());

[asargent_no_longer_on_chrome, 2013/12/14 00:42:39]

note: I removed this block of code because it was not correct and the histogram
would be misleading - it's possible the operation could have been to remove
operation->ids() from the signature, not add them. 

-
     if (!operation->callback.is_null())
-      operation->callback.Run(not_allowed.empty());

[asargent_no_longer_on_chrome, 2013/12/14 00:42:39]

This got updated to return the success of the network request instead of whether
we thought we properly signed all of the ids that may have been requested to be
added. 

It turns out callers are more interested in the network request success/failure.

+      operation->callback.Run(success);
   }
 
   if (!operation_queue_.empty())