Change the extension callback system to work more like events, where we track

contexts rather than frames.

Also change the way we call through to javascript, to avoid a v8::Compile.
This is so we don't skew the histogram stats on our script cache.

BUG=?
TEST=none
Review URL: http://codereview.chromium.org/119369

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

8 files changed, 119 insertions, 126 deletions

diff --git a/chrome/renderer/extensions/bindings_utils.cc b/chrome/renderer/extensions/bindings_utils.cc
index 097c8d0..81e2058 100644
--- a/chrome/renderer/extensions/bindings_utils.cc
+++ b/chrome/renderer/extensions/bindings_utils.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/renderer/extensions/bindings_utils.h"
 
+#include "base/string_util.h"
 #include "chrome/renderer/render_view.h"
 #include "webkit/glue/webframe.h"
 
@@ -21,3 +22,25 @@ RenderView* GetRenderViewForCurrentContext() {
   DCHECK(renderview) << "Encountered a WebView without a WebViewDelegate";
   return renderview;
 }
+
+void CallFunctionInContext(v8::Handle<v8::Context> context,
+                           const std::string& function_name, int argc,
+                           v8::Handle<v8::Value>* argv) {
+  v8::Context::Scope context_scope(context);
+
+  // Look up the function name, which may be a sub-property like
+  // "chrome.handleResponse_" in the global variable.
+  v8::Local<v8::Value> value = context->Global();
+  std::vector<std::string> components;
+  SplitStringDontTrim(function_name, '.', &components);
+  for (size_t i = 0; i < components.size(); ++i) {
+    if (value->IsObject())
+      value = value->ToObject()->Get(v8::String::New(components[i].c_str()));
+  }
+  if (!value->IsFunction())
+    return;
+
+  v8::Local<v8::Function> function = v8::Local<v8::Function>::Cast(value);
+  if (!function.IsEmpty())
+    function->Call(v8::Object::New(), argc, argv);
+}
diff --git a/chrome/renderer/extensions/bindings_utils.h b/chrome/renderer/extensions/bindings_utils.h
index 7596952..b498cc5 100644
--- a/chrome/renderer/extensions/bindings_utils.h
+++ b/chrome/renderer/extensions/bindings_utils.h
@@ -8,6 +8,7 @@
 #include "app/resource_bundle.h"
 #include "base/singleton.h"
 #include "base/string_piece.h"
+#include "v8/include/v8.h"
 
 #include <string>
 
