From 0f605396ab522e1f2f48824c42bb784e9784d479 Mon Sep 17 00:00:00 2001 From: "mpcomplete@chromium.org" Date: Thu, 9 Jul 2009 19:26:35 +0000 Subject: Make the API to open a message channel symmetric, so it works the same whether opening from a tab or extension. Also, move the callback handling back to extension_process_bindings, since I didn't need it in event_bindings to implement this, and it didn't make sense there anyway. BUG=12461 TEST=no Review URL: http://codereview.chromium.org/149237 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@20296 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/renderer/extensions/bindings_utils.cc | 28 ---------- chrome/renderer/extensions/bindings_utils.h | 4 -- chrome/renderer/extensions/event_bindings.cc | 29 ---------- chrome/renderer/extensions/event_bindings.h | 5 -- .../extensions/extension_api_client_unittest.cc | 5 +- .../extensions/extension_process_bindings.cc | 61 +++++++++++++++++++++- .../extensions/extension_process_bindings.h | 7 ++- chrome/renderer/render_view.cc | 4 +- chrome/renderer/renderer_resources.grd | 2 +- chrome/renderer/resources/event_bindings.js | 44 ++-------------- .../resources/extension_process_bindings.js | 43 ++++++++++++++- .../resources/renderer_extension_bindings.js | 32 +++++++----- chrome/renderer/user_script_slave.cc | 5 +- 13 files changed, 142 insertions(+), 127 deletions(-) (limited to 'chrome/renderer') diff --git a/chrome/renderer/extensions/bindings_utils.cc b/chrome/renderer/extensions/bindings_utils.cc index fc9e98d..13738ca 100644 --- a/chrome/renderer/extensions/bindings_utils.cc +++ b/chrome/renderer/extensions/bindings_utils.cc @@ -44,34 +44,6 @@ v8::Handle ExtensionBase::GetChromeHidden( return hidden; } -v8::Handle ExtensionBase::StartRequest( - const v8::Arguments& args) { - // Get the current RenderView so that we can send a routed IPC message from - // the correct source. - RenderView* renderview = bindings_utils::GetRenderViewForCurrentContext(); - 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(); - - v8::Persistent current_context = - v8::Persistent::New(v8::Context::GetCurrent()); - DCHECK(!current_context.IsEmpty()); - GetPendingRequestMap()[request_id].reset(new PendingRequest( - current_context, *v8::String::AsciiValue(args.Data()))); - - renderview->SendExtensionRequest(name, json_args, request_id, has_callback); - - return v8::Undefined(); -} - ContextList& GetContexts() { return Singleton::get()->contexts; } diff --git a/chrome/renderer/extensions/bindings_utils.h b/chrome/renderer/extensions/bindings_utils.h index 1b21023..2e8935a5 100644 --- a/chrome/renderer/extensions/bindings_utils.h +++ b/chrome/renderer/extensions/bindings_utils.h @@ -39,10 +39,6 @@ class ExtensionBase : public v8::Extension { // Returns a hidden variable for use by the bindings that is unreachable // by the page. static v8::Handle GetChromeHidden(const v8::Arguments& args); - - // Starts an API request to the browser, with an optional callback. The - // callback will be dispatched to EventBindings::HandleResponse. - static v8::Handle StartRequest(const v8::Arguments& args); }; template diff --git a/chrome/renderer/extensions/event_bindings.cc b/chrome/renderer/extensions/event_bindings.cc index e9efbad..03cccad 100644 --- a/chrome/renderer/extensions/event_bindings.cc +++ b/chrome/renderer/extensions/event_bindings.cc @@ -23,7 +23,6 @@ using bindings_utils::GetContexts; using bindings_utils::GetStringResource; using bindings_utils::ExtensionBase; using bindings_utils::GetPendingRequestMap; -using bindings_utils::PendingRequest; using bindings_utils::PendingRequestMap; namespace { @@ -62,8 +61,6 @@ class ExtensionImpl : public ExtensionBase { return v8::FunctionTemplate::New(AttachEvent); } else if (name->Equals(v8::String::New("DetachEvent"))) { return v8::FunctionTemplate::New(DetachEvent); - } else if (name->Equals(v8::String::New("GetNextRequestId"))) { - return v8::FunctionTemplate::New(GetNextRequestId); } return ExtensionBase::GetNativeFunction(name); } @@ -100,11 +97,6 @@ class ExtensionImpl : public ExtensionBase { return v8::Undefined(); } - - static v8::Handle GetNextRequestId(const v8::Arguments& args) { - static int next_request_id = 0; - return v8::Integer::New(next_request_id++); - } }; } // namespace @@ -191,24 +183,3 @@ void EventBindings::CallFunction(const std::string& function_name, CallFunctionInContext((*it)->context, function_name, argc, argv); } } - -// static -void EventBindings::HandleResponse(int request_id, bool success, - const std::string& response, - const std::string& error) { - PendingRequest* request = GetPendingRequestMap()[request_id].get(); - if (!request) - return; // The frame went away. - - v8::HandleScope handle_scope; - v8::Handle argv[5]; - argv[0] = v8::Integer::New(request_id); - argv[1] = v8::String::New(request->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( - request->context, "handleResponse", arraysize(argv), argv); - - GetPendingRequestMap().erase(request_id); -} diff --git a/chrome/renderer/extensions/event_bindings.h b/chrome/renderer/extensions/event_bindings.h index 285a379..ea0060f 100644 --- a/chrome/renderer/extensions/event_bindings.h +++ b/chrome/renderer/extensions/event_bindings.h @@ -31,11 +31,6 @@ class EventBindings { // more details. static void CallFunction(const std::string& function_name, int argc, v8::Handle* argv); - - // Handles a response to an API request. - static void HandleResponse(int request_id, bool success, - const std::string& response, - const std::string& error); }; #endif // CHROME_RENDERER_EXTENSIONS_EVENT_BINDINGS_H_ diff --git a/chrome/renderer/extensions/extension_api_client_unittest.cc b/chrome/renderer/extensions/extension_api_client_unittest.cc index 6cef5750..663a892 100644 --- a/chrome/renderer/extensions/extension_api_client_unittest.cc +++ b/chrome/renderer/extensions/extension_api_client_unittest.cc @@ -7,7 +7,7 @@ #include "base/string_util.h" #include "chrome/common/chrome_paths.h" #include "chrome/common/render_messages.h" -#include "chrome/renderer/extensions/event_bindings.h" +#include "chrome/renderer/extensions/extension_process_bindings.h" #include "chrome/renderer/extensions/renderer_extension_bindings.h" #include "chrome/test/render_view_test.h" #include "testing/gtest/include/gtest/gtest.h" @@ -91,7 +91,8 @@ TEST_F(ExtensionAPIClientTest, CallbackDispatching) { ASSERT_GE(callback_id, 0); // Now send the callback a response - EventBindings::HandleResponse(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 6b568a1..e68fb82 100644 --- a/chrome/renderer/extensions/extension_process_bindings.cc +++ b/chrome/renderer/extensions/extension_process_bindings.cc @@ -19,6 +19,8 @@ using bindings_utils::GetStringResource; using bindings_utils::ContextInfo; using bindings_utils::ContextList; using bindings_utils::GetContexts; +using bindings_utils::GetPendingRequestMap; +using bindings_utils::PendingRequest; using bindings_utils::ExtensionBase; namespace { @@ -58,8 +60,10 @@ class ExtensionImpl : public ExtensionBase { if (name->Equals(v8::String::New("GetViews"))) { return v8::FunctionTemplate::New(GetViews); + } else if (name->Equals(v8::String::New("GetNextRequestId"))) { + return v8::FunctionTemplate::New(GetNextRequestId); } else if (names->find(*v8::String::AsciiValue(name)) != names->end()) { - return v8::FunctionTemplate::New(ExtensionBase::StartRequest, name); + return v8::FunctionTemplate::New(StartRequest, name); } return ExtensionBase::GetNativeFunction(name); @@ -88,6 +92,40 @@ class ExtensionImpl : public ExtensionBase { } return views; } + + static v8::Handle GetNextRequestId(const v8::Arguments& args) { + static int next_request_id = 0; + return v8::Integer::New(next_request_id++); + } + + // Starts an API request to the browser, with an optional callback. The + // callback will be dispatched to EventBindings::HandleResponse. + static v8::Handle StartRequest(const v8::Arguments& args) { + // Get the current RenderView so that we can send a routed IPC message from + // the correct source. + RenderView* renderview = bindings_utils::GetRenderViewForCurrentContext(); + 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(); + + v8::Persistent current_context = + v8::Persistent::New(v8::Context::GetCurrent()); + DCHECK(!current_context.IsEmpty()); + GetPendingRequestMap()[request_id].reset(new PendingRequest( + current_context, *v8::String::AsciiValue(args.Data()))); + + renderview->SendExtensionRequest(name, json_args, request_id, has_callback); + + return v8::Undefined(); + } }; } // namespace @@ -100,3 +138,24 @@ void ExtensionProcessBindings::SetFunctionNames( const std::vector& names) { ExtensionImpl::SetFunctionNames(names); } + +// static +void ExtensionProcessBindings::HandleResponse(int request_id, bool success, + const std::string& response, + const std::string& error) { + PendingRequest* request = GetPendingRequestMap()[request_id].get(); + if (!request) + return; // The frame went away. + + v8::HandleScope handle_scope; + v8::Handle argv[5]; + argv[0] = v8::Integer::New(request_id); + argv[1] = v8::String::New(request->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()); + bindings_utils::CallFunctionInContext( + request->context, "handleResponse", arraysize(argv), argv); + + GetPendingRequestMap().erase(request_id); +} diff --git a/chrome/renderer/extensions/extension_process_bindings.h b/chrome/renderer/extensions/extension_process_bindings.h index f703914..9e2c814 100644 --- a/chrome/renderer/extensions/extension_process_bindings.h +++ b/chrome/renderer/extensions/extension_process_bindings.h @@ -12,12 +12,15 @@ #include "v8/include/v8.h" -class WebFrame; - class ExtensionProcessBindings { public: static void SetFunctionNames(const std::vector& names); static v8::Extension* Get(); + + // Handles a response to an API request. + 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 cc69619..5f72843 100644 --- a/chrome/renderer/render_view.cc +++ b/chrome/renderer/render_view.cc @@ -37,6 +37,7 @@ #include "chrome/renderer/devtools_agent.h" #include "chrome/renderer/devtools_client.h" #include "chrome/renderer/extensions/event_bindings.h" +#include "chrome/renderer/extensions/extension_process_bindings.h" #include "chrome/renderer/localized_error.h" #include "chrome/renderer/media/audio_renderer_impl.h" #include "chrome/renderer/media/buffered_data_source.h" @@ -2798,7 +2799,8 @@ void RenderView::OnExtensionResponse(int request_id, bool success, const std::string& response, const std::string& error) { - EventBindings::HandleResponse(request_id, success, response, error); + ExtensionProcessBindings::HandleResponse( + request_id, success, response, error); } // Dump all load time histograms. diff --git a/chrome/renderer/renderer_resources.grd b/chrome/renderer/renderer_resources.grd index 241abb3..ed66407 100644 --- a/chrome/renderer/renderer_resources.grd +++ b/chrome/renderer/renderer_resources.grd @@ -1,6 +1,6 @@ +without changes to the corresponding grd file. mp12 --> diff --git a/chrome/renderer/resources/event_bindings.js b/chrome/renderer/resources/event_bindings.js index a58cf37..5fb16f9 100644 --- a/chrome/renderer/resources/event_bindings.js +++ b/chrome/renderer/resources/event_bindings.js @@ -12,7 +12,6 @@ var chrome = chrome || {}; native function GetChromeHidden(); native function AttachEvent(eventName); native function DetachEvent(eventName); - native function GetNextRequestId(); var chromeHidden = GetChromeHidden(); @@ -149,45 +148,6 @@ var chrome = chrome || {}; delete attachedNamedEvents[this.eventName_]; }; - // Callback handling. - var callbacks = []; - chromeHidden.handleResponse = function(requestId, name, - success, response, error) { - try { - if (!success) { - if (!error) - error = "Unknown error." - console.error("Error during " + name + ": " + error); - return; - } - - if (callbacks[requestId]) { - if (response) { - callbacks[requestId](JSON.parse(response)); - } else { - callbacks[requestId](); - } - } - } finally { - delete callbacks[requestId]; - } - }; - - // Send an API request and optionally register a callback. - chromeHidden.sendRequest = function(request, args, callback) { - // JSON.stringify doesn't support a root object which is undefined. - if (args === undefined) - args = null; - var sargs = JSON.stringify(args); - var requestId = GetNextRequestId(); - var hasCallback = false; - if (callback) { - hasCallback = true; - callbacks[requestId] = callback; - } - request(sargs, requestId, hasCallback); - } - // Special load events: we don't use the DOM unload because that slows // down tab shutdown. On the other hand, onUnload might not always fire, // since Chrome will terminate renderers on shutdown (SuddenTermination). @@ -203,4 +163,8 @@ var chrome = chrome || {}; for (var i in allAttachedEvents) allAttachedEvents[i].detach_(); } + + chromeHidden.dispatchError = function(msg) { + console.error(msg); + } })(); diff --git a/chrome/renderer/resources/extension_process_bindings.js b/chrome/renderer/resources/extension_process_bindings.js index 5b6239c..f3893fb 100644 --- a/chrome/renderer/resources/extension_process_bindings.js +++ b/chrome/renderer/resources/extension_process_bindings.js @@ -35,6 +35,7 @@ var chrome = chrome || {}; native function MoveBookmark(); native function SetBookmarkTitle(); native function GetChromeHidden(); + native function GetNextRequestId(); if (!chrome) chrome = {}; @@ -72,7 +73,44 @@ var chrome = chrome || {}; } } - var sendRequest = chromeHidden.sendRequest; + // Callback handling. + var callbacks = []; + chromeHidden.handleResponse = function(requestId, name, + success, response, error) { + try { + if (!success) { + if (!error) + error = "Unknown error." + console.error("Error during " + name + ": " + error); + return; + } + + if (callbacks[requestId]) { + if (response) { + callbacks[requestId](JSON.parse(response)); + } else { + callbacks[requestId](); + } + } + } finally { + delete callbacks[requestId]; + } + }; + + // Send an API request and optionally register a callback. + function sendRequest(request, args, callback) { + // JSON.stringify doesn't support a root object which is undefined. + if (args === undefined) + args = null; + var sargs = JSON.stringify(args); + var requestId = GetNextRequestId(); + var hasCallback = false; + if (callback) { + hasCallback = true; + callbacks[requestId] = callback; + } + request(sargs, requestId, hasCallback); + } //---------------------------------------------------------------------------- @@ -492,6 +530,9 @@ var chrome = chrome || {}; chrome.self = chrome.self || {}; chromeHidden.onLoad.addListener(function (extensionId) { + chrome.extension = new chrome.Extension(extensionId); + // TODO(mpcomplete): self.onConnect is deprecated. Remove it at 1.0. + // http://code.google.com/p/chromium/issues/detail?id=16356 chrome.self.onConnect = new chrome.Event("channel-connect:" + extensionId); }); diff --git a/chrome/renderer/resources/renderer_extension_bindings.js b/chrome/renderer/resources/renderer_extension_bindings.js index b27a2df..2a41d5f 100644 --- a/chrome/renderer/resources/renderer_extension_bindings.js +++ b/chrome/renderer/resources/renderer_extension_bindings.js @@ -22,21 +22,26 @@ var chrome = chrome || {}; // Port object. Represents a connection to another script context through // which messages can be passed. chrome.Port = function(portId) { - if (ports[portId]) { - throw new Error("Port '" + portId + "' already exists."); - } - this.portId_ = portId; // TODO(mpcomplete): readonly + this.portId_ = portId; this.onDisconnect = new chrome.Event(); this.onMessage = new chrome.Event(); - ports[portId] = this; + }; - var port = this; + chromeHidden.Port = {}; + + // Hidden port creation function. We don't want to expose an API that lets + // people add arbitrary port IDs to the port list. + chromeHidden.Port.createPort = function(portId) { + if (ports[portId]) { + throw new Error("Port '" + portId + "' already exists."); + } + var port = new chrome.Port(portId); + ports[portId] = port; chromeHidden.onUnload.addListener(function() { port.disconnect(); }); - }; - - chromeHidden.Port = {}; + return port; + } // Called by native code when a channel has been opened to this context. chromeHidden.Port.dispatchOnConnect = function(portId, tab, extensionId) { @@ -44,9 +49,9 @@ var chrome = chrome || {}; // In addition to being an optimization, this also fixes a bug where if 2 // channels were opened to and from the same process, closing one would // close both. - var connectEvent = "channel-connect:" + (extensionId || ""); + var connectEvent = "channel-connect:" + extensionId; if (chromeHidden.Event.hasListener(connectEvent)) { - var port = new chrome.Port(portId); + var port = chromeHidden.Port.createPort(portId); if (tab) { tab = JSON.parse(tab); } @@ -93,6 +98,7 @@ var chrome = chrome || {}; // Extension object. chrome.Extension = function(id) { this.id_ = id; + this.onConnect = new chrome.Event('channel-connect:' + id); }; // Opens a message channel to the extension. Returns a Port for @@ -101,7 +107,7 @@ var chrome = chrome || {}; var portId = OpenChannelToExtension(this.id_); if (portId == -1) throw new Error("No such extension: '" + this.id_ + "'"); - return new chrome.Port(portId); + return chromeHidden.Port.createPort(portId); }; // Returns a resource URL that can be used to fetch a resource from this @@ -109,4 +115,6 @@ var chrome = chrome || {}; chrome.Extension.prototype.getURL = function(path) { return "chrome-extension://" + this.id_ + "/" + path; }; + + chrome.self = chrome.self || {}; })(); diff --git a/chrome/renderer/user_script_slave.cc b/chrome/renderer/user_script_slave.cc index fa4eb7d..afd065f 100644 --- a/chrome/renderer/user_script_slave.cc +++ b/chrome/renderer/user_script_slave.cc @@ -26,8 +26,11 @@ static const char kUserScriptHead[] = "(function (unsafeWindow) {\n"; static const char kUserScriptTail[] = "\n})(window);"; // Creates a convenient reference to a content script's parent extension. +// TODO(mpcomplete): self.onConnect is deprecated. Remove it at 1.0. +// http://code.google.com/p/chromium/issues/detail?id=16356 static const char kInitExtension[] = - "chrome.extension = new chrome.Extension('%s')"; + "chrome.extension = new chrome.Extension('%s');" + "chrome.self.onConnect = chrome.extension.onConnect;"; UserScriptSlave::UserScriptSlave() : shared_memory_(NULL), -- cgit v1.1