ntp4: improved app install, try 2

chrome/browser/ui/webui/ntp/app_launcher_handler.cc

 // Copyright (c) 2011 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/ui/webui/ntp/app_launcher_handler.h"
 
 #include <string>
 #include <vector>
 
 #include "base/auto_reset.h"
@@ skipping 57 matching lines @@
   CHECK(bucket < extension_misc::APP_LAUNCH_BUCKET_BOUNDARY);
   return bucket;
 }
 
 }  // namespace
 
 AppLauncherHandler::AppLauncherHandler(ExtensionService* extension_service)
     : extension_service_(extension_service),
       promo_active_(false),
       ignore_changes_(false),
-      attempted_bookmark_app_install_(false) {
+      attempted_bookmark_app_install_(false),
+      has_loaded_apps_(false) {
 }
 
 AppLauncherHandler::~AppLauncherHandler() {}
 
 // Serializes |notification| into a new DictionaryValue which the caller then
 // owns.
 static DictionaryValue* SerializeNotification(
     const AppNotification& notification) {
   DictionaryValue* dictionary = new DictionaryValue();
   dictionary->SetString("title", notification.title);
@@ skipping 125 matching lines @@
     RecordWebStoreLaunch(is_promo_active);
   }  else {
     CHECK(params.size() == 3);
     RecordAppLaunchByID(is_promo_active, ParseLaunchSource(params.at(2)));
   }
 
   return true;
 }
 
 WebUIMessageHandler* AppLauncherHandler::Attach(WebUI* web_ui) {
+  registrar_.Add(this, chrome::NOTIFICATION_APP_INSTALLED_TO_NTP,
+      Source<TabContents>(web_ui->tab_contents()));
   return WebUIMessageHandler::Attach(web_ui);
 }
 
 void AppLauncherHandler::RegisterMessages() {
   web_ui_->RegisterMessageCallback("getApps",
       NewCallback(this, &AppLauncherHandler::HandleGetApps));
   web_ui_->RegisterMessageCallback("launchApp",
       NewCallback(this, &AppLauncherHandler::HandleLaunchApp));
   web_ui_->RegisterMessageCallback("setLaunchType",
       NewCallback(this, &AppLauncherHandler::HandleSetLaunchType));
@@ skipping 28 matching lines @@
       const std::string& id = *Details<const std::string>(details).ptr();
       const AppNotification* notification =
           extension_service_->app_notification_manager()->GetLast(id);
       ListValue args;
       args.Append(new StringValue(id));
       if (notification)
         args.Append(SerializeNotification(*notification));
       web_ui_->CallJavascriptFunction("appNotificationChanged", args);
       break;
     }
+    case chrome::NOTIFICATION_APP_INSTALLED_TO_NTP: {
+      if (!NewTabUI::NTP4Enabled())
+        break;
+
+      highlight_app_id_ = *Details<const std::string>(details).ptr();
+      if (has_loaded_apps_)
+        SetAppToBeHighlighted();

[Rick Byers, 2011/08/29 19:51:12]

I assume we can't do this unconditionally because it may be too early for the
setAppToBeHighlighted function to even run, right?  It seems a shame to have an
"app to highlight" here, which at some later point gets pushed to JS and cached
again, which at some later point actually causes a behavior change.  But if we
really can't reliably call into the JS this early, then I don't see an obviously
simpler option.

[Evan Stade, 2011/08/29 20:30:51]

yes, the js isn't ready yet. 

+      break;
+    }
     case chrome::NOTIFICATION_EXTENSION_LOADED: {
       const Extension* extension = Details<const Extension>(details).ptr();
       if (!extension->is_app())
         return;
 
       if (!NewTabUI::NTP4Enabled()) {
         HandleGetApps(NULL);
         break;
       }
 
@@ skipping 174 matching lines @@
 
   // If the default apps have just expired (user viewed them too many times with
   // no interaction), then we uninstall them and focus the recent sites section.
   if (apps_promo_just_expired) {
     ignore_changes_ = true;
     UninstallDefaultApps();
     ignore_changes_ = false;
     ShownSectionsHandler::SetShownSection(prefs, THUMB);
   }
 
+  SetAppToBeHighlighted();
   FillAppDictionary(&dictionary);
   web_ui_->CallJavascriptFunction("getAppsCallback", dictionary);
 
   // First time we get here we set up the observer so that we can tell update
   // the apps as they change.
-  if (registrar_.IsEmpty()) {
+  if (!has_loaded_apps_) {
+    pref_change_registrar_.Init(
+        extension_service_->extension_prefs()->pref_service());
+    pref_change_registrar_.Add(ExtensionPrefs::kExtensionsPref, this);
+    pref_change_registrar_.Add(prefs::kNTPAppPageNames, this);
+
     registrar_.Add(this, chrome::NOTIFICATION_APP_NOTIFICATION_STATE_CHANGED,
         NotificationService::AllSources());
     registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_LOADED,
         NotificationService::AllSources());
     registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_UNLOADED,
         NotificationService::AllSources());
     registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_LAUNCHER_REORDERED,
         NotificationService::AllSources());
     registrar_.Add(this, chrome::NOTIFICATION_WEB_STORE_PROMO_LOADED,
         NotificationService::AllSources());
     registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_INSTALL_ERROR,
         NotificationService::AllSources());
   }
