Fix bug where transient pages would miss events dispatched while it was

chrome/browser/extensions/lazy_background_task_queue.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/extensions/lazy_background_task_queue.h"
 
 #include "base/callback.h"
+#include "base/message_loop.h"
 #include "chrome/browser/extensions/extension_host.h"
 #include "chrome/browser/extensions/extension_process_manager.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/extensions/process_map.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
 #include "chrome/common/chrome_notification_types.h"
@@ skipping 18 matching lines @@
                  content::Source<Profile>(profile));
 }
 
 LazyBackgroundTaskQueue::~LazyBackgroundTaskQueue() {
 }
 
 bool LazyBackgroundTaskQueue::ShouldEnqueueTask(
     Profile* profile, const Extension* extension) {
   DCHECK(extension);
   if (extension->has_lazy_background_page()) {
-    ExtensionProcessManager* pm = profile->GetExtensionProcessManager();
+    ExtensionProcessManager* pm =
+         ExtensionSystem::Get(profile)->process_manager();
     ExtensionHost* background_host =
         pm->GetBackgroundHostForExtension(extension->id());
-    if (!background_host || !background_host->did_stop_loading())
+    if (!background_host || !background_host->did_stop_loading() ||
+        pm->IsBackgroundHostClosing(extension->id()))
       return true;
   }
 
   return false;
 }
 
 void LazyBackgroundTaskQueue::AddPendingTask(
     Profile* profile,
     const std::string& extension_id,
     const PendingTask& task) {
   PendingTasksList* tasks_list = NULL;
   PendingTasksKey key(profile, extension_id);
   PendingTasksMap::iterator it = pending_tasks_.find(key);
   if (it == pending_tasks_.end()) {
     tasks_list = new PendingTasksList();
     pending_tasks_[key] = linked_ptr<PendingTasksList>(tasks_list);
 
     // If this is the first enqueued task, ensure the background page

[Yoyo Zhou, 2012/04/19 21:47:13]

This comment needs an "and we are not waiting for the background page to
unload".

[Matt Perry, 2012/04/19 22:21:53]

Done.

     // is loaded.
-    const Extension* extension =
-        ExtensionSystem::Get(profile)->extension_service()->
-            extensions()->GetByID(extension_id);
-    DCHECK(extension->has_lazy_background_page());
-    ExtensionProcessManager* pm =
-        ExtensionSystem::Get(profile)->process_manager();
-    pm->IncrementLazyKeepaliveCount(extension);
-    pm->CreateBackgroundHost(extension, extension->GetBackgroundURL());
+    if (pending_page_loads_.count(key) == 0)
+      StartLazyBackgroundPage(profile, extension_id);
   } else {
     tasks_list = it->second.get();
   }
 
   tasks_list->push_back(task);
 }
 
+void LazyBackgroundTaskQueue::StartLazyBackgroundPage(
+    Profile* profile, const std::string& extension_id) {
+  ExtensionProcessManager* pm =
+       ExtensionSystem::Get(profile)->process_manager();
+  if (pm->IsBackgroundHostClosing(extension_id)) {
+    // When the background host finishes closing, we will reload it.
+    pending_page_loads_.insert(PendingTasksKey(profile, extension_id));
+    return;
+  }
+
+  const Extension* extension =
+      ExtensionSystem::Get(profile)->extension_service()->
+          extensions()->GetByID(extension_id);
+  DCHECK(extension->has_lazy_background_page());
+  pm->IncrementLazyKeepaliveCount(extension);
+  pm->CreateBackgroundHost(extension, extension->GetBackgroundURL());
+
+  pending_page_loads_.erase(PendingTasksKey(profile, extension_id));
+}
+
 void LazyBackgroundTaskQueue::ProcessPendingTasks(
     ExtensionHost* host,
     Profile* profile,
     const Extension* extension) {
   if (!profile->IsSameProfile(profile_) ||
       !extension->has_lazy_background_page())
     return;
 
   PendingTasksKey key(profile, extension->id());
   PendingTasksMap::iterator map_it = pending_tasks_.find(key);
@@ skipping 29 matching lines @@
       // events for it.
       ExtensionHost* host = content::Details<ExtensionHost>(details).ptr();
       if (host->extension_host_type() ==
               chrome::VIEW_TYPE_EXTENSION_BACKGROUND_PAGE) {
         CHECK(host->did_stop_loading());
         ProcessPendingTasks(host, host->profile(), host->extension());
       }
       break;
     }
     case chrome::NOTIFICATION_EXTENSION_HOST_DESTROYED: {
-      // Notify consumers about the load failure when the background host dies.
-      // This can happen if the extension crashes. This is not strictly
-      // necessary, since we also unload the extension in that case (which
-      // dispatches the tasks below), but is a good extra precaution.
       Profile* profile = content::Source<Profile>(source).ptr();
       ExtensionHost* host = content::Details<ExtensionHost>(details).ptr();
       if (host->extension_host_type() ==
               chrome::VIEW_TYPE_EXTENSION_BACKGROUND_PAGE) {
-        ProcessPendingTasks(NULL, profile, host->extension());
+        PendingTasksKey key(profile,  host->extension()->id());
+        if (pending_page_loads_.count(key) > 0) {
+          // We were waiting for the background page to unload. We can start it
+          // up again and dispatch any queued events.
+          MessageLoop::current()->PostTask(FROM_HERE, base::Bind(
+              &LazyBackgroundTaskQueue::StartLazyBackgroundPage,
+              base::Unretained(this), profile, host->extension()->id()));

[Yoyo Zhou, 2012/04/19 21:47:13]

If we might be receiving this notification near shutdown, WeakPtrFactory might
be safer than Unretained.

[Matt Perry, 2012/04/19 22:21:53]

Done.

+        } else {
+          // This may be a load failure (e.g. a crash). In that case, notify
+          // consumers about the load failure. This is not strictly necessary,
+          // since we also unload the extension in that case (which dispatches
+          // the tasks below), but is a good extra precaution.
+          ProcessPendingTasks(NULL, profile, host->extension());
+        }
       }
       break;
     }
     case chrome::NOTIFICATION_EXTENSION_UNLOADED: {
       // Notify consumers that the page failed to load.
       Profile* profile = content::Source<Profile>(source).ptr();
       UnloadedExtensionInfo* unloaded =
           content::Details<UnloadedExtensionInfo>(details).ptr();
       ProcessPendingTasks(NULL, profile, unloaded->extension);
       if (profile->HasOffTheRecordProfile()) {
         ProcessPendingTasks(NULL, profile->GetOffTheRecordProfile(),
                             unloaded->extension);
       }
       break;
     }
     default:
       NOTREACHED();
       break;
   }
 }
 
 }  // namespace extensions