Revert 95009. Causing pyauto NTP test failures on all platforms.

Original CL: - Show crashed apps on NTP, desaturated. Click to reload.

Also refactored CreateAppInfo a bit.

BUG=90433
TEST=Crash a packaged app with a background page. It appears on the NTP, and clicking its icon relaunches it.

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

TBR=yoz@chromium.org
Review URL: http://codereview.chromium.org/7553001

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

4 files changed, 82 insertions, 93 deletions

diff --git a/chrome/browser/automation/automation_provider_observers.cc b/chrome/browser/automation/automation_provider_observers.cc
index dd0cc47..155af6c 100644
--- a/chrome/browser/automation/automation_provider_observers.cc
+++ b/chrome/browser/automation/automation_provider_observers.cc
@@ -1981,10 +1981,8 @@ std::vector<DictionaryValue*>* GetAppInfoFromExtensions(
        ext != extensions->end(); ++ext) {
     // Only return information about extensions that are actually apps.
     if ((*ext)->is_app()) {
-      DictionaryValue* app_info =
-          AppLauncherHandler::CreateAppInfo(*ext, NULL, ext_service);
-      if (!app_info)
-        continue;
+      DictionaryValue* app_info = new DictionaryValue();
+      AppLauncherHandler::CreateAppInfo(*ext, NULL, ext_service, app_info);
       app_info->SetBoolean("is_component_extension",
                            (*ext)->location() == Extension::COMPONENT);
 
diff --git a/chrome/browser/ui/webui/extension_icon_source.cc b/chrome/browser/ui/webui/extension_icon_source.cc
index 482c3ee..78c7b27 100644
--- a/chrome/browser/ui/webui/extension_icon_source.cc
+++ b/chrome/browser/ui/webui/extension_icon_source.cc
@@ -278,7 +278,7 @@ bool ExtensionIconSource::ParseData(const std::string& path,
 
   std::string extension_id = path_parts.at(0);
   const Extension* extension =
-      profile_->GetExtensionService()->GetInstalledExtension(extension_id);
+      profile_->GetExtensionService()->GetExtensionById(extension_id, true);
   if (!extension)
     return false;
 
diff --git a/chrome/browser/ui/webui/ntp/app_launcher_handler.cc b/chrome/browser/ui/webui/ntp/app_launcher_handler.cc
index c36d8c4..1af426d 100644
--- a/chrome/browser/ui/webui/ntp/app_launcher_handler.cc
+++ b/chrome/browser/ui/webui/ntp/app_launcher_handler.cc
@@ -74,7 +74,7 @@ extension_misc::AppLaunchBucket ParseLaunchSource(
 }  // namespace
 
 AppLauncherHandler::AppLauncherHandler(ExtensionService* extension_service)
-    : extension_service_(extension_service),
+    : extensions_service_(extension_service),
       promo_active_(false),
       ignore_changes_(false) {
 }
@@ -96,23 +96,11 @@ static DictionaryValue* SerializeNotification(
 }
 
 // static
-DictionaryValue* AppLauncherHandler::CreateAppInfo(
-    const Extension* extension,
-    const AppNotification* notification,
-    ExtensionService* service) {
-  // Don't include the WebStore and the Cloud Print app.
-  // The WebStore launcher gets special treatment in ntp/apps.js.
-  // The Cloud Print app should never be displayed in the NTP.
-  bool ntp3 =
-      !CommandLine::ForCurrentProcess()->HasSwitch(switches::kNewTabPage4);
-  if (!extension->is_app() ||
-      (ntp3 && extension->id() == extension_misc::kWebStoreAppId) ||
-      (extension->id() == extension_misc::kCloudPrintAppId)) {
-    return NULL;
-  }
-
-  bool enabled = service->IsExtensionEnabled(extension->id()) &&
-      !service->GetTerminatedExtension(extension->id());
+void AppLauncherHandler::CreateAppInfo(const Extension* extension,
+                                       const AppNotification* notification,
+                                       ExtensionService* service,
+                                       DictionaryValue* value) {
+  bool enabled = service->IsExtensionEnabled(extension->id());
   GURL icon_big =
       ExtensionIconSource::GetIconURL(extension,
                                       Extension::EXTENSION_ICON_LARGE,
@@ -124,7 +112,6 @@ DictionaryValue* AppLauncherHandler::CreateAppInfo(
                                       ExtensionIconSet::MATCH_BIGGER,
                                       !enabled);
 
-  DictionaryValue* value = new DictionaryValue();
   value->Clear();
   value->SetString("id", extension->id());
   value->SetString("name", extension->name());
@@ -160,7 +147,6 @@ DictionaryValue* AppLauncherHandler::CreateAppInfo(
     // Only provide a value if one is stored
     value->SetInteger("page_index", page_index);
   }
-  return value;
 }
 
 // static
@@ -244,7 +230,7 @@ void AppLauncherHandler::Observe(int type,
     case chrome::NOTIFICATION_APP_NOTIFICATION_STATE_CHANGED: {
       const std::string& id = *Details<const std::string>(details).ptr();
       const AppNotification* notification =
-          extension_service_->app_notification_manager()->GetLast(id);
+          extensions_service_->app_notification_manager()->GetLast(id);
       ListValue args;
       args.Append(new StringValue(id));
       if (notification)
@@ -305,30 +291,32 @@ void AppLauncherHandler::Observe(int type,
 
 void AppLauncherHandler::FillAppDictionary(DictionaryValue* dictionary) {
   ListValue* list = new ListValue();
-  const ExtensionList* extensions = extension_service_->extensions();
+  const ExtensionList* extensions = extensions_service_->extensions();
   ExtensionList::const_iterator it;
   for (it = extensions->begin(); it != extensions->end(); ++it) {
-    DictionaryValue* app_info = GetAppInfo(*it);
-    if (app_info)
-      list->Append(app_info);
-  }
-
-  extensions = extension_service_->disabled_extensions();
-  for (it = extensions->begin(); it != extensions->end(); ++it) {
-    DictionaryValue* app_info = CreateAppInfo(*it,
-                                              NULL,
-                                              extension_service_);
+    // Don't include the WebStore and the Cloud Print app.
+    // The WebStore launcher gets special treatment in ntp/apps.js.
+    // The Cloud Print app should never be displayed in the NTP.
+    const Extension* extension = *it;
+    DictionaryValue* app_info = GetAppInfo(extension);
     if (app_info)
       list->Append(app_info);
   }
 
-  extensions = extension_service_->terminated_extensions();
+  extensions = extensions_service_->disabled_extensions();
   for (it = extensions->begin(); it != extensions->end(); ++it) {
-    DictionaryValue* app_info = CreateAppInfo(*it,
-                                              NULL,
-                                              extension_service_);
-    if (app_info)
+    bool ntp3 =
+        !CommandLine::ForCurrentProcess()->HasSwitch(switches::kNewTabPage4);
+    if ((*it)->is_app() &&
+        !(ntp3 && (*it)->id() == extension_misc::kWebStoreAppId) &&
+        ((*it)->id() != extension_misc::kCloudPrintAppId)) {
+      DictionaryValue* app_info = new DictionaryValue();
+      CreateAppInfo(*it,
+                    NULL,
+                    extensions_service_,
+                    app_info);
       list->Append(app_info);
+    }
   }
 
   dictionary->Set("apps", list);
@@ -349,8 +337,8 @@ void AppLauncherHandler::FillAppDictionary(DictionaryValue* dictionary) {
 
   dictionary->SetBoolean(
       "showLauncher",
-      extension_service_->apps_promo()->ShouldShowAppLauncher(
-          extension_service_->GetAppIds()));
+      extensions_service_->apps_promo()->ShouldShowAppLauncher(
+          extensions_service_->GetAppIds()));
 
   if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kNewTabPage4)) {
     PrefService* prefs = web_ui_->GetProfile()->GetPrefs();
@@ -363,11 +351,22 @@ void AppLauncherHandler::FillAppDictionary(DictionaryValue* dictionary) {
 }
 
 DictionaryValue* AppLauncherHandler::GetAppInfo(const Extension* extension) {
+  bool ntp3 =
+      !CommandLine::ForCurrentProcess()->HasSwitch(switches::kNewTabPage4);
+  if (!extension->is_app() ||
+      (ntp3 && extension->id() == extension_misc::kWebStoreAppId) ||
+      (extension->id() == extension_misc::kCloudPrintAppId)) {
+    return NULL;
+  }
+
+  DictionaryValue* app_info = new DictionaryValue();
   AppNotificationManager* notification_manager =
-      extension_service_->app_notification_manager();
-  return CreateAppInfo(extension,
-                       notification_manager->GetLast(extension->id()),
-                       extension_service_);
+      extensions_service_->app_notification_manager();
+  CreateAppInfo(extension,
+                notification_manager->GetLast(extension->id()),
+                extensions_service_,
+                app_info);
+  return app_info;
 }
 
 void AppLauncherHandler::FillPromoDictionary(DictionaryValue* dictionary) {
@@ -389,10 +388,10 @@ void AppLauncherHandler::HandleGetApps(const ListValue* args) {
   //    expired.
   // b) Conceptually, it doesn't really make sense to count a
   //    prefchange-triggered refresh as a promo 'view'.
-  AppsPromo* apps_promo = extension_service_->apps_promo();
+  AppsPromo* apps_promo = extensions_service_->apps_promo();
   PrefService* prefs = web_ui_->GetProfile()->GetPrefs();
   bool apps_promo_just_expired = false;
-  if (apps_promo->ShouldShowPromo(extension_service_->GetAppIds(),
+  if (apps_promo->ShouldShowPromo(extensions_service_->GetAppIds(),
                                   &apps_promo_just_expired)) {
     apps_promo->MaximizeAppsIfNecessary();
     dictionary.SetBoolean("showPromo", true);
@@ -431,7 +430,7 @@ void AppLauncherHandler::HandleGetApps(const ListValue* args) {
   }
   if (pref_change_registrar_.IsEmpty()) {
     pref_change_registrar_.Init(
-        extension_service_->extension_prefs()->pref_service());
+        extensions_service_->extension_prefs()->pref_service());
     pref_change_registrar_.Add(ExtensionPrefs::kExtensionsPref, this);
     pref_change_registrar_.Add(prefs::kNTPAppPageNames, this);
   }
@@ -463,7 +462,7 @@ void AppLauncherHandler::HandleLaunchApp(const ListValue* args) {
         launch_bucket < extension_misc::APP_LAUNCH_BUCKET_BOUNDARY);
 
   const Extension* extension =
-      extension_service_->GetExtensionById(extension_id, false);
+      extensions_service_->GetExtensionById(extension_id, false);
 
   // Prompt the user to re-enable the application if disabled.
   if (!extension) {
@@ -471,7 +470,7 @@ void AppLauncherHandler::HandleLaunchApp(const ListValue* args) {
     return;
   }
 
-  Profile* profile = extension_service_->profile();
+  Profile* profile = extensions_service_->profile();
 
   // If the user pressed special keys when clicking, override the saved
   // preference for launch container.
@@ -482,7 +481,7 @@ void AppLauncherHandler::HandleLaunchApp(const ListValue* args) {
 
   if (extension_id != extension_misc::kWebStoreAppId) {
     RecordAppLaunchByID(promo_active_, launch_bucket);
-    extension_service_->apps_promo()->ExpireDefaultApps();
+    extensions_service_->apps_promo()->ExpireDefaultApps();
   }
 
   if (disposition == NEW_FOREGROUND_TAB || disposition == NEW_BACKGROUND_TAB) {
@@ -497,7 +496,7 @@ void AppLauncherHandler::HandleLaunchApp(const ListValue* args) {
     // Look at preference to find the right launch container.  If no preference
     // is set, launch as a regular tab.
     extension_misc::LaunchContainer launch_container =
-        extension_service_->extension_prefs()->GetLaunchContainer(
+        extensions_service_->extension_prefs()->GetLaunchContainer(
             extension, ExtensionPrefs::LAUNCH_REGULAR);
 
     // To give a more "launchy" experience when using the NTP launcher, we close
@@ -524,7 +523,7 @@ void AppLauncherHandler::HandleSetLaunchType(const ListValue* args) {
   CHECK(args->GetDouble(1, &launch_type));
 
   const Extension* extension =
-      extension_service_->GetExtensionById(extension_id, true);
+      extensions_service_->GetExtensionById(extension_id, true);
   CHECK(extension);
 
   // Don't update the page; it already knows about the launch type change.
@@ -532,7 +531,7 @@ void AppLauncherHandler::HandleSetLaunchType(const ListValue* args) {
   if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kNewTabPage4))
     auto_reset.reset(new AutoReset<bool>(&ignore_changes_, true));
 
-  extension_service_->extension_prefs()->SetLaunchType(
+  extensions_service_->extension_prefs()->SetLaunchType(
       extension_id,
       static_cast<ExtensionPrefs::LaunchType>(
           static_cast<int>(launch_type)));
@@ -542,7 +541,7 @@ void AppLauncherHandler::HandleUninstallApp(const ListValue* args) {
   std::string extension_id;
   CHECK(args->GetString(0, &extension_id));
 
-  const Extension* extension = extension_service_->GetExtensionById(
+  const Extension* extension = extensions_service_->GetExtensionById(
       extension_id, true);
   if (!extension)
     return;
@@ -574,7 +573,7 @@ void AppLauncherHandler::HandleHideAppsPromo(const ListValue* args) {
   // this point, or the promotion wouldn't have been shown).
   ignore_changes_ = true;
   UninstallDefaultApps();
-  extension_service_->apps_promo()->HidePromo();
+  extensions_service_->apps_promo()->HidePromo();
   ignore_changes_ = false;
   HandleGetApps(NULL);
 }
@@ -587,7 +586,7 @@ void AppLauncherHandler::HandleCreateAppShortcut(const ListValue* args) {
   }
 
   const Extension* extension =
-      extension_service_->GetExtensionById(extension_id, true);
+      extensions_service_->GetExtensionById(extension_id, true);
   CHECK(extension);
 
   Browser* browser = BrowserList::GetLastActive();
@@ -617,8 +616,8 @@ void AppLauncherHandler::HandleReorderApps(const ListValue* args) {
   if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kNewTabPage4))
     auto_reset.reset(new AutoReset<bool>(&ignore_changes_, true));
 
-  extension_service_->extension_prefs()->SetAppDraggedByUser(dragged_app_id);
-  extension_service_->extension_prefs()->SetAppLauncherOrder(extension_ids);
+  extensions_service_->extension_prefs()->SetAppDraggedByUser(dragged_app_id);
+  extensions_service_->extension_prefs()->SetAppLauncherOrder(extension_ids);
 }
 
 void AppLauncherHandler::HandleSetPageIndex(const ListValue* args) {
@@ -632,7 +631,7 @@ void AppLauncherHandler::HandleSetPageIndex(const ListValue* args) {
   if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kNewTabPage4))
     auto_reset.reset(new AutoReset<bool>(&ignore_changes_, true));
 
-  extension_service_->extension_prefs()->SetPageIndex(extension_id,
+  extensions_service_->extension_prefs()->SetPageIndex(extension_id,
       static_cast<int>(page_index));
 }
 
@@ -680,7 +679,7 @@ void AppLauncherHandler::HandleGenerateAppForLink(const ListValue* args) {
   if (!favicon_service) {
     LOG(ERROR) << "No favicon service";
     scoped_refptr<CrxInstaller> installer(
-        extension_service_->MakeCrxInstaller(NULL));
+        extensions_service_->MakeCrxInstaller(NULL));
     installer->InstallWebApp(*web_app);
     return;
   }
@@ -706,7 +705,7 @@ void AppLauncherHandler::OnFaviconForApp(FaviconService::Handle handle,
   }
 
   scoped_refptr<CrxInstaller> installer(
-      extension_service_->MakeCrxInstaller(NULL));
+      extensions_service_->MakeCrxInstaller(NULL));
   installer->InstallWebApp(*web_app);
 }
 
@@ -763,24 +762,16 @@ void AppLauncherHandler::RecordAppLaunchByURL(
 
 void AppLauncherHandler::PromptToEnableApp(const std::string& extension_id) {
   const Extension* extension =
-      extension_service_->GetExtensionById(extension_id, true);
-  if (!extension) {
-    extension = extension_service_->GetTerminatedExtension(extension_id);
-    CHECK(extension);
-    // If the app was terminated, reload it first. (This reallocates the
-    // Extension object.)
-    extension_service_->ReloadExtension(extension_id);
-    extension = extension_service_->GetExtensionById(extension_id, true);
-  }
+      extensions_service_->GetExtensionById(extension_id, true);
+  CHECK(extension);
 
-  ExtensionPrefs* extension_prefs = extension_service_->extension_prefs();
+  ExtensionPrefs* extension_prefs = extensions_service_->extension_prefs();
   if (!extension_prefs->DidExtensionEscalatePermissions(extension_id)) {
     // Enable the extension immediately if its privileges weren't escalated.
-    // This is a no-op if the extension was previously terminated.
-    extension_service_->EnableExtension(extension_id);
+    extensions_service_->EnableExtension(extension_id);
 
     // Launch app asynchronously so the image will update.
-    StringValue* app_id = Value::CreateStringValue(extension_id);
+    StringValue* app_id = Value::CreateStringValue(extension->id());
     web_ui_->CallJavascriptFunction("launchAppAfterEnable", *app_id);
     return;
   }
@@ -799,12 +790,12 @@ void AppLauncherHandler::ExtensionDialogAccepted() {
   // The extension can be uninstalled in another window while the UI was
   // showing. Do nothing in that case.
   const Extension* extension =
-      extension_service_->GetExtensionById(extension_id_prompting_, true);
+      extensions_service_->GetExtensionById(extension_id_prompting_, true);
   if (!extension)
     return;
 
-  extension_service_->UninstallExtension(extension_id_prompting_,
-                                         false /* external_uninstall */, NULL);
+  extensions_service_->UninstallExtension(extension_id_prompting_,
+                                          false /* external_uninstall */, NULL);
 
   extension_id_prompting_ = "";
 }
@@ -820,11 +811,11 @@ void AppLauncherHandler::InstallUIProceed() {
   // The extension can be uninstalled in another window while the UI was
   // showing. Do nothing in that case.
   const Extension* extension =
-      extension_service_->GetExtensionById(extension_id_prompting_, true);
+      extensions_service_->GetExtensionById(extension_id_prompting_, true);
   if (!extension)
     return;
 
-  extension_service_->GrantPermissionsAndEnableExtension(extension);
+  extensions_service_->GrantPermissionsAndEnableExtension(extension);
 
   // We bounce this off the NTP so the browser can update the apps icon.
   // If we don't launch the app asynchronously, then the app's disabled
@@ -840,7 +831,7 @@ void AppLauncherHandler::InstallUIAbort(bool user_initiated) {
   // We record the histograms here because ExtensionDialogCanceled is also
   // called when the extension uninstall dialog is canceled.
   const Extension* extension =
-      extension_service_->GetExtensionById(extension_id_prompting_, true);
+      extensions_service_->GetExtensionById(extension_id_prompting_, true);
   std::string histogram_name = user_initiated ?
       "Extensions.Permissions_ReEnableCancel" :
       "Extensions.Permissions_ReEnableAbort";
@@ -867,11 +858,11 @@ ExtensionInstallUI* AppLauncherHandler::GetExtensionInstallUI() {
 }
 
 void AppLauncherHandler::UninstallDefaultApps() {
-  AppsPromo* apps_promo = extension_service_->apps_promo();
+  AppsPromo* apps_promo = extensions_service_->apps_promo();
   const ExtensionIdSet& app_ids = apps_promo->old_default_apps();
   for (ExtensionIdSet::const_iterator iter = app_ids.begin();
        iter != app_ids.end(); ++iter) {
-    if (extension_service_->GetExtensionById(*iter, true))
-      extension_service_->UninstallExtension(*iter, false, NULL);
+    if (extensions_service_->GetExtensionById(*iter, true))
+      extensions_service_->UninstallExtension(*iter, false, NULL);
   }
 }
diff --git a/chrome/browser/ui/webui/ntp/app_launcher_handler.h b/chrome/browser/ui/webui/ntp/app_launcher_handler.h
index 30a97cb..7d6305c 100644
--- a/chrome/browser/ui/webui/ntp/app_launcher_handler.h
+++ b/chrome/browser/ui/webui/ntp/app_launcher_handler.h
@@ -41,10 +41,10 @@ class AppLauncherHandler : public WebUIMessageHandler,
   virtual ~AppLauncherHandler();
 
   // Populate a dictionary with the information from an extension.
-  static base::DictionaryValue* CreateAppInfo(
-      const Extension* extension,
-      const AppNotification* notification,
-      ExtensionService* service);
+  static void CreateAppInfo(const Extension* extension,
+                            const AppNotification* notification,
+                            ExtensionService* service,
+                            base::DictionaryValue* value);
 
   // Callback for pings related to launching apps on the NTP.
   static bool HandlePing(Profile* profile, const std::string& path);
@@ -148,8 +148,8 @@ class AppLauncherHandler : public WebUIMessageHandler,
                        history::FaviconData data);
 
   // The apps are represented in the extensions model, which
-  // outlives us since it's owned by our containing profile.
-  ExtensionService* const extension_service_;
+  // outlives us since its owned by our containing profile.
+  ExtensionService* const extensions_service_;
 
   // We monitor changes to the extension system so that we can reload the apps
   // when necessary.