Fix some issues with extension messaging:

- Disconnect ports properly (javascript mistake).
- Use the right port ID when dispatching the disconnect event.
- Fix a bug with 2 extensions loaded in the same process.

BUG=12686
BUG=15798
TEST=Load an extension that uses messaging, and make sure it disconnects when you navigate or close the connecting.

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

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

4 files changed, 25 insertions, 18 deletions

diff --git a/chrome/browser/extensions/extension_host.cc b/chrome/browser/extensions/extension_host.cc
index 669dbdd..48853d8 100644
--- a/chrome/browser/extensions/extension_host.cc
+++ b/chrome/browser/extensions/extension_host.cc
@@ -149,7 +149,6 @@ void ExtensionHost::UpdatePreferredWidth(int pref_width) {
 void ExtensionHost::RenderViewGone(RenderViewHost* render_view_host) {
   DCHECK_EQ(render_view_host_, render_view_host);
   TabContents* current_tab = GetBrowser()->GetSelectedTabContents();
-  DCHECK(current_tab);
   if (current_tab) {
     current_tab->AddInfoBar(
         new CrashedExtensionInfobarDelegate(current_tab, this));
diff --git a/chrome/browser/extensions/extension_message_service.cc b/chrome/browser/extensions/extension_message_service.cc
index 0ee4579..4929eea 100644
--- a/chrome/browser/extensions/extension_message_service.cc
+++ b/chrome/browser/extensions/extension_message_service.cc
@@ -43,10 +43,12 @@ struct SingletonData {
 };
 
 static void DispatchOnConnect(IPC::Message::Sender* channel, int source_port_id,
-                              const std::string& tab_json) {
+                              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));
   channel->Send(new ViewMsg_ExtensionMessageInvoke(
       ExtensionMessageService::kDispatchOnConnect, args));
 }
@@ -221,23 +223,25 @@ int ExtensionMessageService::OpenChannelToExtension(
 
   ui_loop_->PostTask(FROM_HERE,
       NewRunnableMethod(this, &ExtensionMessageService::OpenChannelOnUIThread,
-          routing_id, port1_id, source->GetProcessId(), port2_id, process_id));
+          routing_id, port1_id, source->GetProcessId(), port2_id, process_id,
+          extension_id));
 
   return port2_id;
 }
 
 void ExtensionMessageService::OpenChannelOnUIThread(
     int source_routing_id, int source_port_id, int source_process_id,
-    int dest_port_id, int dest_process_id) {
+    int dest_port_id, int dest_process_id, const std::string& extension_id) {
   RenderProcessHost* source = RenderProcessHost::FromID(source_process_id);
   OpenChannelOnUIThreadImpl(source_routing_id, source_port_id,
-                            source, dest_port_id, dest_process_id,
-                            source_process_id);
+                            source_process_id, source, dest_port_id,
+                            dest_process_id, extension_id);
 }
 
 void ExtensionMessageService::OpenChannelOnUIThreadImpl(
-    int source_routing_id, int source_port_id, IPC::Message::Sender* source,
-    int dest_port_id, int dest_process_id, int source_process_id) {
+    int source_routing_id, int source_port_id, int source_process_id,
+    IPC::Message::Sender* source, int dest_port_id, int dest_process_id,
+    const std::string& extension_id) {
   DCHECK_EQ(MessageLoop::current()->type(), MessageLoop::TYPE_UI);
 
   MessageChannel channel;
@@ -259,7 +263,7 @@ void ExtensionMessageService::OpenChannelOnUIThreadImpl(
   }
 
   // Send the process the id for the opposite port.
-  DispatchOnConnect(channel.port2, source_port_id, tab_json);
+  DispatchOnConnect(channel.port2, source_port_id, tab_json, extension_id);
 }
 
 int ExtensionMessageService::OpenAutomationChannelToExtension(
@@ -284,8 +288,8 @@ 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, port2_id,
-                            process_id, source_process_id);
+  OpenChannelOnUIThreadImpl(routing_id, port1_id, source_process_id,
+                            source, port2_id, process_id, extension_id);
 
   return port2_id;
 }
@@ -305,10 +309,12 @@ void ExtensionMessageService::CloseChannelImpl(
 
   // Notify the other side.
   if (port_id == GET_CHANNEL_PORT1(channel_iter->first)) {
-    DispatchOnDisconnect(channel_iter->second.port2, port_id);
+    DispatchOnDisconnect(channel_iter->second.port2,
+                         GET_OPPOSITE_PORT_ID(port_id));
   } else {
     DCHECK_EQ(port_id, GET_CHANNEL_PORT2(channel_iter->first));
-    DispatchOnDisconnect(channel_iter->second.port1, port_id);
+    DispatchOnDisconnect(channel_iter->second.port1,
+                         GET_OPPOSITE_PORT_ID(port_id));
   }
 
   channels_.erase(channel_iter);
diff --git a/chrome/browser/extensions/extension_message_service.h b/chrome/browser/extensions/extension_message_service.h
index f128103..e3b7ba8 100644
--- a/chrome/browser/extensions/extension_message_service.h
+++ b/chrome/browser/extensions/extension_message_service.h
@@ -140,13 +140,15 @@ class ExtensionMessageService : public NotificationObserver {
   // opened.
   void OpenChannelOnUIThread(int source_routing_id,
       int source_port_id, int source_process_id,
-      int dest_port_id, int dest_process_id);
+      int dest_port_id, int dest_process_id,
+      const std::string& extension_id);
 
   // Common between OpenChannelOnUIThread and
   // OpenAutomationChannelToExtension.
   void OpenChannelOnUIThreadImpl(
-    int source_routing_id, int source_port_id, IPC::Message::Sender* source,
-    int dest_port_id, int dest_process_id, int source_process_id);
+    int source_routing_id, int source_port_id, int source_process_id,
+    IPC::Message::Sender* source, int dest_port_id, int dest_process_id,
+    const std::string& extension_id);
 
   MessageChannelMap channels_;
 
diff --git a/chrome/browser/extensions/extension_messages_unittest.cc b/chrome/browser/extensions/extension_messages_unittest.cc
index 45de6c7..86a3180 100644
--- a/chrome/browser/extensions/extension_messages_unittest.cc
+++ b/chrome/browser/extensions/extension_messages_unittest.cc
@@ -34,7 +34,7 @@ static void DispatchOnMessage(const std::string& message, int source_port_id) {
 
 // Tests that the bindings for opening a channel to an extension and sending
 // and receiving messages through that channel all works.
-TEST_F(RenderViewTest, DISABLED_ExtensionMessagesOpenChannel) {
+TEST_F(RenderViewTest, ExtensionMessagesOpenChannel) {
   render_thread_.sink().ClearMessages();
   LoadHTML("<body></body>");
   ExecuteJavaScript(
@@ -78,7 +78,7 @@ TEST_F(RenderViewTest, DISABLED_ExtensionMessagesOpenChannel) {
 
 // Tests that the bindings for handling a new channel connection and channel
 // closing all works.
-TEST_F(RenderViewTest, DISABLED_ExtensionMessagesOnConnect) {
+TEST_F(RenderViewTest, ExtensionMessagesOnConnect) {
   LoadHTML("<body></body>");
   ExecuteJavaScript(
     "chrome.self.onConnect.addListener(function (port) {"