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

9 files changed, 64 insertions, 32 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) {"
diff --git a/chrome/renderer/extensions/event_bindings.cc b/chrome/renderer/extensions/event_bindings.cc
index 240c330..e9efbad 100644
--- a/chrome/renderer/extensions/event_bindings.cc
+++ b/chrome/renderer/extensions/event_bindings.cc
@@ -144,6 +144,10 @@ void EventBindings::HandleContextCreated(WebFrame* frame) {
       v8::Persistent<v8::Context>::New(context);
   GetContexts().push_back(linked_ptr<ContextInfo>(
       new ContextInfo(persistent_context, extension_id)));
+
+  v8::Handle<v8::Value> argv[1];
+  argv[0] = v8::String::New(extension_id.c_str());
+  CallFunctionInContext(context, "dispatchOnLoad", arraysize(argv), argv);
 }
 
 // static
@@ -182,7 +186,6 @@ void EventBindings::HandleContextDestroyed(WebFrame* frame) {
 // static
 void EventBindings::CallFunction(const std::string& function_name,
                                  int argc, v8::Handle<v8::Value>* argv) {
-  v8::HandleScope handle_scope;
   for (ContextList::iterator it = GetContexts().begin();
        it != GetContexts().end(); ++it) {
     CallFunctionInContext((*it)->context, function_name, argc, argv);
diff --git a/chrome/renderer/renderer_resources.grd b/chrome/renderer/renderer_resources.grd
index cda253b..d16c963 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. -->
+without changes to the corresponding grd file. mp6 -->
 <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 78a1ceb..a58cf37 100644
--- a/chrome/renderer/resources/event_bindings.js
+++ b/chrome/renderer/resources/event_bindings.js
@@ -59,6 +59,12 @@ var chrome = chrome || {};
     }
   };
 
+  // Test if a named event has any listeners.
+  chromeHidden.Event.hasListener = function(name) {
+    return (attachedNamedEvents[name] &&
+            attachedNamedEvents[name].listeners_.length > 0);
+  }
+
   // Registers a callback to be called when this event is dispatched.
   chrome.Event.prototype.addListener = function(cb) {
     this.listeners_.push(cb);
@@ -182,11 +188,16 @@ var chrome = chrome || {};
     request(sargs, requestId, hasCallback);
   }
 
-  // Special unload event: we don't use the DOM unload because that slows
-  // down tab shutdown.  On the other hand, this might not always fire, since
-  // Chrome will terminate renderers on shutdown (SuddenTermination).
+  // 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).
+  chromeHidden.onLoad = new chrome.Event();
   chromeHidden.onUnload = new chrome.Event();
 
+  chromeHidden.dispatchOnLoad = function(extensionId) {
+    chromeHidden.onLoad.dispatch(extensionId);
+  }
+
   chromeHidden.dispatchOnUnload = function() {
     chromeHidden.onUnload.dispatch();
     for (var i in allAttachedEvents)
diff --git a/chrome/renderer/resources/extension_process_bindings.js b/chrome/renderer/resources/extension_process_bindings.js
index 2e76723..5b6239c 100644
--- a/chrome/renderer/resources/extension_process_bindings.js
+++ b/chrome/renderer/resources/extension_process_bindings.js
@@ -490,7 +490,10 @@ var chrome = chrome || {};
 
   // Self.
   chrome.self = chrome.self || {};
-  chrome.self.onConnect = new chrome.Event("channel-connect");
+
+  chromeHidden.onLoad.addListener(function (extensionId) {
+    chrome.self.onConnect = new chrome.Event("channel-connect:" + extensionId);
+  });
 
   chrome.self.getViews = function() {
     return GetViews();
diff --git a/chrome/renderer/resources/renderer_extension_bindings.js b/chrome/renderer/resources/renderer_extension_bindings.js
index f9d354e..b27a2df 100644
--- a/chrome/renderer/resources/renderer_extension_bindings.js
+++ b/chrome/renderer/resources/renderer_extension_bindings.js
@@ -29,22 +29,30 @@ var chrome = chrome || {};
     this.onDisconnect = new chrome.Event();
     this.onMessage = new chrome.Event();
     ports[portId] = this;
-    
+
+    var port = this;
     chromeHidden.onUnload.addListener(function() {
-      this.disconnect();
+      port.disconnect();
     });
   };
 
   chromeHidden.Port = {};
 
   // Called by native code when a channel has been opened to this context.
-  chromeHidden.Port.dispatchOnConnect = function(portId, tab) {
-    var port = new chrome.Port(portId);
-    if (tab) {
-      tab = JSON.parse(tab);
+  chromeHidden.Port.dispatchOnConnect = function(portId, tab, extensionId) {
+    // Only create a new Port if someone is actually listening for a connection.
+    // 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 || "");
+    if (chromeHidden.Event.hasListener(connectEvent)) {
+      var port = new chrome.Port(portId);
+      if (tab) {
+        tab = JSON.parse(tab);
+      }
+      port.tab = tab;
+      chromeHidden.Event.dispatch(connectEvent, [port]);
     }
-    port.tab = tab;
-    chromeHidden.Event.dispatch("channel-connect", [port]);
   };
 
   // Called by native code when a channel has been closed.