@@ -32,4 +33,11 @@ const char* GetStringResource() {
 // an error to call this when not in a V8 context.
 RenderView* GetRenderViewForCurrentContext();
 
+// Call the named javascript function with the given arguments in a context.
+// The function name should be reachable from the global object, and can be a
+// sub-property like "chrome.handleResponse_".
+void CallFunctionInContext(v8::Handle<v8::Context> context,
+                           const std::string& function_name, int argc,
+                           v8::Handle<v8::Value>* argv);
+
 #endif  // CHROME_RENDERER_EXTENSIONS_BINDINGS_UTILS_H_
diff --git a/chrome/renderer/extensions/event_bindings.cc b/chrome/renderer/extensions/event_bindings.cc
index 19a9245..576beba 100644
--- a/chrome/renderer/extensions/event_bindings.cc
+++ b/chrome/renderer/extensions/event_bindings.cc
@@ -168,19 +168,6 @@ void EventBindings::CallFunction(const std::string& function_name,
                                  int argc, v8::Handle<v8::Value>* argv) {
   for (ContextList::iterator it = GetRegisteredContexts().begin();
        it != GetRegisteredContexts().end(); ++it) {
-    DCHECK(!it->IsEmpty());
-    v8::Context::Scope context_scope(*it);
-    v8::Local<v8::Object> global = (*it)->Global();
-
-    v8::Local<v8::Script> script = v8::Script::Compile(
-        v8::String::New(function_name.c_str()));
-    v8::Local<v8::Value> function_obj = script->Run();
-    if (!function_obj->IsFunction())
-      continue;
-
-    v8::Local<v8::Function> function =
-        v8::Local<v8::Function>::Cast(function_obj);
-    if (!function.IsEmpty())
-      function->Call(v8::Object::New(), argc, argv);
+    CallFunctionInContext(*it, function_name, argc, argv);
   }
 }
diff --git a/chrome/renderer/extensions/extension_api_client_unittest.cc b/chrome/renderer/extensions/extension_api_client_unittest.cc
index ddc7c42..dd95e83 100644
--- a/chrome/renderer/extensions/extension_api_client_unittest.cc
+++ b/chrome/renderer/extensions/extension_api_client_unittest.cc
@@ -91,9 +91,8 @@ TEST_F(ExtensionAPIClientTest, CallbackDispatching) {
   ASSERT_TRUE(callback_id >= 0);
 
   // Now send the callback a response
-  ExtensionProcessBindings::CallContext call(GetMainFrame(), "CreateTab");
-  ExtensionProcessBindings::ExecuteResponseInFrame(
-    &call, callback_id, true, "{\"foo\":\"bar\"}", "");
+  ExtensionProcessBindings::HandleResponse(
+      callback_id, true, "{\"foo\":\"bar\"}", "");
 
   // And verify that it worked
   ASSERT_EQ("pass", GetConsoleMessage());
diff --git a/chrome/renderer/extensions/extension_process_bindings.cc b/chrome/renderer/extensions/extension_process_bindings.cc
index 6057a3e..1ce7ea2 100644
--- a/chrome/renderer/extensions/extension_process_bindings.cc
+++ b/chrome/renderer/extensions/extension_process_bindings.cc
@@ -5,6 +5,7 @@
 #include "chrome/renderer/extensions/extension_process_bindings.h"
 
 #include "base/singleton.h"
+#include "base/stl_util-inl.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/renderer/extensions/bindings_utils.h"
@@ -33,10 +34,41 @@ const char* kExtensionDeps[] = {
 // |extension_id| -> <List of v8 Contexts for the "views" of that extension>
 typedef std::list< v8::Persistent<v8::Context> > ContextList;
 typedef std::map<std::string, ContextList> ExtensionIdContextsMap;
-struct ExtensionViewContexts {
+
+// Contains info relevant to a pending request.
+struct CallContext {
+ public :
+  CallContext(v8::Persistent<v8::Context> context, const std::string& name)
+      : context(context), name(name) {
+  }
+  v8::Persistent<v8::Context> context;
+  std::string name;
+};
+typedef std::map<int, CallContext*> PendingRequestMap;
+
+struct SingletonData {
+  std::set<std::string> function_names_;
   ExtensionIdContextsMap contexts;
+  PendingRequestMap pending_requests;
+
+  ~SingletonData() {
+    STLDeleteContainerPairSecondPointers(pending_requests.begin(),
+                                         pending_requests.end());
+  }
 };
 
+static std::set<std::string>* GetFunctionNameSet() {
+  return &Singleton<SingletonData>()->function_names_;
+}
+
+static ContextList& GetRegisteredContexts(std::string extension_id) {
+  return Singleton<SingletonData>::get()->contexts[extension_id];
+}
+
+static PendingRequestMap& GetPendingRequestMap() {
+  return Singleton<SingletonData>::get()->pending_requests;
+}
+
 class ExtensionImpl : public v8::Extension {
  public:
   ExtensionImpl() : v8::Extension(
@@ -69,18 +101,6 @@ class ExtensionImpl : public v8::Extension {
   }
 
  private:
-  struct SingletonData {
-    std::set<std::string> function_names_;
-  };
-
-  static std::set<std::string>* GetFunctionNameSet() {
-    return &Singleton<SingletonData>()->function_names_;
-  }
-
-  static ContextList& GetRegisteredContexts(std::string extension_id) {
-    return Singleton<ExtensionViewContexts>::get()->contexts[extension_id];
-  }
-
   static v8::Handle<v8::Value> RegisterExtension(const v8::Arguments& args) {
     RenderView* renderview = GetRenderViewForCurrentContext();
     DCHECK(renderview);
@@ -100,11 +120,24 @@ class ExtensionImpl : public v8::Extension {
     DCHECK_EQ(args.Length(), 1);
     DCHECK(args[0]->IsString());
 
-    std::string extension_id(*v8::String::Utf8Value(args[0]));
-    ContextList& contexts = GetRegisteredContexts(extension_id);
     v8::Local<v8::Context> current_context = v8::Context::GetCurrent();
     DCHECK(!current_context.IsEmpty());
 
+    // Remove all pending requests for this context.
+    PendingRequestMap& pending_requests = GetPendingRequestMap();
+    for (PendingRequestMap::iterator it = pending_requests.begin();
+         it != pending_requests.end(); ) {
+      PendingRequestMap::iterator current = it++;
+      if (current->second->context == current_context) {
+        current->second->context.Dispose();
+        current->second->context.Clear();
+        delete current->second;
+        pending_requests.erase(current);
+      }
+    }
+
+    std::string extension_id(*v8::String::Utf8Value(args[0]));
+    ContextList& contexts = GetRegisteredContexts(extension_id);
     ContextList::iterator it = std::find(contexts.begin(), contexts.end(),
                                          current_context);
     if (it == contexts.end()) {
@@ -149,22 +182,26 @@ class ExtensionImpl : public v8::Extension {
   static v8::Handle<v8::Value> StartRequest(const v8::Arguments& args) {
     // Get the current RenderView so that we can send a routed IPC message from
     // the correct source.
-    WebFrame* webframe = WebFrame::RetrieveFrameForCurrentContext();
     RenderView* renderview = GetRenderViewForCurrentContext();
-    if (!webframe || !renderview)
+    if (!renderview)
       return v8::Undefined();
 
     if (args.Length() != 3 || !args[0]->IsString() || !args[1]->IsInt32() ||
         !args[2]->IsBoolean())
       return v8::Undefined();
 
+    std::string name = *v8::String::AsciiValue(args.Data());
+    std::string json_args = *v8::String::Utf8Value(args[0]);
     int request_id = args[1]->Int32Value();
     bool has_callback = args[2]->BooleanValue();
 
-    renderview->SendExtensionRequest(
-        std::string(*v8::String::AsciiValue(args.Data())),
-        std::string(*v8::String::Utf8Value(args[0])),
-        request_id, has_callback, webframe);
+    v8::Persistent<v8::Context> current_context =
+        v8::Persistent<v8::Context>::New(v8::Context::GetCurrent());
+    DCHECK(!current_context.IsEmpty());
+    GetPendingRequestMap()[request_id] =
+        new CallContext(current_context, *v8::String::AsciiValue(args.Data()));
+
+    renderview->SendExtensionRequest(name, json_args, request_id, has_callback);
 
     return v8::Undefined();
   }
@@ -186,31 +223,23 @@ void ExtensionProcessBindings::RegisterExtensionContext(WebFrame* frame) {
       "chrome.self.register_();")));
 }
 
-void ExtensionProcessBindings::ExecuteResponseInFrame(
-    CallContext *call, int request_id, bool success,
-    const std::string& response,
-    const std::string& error) {
-  std::string code = "chrome.handleResponse_(";
-  code += IntToString(request_id);
-
-  code += ", '" + call->name_;
-
-  if (success)
-    code += "', true";
-  else
-    code += "', false";
-
-  code += ", '";
-  size_t offset = code.length();
-  code += response;
-  ReplaceSubstringsAfterOffset(&code, offset, "\\", "\\\\");
-  ReplaceSubstringsAfterOffset(&code, offset, "'", "\\'");
-  code += "', '";
-  offset = code.length();
-  code += error;
-  ReplaceSubstringsAfterOffset(&code, offset, "\\", "\\\\");
-  ReplaceSubstringsAfterOffset(&code, offset, "'", "\\'");
-  code += "')";
-
-  call->frame_->ExecuteScript(WebScriptSource(WebString::fromUTF8(code)));
+void ExtensionProcessBindings::HandleResponse(int request_id, bool success,
+                                              const std::string& response,
+                                              const std::string& error) {
+  CallContext* call = GetPendingRequestMap()[request_id];
+  if (!call)
+    return;  // The frame went away.
+
+  v8::HandleScope handle_scope;
+  v8::Handle<v8::Value> argv[5];
+  argv[0] = v8::Integer::New(request_id);
+  argv[1] = v8::String::New(call->name.c_str());
+  argv[2] = v8::Boolean::New(success);
+  argv[3] = v8::String::New(response.c_str());
+  argv[4] = v8::String::New(error.c_str());
+  CallFunctionInContext(call->context, "chrome.handleResponse_",
+                        arraysize(argv), argv);
+
+  GetPendingRequestMap().erase(request_id);
+  delete call;
 }
diff --git a/chrome/renderer/extensions/extension_process_bindings.h b/chrome/renderer/extensions/extension_process_bindings.h
index d5798cd..9639090 100644
--- a/chrome/renderer/extensions/extension_process_bindings.h
+++ b/chrome/renderer/extensions/extension_process_bindings.h
@@ -16,22 +16,12 @@ class WebFrame;
 
 class ExtensionProcessBindings {
  public:
-  struct CallContext {
-   public :
-     CallContext(WebFrame *frame, const std::string& name)
-        : frame_(frame),
-          name_(name) {}
-    WebFrame* frame_;
-    std::string name_;
-  };
-
   static void SetFunctionNames(const std::vector<std::string>& names);
   static v8::Extension* Get();
   static void RegisterExtensionContext(WebFrame* frame);
-  static void ExecuteResponseInFrame(CallContext *call, int request_id,
-                                     bool success,
-                                     const std::string& response,
-                                     const std::string& error);
+  static void HandleResponse(int request_id, bool success,
+                             const std::string& response,
+                             const std::string& error);
 };
 
 #endif  // CHROME_RENDERER_EXTENSIONS_EXTENSION_PROCESS_BINDINGS_H_
diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index 7882860..557bc0d 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -207,13 +207,6 @@ RenderView::~RenderView() {
     it = plugin_delegates_.erase(it);
   }
 
-  // Clear any pending extension api call requests.
-  IDMap<ExtensionProcessBindings::CallContext>::const_iterator call =
-      pending_extension_requests_.begin();
-  for (; call != pending_extension_requests_.end(); ++call) {
-    delete call->second;
-  }
-
   render_thread_->RemoveFilter(debug_message_handler_);
   render_thread_->RemoveFilter(audio_message_filter_);
 }
@@ -1344,23 +1337,6 @@ void RenderView::DidCancelClientRedirect(WebView* webview,
 }
 
 void RenderView::WillCloseFrame(WebView* view, WebFrame* frame) {
-  // Remove all the pending extension callbacks for this frame.
-  if (pending_extension_requests_.IsEmpty())
-    return;
-
-  std::vector<int> orphaned_requests;
-  for (IDMap<ExtensionProcessBindings::CallContext>::const_iterator iter =
-       pending_extension_requests_.begin();
-       iter != pending_extension_requests_.end(); ++iter) {
-    if (iter->second->frame_ == frame)
-      orphaned_requests.push_back(iter->first);
-  }
-
-  for (std::vector<int>::const_iterator iter = orphaned_requests.begin();
-       iter != orphaned_requests.end(); ++iter) {
-    delete pending_extension_requests_.Lookup(*iter);
-    pending_extension_requests_.Remove(*iter);
-  }
 }
 
 void RenderView::DidCompleteClientRedirect(WebView* webview,
@@ -2747,32 +2723,17 @@ void RenderView::OnSetBackground(const SkBitmap& background) {
 void RenderView::SendExtensionRequest(const std::string& name,
                                       const std::string& args,
                                       int request_id,
-                                      bool has_callback,
-                                      WebFrame* request_frame) {
-  DCHECK(request_frame) << "Request specified without frame";
-  pending_extension_requests_.AddWithID(
-      new ExtensionProcessBindings::CallContext(request_frame, name),
-      request_id);
-
+                                      bool has_callback) {
   Send(new ViewHostMsg_ExtensionRequest(routing_id_, name, args, request_id,
-      has_callback));
+                                        has_callback));
 }
 
 void RenderView::OnExtensionResponse(int request_id,
                                      bool success,
                                      const std::string& response,
                                      const std::string& error) {
-  ExtensionProcessBindings::CallContext* call =
-      pending_extension_requests_.Lookup(request_id);
-
-  if (!call)
-    return;  // The frame went away.
-
-  ExtensionProcessBindings::ExecuteResponseInFrame(call, request_id,
-                                                   success, response,
-                                                   error);
-  pending_extension_requests_.Remove(request_id);
-  delete call;
+  ExtensionProcessBindings::HandleResponse(request_id, success, response,
+                                           error);
 }
 
 // Dump all load time histograms.
diff --git a/chrome/renderer/render_view.h b/chrome/renderer/render_view.h
index 926ea72..a1928ba 100644
--- a/chrome/renderer/render_view.h
+++ b/chrome/renderer/render_view.h
@@ -380,8 +380,7 @@ class RenderView : public RenderWidget,
   void OnClearFocusedNode();
 
   void SendExtensionRequest(const std::string& name, const std::string& args,
-                            int request_id, bool has_callback,
-                            WebFrame* web_frame);
+                            int request_id, bool has_callback);
   void OnExtensionResponse(int request_id, bool success,
                            const std::string& response,
                            const std::string& error);
@@ -775,9 +774,6 @@ class RenderView : public RenderWidget,
   // change but is overridden by tests.
   int delay_seconds_for_form_state_sync_;
 
-  // Maps pending request IDs to their frames.
-  IDMap<ExtensionProcessBindings::CallContext> pending_extension_requests_;
-
   scoped_refptr<AudioMessageFilter> audio_message_filter_;
 
   // The currently selected text. This is currently only updated on Linux, where