-  if (pref_change_registrar_.IsEmpty()) {
-    pref_change_registrar_.Init(
-        extension_service_->extension_prefs()->pref_service());
-    pref_change_registrar_.Add(ExtensionPrefs::kExtensionsPref, this);
-    pref_change_registrar_.Add(prefs::kNTPAppPageNames, this);
-  }
+
+  has_loaded_apps_ = true;
 }
 
 void AppLauncherHandler::HandleLaunchApp(const ListValue* args) {
   std::string extension_id;
   double source = -1.0;
   bool alt_key = false;
   bool ctrl_key = false;
   bool meta_key = false;
   bool shift_key = false;
   double button = 0.0;
@@ skipping 287 matching lines @@
     web_app->icons.clear();
   }
 
   scoped_refptr<CrxInstaller> installer(
       extension_service_->MakeCrxInstaller(NULL));
   installer->set_page_index(install_info->page_index);
   installer->InstallWebApp(*web_app);
   attempted_bookmark_app_install_ = true;
 }
 
+void AppLauncherHandler::SetAppToBeHighlighted() {
+  if (highlight_app_id_.empty())
+    return;
+
+  scoped_ptr<StringValue> app_id(Value::CreateStringValue(highlight_app_id_));
+  web_ui_->CallJavascriptFunction("ntp4.setAppToBeHighlighted", *app_id);
+  highlight_app_id_.clear();
+}
+
 // static
 void AppLauncherHandler::RegisterUserPrefs(PrefService* pref_service) {
   // TODO(csilv): We will want this to be a syncable preference instead.
   pref_service->RegisterListPref(prefs::kNTPAppPageNames,
                                  PrefService::UNSYNCABLE_PREF);
 }
 
 // statiic
 void AppLauncherHandler::RecordWebStoreLaunch(bool promo_active) {
   UMA_HISTOGRAM_ENUMERATION(extension_misc::kAppLaunchHistogram,
@@ skipping 50 matching lines @@
     extension = extension_service_->GetExtensionById(extension_id, true);
   }
 
   ExtensionPrefs* extension_prefs = extension_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);
 
     // Launch app asynchronously so the image will update.
-    StringValue* app_id = Value::CreateStringValue(extension_id);
+    scoped_ptr<StringValue> app_id(Value::CreateStringValue(extension_id));
     web_ui_->CallJavascriptFunction("launchAppAfterEnable", *app_id);
     return;
   }
 
   if (!extension_id_prompting_.empty())
     return;  // Only one prompt at a time.
 
   extension_id_prompting_ = extension_id;
   GetExtensionInstallUI()->ConfirmReEnable(this, extension);
 }
@@ skipping 29 matching lines @@
       extension_service_->GetExtensionById(extension_id_prompting_, true);
   if (!extension)
     return;
 
   extension_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
   // icon disappears but isn't replaced by the enabled icon, making a poor
   // visual experience.
-  StringValue* app_id = Value::CreateStringValue(extension->id());
+  scoped_ptr<StringValue> app_id(Value::CreateStringValue(extension->id()));
   web_ui_->CallJavascriptFunction("launchAppAfterEnable", *app_id);
 
   extension_id_prompting_ = "";
 }
 
 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);
@@ skipping 24 matching lines @@
 
 void AppLauncherHandler::UninstallDefaultApps() {
   AppsPromo* apps_promo = extension_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);
   }
 }