Navi: Cache Google Apps bookmark icons from web ui.

Bug: 889222
Change-Id: I5af4c643bd14e3e52b37a931eb04b7d3c533c6f1
Reviewed-on: https://chromium-review.googlesource.com/c/1287437
Commit-Queue: Hector Carmona <hcarmona@chromium.org>
Reviewed-by: Scott Chen <scottchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602971}

chrome/browser/resources/welcome/onboarding_welcome/google_apps/apps_chooser.js

@@ -42,26 +42,28 @@ Polymer({
   },
 
   /** @private {nux.NuxGoogleAppsProxy} */
-  appProxy_: null,
+  appsProxy_: null,
 
   /** @private {nux.BookmarkProxy} */
   bookmarkProxy_: null,
 
   /** @override */
   ready() {
-    this.appProxy_ = nux.NuxGoogleAppsProxyImpl.getInstance();
+    this.appsProxy_ = nux.NuxGoogleAppsProxyImpl.getInstance();
     this.bookmarkProxy_ = nux.BookmarkProxyImpl.getInstance();
   },
 
   /** Called when bookmarks should be created for all selected apps. */
   populateAllBookmarks() {
     if (!this.appList_) {
-      this.appProxy_.getGoogleAppsList().then(list => {
+      this.appsProxy_.getGoogleAppsList().then(list => {
         this.appList_ = list;
         this.appList_.forEach(app => {
           // Default select all items.
           app.selected = true;
           this.updateBookmark(app);
+          // Icons only need to be added to the cache once.
+          this.appsProxy_.cacheBookmarkIcon(app.id);
         });
       });
     }

chrome/browser/resources/welcome/onboarding_welcome/google_apps/nux_google_apps.js

@@ -28,14 +28,13 @@ Polymer({
 
   /** @private */
   onNoThanksClicked_: function() {
-    chrome.send('rejectGoogleApps');
+    // TODO(hcarmona): Add metrics.
     welcome.navigateToNextStep();
   },
 
   /** @private */
   onGetStartedClicked_: function() {
-    let selectedApps = this.$.appChooser.getSelectedAppList();
-    nux.NuxGoogleAppsProxyImpl.getInstance().addGoogleApps(selectedApps);
+    // TODO(hcarmona): Add metrics.
     welcome.navigateToNextStep();
   },
 });

chrome/browser/resources/welcome/onboarding_welcome/google_apps/nux_google_apps_proxy.js

@@ -6,10 +6,11 @@ cr.define('nux', function() {
   /** @interface */
   class NuxGoogleAppsProxy {
     /**
-     * Adds the selected apps to the bookmark bar.
-     * @param {!Array<boolean>} selectedApps
+     * Google app IDs are local to the list of Google apps, so their icon must
+     * be cached by the handler that provided the IDs.
+     * @param {number} appId
      */
-    addGoogleApps(selectedApps) {}
+    cacheBookmarkIcon(appId) {}
 
     /**
      * Returns a promise for an array of Google apps.
@@ -21,8 +22,8 @@ cr.define('nux', function() {
   /** @implements {nux.NuxGoogleAppsProxy} */
   class NuxGoogleAppsProxyImpl {
     /** @override */
-    addGoogleApps(selectedApps) {
-      chrome.send('addGoogleApps', selectedApps);
+    cacheBookmarkIcon(appId) {
+      chrome.send('cacheGoogleAppIcon', [appId]);
     }
 
     /** @override */
@@ -37,4 +38,4 @@ cr.define('nux', function() {
     NuxGoogleAppsProxy: NuxGoogleAppsProxy,
     NuxGoogleAppsProxyImpl: NuxGoogleAppsProxyImpl,
   };
-});
+});
\ No newline at end of file

chrome/browser/ui/webui/welcome/nux/google_apps_handler.cc

@@ -61,13 +61,8 @@ GoogleAppsHandler::~GoogleAppsHandler() {}
 
 void GoogleAppsHandler::RegisterMessages() {
   web_ui()->RegisterMessageCallback(
-      "rejectGoogleApps",
-      base::BindRepeating(&GoogleAppsHandler::HandleRejectGoogleApps,
-                          base::Unretained(this)));
-
-  web_ui()->RegisterMessageCallback(
-      "addGoogleApps",
-      base::BindRepeating(&GoogleAppsHandler::HandleAddGoogleApps,
+      "cacheGoogleAppIcon",
+      base::BindRepeating(&GoogleAppsHandler::HandleCacheGoogleAppIcon,
                           base::Unretained(this)));
 
   web_ui()->RegisterMessageCallback(
@@ -76,38 +71,28 @@ void GoogleAppsHandler::RegisterMessages() {
                           base::Unretained(this)));
 }
 
-void GoogleAppsHandler::HandleRejectGoogleApps(const base::ListValue* args) {
-  UMA_HISTOGRAM_ENUMERATION(kGoogleAppsInteractionHistogram,
-                            GoogleAppsInteraction::kNoThanks,
-                            GoogleAppsInteraction::kCount);
-}
+void GoogleAppsHandler::HandleCacheGoogleAppIcon(const base::ListValue* args) {
+  int appId;
+  args->GetInteger(0, &appId);
 
-void GoogleAppsHandler::HandleAddGoogleApps(const base::ListValue* args) {
-  // Add bookmarks for all selected apps.
-  for (size_t i = 0; i < base::size(kGoogleApps); ++i) {
-    bool selected = false;
-    CHECK(args->GetBoolean(i, &selected));
-    if (selected) {
-      UMA_HISTOGRAM_ENUMERATION(
-          "FirstRun.NewUserExperience.GoogleAppsSelection",
-          (GoogleApps)kGoogleApps[i].id, GoogleApps::kCount);
-      GURL app_url = GURL(kGoogleApps[i].url);
-      // TODO(hcarmona): Add bookmark from JS.
-
-      // Preload the favicon cache with Chrome-bundled images. Otherwise, the
-      // pre-populated bookmarks don't have favicons and look bad. Favicons are
-      // updated automatically when a user visits a site.
-      favicon_service_->MergeFavicon(
-          app_url, app_url, favicon_base::IconType::kFavicon,
-          ui::ResourceBundle::GetSharedInstance().LoadDataResourceBytes(
-              kGoogleApps[i].icon),
-          gfx::Size(kGoogleAppIconSize, kGoogleAppIconSize));
+  const BookmarkItem* selectedApp = NULL;
+  for (size_t i = 0; i < base::size(kGoogleApps); i++) {
+    if (static_cast<int>(kGoogleApps[i].id) == appId) {
+      selectedApp = &kGoogleApps[i];
+      break;
     }
   }
-
-  UMA_HISTOGRAM_ENUMERATION(kGoogleAppsInteractionHistogram,
-                            GoogleAppsInteraction::kGetStarted,
-                            GoogleAppsInteraction::kCount);
+  CHECK(selectedApp);  // WebUI should not be able to pass non-existent ID.
+
+  // Preload the favicon cache with Chrome-bundled images. Otherwise, the
+  // pre-populated bookmarks don't have favicons and look bad. Favicons are
+  // updated automatically when a user visits a site.
+  GURL app_url = GURL(selectedApp->url);
+  favicon_service_->MergeFavicon(
+      app_url, app_url, favicon_base::IconType::kFavicon,
+      ui::ResourceBundle::GetSharedInstance().LoadDataResourceBytes(
+          selectedApp->icon),
+      gfx::Size(kGoogleAppIconSize, kGoogleAppIconSize));
 }
 
 void GoogleAppsHandler::HandleGetGoogleAppsList(const base::ListValue* args) {

chrome/browser/ui/webui/welcome/nux/google_apps_handler.h

@@ -39,8 +39,7 @@ class GoogleAppsHandler : public content::WebUIMessageHandler {
   void RegisterMessages() override;
 
   // Callbacks for JS APIs.
-  void HandleRejectGoogleApps(const base::ListValue* args);
-  void HandleAddGoogleApps(const base::ListValue* args);
+  void HandleCacheGoogleAppIcon(const base::ListValue* args);
   void HandleGetGoogleAppsList(const base::ListValue* args);
 
   // Adds webui sources.