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

13 files changed, 142 insertions, 127 deletions

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<v8::Value> ExtensionBase::GetChromeHidden(
   return hidden;
 }
 
-v8::Handle<v8::Value> 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<v8::Context> current_context =
-      v8::Persistent<v8::Context>::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<SingletonData>::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<v8::Value> 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<v8::Value> StartRequest(const v8::Arguments& args);
 };
 
 template<int kResourceId>
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<v8::Value> 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<v8::Value> 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<v8::Value>* 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<v8::Value> 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<v8::Value> 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<v8::Context> current_context =
+        v8::Persistent<v8::Context>::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<std::string>& 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<v8::Value> 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<std::string>& 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 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <!-- This comment is only here because changes to resources are not picked up
-without changes to the corresponding grd file. mp6 -->
+without changes to the corresponding grd file. mp12 -->
 <grit latest_public_release="0" current_release="1">
   <outputs>
     <output filename="grit/renderer_resources.h" type="rc_header">
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),