Run any pending injections in ActiveScriptController if permission is granted

chrome/browser/extensions/active_script_controller.cc

 // Copyright 2014 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 "chrome/browser/extensions/active_script_controller.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/metrics/histogram.h"
@@ skipping 83 matching lines @@
   CHECK(extension);
   PendingRequestList& list = pending_requests_[extension->id()];
   list.push_back(PendingRequest(callback, page_id));
 
   // If this was the first entry, notify the location bar that there's a new
   // icon.
   if (list.size() == 1u)
     LocationBarController::NotifyChange(web_contents());
 }
 
+void ActiveScriptController::OnActiveTabPermissionGranted(
+    const Extension* extension) {
+  DCHECK(extension);
+  RunPendingForExtension(extension);
+}
+
 void ActiveScriptController::OnAdInjectionDetected(
     const std::vector<std::string> ad_injectors) {
   // We're only interested in data if there are ad injectors detected.
   if (ad_injectors.empty())
     return;
 
   size_t num_preventable_ad_injectors =
       base::STLSetIntersection<std::set<std::string> >(
           ad_injectors, permitted_extensions_).size();
 
@@ skipping 29 matching lines @@
         make_scoped_ptr(new ExtensionIconSet(action_info->default_icon)));
   }
 
   active_script_actions_[extension->id()] = action;
   return action.get();
 }
 
 LocationBarController::Action ActiveScriptController::OnClicked(
     const Extension* extension) {
   DCHECK(extension);
+  DCHECK(ContainsKey(pending_requests_, extension->id()));
+  RunPendingForExtension(extension);
+  return LocationBarController::ACTION_NONE;
+}
+
+void ActiveScriptController::OnNavigated() {
+  LogUMA();
+  permitted_extensions_.clear();
+  pending_requests_.clear();
+}
+
+void ActiveScriptController::RunPendingForExtension(
+    const Extension* extension) {
+  DCHECK(extension);

[not at google - send to devlin, 2014/05/22 20:44:49]

you already DCHECK the extension at all call sites. given it's actually only
this method that cares about it, I'd just have it here (if at all, realy) and
not in those other 2 methods.

[Devlin, 2014/05/22 22:12:36]

Fair enough - maybe I was a bit DCHECK() happy. :)

   PendingRequestMap::iterator iter =
       pending_requests_.find(extension->id());
-  DCHECK(iter != pending_requests_.end());
+  if (iter ==pending_requests_.end())

[not at google - send to devlin, 2014/05/22 20:44:49]

s/ ==/ == /

[Devlin, 2014/05/22 22:12:36]

Done.

+    return;
 
   content::NavigationEntry* visible_entry =
       web_contents()->GetController().GetVisibleEntry();
   // Refuse to run if there's no visible entry, because we have no idea of
   // determining if it's the proper page. This should rarely, if ever, happen.
   if (!visible_entry)
-    return LocationBarController::ACTION_NONE;
+    return;
 
   int page_id = visible_entry->GetPageID();
 
   // We add this to the list of permitted extensions and erase pending entries
   // *before* running them to guard against the crazy case where running the
   // callbacks adds more entries.
   permitted_extensions_.insert(extension->id());
   PendingRequestList requests;
   iter->second.swap(requests);
   pending_requests_.erase(extension->id());
 
   // Clicking to run the extension counts as granting it permission to run on
   // the given tab.
+  // The extension may already have active tab at this point, but granting
+  // it twice is essentially a no-op.
   TabHelper::FromWebContents(web_contents())->
       active_tab_permission_granter()->GrantIfRequested(extension);
 
   // Run all pending injections for the given extension.
   for (PendingRequestList::iterator request = requests.begin();
        request != requests.end();
        ++request) {
     // Only run if it's on the proper page.
     if (request->page_id == page_id)
       request->closure.Run();
   }
 
   // Inform the location bar that the action is now gone.
   LocationBarController::NotifyChange(web_contents());
-
-  return LocationBarController::ACTION_NONE;
-}
-
-void ActiveScriptController::OnNavigated() {
-  LogUMA();
-  permitted_extensions_.clear();
-  pending_requests_.clear();
 }
 
 void ActiveScriptController::OnNotifyExtensionScriptExecution(
     const std::string& extension_id,
     int page_id) {
   if (!Extension::IdIsValid(extension_id)) {
     NOTREACHED() << "'" << extension_id << "' is not a valid id.";
     return;
   }
 
@@ skipping 33 matching lines @@
     UMA_HISTOGRAM_COUNTS_100(
         "Extensions.ActiveScriptController.PermittedExtensions",
         permitted_extensions_.size());
     UMA_HISTOGRAM_COUNTS_100(
         "Extensions.ActiveScriptController.DeniedExtensions",
         pending_requests_.size());
   }
 }
 
 }  // namespace extensions