Send port disconnect events when a frame is unloaded.

This is the other half of CL http://codereview.chromium.org/125280, which I split into http://codereview.chromium.org/147033 and this CL.

BUG=12686
TEST=no

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


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

5 files changed, 57 insertions, 39 deletions

diff --git a/chrome/browser/automation/extension_port_container.cc b/chrome/browser/automation/extension_port_container.cc
index 15c2c8e..581da75 100644
--- a/chrome/browser/automation/extension_port_container.cc
+++ b/chrome/browser/automation/extension_port_container.cc
@@ -34,7 +34,7 @@ ExtensionPortContainer::~ExtensionPortContainer() {
   DCHECK_EQ(MessageLoop::current()->type(), MessageLoop::TYPE_UI);
 
   if (port_id_ != -1)
-    service_->CloseAutomationChannel(port_id_);
+    service_->CloseChannel(port_id_);
 }
 
 bool ExtensionPortContainer::PostResponseToExternalPort(
diff --git a/chrome/browser/extensions/extension_message_service.cc b/chrome/browser/extensions/extension_message_service.cc
index 7a65078..0ee4579 100644
--- a/chrome/browser/extensions/extension_message_service.cc
+++ b/chrome/browser/extensions/extension_message_service.cc
@@ -290,13 +290,28 @@ int ExtensionMessageService::OpenAutomationChannelToExtension(
   return port2_id;
 }
 
-void ExtensionMessageService::CloseAutomationChannel(int port_id) {
+void ExtensionMessageService::CloseChannel(int port_id) {
   DCHECK_EQ(MessageLoop::current()->type(), MessageLoop::TYPE_UI);
 
-  // TODO(siggi): Cleanup from the tab seems to beat this to the punch.
-  // DCHECK(channels_[GET_CHANNEL_ID(port_id)].port1 != NULL);
-  // TODO(siggi): should we notify the other side of the port?
-  channels_.erase(GET_CHANNEL_ID(port_id));
+  // Note: The channel might be gone already, if the other side closed first.
+  MessageChannelMap::iterator it = channels_.find(GET_CHANNEL_ID(port_id));
+  if (it != channels_.end())
+    CloseChannelImpl(it, port_id);
+}
+
+void ExtensionMessageService::CloseChannelImpl(
+    MessageChannelMap::iterator channel_iter, int port_id) {
+  DCHECK_EQ(MessageLoop::current()->type(), MessageLoop::TYPE_UI);
+
+  // Notify the other side.
+  if (port_id == GET_CHANNEL_PORT1(channel_iter->first)) {
+    DispatchOnDisconnect(channel_iter->second.port2, port_id);
+  } else {
+    DCHECK_EQ(port_id, GET_CHANNEL_PORT2(channel_iter->first));
+    DispatchOnDisconnect(channel_iter->second.port1, port_id);
+  }
+
+  channels_.erase(channel_iter);
 }
 
 void ExtensionMessageService::PostMessageFromRenderer(
@@ -365,13 +380,9 @@ void ExtensionMessageService::Observe(NotificationType type,
        it != channels_.end(); ) {
     MessageChannelMap::iterator current = it++;
     if (current->second.port1 == renderer) {
-      DispatchOnDisconnect(current->second.port2,
-                           GET_CHANNEL_PORT1(current->first));
-      channels_.erase(current);
+      CloseChannelImpl(current, GET_CHANNEL_PORT1(current->first));
     } else if (current->second.port2 == renderer) {
-      DispatchOnDisconnect(current->second.port1,
-                           GET_CHANNEL_PORT2(current->first));
-      channels_.erase(current);
+      CloseChannelImpl(current, GET_CHANNEL_PORT2(current->first));
     }
   }
 }
diff --git a/chrome/browser/extensions/extension_message_service.h b/chrome/browser/extensions/extension_message_service.h
index dde7280..f128103 100644
--- a/chrome/browser/extensions/extension_message_service.h
+++ b/chrome/browser/extensions/extension_message_service.h
@@ -54,22 +54,17 @@ class ExtensionMessageService : public NotificationObserver {
   void AddEventListener(std::string event_name, int render_process_id);
   void RemoveEventListener(std::string event_name, int render_process_id);
 
-  // Closes an extension channel for test automation.
-  void CloseAutomationChannel(int port_id);
+  // Closes the message channel associated with the given port, and notifies
+  // the other side.
+  void CloseChannel(int port_id);
 
   // Sends a message from a renderer to the given port.
-  // TODO(mpcomplete): include the source tab.
   void PostMessageFromRenderer(int port_id, const std::string& message);
 
   // Send an event to every registered extension renderer.
   void DispatchEventToRenderers(
       const std::string& event_name, const std::string& event_args);
 
-  // NotificationObserver interface.
-  void Observe(NotificationType type,
-               const NotificationSource& source,
-               const NotificationDetails& details);
-
   // Given an extension's ID, opens a channel between the given automation
   // "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.
@@ -78,6 +73,11 @@ class ExtensionMessageService : public NotificationObserver {
                                        const std::string& extension_id,
                                        IPC::Message::Sender* source);
 
+  // NotificationObserver interface.
+  void Observe(NotificationType type,
+               const NotificationSource& source,
+               const NotificationDetails& details);
+
   // --- IO thread only:
 
   // Given an extension's ID, opens a channel between the given renderer "port"
@@ -89,6 +89,16 @@ class ExtensionMessageService : public NotificationObserver {
                              ResourceMessageFilter* source);
 
  private:
+  // The connection between two ports.  It is possible that both ports
+  // refer to the same renderer.
+  struct MessageChannel {
+    IPC::Message::Sender* port1;
+    IPC::Message::Sender* port2;
+  };
+
+  // A map of channel ID to its channel object.
+  typedef std::map<int, MessageChannel> MessageChannelMap;
+
   // Allocates a pair of port ids.
   // NOTE: this can be called from any thread.
   void AllocatePortIdPair(int* port1, int* port2);
@@ -97,6 +107,8 @@ class ExtensionMessageService : public NotificationObserver {
   // NOTE: this can be called from any thread.
   int GetProcessIdForExtension(const std::string& extension_id);
 
+  void CloseChannelImpl(MessageChannelMap::iterator channel_iter, int port_id);
+
   int OpenChannelToExtensionImpl(const std::string& extension_id,
                                  IPC::Message::Sender* source);
 
@@ -136,15 +148,6 @@ class ExtensionMessageService : public NotificationObserver {
     int source_routing_id, int source_port_id, IPC::Message::Sender* source,
     int dest_port_id, int dest_process_id, int source_process_id);
 
-  // The connection between two ports.  It is possible that both ports
-  // refer to the same renderer.
-  struct MessageChannel {
-    IPC::Message::Sender* port1;
-    IPC::Message::Sender* port2;
-  };
-
-  // A map of channel ID to its channel object.
-  typedef std::map<int, MessageChannel> MessageChannelMap;
   MessageChannelMap channels_;
 
   // True if Init has been called.
diff --git a/chrome/browser/renderer_host/browser_render_process_host.cc b/chrome/browser/renderer_host/browser_render_process_host.cc
index 03c961a..a462862 100644
--- a/chrome/browser/renderer_host/browser_render_process_host.cc
+++ b/chrome/browser/renderer_host/browser_render_process_host.cc
@@ -671,6 +671,8 @@ void BrowserRenderProcessHost::OnMessageReceived(const IPC::Message& msg) {
                           OnExtensionAddListener)
       IPC_MESSAGE_HANDLER(ViewHostMsg_ExtensionRemoveListener,
                           OnExtensionRemoveListener)
+      IPC_MESSAGE_HANDLER(ViewHostMsg_ExtensionCloseChannel,
+                          OnExtensionCloseChannel)
       IPC_MESSAGE_UNHANDLED_ERROR()
     IPC_END_MESSAGE_MAP_EX()
 
@@ -887,14 +889,17 @@ void BrowserRenderProcessHost::Observe(NotificationType type,
 
 void BrowserRenderProcessHost::OnExtensionAddListener(
     const std::string& event_name) {
-  URLRequestContext* context = profile()->GetRequestContext();
-  ExtensionMessageService* ems = ExtensionMessageService::GetInstance(context);
-  ems->AddEventListener(event_name, pid());
+  ExtensionMessageService::GetInstance(profile()->GetRequestContext())->
+      AddEventListener(event_name, pid());
 }
 
 void BrowserRenderProcessHost::OnExtensionRemoveListener(
     const std::string& event_name) {
-  URLRequestContext* context = profile()->GetRequestContext();
-  ExtensionMessageService* ems = ExtensionMessageService::GetInstance(context);
-  ems->RemoveEventListener(event_name, pid());
+  ExtensionMessageService::GetInstance(profile()->GetRequestContext())->
+      RemoveEventListener(event_name, pid());
+}
+
+void BrowserRenderProcessHost::OnExtensionCloseChannel(int port_id) {
+  ExtensionMessageService::GetInstance(profile()->GetRequestContext())->
+      CloseChannel(port_id);
 }
diff --git a/chrome/browser/renderer_host/browser_render_process_host.h b/chrome/browser/renderer_host/browser_render_process_host.h
index aee8d2b..d92d9fd 100644
--- a/chrome/browser/renderer_host/browser_render_process_host.h
+++ b/chrome/browser/renderer_host/browser_render_process_host.h
@@ -87,16 +87,15 @@ class BrowserRenderProcessHost : public RenderProcessHost,
                        const NotificationSource& source,
                        const NotificationDetails& details);
 
-  // An extension process started or stopped listening to an event.
-  void OnExtensionAddListener(const std::string& event_name);
-  void OnExtensionRemoveListener(const std::string& event_name);
-
  private:
   // Control message handlers.
   void OnPageContents(const GURL& url, int32 page_id,
                       const std::wstring& contents);
   void OnUpdatedCacheStats(const WebKit::WebCache::UsageStats& stats);
   void SuddenTerminationChanged(bool enabled);
+  void OnExtensionAddListener(const std::string& event_name);
+  void OnExtensionRemoveListener(const std::string& event_name);
+  void OnExtensionCloseChannel(int port_id);
 
   // Initialize support for visited links. Send the renderer process its initial
   // set of visited links.