Block tabs.executeScript() from executing until user grants permission

chrome/browser/extensions/script_executor.cc

Index: chrome/browser/extensions/script_executor.cc
diff --git a/chrome/browser/extensions/script_executor.cc b/chrome/browser/extensions/script_executor.cc
index b7711cbc541e1452992eca8978b9dd8225f9e7ad..be510b5bf4a16f2a1be80dd692e036303e9ef4cb 100644
--- a/chrome/browser/extensions/script_executor.cc
+++ b/chrome/browser/extensions/script_executor.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/extensions/script_executor.h"
 
+#include "base/bind.h"
 #include "base/callback.h"
 #include "base/logging.h"
 #include "base/pickle.h"
@@ -126,17 +127,6 @@ void ScriptExecutor::ExecuteScript(const std::string& extension_id,
                                    bool user_gesture,
                                    ScriptExecutor::ResultType result_type,
                                    const ExecuteScriptCallback& callback) {
-  ActiveScriptController* active_script_controller =
-      ActiveScriptController::GetForWebContents(web_contents_);
-  content::NavigationEntry* visible_entry =
-      web_contents_->GetController().GetVisibleEntry();
-  if (active_script_controller && visible_entry) {
-    // TODO(rdevlin.cronin): Now, this is just a notification. Soon, it should
-    // block until the user gives the OK to execute.
-    active_script_controller->NotifyScriptExecuting(extension_id,
-                                                    visible_entry->GetPageID());
-  }
-
   ExtensionMsg_ExecuteCode_Params params;
   params.request_id = next_request_id_++;
   params.extension_id = extension_id;
@@ -151,6 +141,31 @@ void ScriptExecutor::ExecuteScript(const std::string& extension_id,
   params.wants_result = (result_type == JSON_SERIALIZED_RESULT);
   params.user_gesture = user_gesture;
 
+  ActiveScriptController* active_script_controller =
+      ActiveScriptController::GetForWebContents(web_contents_);
+  content::NavigationEntry* visible_entry =
+      web_contents_->GetController().GetVisibleEntry();
+  if (active_script_controller && visible_entry) {
+    // The base::Unretained(this) is safe, because this and the
+    // ActiveScriptController are both attached to the TabHelper. Thus, if the
+    // ActiveScriptController is still alive to invoke the callback, this is
+    // alive, too.
+    active_script_controller->GetPermissionForInjection(
+        extension_id,
+        visible_entry->GetPageID(),
+        scoped_ptr<const base::Closure>(new base::Closure(
+            base::Bind(&ScriptExecutor::ExecuteScriptHelper,
+                       base::Unretained(this),
+                       params,

[not at google - send to devlin, 2014/05/15 00:12:36]

yes, it would make sense to Pass() this.

[Devlin, 2014/05/15 17:45:59]

Done.

+                       callback))));
+  } else {
+    ExecuteScriptHelper(params, callback);
+  }
+}
+
+void ScriptExecutor::ExecuteScriptHelper(
+    const ExtensionMsg_ExecuteCode_Params& params,
+    const ExecuteScriptCallback& callback) {
   // Handler handles IPCs and deletes itself on completion.
   new Handler(script_observers_, web_contents_, params, callback);
 }