Add an optional channel name to the extension message connect event.

Also changed the way port IDs work internally to reduce confusion.  Each end of the port has its own ID, not the ID of its opposite end.

BUG=13706
TEST=no

Review URL: http://codereview.chromium.org/155381

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

5 files changed, 82 insertions, 47 deletions

diff --git a/chrome/browser/extensions/extension_message_service.cc b/chrome/browser/extensions/extension_message_service.cc
index 78dd1f2..70bfda7 100644
--- a/chrome/browser/extensions/extension_message_service.cc
+++ b/chrome/browser/extensions/extension_message_service.cc
@@ -42,13 +42,15 @@ struct SingletonData {
   InstanceMap map;
 };
 
-static void DispatchOnConnect(IPC::Message::Sender* channel, int source_port_id,
+static void DispatchOnConnect(IPC::Message::Sender* channel, int dest_port_id,
+                              const std::string& channel_name,
                               const std::string& tab_json,
                               const std::string& extension_id) {
   ListValue args;
-  args.Set(0, Value::CreateIntegerValue(source_port_id));
-  args.Set(1, Value::CreateStringValue(tab_json));
-  args.Set(2, Value::CreateStringValue(extension_id));
+  args.Set(0, Value::CreateIntegerValue(dest_port_id));
+  args.Set(1, Value::CreateStringValue(channel_name));
+  args.Set(2, Value::CreateStringValue(tab_json));
+  args.Set(3, Value::CreateStringValue(extension_id));
   channel->Send(new ViewMsg_ExtensionMessageInvoke(
       ExtensionMessageService::kDispatchOnConnect, args));
 }
@@ -172,7 +174,7 @@ void ExtensionMessageService::AllocatePortIdPair(int* port1, int* port2) {
 
 int ExtensionMessageService::OpenChannelToExtension(
     int routing_id, const std::string& extension_id,
-    ResourceMessageFilter* source) {
+    const std::string& channel_name, ResourceMessageFilter* source) {
   DCHECK_EQ(MessageLoop::current(),
             ChromeThread::GetMessageLoop(ChromeThread::IO));
   DCHECK(initialized_);
@@ -182,24 +184,29 @@ int ExtensionMessageService::OpenChannelToExtension(
   int port2_id = -1;
   AllocatePortIdPair(&port1_id, &port2_id);
 
+  // Each side of the port is given his own port ID.  When they send messages,
+  // we convert to the opposite port ID.  See PostMessageFromRenderer.
   ui_loop_->PostTask(FROM_HERE,
       NewRunnableMethod(this, &ExtensionMessageService::OpenChannelOnUIThread,
-          routing_id, port1_id, source->GetProcessId(), extension_id));
+          routing_id, port2_id, source->GetProcessId(), extension_id,
+          channel_name));
 
-  return port2_id;
+  return port1_id;
 }
 
 void ExtensionMessageService::OpenChannelOnUIThread(
-    int source_routing_id, int source_port_id, int source_process_id,
-    const std::string& extension_id) {
+    int source_routing_id, int receivers_port_id, int source_process_id,
+    const std::string& extension_id, const std::string& channel_name) {
   RenderProcessHost* source = RenderProcessHost::FromID(source_process_id);
-  OpenChannelOnUIThreadImpl(source_routing_id, source_port_id,
-                            source_process_id, source, extension_id);
+  OpenChannelOnUIThreadImpl(source_routing_id, receivers_port_id,
+                            source_process_id, source, extension_id,
+                            channel_name);
 }
 
 void ExtensionMessageService::OpenChannelOnUIThreadImpl(
-    int source_routing_id, int source_port_id, int source_process_id,
-    IPC::Message::Sender* source, const std::string& extension_id) {
+    int source_routing_id, int receivers_port_id, int source_process_id,
+    IPC::Message::Sender* source, const std::string& extension_id,
+    const std::string& channel_name) {
   DCHECK_EQ(MessageLoop::current()->type(), MessageLoop::TYPE_UI);
 
   if (!source)
@@ -224,7 +231,7 @@ void ExtensionMessageService::OpenChannelOnUIThreadImpl(
     return;
   }
 
-  channels_[GET_CHANNEL_ID(source_port_id)] = channel;
+  channels_[GET_CHANNEL_ID(receivers_port_id)] = channel;
 
   // Include info about the opener's tab (if it was a tab).
   std::string tab_json = "null";
@@ -239,7 +246,8 @@ void ExtensionMessageService::OpenChannelOnUIThreadImpl(
   // port ID (the opener has the opposite port ID).
   for (MessageChannel::Ports::iterator it = channel->receivers.begin();
        it != channel->receivers.end(); ++it) {
-    DispatchOnConnect(*it, source_port_id, tab_json, extension_id);
+    DispatchOnConnect(*it, receivers_port_id, channel_name, tab_json,
+                      extension_id);
   }
 }
 
@@ -259,10 +267,10 @@ int ExtensionMessageService::OpenAutomationChannelToExtension(
   //      This isn't really appropriate here, the originating tab
   //      information should be supplied by the caller for
   //      automation-initiated ports.
-  OpenChannelOnUIThreadImpl(routing_id, port1_id, source_process_id,
-                            source, extension_id);
+  OpenChannelOnUIThreadImpl(routing_id, port2_id, source_process_id,
+                            source, extension_id, "");
 
-  return port2_id;
+  return port1_id;
 }
 
 void ExtensionMessageService::CloseChannel(int port_id) {
@@ -292,23 +300,23 @@ void ExtensionMessageService::CloseChannelImpl(
 }
 
 void ExtensionMessageService::PostMessageFromRenderer(
-    int dest_port_id, const std::string& message) {
+    int source_port_id, const std::string& message) {
   DCHECK_EQ(MessageLoop::current()->type(), MessageLoop::TYPE_UI);
 
   MessageChannelMap::iterator iter =
-      channels_.find(GET_CHANNEL_ID(dest_port_id));
+      channels_.find(GET_CHANNEL_ID(source_port_id));
   if (iter == channels_.end())
     return;
 
   // Figure out which port the ID corresponds to.
+  int dest_port_id = GET_OPPOSITE_PORT_ID(source_port_id);
   MessageChannel::Ports* ports =
       IS_OPENER_PORT_ID(dest_port_id) ?
           &iter->second->opener : &iter->second->receivers;
-  int source_port_id = GET_OPPOSITE_PORT_ID(dest_port_id);
 
   for (MessageChannel::Ports::iterator it = ports->begin();
        it != ports->end(); ++it) {
-    DispatchOnMessage(*it, message, source_port_id);
+    DispatchOnMessage(*it, message, dest_port_id);
   }
 }
 
diff --git a/chrome/browser/extensions/extension_message_service.h b/chrome/browser/extensions/extension_message_service.h
index 7a87adf..77970bc 100644
--- a/chrome/browser/extensions/extension_message_service.h
+++ b/chrome/browser/extensions/extension_message_service.h
@@ -19,15 +19,27 @@ class RenderProcessHost;
 class ResourceMessageFilter;
 class URLRequestContext;
 
-// This class manages message passing between renderer processes.  It maintains
-// a list of available extensions and which renderers each lives in, as well as
-// a set of open channels.
+// This class manages message and event passing between renderer processes.
+// It maintains a list of processes that are listening to events (including
+// messaging events), as well as a set of open channels.
+// 
+// Messaging works this way:
+// - An extension-owned script context (like a toolstrip or a content script)
+// adds an event listener to the "onConnect" event.  We keep track here of a
+// list of "listeners" that registered interest in receiving extension
+// messages.
+// - Another context calls "connect()" to open a channel to every listener
+// owned by the same extension.  This is a broadcast event, so every listener
+// will get notified.
+// - Once the channel is established, either side can call postMessage to send
+// a message to the opposite side of the channel, which may have multiple
+// listeners.
 //
 // Terminology:
-// channel: connection between two ports (one of which belongs to an extension)
-// port: an IPC::Message::Sender interface through which we communicate to a
-//   process.  We use MessageFilters for this since that allows us to send our
-//   messages on the IO thread.
+// channel: connection between two ports (one side of which can have multiple
+// listeners)
+// port: one or more IPC::Message::Sender interfaces through which we
+// communicate to process(es).  These are generally RenderProcessHosts.
 class ExtensionMessageService : public NotificationObserver {
  public:
   // Javascript function name constants.
@@ -35,6 +47,7 @@ class ExtensionMessageService : public NotificationObserver {
   static const char kDispatchOnDisconnect[];
   static const char kDispatchOnMessage[];
   static const char kDispatchEvent[];
+  static const char kDispatchError[];
 
   // Returns the message service for the given context.  Messages can only
   // be sent within a single context.
@@ -78,11 +91,13 @@ class ExtensionMessageService : public NotificationObserver {
   // --- IO thread only:
 
   // Given an extension's ID, opens a channel between the given renderer "port"
-  // and that extension.  Returns a channel ID to be used for posting messages
-  // between the processes, or -1 if the extension doesn't exist.
+  // and every listening context owned by that extension.  Returns a port ID
+  // to be used for posting messages between the processes.  |channel_name| is
+  // an optional identifier for use by extension developers.
   // This runs on the IO thread so that it can be used in a synchronous IPC
   // message.
   int OpenChannelToExtension(int routing_id, const std::string& extension_id,
+                             const std::string& channel_name,
                              ResourceMessageFilter* source);
   
  private:
@@ -110,16 +125,17 @@ class ExtensionMessageService : public NotificationObserver {
 
   // --- UI thread only:
 
-  // Handles channel creation and notifies the destination that a channel was
+  // Handles channel creation and notifies the destinations that a channel was
   // opened.
   void OpenChannelOnUIThread(int source_routing_id,
       int source_port_id, int source_process_id,
-      const std::string& extension_id);
+      const std::string& extension_id, const std::string& channel_name);
 
   // Common between OpenChannelOnUIThread and OpenAutomationChannelToExtension.
   void OpenChannelOnUIThreadImpl(
     int source_routing_id, int source_port_id, int source_process_id,
-    IPC::Message::Sender* source, const std::string& extension_id);
+    IPC::Message::Sender* source, const std::string& extension_id,
+    const std::string& channel_name);
 
   NotificationRegistrar registrar_;
 
diff --git a/chrome/browser/extensions/extension_messages_unittest.cc b/chrome/browser/extensions/extension_messages_unittest.cc
index 27a5bd9..c2e4540 100644
--- a/chrome/browser/extensions/extension_messages_unittest.cc
+++ b/chrome/browser/extensions/extension_messages_unittest.cc
@@ -9,11 +9,13 @@
 #include "chrome/test/render_view_test.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-static void DispatchOnConnect(int source_port_id, const std::string& tab_json) {
+static void DispatchOnConnect(int source_port_id, const std::string& name,
+                              const std::string& tab_json) {
   ListValue args;
   args.Set(0, Value::CreateIntegerValue(source_port_id));
-  args.Set(1, Value::CreateStringValue(tab_json));
-  args.Set(2, Value::CreateStringValue(""));  // extension ID is empty for tests
+  args.Set(1, Value::CreateStringValue(name));
+  args.Set(2, Value::CreateStringValue(tab_json));
+  args.Set(3, Value::CreateStringValue(""));  // extension ID is empty for tests
   RendererExtensionBindings::Invoke(
       ExtensionMessageService::kDispatchOnConnect, args);
 }
@@ -40,7 +42,7 @@ TEST_F(RenderViewTest, ExtensionMessagesOpenChannel) {
   LoadHTML("<body></body>");
   ExecuteJavaScript(
     "var e = new chrome.Extension('foobar');"
-    "var port = e.connect();"
+    "var port = e.connect('testName');"
     "port.onMessage.addListener(doOnMessage);"
     "port.postMessage({message: 'content ready'});"
     "function doOnMessage(msg, port) {"
@@ -51,7 +53,11 @@ TEST_F(RenderViewTest, ExtensionMessagesOpenChannel) {
   const IPC::Message* open_channel_msg =
       render_thread_.sink().GetUniqueMessageMatching(
           ViewHostMsg_OpenChannelToExtension::ID);
-  EXPECT_TRUE(open_channel_msg);
+  ASSERT_TRUE(open_channel_msg);
+  void* iter = IPC::SyncMessage::GetDataIterator(open_channel_msg);
+  ViewHostMsg_OpenChannelToExtension::SendParam open_params;
+  ASSERT_TRUE(IPC::ReadParam(open_channel_msg, &iter, &open_params));
+  EXPECT_EQ("testName", open_params.c);
 
   const IPC::Message* post_msg =
       render_thread_.sink().GetUniqueMessageMatching(
@@ -71,7 +77,7 @@ TEST_F(RenderViewTest, ExtensionMessagesOpenChannel) {
       render_thread_.sink().GetUniqueMessageMatching(
           ViewHostMsg_RunJavaScriptMessage::ID);
   ASSERT_TRUE(alert_msg);
-  void* iter = IPC::SyncMessage::GetDataIterator(alert_msg);
+  iter = IPC::SyncMessage::GetDataIterator(alert_msg);
   ViewHostMsg_RunJavaScriptMessage::SendParam alert_param;
   ASSERT_TRUE(IPC::ReadParam(alert_msg, &iter, &alert_param));
   EXPECT_EQ(L"content got: 42", alert_param.a);
@@ -86,7 +92,8 @@ TEST_F(RenderViewTest, ExtensionMessagesOnConnect) {
     "  port.test = 24;"
     "  port.onMessage.addListener(doOnMessage);"
     "  port.onDisconnect.addListener(doOnDisconnect);"
-    "  port.postMessage({message: 'onconnect from ' + port.tab.url});"
+    "  port.postMessage({message: 'onconnect from ' + port.tab.url + "
+    "                   ' name ' + port.name});"
     "});"
     "function doOnMessage(msg, port) {"
     "  alert('got: ' + msg.val);"
@@ -99,7 +106,8 @@ TEST_F(RenderViewTest, ExtensionMessagesOnConnect) {
 
   // Simulate a new connection being opened.
   const int kPortId = 0;
-  DispatchOnConnect(kPortId, "{\"url\":\"foo://bar\"}");
+  const std::string kPortName = "testName";
+  DispatchOnConnect(kPortId, kPortName, "{\"url\":\"foo://bar\"}");
 
   // Verify that we handled the new connection by posting a message.
   const IPC::Message* post_msg =
@@ -108,7 +116,9 @@ TEST_F(RenderViewTest, ExtensionMessagesOnConnect) {
   ASSERT_TRUE(post_msg);
   ViewHostMsg_ExtensionPostMessage::Param post_params;
   ViewHostMsg_ExtensionPostMessage::Read(post_msg, &post_params);
-  EXPECT_EQ("{\"message\":\"onconnect from foo://bar\"}", post_params.b);
+  std::string expected_msg =
+      "{\"message\":\"onconnect from foo://bar name " + kPortName + "\"}";
+  EXPECT_EQ(expected_msg, post_params.b);
 
   // Now simulate getting a message back from the channel opener.
   render_thread_.sink().ClearMessages();
diff --git a/chrome/browser/renderer_host/resource_message_filter.cc b/chrome/browser/renderer_host/resource_message_filter.cc
index 6858430..d913aec 100644
--- a/chrome/browser/renderer_host/resource_message_filter.cc
+++ b/chrome/browser/renderer_host/resource_message_filter.cc
@@ -884,9 +884,10 @@ void ResourceMessageFilter::OnFreeTransportDIB(
 #endif
 
 void ResourceMessageFilter::OnOpenChannelToExtension(
-    int routing_id, const std::string& extension_id, int* port_id) {
+    int routing_id, const std::string& extension_id,
+    const std::string& channel_name, int* port_id) {
   *port_id = ExtensionMessageService::GetInstance(request_context_.get())->
-      OpenChannelToExtension(routing_id, extension_id, this);
+      OpenChannelToExtension(routing_id, extension_id, channel_name, this);
 }
 
 bool ResourceMessageFilter::CheckBenchmarkingEnabled() {
diff --git a/chrome/browser/renderer_host/resource_message_filter.h b/chrome/browser/renderer_host/resource_message_filter.h
index 70eeaf8..613dd77 100644
--- a/chrome/browser/renderer_host/resource_message_filter.h
+++ b/chrome/browser/renderer_host/resource_message_filter.h
@@ -218,8 +218,8 @@ class ResourceMessageFilter : public IPC::ChannelProxy::MessageFilter,
                            TransportDIB::Handle* result);
   void OnFreeTransportDIB(TransportDIB::Id dib_id);
 
-  void OnOpenChannelToExtension(int routing_id,
-                                const std::string& extension_id, int* port_id);
+  void OnOpenChannelToExtension(int routing_id, const std::string& extension_id,
+                                const std::string& channel_name, int* port_id);
 
   void OnCloseIdleConnections();
   void OnSetCacheMode(bool enabled);