Have LazyBackgroundTaskQueue notify consumers when the transient page fails to

load, so they can do whatever cleanup is necessary.

BUG=119620
TEST=no
TBR=arv

Review URL: https://chromiumcodereview.appspot.com/10082019

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

chrome/browser/extensions/api/runtime/runtime_api.cc

@@ -17,6 +17,7 @@ namespace {
 
 const char kOnInstalledEvent[] = "experimental.runtime.onInstalled";
 const char kNoBackgroundPageError[] = "You do not have a background page.";
+const char kPageLoadError[] = "Background page failed to load.";
 
 }
 
@@ -56,8 +57,13 @@ bool RuntimeGetBackgroundPageFunction::RunImpl() {
   return true;
 }
 
-void RuntimeGetBackgroundPageFunction::OnPageLoaded(ExtensionHost*) {
-  SendResponse(true);
+void RuntimeGetBackgroundPageFunction::OnPageLoaded(ExtensionHost* host) {
+  if (host) {
+    SendResponse(true);
+  } else {
+    error_ = kPageLoadError;
+    SendResponse(false);
+  }
 }
 
 }   // namespace extensions

chrome/browser/extensions/extension_event_router.cc

@@ -423,6 +423,9 @@ void ExtensionEventRouter::OnExtensionEventAck(
 
 void ExtensionEventRouter::DispatchPendingEvent(
     const linked_ptr<ExtensionEvent>& event, ExtensionHost* host) {
+  if (!host)
+    return;
+
   ListenerProcess listener(host->render_process_host(),
                            host->extension()->id());
   if (listeners_[event->event_name].count(listener) > 0u)

chrome/browser/extensions/extension_message_service.cc

@@ -442,6 +442,9 @@ void ExtensionMessageService::PendingOpenChannel(
     const OpenChannelParams& params_in,
     int source_process_id,
     ExtensionHost* host) {
+  if (!host)
+    return;  // TODO(mpcomplete): notify source of disconnect?
+
   // Re-lookup the source process since it may no longer be valid.
   OpenChannelParams params = params_in;
   params.source = content::RenderProcessHost::FromID(source_process_id);

chrome/browser/extensions/extension_message_service.h

@@ -126,13 +126,17 @@ class ExtensionMessageService : public content::NotificationObserver {
   void PendingOpenChannel(const OpenChannelParams& params,
                           int source_process_id,
                           ExtensionHost* host);
-  void PendingCloseChannel(int port_id, bool connection_error, ExtensionHost*) {
-    CloseChannel(port_id, connection_error);
+  void PendingCloseChannel(int port_id,
+                           bool connection_error,
+                           ExtensionHost* host) {
+    if (host)
+      CloseChannel(port_id, connection_error);
   }
   void PendingPostMessage(int port_id,
                           const std::string& message,
-                          ExtensionHost*) {
-    PostMessageFromRenderer(port_id, message);
+                          ExtensionHost* host) {
+    if (host)
+      PostMessageFromRenderer(port_id, message);
   }
 
   content::NotificationRegistrar registrar_;

chrome/browser/extensions/lazy_background_task_queue.cc

@@ -8,6 +8,7 @@
 #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"
@@ -65,10 +66,12 @@ void LazyBackgroundTaskQueue::AddPendingTask(
 
     // If this is the first enqueued task, ensure the background page
     // is loaded.
-    const Extension* extension = profile->GetExtensionService()->
-        extensions()->GetByID(extension_id);
+    const Extension* extension =
+        ExtensionSystem::Get(profile)->extension_service()->
+            extensions()->GetByID(extension_id);
     DCHECK(extension->has_lazy_background_page());
-    ExtensionProcessManager* pm = profile->GetExtensionProcessManager();
+    ExtensionProcessManager* pm =
+        ExtensionSystem::Get(profile)->process_manager();
     pm->IncrementLazyKeepaliveCount(extension);
     pm->CreateBackgroundHost(extension, extension->GetBackgroundURL());
   } else {
@@ -78,11 +81,18 @@ void LazyBackgroundTaskQueue::AddPendingTask(
   tasks_list->push_back(task);
 }
 
-void LazyBackgroundTaskQueue::ProcessPendingTasks(ExtensionHost* host) {
-  PendingTasksKey key(host->profile(), host->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);
   if (map_it == pending_tasks_.end()) {
-    NOTREACHED();  // lazy page should not load without any pending tasks
+    CHECK(!host);  // lazy page should not load without any pending tasks
     return;
   }
 
@@ -95,9 +105,12 @@ void LazyBackgroundTaskQueue::ProcessPendingTasks(ExtensionHost* host) {
   tasks->clear();
   pending_tasks_.erase(map_it);
 
-  // Balance the keepalive in AddPendingTask.
-  host->profile()->GetExtensionProcessManager()->
-      DecrementLazyKeepaliveCount(host->extension());
+  // Balance the keepalive in AddPendingTask. Note we don't do this on a
+  // failure to load, because the keepalive count is reset in that case.
+  if (host) {
+    ExtensionSystem::Get(profile)->process_manager()->
+        DecrementLazyKeepaliveCount(extension);
+  }
 }
 
 void LazyBackgroundTaskQueue::Observe(
@@ -109,38 +122,36 @@ void LazyBackgroundTaskQueue::Observe(
       // If an on-demand background page finished loading, dispatch queued up
       // events for it.
       ExtensionHost* host = content::Details<ExtensionHost>(details).ptr();
-      if (host->profile()->IsSameProfile(profile_) &&
-          host->extension_host_type() ==
-              chrome::VIEW_TYPE_EXTENSION_BACKGROUND_PAGE &&
-          host->extension()->has_lazy_background_page()) {
+      if (host->extension_host_type() ==
+              chrome::VIEW_TYPE_EXTENSION_BACKGROUND_PAGE) {
         CHECK(host->did_stop_loading());
-        ProcessPendingTasks(host);
+        ProcessPendingTasks(host, host->profile(), host->extension());
       }
       break;
     }
     case chrome::NOTIFICATION_EXTENSION_HOST_DESTROYED: {
-      // Clear pending tasks 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 clears the tasks below),
-      // but is a good extra precaution.
+      // 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) {
-        pending_tasks_.erase(PendingTasksKey(profile, host->extension()->id()));
+        ProcessPendingTasks(NULL, profile, host->extension());
       }
       break;
     }
     case chrome::NOTIFICATION_EXTENSION_UNLOADED: {
-      // Clear pending tasks for this extension.
+      // Notify consumers that the page failed to load.
       Profile* profile = content::Source<Profile>(source).ptr();
       UnloadedExtensionInfo* unloaded =
           content::Details<UnloadedExtensionInfo>(details).ptr();
-      pending_tasks_.erase(PendingTasksKey(
-          profile, unloaded->extension->id()));
-      if (profile->HasOffTheRecordProfile())
-        pending_tasks_.erase(PendingTasksKey(
-            profile->GetOffTheRecordProfile(), unloaded->extension->id()));
+      ProcessPendingTasks(NULL, profile, unloaded->extension);
+      if (profile->HasOffTheRecordProfile()) {
+        ProcessPendingTasks(NULL, profile->GetOffTheRecordProfile(),
+                            unloaded->extension);
+      }
       break;
     }
     default:

chrome/browser/extensions/lazy_background_task_queue.h

@@ -40,6 +40,9 @@ class LazyBackgroundTaskQueue : public content::NotificationObserver {
 
   // Adds a task to the queue for a given extension. If this is the first
   // task added for the extension, its lazy background page will be loaded.
+  // The task will be called either when the page is loaded, or when the
+  // page fails to load for some reason (e.g. a crash). In the latter case,
+  // the ExtensionHost parameter is NULL.
   void AddPendingTask(
       Profile* profile,
       const std::string& extension_id,
@@ -59,9 +62,12 @@ class LazyBackgroundTaskQueue : public content::NotificationObserver {
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details) OVERRIDE;
 
-  // Called when a lazy background page has finished loading. All enqueued
-  // tasks are run in order.
-  void ProcessPendingTasks(ExtensionHost* host);
+  // Called when a lazy background page has finished loading, or has failed to
+  // load (host is NULL in that case). All enqueued tasks are run in order.
+  void ProcessPendingTasks(
+      ExtensionHost* host,
+      Profile* profile,
+      const Extension* extension);
 
   Profile* profile_;
   content::NotificationRegistrar registrar_;

chrome/browser/ui/webui/extensions/extension_settings_handler.cc

@@ -822,5 +822,6 @@ ExtensionSettingsHandler::GetExtensionUninstallDialog() {
 }
 
 void ExtensionSettingsHandler::InspectExtensionHost(ExtensionHost* host) {
-  DevToolsWindow::OpenDevToolsWindow(host->render_view_host());
+  if (host)
+    DevToolsWindow::OpenDevToolsWindow(host->render_view_host());
 }