Support "always allow" for runtime script execution

chrome/browser/extensions/extension_context_menu_model.cc

Index: chrome/browser/extensions/extension_context_menu_model.cc
diff --git a/chrome/browser/extensions/extension_context_menu_model.cc b/chrome/browser/extensions/extension_context_menu_model.cc
index 3334fd9ccc063a8409a9b146992717331460d3b8..6b914fa86130babe60ae9f508bf234b06f1aa6ed 100644
--- a/chrome/browser/extensions/extension_context_menu_model.cc
+++ b/chrome/browser/extensions/extension_context_menu_model.cc
@@ -6,6 +6,7 @@
 
 #include "base/prefs/pref_service.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/extensions/active_script_controller.h"
 #include "chrome/browser/extensions/api/extension_action/extension_action_api.h"
 #include "chrome/browser/extensions/extension_action.h"
 #include "chrome/browser/extensions/extension_action_manager.h"
@@ -26,6 +27,7 @@
 #include "extensions/browser/management_policy.h"
 #include "extensions/browser/uninstall_reason.h"
 #include "extensions/common/extension.h"
+#include "extensions/common/manifest_constants.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -79,8 +81,7 @@ bool ExtensionContextMenuModel::IsCommandIdEnabled(int command_id) const {
     // homepage, we just disable this menu item.
     return extensions::ManifestURL::GetHomepageURL(extension).is_valid();
   } else if (command_id == INSPECT_POPUP) {
-    WebContents* web_contents =
-        browser_->tab_strip_model()->GetActiveWebContents();
+    WebContents* web_contents = GetActiveWebContents();
     if (!web_contents)
       return false;
 
@@ -113,6 +114,15 @@ void ExtensionContextMenuModel::ExecuteCommand(int command_id,
       browser_->OpenURL(params);
       break;
     }
+    case ALWAYS_RUN: {
+      WebContents* web_contents = GetActiveWebContents();
+      if (!web_contents)

[not at google - send to devlin, 2014/08/11 22:59:30]

please phrase this

if (web_contents) {
  extensions::ActiveScriptController::GetForWebContents(web_contents)
      ->AlwaysRunOnVisibleHost(extension);
}
break;

[gpdavis, 2014/08/12 00:52:05]

Solid.  I think I'm starting to get used to where early returning is
appropriate.

[not at google - send to devlin, 2014/08/12 19:49:26]

I admit that a lot of it is aesthetic, but here, returning in the middle of a
switch statement in one of the cases is a bit confusing.

+        return;
+
+      extensions::ActiveScriptController::GetForWebContents(web_contents)
+          ->AlwaysRunOnVisibleHost(extension);
+      break;
+    }
     case CONFIGURE:
       DCHECK(!extensions::ManifestURL::GetOptionsPage(extension).is_empty());
       extensions::ExtensionTabUtil::OpenOptionsPage(extension, browser_);
@@ -177,6 +187,15 @@ void ExtensionContextMenuModel::InitMenu(const Extension* extension) {
   base::ReplaceChars(extension_name, "&", "&&", &extension_name);
   AddItem(NAME, base::UTF8ToUTF16(extension_name));
   AddSeparator(ui::NORMAL_SEPARATOR);
+
+  // Add the "Always Allow" item for adding persisted permissions for script
+  // injections if there is an active action for this extension (this action).
+  WebContents* web_contents = GetActiveWebContents();
+  if (web_contents && extensions::ActiveScriptController::GetForWebContents(
+      web_contents)->HasActiveScriptAction(extension)) {
+    AddItemWithStringId(ALWAYS_RUN, IDS_EXTENSIONS_ALWAYS_RUN);

[not at google - send to devlin, 2014/08/11 22:59:30]

This is actually really subtle. The way it's written now, if the extension is
showing a script badge then it's going to get this menu item, *whether or not
the context menu is actually on the badge*.

E.g. you'll see "always run" on the browser action context menu.

Fixing this is probably going to be a PITA, threading through knowledge of where
in the UI this ExtensionContextMenuModel is hosted, which I don't think it's
done.

I'm also not sure it really matters, particularly given eventually there will
only be 1 action anyway. Plus, this UI is just a prototype.

So no need to change this code, but you might want to mention it in a comment,
like

// Add the "always allow" if there is a script injection request for
// this extension. Note that this will add it to *all* extension action context
// menus, not just the one attached to the script injection request icon,
// but that's ok.

[gpdavis, 2014/08/12 00:52:05]

Wow, that is subtle.  Nice catch.  I added the comment.

+  }
+
   AddItemWithStringId(CONFIGURE, IDS_EXTENSIONS_OPTIONS_MENU_ITEM);
   AddItem(UNINSTALL, l10n_util::GetStringUTF16(IDS_EXTENSIONS_UNINSTALL));
   if (extension_action_manager->GetBrowserAction(*extension))
@@ -190,3 +209,7 @@ const Extension* ExtensionContextMenuModel::GetExtension() const {
       extensions::ExtensionSystem::Get(profile_)->extension_service();
   return extension_service->GetExtensionById(extension_id_, false);
 }
+
+content::WebContents* ExtensionContextMenuModel::GetActiveWebContents() const {
+  return browser_->tab_strip_model()->GetActiveWebContents();
+}