From 2f25d7b91d75afea74cf4ba9e3b2a2db0d853f50 Mon Sep 17 00:00:00 2001
From: "mpcomplete@google.com"
<mpcomplete@google.com@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Wed, 10 Jun 2009 00:06:47 +0000
Subject: 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
---
chrome/renderer/extensions/bindings_utils.cc | 23 ++++
chrome/renderer/extensions/bindings_utils.h | 8 ++
chrome/renderer/extensions/event_bindings.cc | 15 +--
.../extensions/extension_api_client_unittest.cc | 5 +-
.../extensions/extension_process_bindings.cc | 125 +++++++++++++--------
.../extensions/extension_process_bindings.h | 16 +--
chrome/renderer/render_view.cc | 47 +-------
chrome/renderer/render_view.h | 6 +-
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
--
cgit v1.1