Run any pending injections in ActiveScriptController if permission is granted

If ActiveTab permission is granted to an extension through a means other than
ActiveScriptController, have ActiveScriptController run any pending tasks for
that extension.

BUG=362353

Review URL: https://codereview.chromium.org/296483011

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@272413 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/extensions/active_script_controller.cc

@@ -101,6 +101,11 @@ void ActiveScriptController::RequestScriptInjection(
     LocationBarController::NotifyChange(web_contents());
 }
 
+void ActiveScriptController::OnActiveTabPermissionGranted(
+    const Extension* 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.
@@ -149,17 +154,31 @@ ExtensionAction* ActiveScriptController::GetActionForExtension(
 
 LocationBarController::Action ActiveScriptController::OnClicked(
     const Extension* 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);
   PendingRequestMap::iterator iter =
       pending_requests_.find(extension->id());
-  DCHECK(iter != pending_requests_.end());
+  if (iter == pending_requests_.end())
+    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();
 
@@ -173,6 +192,8 @@ LocationBarController::Action ActiveScriptController::OnClicked(
 
   // 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);
 
@@ -187,14 +208,6 @@ LocationBarController::Action ActiveScriptController::OnClicked(
 
   // 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(

chrome/browser/extensions/active_script_controller.h

@@ -56,6 +56,11 @@ class ActiveScriptController : public LocationBarController::ActionProvider,
                               int page_id,
                               const base::Closure& callback);
 
+  // Notifies the ActiveScriptController that an extension has been granted
+  // active tab permissions. This will run any pending injections for that
+  // extension.
+  void OnActiveTabPermissionGranted(const Extension* extension);
+
   // Notifies the ActiveScriptController of detected ad injection.
   void OnAdInjectionDetected(const std::vector<std::string> ad_injectors);
 
@@ -80,6 +85,9 @@ class ActiveScriptController : public LocationBarController::ActionProvider,
   typedef std::vector<PendingRequest> PendingRequestList;
   typedef std::map<std::string, PendingRequestList> PendingRequestMap;
 
+  // Runs any pending injections for the corresponding extension.
+  void RunPendingForExtension(const Extension* extension);
+
   // Handles the NotifyExtensionScriptExecution message.
   void OnNotifyExtensionScriptExecution(const std::string& extension_id,
                                         int page_id);

chrome/browser/extensions/active_script_controller_unittest.cc

@@ -276,11 +276,27 @@ TEST_F(ActiveScriptControllerUnitTest, ActiveScriptsUseActiveTabPermissions) {
 
   // Since we have active tab permissions, we shouldn't need user consent
   // anymore.
-  EXPECT_FALSE(
-      controller()->RequiresUserConsentForScriptInjection(extension));
+  EXPECT_FALSE(controller()->RequiresUserConsentForScriptInjection(extension));
+
+  // Also test that granting active tab runs any pending tasks.
+  NavigateAndCommit(GURL("https://www.google.com"));
+  // Navigating should mean we need permission again.
+  EXPECT_TRUE(controller()->RequiresUserConsentForScriptInjection(extension));
 
-  // TODO(rdevlin.cronin): We should also implement/test that granting active
-  // tab permissions automatically runs any pending injections.
+  controller()->RequestScriptInjection(
+      extension,
+      GetPageId(),
+      GetExecutionCallbackForExtension(extension->id()));
+  EXPECT_TRUE(controller()->GetActionForExtension(extension));
+  EXPECT_EQ(0u, GetExecutionCountForExtension(extension->id()));
+
+  // Grant active tab.
+  active_tab_permission_granter->GrantIfRequested(extension);
+
+  // The pending injections should have run since active tab permission was
+  // granted.
+  EXPECT_EQ(1u, GetExecutionCountForExtension(extension->id()));
+  EXPECT_FALSE(controller()->GetActionForExtension(extension));
 }
 
 }  // namespace extensions

chrome/browser/extensions/active_tab_permission_granter.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/extensions/active_tab_permission_granter.h"
 
+#include "chrome/browser/extensions/active_script_controller.h"
 #include "chrome/browser/profiles/profile.h"
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/navigation_entry.h"
@@ -83,6 +84,13 @@ void ActiveTabPermissionGranter::GrantIfRequested(const Extension* extension) {
           tab_id_,
           extension->id(),
           new_hosts));
+      // If more things ever need to know about this, we should consider making
+      // an observer class.
+      // It's important that this comes after the IPC is sent to the renderer,
+      // so that any tasks executing in the renderer occur after it has the
+      // updated permissions.
+      ActiveScriptController::GetForWebContents(web_contents())
+          ->OnActiveTabPermissionGranted(extension);
     }
   }
 }