Don't close background pages if there is a pending response (callback).

BUG=81752
TEST=


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

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

chrome/browser/extensions/extension_event_router.cc

@@ -288,15 +288,36 @@ void ExtensionEventRouter::DispatchEventImpl(
         DispatchEvent(listener->process, listener->extension_id,
                       event->event_name, event->cross_incognito_args,
                       event->event_url);
+        IncrementInFlightEvents(listener->extension_id);
       }
       continue;
     }
 
     DispatchEvent(listener->process, listener->extension_id,
                   event->event_name, event->event_args, event->event_url);
+    IncrementInFlightEvents(listener->extension_id);
   }
 }
 
+void ExtensionEventRouter::IncrementInFlightEvents(
+    const std::string& extension_id) {
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableLazyBackgroundPages))
+    in_flight_events_[extension_id]++;
+}
+
+void ExtensionEventRouter::OnExtensionEventAck(
+    const std::string& extension_id) {
+  CHECK(CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableLazyBackgroundPages));
+  CHECK(in_flight_events_[extension_id] > 0);
+  in_flight_events_[extension_id]--;
+}
+
+bool ExtensionEventRouter::HasInFlightEvents(const std::string& extension_id) {
+  return in_flight_events_[extension_id] > 0;
+}
+
 void ExtensionEventRouter::AppendEvent(
     const linked_ptr<ExtensionEvent>& event) {
   PendingEventsList* events_list = NULL;

chrome/browser/extensions/extension_event_router.h

@@ -88,6 +88,13 @@ class ExtensionEventRouter : public content::NotificationObserver {
       const std::string& cross_incognito_args,
       const GURL& event_url);
 
+  // Record the Event Ack from the renderer. (One less event in-flight.)
+  void OnExtensionEventAck(const std::string& extension_id);
+
+  // Check if there are any Extension Events that have not yet been acked by
+  // the renderer.
+  bool HasInFlightEvents(const std::string& extension_id);
+
  protected:
   // The details of an event to be dispatched.
   struct ExtensionEvent;
@@ -132,6 +139,11 @@ class ExtensionEventRouter : public content::NotificationObserver {
                    linked_ptr<PendingEventsList> > PendingEventsPerExtMap;
   PendingEventsPerExtMap pending_events_;
 
+  // Track of the number of dispatched events that have not yet sent an
+  // ACK from the renderer.
+  void IncrementInFlightEvents(const std::string& extension_id);
+  std::map<std::string, int> in_flight_events_;
+
   DISALLOW_COPY_AND_ASSIGN(ExtensionEventRouter);
 };
 

chrome/browser/extensions/extension_process_manager.cc

@@ -8,6 +8,7 @@
 
 #include "chrome/browser/ui/browser_window.h"
 #include "content/browser/browsing_instance.h"
+#include "chrome/browser/extensions/extension_event_router.h"
 #if defined(OS_MACOSX)
 #include "chrome/browser/extensions/extension_host_mac.h"
 #endif
@@ -429,8 +430,12 @@ bool ExtensionProcessManager::HasExtensionHost(ExtensionHost* host) const {
 
 void ExtensionProcessManager::OnExtensionIdle(const std::string& extension_id) {
   ExtensionHost* host = GetBackgroundHostForExtension(extension_id);
-  if (host && !HasVisibleViews(extension_id))
+  if (host && !HasVisibleViews(extension_id)) {
+    Profile* profile =
+        Profile::FromBrowserContext(browsing_instance_->browser_context());
+    if (!profile->GetExtensionEventRouter()->HasInFlightEvents(extension_id))
       CloseBackgroundHost(host);
+  }
 }
 
 bool ExtensionProcessManager::HasVisibleViews(const std::string& extension_id) {

chrome/browser/extensions/lazy_background_page_apitest.cc

@@ -87,10 +87,9 @@ IN_PROC_BROWSER_TEST_F(LazyBackgroundPageApiTest,
   bg_pg_created.Wait();
   bg_pg_closed.Wait();
 
-  // Background page is closed before it created a new tab.
-  // TODO(tessamac): Implement! Close background page after callback.
+  // Background page is closed after creating a new tab.
   EXPECT_FALSE(pm->GetBackgroundHostForExtension(last_loaded_extension_id_));
-  EXPECT_EQ(num_tabs_before, browser()->tab_count());
+  EXPECT_EQ(num_tabs_before + 1, browser()->tab_count());
 }
 
 IN_PROC_BROWSER_TEST_F(LazyBackgroundPageApiTest,

chrome/browser/renderer_host/chrome_render_message_filter.cc

@@ -130,6 +130,7 @@ bool ChromeRenderMessageFilter::OnMessageReceived(const IPC::Message& message,
     IPC_MESSAGE_HANDLER(ExtensionHostMsg_RemoveListener,
                         OnExtensionRemoveListener)
     IPC_MESSAGE_HANDLER(ExtensionHostMsg_ExtensionIdle, OnExtensionIdle)
+    IPC_MESSAGE_HANDLER(ExtensionHostMsg_ExtensionEventAck, OnExtensionEventAck)
     IPC_MESSAGE_HANDLER(ExtensionHostMsg_CloseChannel, OnExtensionCloseChannel)
     IPC_MESSAGE_HANDLER(ExtensionHostMsg_RequestForIOThread,
                         OnExtensionRequestForIOThread)
@@ -192,6 +193,7 @@ void ChromeRenderMessageFilter::OverrideThreadForMessage(
     case ExtensionHostMsg_AddListener::ID:
     case ExtensionHostMsg_RemoveListener::ID:
     case ExtensionHostMsg_ExtensionIdle::ID:
+    case ExtensionHostMsg_ExtensionEventAck::ID:
     case ExtensionHostMsg_CloseChannel::ID:
     case ChromeViewHostMsg_UpdatedCacheStats::ID:
       *thread = BrowserThread::UI;
@@ -381,6 +383,12 @@ void ChromeRenderMessageFilter::OnExtensionIdle(
     profile_->GetExtensionProcessManager()->OnExtensionIdle(extension_id);
 }
 
+void ChromeRenderMessageFilter::OnExtensionEventAck(
+    const std::string& extension_id) {
+  if (profile_->GetExtensionEventRouter())
+    profile_->GetExtensionEventRouter()->OnExtensionEventAck(extension_id);
+}
+
 void ChromeRenderMessageFilter::OnExtensionCloseChannel(int port_id) {
   if (!RenderProcessHost::FromID(render_process_id_))
     return;  // To guard against crash in browser_tests shutdown.

chrome/browser/renderer_host/chrome_render_message_filter.h

@@ -93,6 +93,7 @@ class ChromeRenderMessageFilter : public BrowserMessageFilter {
   void OnExtensionRemoveListener(const std::string& extension_id,
                                  const std::string& event_name);
   void OnExtensionIdle(const std::string& extension_id);
+  void OnExtensionEventAck(const std::string& extension_id);
   void OnExtensionCloseChannel(int port_id);
   void OnExtensionRequestForIOThread(
       int routing_id,

chrome/common/extensions/extension_messages.h

@@ -273,6 +273,11 @@ IPC_MESSAGE_CONTROL2(ExtensionHostMsg_RemoveListener,
 IPC_MESSAGE_CONTROL1(ExtensionHostMsg_ExtensionIdle,
                      std::string /* extension_id */)
 
+// Notify the browser that an event has finished being dispatched.
+IPC_MESSAGE_CONTROL1(ExtensionHostMsg_ExtensionEventAck,
+                     std::string /* extension_id */)
+
+
 // Open a channel to all listening contexts owned by the extension with
 // the given ID.  This always returns a valid port ID which can be used for
 // sending messages.  If an error occurred, the opener will be notified

chrome/renderer/extensions/extension_dispatcher.cc

@@ -154,10 +154,18 @@ void ExtensionDispatcher::OnMessageInvoke(const std::string& extension_id,
         kInitialExtensionIdleHandlerDelayS);
   }
 
-  // Tell the browser process that we're idle.
+  // Tell the browser process that the event is dispatched and we're idle.
   if (CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableLazyBackgroundPages) &&
-      function_name == "Event.dispatchJSON")  // may always be true
+      function_name == "Event.dispatchJSON") { // may always be true
+    RenderThread::Get()->Send(
+        new ExtensionHostMsg_ExtensionEventAck(extension_id));
+    CheckIdleStatus(extension_id);
+  }
+}
+
+void ExtensionDispatcher::CheckIdleStatus(const std::string& extension_id) {
+  if (!ExtensionProcessBindings::HasPendingRequests(extension_id))
     RenderThread::Get()->Send(new ExtensionHostMsg_ExtensionIdle(extension_id));
 }
 

chrome/renderer/extensions/extension_dispatcher.h

@@ -81,6 +81,10 @@ class ExtensionDispatcher : public content::RenderProcessObserver {
     return webrequest_other_;
   }
 
+  // If the extension is in fact idle, tell the browser process to close
+  // the background page.
+  void CheckIdleStatus(const std::string& extension_id);
+
  private:
   friend class RenderViewTest;
 

chrome/renderer/extensions/extension_helper.cc

@@ -197,9 +197,14 @@ void ExtensionHelper::OnExtensionResponse(int request_id,
                                           bool success,
                                           const std::string& response,
                                           const std::string& error) {
+  std::string extension_id;
   ExtensionProcessBindings::HandleResponse(
       extension_dispatcher_->v8_context_set(), request_id, success,
-      response, error);
+      response, error, &extension_id);
+
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableLazyBackgroundPages))
+    extension_dispatcher_->CheckIdleStatus(extension_id);
 }
 
 void ExtensionHelper::OnExtensionMessageInvoke(const std::string& extension_id,

chrome/renderer/extensions/extension_process_bindings.cc

@@ -66,11 +66,13 @@ const char* kExtensionDeps[] = {
 // Contains info relevant to a pending API request.
 struct PendingRequest {
  public :
-  PendingRequest(v8::Persistent<v8::Context> context, const std::string& name)
-      : context(context), name(name) {
+  PendingRequest(v8::Persistent<v8::Context> context, const std::string& name,
+                 const std::string& extension_id)
+      : context(context), name(name), extension_id(extension_id) {
   }
   v8::Persistent<v8::Context> context;
   std::string name;
+  std::string extension_id;
 };
 typedef std::map<int, linked_ptr<PendingRequest> > PendingRequestMap;
 
@@ -473,7 +475,7 @@ class ExtensionImpl : public ChromeV8Extension {
         v8::Persistent<v8::Context>::New(v8::Context::GetCurrent());
     DCHECK(!v8_context.IsEmpty());
     g_pending_requests.Get()[request_id].reset(new PendingRequest(
-        v8_context, name));
+        v8_context, name, current_context->extension_id()));
 
     ExtensionHostMsg_Request_Params params;
     params.name = name;
@@ -605,7 +607,8 @@ void ExtensionProcessBindings::HandleResponse(
     int request_id,
     bool success,
     const std::string& response,
-    const std::string& error) {
+    const std::string& error,
+    std::string* extension_id) {
   PendingRequestMap::iterator request =
       g_pending_requests.Get().find(request_id);
   if (request == g_pending_requests.Get().end()) {
@@ -642,7 +645,21 @@ void ExtensionProcessBindings::HandleResponse(
   }
 #endif
 
+  // Save the extension id before erasing the request.
+  *extension_id = request->second->extension_id;
+
   request->second->context.Dispose();
   request->second->context.Clear();
   g_pending_requests.Get().erase(request);
 }
+
+// static
+bool ExtensionProcessBindings::HasPendingRequests(
+    const std::string& extension_id) {
+  for (PendingRequestMap::const_iterator it = g_pending_requests.Get().begin();
+       it != g_pending_requests.Get().end(); ++it) {
+    if (it->second->extension_id == extension_id)
+      return true;
+  }
+  return false;
+}

chrome/renderer/extensions/extension_process_bindings.h

@@ -27,12 +27,15 @@ class ExtensionProcessBindings {
  public:
   static v8::Extension* Get(ExtensionDispatcher* extension_dispatcher);
 
-  // Handles a response to an API request.
+  // Handles a response to an API request.  Sets |extension_id|.
   static void HandleResponse(const ChromeV8ContextSet& contexts,
                              int request_id,
                              bool success,
                              const std::string& response,
-                             const std::string& error);
+                             const std::string& error,
+                             std::string* extension_id);
+
+  static bool HasPendingRequests(const std::string& extension_id);
 };
 
 #endif  // CHROME_RENDERER_EXTENSIONS_EXTENSION_PROCESS_BINDINGS_H_