Navi: Remove bookmarks when moving back/forward through history.

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

chrome/browser/resources/welcome/onboarding_welcome/email/email_chooser.js

@@ -10,7 +10,7 @@ cr.exportPath('nuxEmail');
  *   name: string,
  *   icon: string,
  *   url: string,
- *   bookmarkId: (string|undefined),
+ *   bookmarkId: (string|undefined|null),
  * }}
  */
 nuxEmail.EmailProviderModel;
@@ -73,14 +73,36 @@ Polymer({
         return;
 
       if (this.selectedEmailProvider_) {
-        this.emailProxy_.recordProviderSelected(
-            this.selectedEmailProvider_.id, this.emailList_.length);
+        // TODO(hcarmona): metrics.
+        this.revertBookmark_();
+        this.bookmarkProxy_.toggleBookmarkBar(this.bookmarkBarWasShown);
       }
 
       this.emailProxy_.recordFinalize();
     });
   },
 
+  /** Initializes the section when navigated to. */
+  initializeSection: function() {
+    this.finalized_ = false;
+    if (this.selectedEmailProvider_) {
+      this.addBookmark_(this.selectedEmailProvider_);
+      this.bookmarkProxy_.toggleBookmarkBar(true);
+    }
+  },
+
+  /** Finalizes the section when navigated away from. */
+  finalizeSection: function() {
+    if (this.finalized_)
+      return;
+
+    if (this.selectedEmailProvider_) {
+      // TODO(hcarmona): metrics?
+      this.revertBookmark_();
+      this.bookmarkProxy_.toggleBookmarkBar(this.bookmarkBarWasShown);
+    }
+  },
+
   /**
    * Handle toggling the email selected.
    * @param {!{model: {item: !nuxEmail.EmailProviderModel}}} e
@@ -122,6 +144,27 @@ Polymer({
         item.name === this.selectedEmailProvider_.name;
   },
 
+  /**
+   * @param {nuxEmail.EmailProviderModel} emailProvider
+   * @private
+   */
+  addBookmark_: function(emailProvider) {
+    if (emailProvider.bookmarkId)
+      return;
+
+    this.emailProxy_.cacheBookmarkIcon(emailProvider.id);
+    this.bookmarkProxy_.toggleBookmarkBar(true);
+    this.bookmarkProxy_.addBookmark(
+        {
+          title: emailProvider.name,
+          url: emailProvider.url,
+          parentId: '1',
+        },
+        results => {
+          this.selectedEmailProvider_.bookmarkId = results.id;
+        });
+  },
+
   /**
    * @param {nuxEmail.EmailProviderModel=} opt_emailProvider
    * @private
@@ -129,8 +172,10 @@ Polymer({
   revertBookmark_: function(opt_emailProvider) {
     let emailProvider = opt_emailProvider || this.selectedEmailProvider_;
 
-    if (emailProvider && emailProvider.bookmarkId)
+    if (emailProvider && emailProvider.bookmarkId) {
       this.bookmarkProxy_.removeBookmark(emailProvider.bookmarkId);
+      emailProvider.bookmarkId = null;
+    }
   },
 
   /**
@@ -148,21 +193,10 @@ Polymer({
       this.revertBookmark_(prevEmail);
     }
 
-    if (newEmail) {
-      this.emailProxy_.cacheBookmarkIcon(newEmail.id);
-      this.bookmarkProxy_.toggleBookmarkBar(true);
-      this.bookmarkProxy_.addBookmark(
-          {
-            title: newEmail.name,
-            url: newEmail.url,
-            parentId: '1',
-          },
-          results => {
-            this.selectedEmailProvider_.bookmarkId = results.id;
-          });
-    } else {
+    if (newEmail)
+      this.addBookmark_(newEmail);
+    else
       this.bookmarkProxy_.toggleBookmarkBar(this.bookmarkBarWasShown);
-    }
 
     // Announcements are mutually exclusive, so keeping separate.
     if (prevEmail && newEmail) {

chrome/browser/resources/welcome/onboarding_welcome/email/nux_email.js

@@ -12,6 +12,18 @@ Polymer({
     indicatorModel: Object,
   },
 
+  /**
+   * This element can receive an |onRouteChange| notification after it's
+   * detached. This will make it no-op.
+   * @private
+   */
+  isDetached_: false,
+
+  /** @override */
+  detached: function() {
+    this.isDetached_ = true;
+  },
+
   /**
    * Elements can override onRouteChange to handle route changes.
    * Overrides function in behavior.
@@ -22,8 +34,14 @@ Polymer({
     if (`step-${step}` == this.id) {
       nux.BookmarkProxyImpl.getInstance().isBookmarkBarShown().then(
           bookmarkBarShown => {
+            if (this.isDetached_)
+              return;
+
             this.$.emailChooser.bookmarkBarWasShown = bookmarkBarShown;
+            this.$.emailChooser.addBookmark();
           });
+    } else {
+      this.$.emailChooser.removeBookmark();
     }
   },
 });

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

@@ -66,12 +66,14 @@ Polymer({
 
   /** Called when bookmarks should be created for all selected apps. */
   populateAllBookmarks() {
-    if (!this.appList_) {
+    if (this.appList_) {
+      this.appList_.forEach(app => this.updateBookmark(app));
+    } else {
       this.appsProxy_.getGoogleAppsList().then(list => {
         this.appList_ = list;
-        this.appList_.forEach(app => {
-          // Default select all items.
-          app.selected = true;
+        this.appList_.forEach((app, index) => {
+          // Default select first few items.
+          app.selected = index < 3;
           this.updateBookmark(app);
         });
         this.updateHasAppsSelected();
@@ -82,17 +84,21 @@ Polymer({
 
   /** Called when bookmarks should be removed for all selected apps. */
   removeAllBookmarks() {
+    if (!this.appList_)
+      return;  // No apps to remove.
+
     let removedBookmarks = false;
     this.appList_.forEach(app => {
-      if (app.selected) {
-        app.selected = false;
-        this.updateBookmark(app);
+      if (app.selected && app.bookmarkId) {
+        // Don't call |updateBookmark| b/c we want to save the selection.
+        this.bookmarkProxy_.removeBookmark(app.bookmarkId);
+        app.bookmarkId = null;
         removedBookmarks = true;
       }
     });
     // Only update and announce if we removed bookmarks.
     if (removedBookmarks) {
-      this.updateHasAppsSelected();
+      this.bookmarkProxy_.toggleBookmarkBar(this.bookmarkBarWasShown);
       this.fire('iron-announce', {text: this.i18n('bookmarksRemoved')});
     }
   },

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

@@ -15,6 +15,19 @@ Polymer({
     indicatorModel: Object,
   },
 
+  /** @private */
+  finalized_: false,
+
+  /** @override */
+  ready: function() {
+    window.addEventListener('beforeunload', () => {
+      if (this.finalized_)
+        return;
+      // TODO(hcarmona): Add metrics.
+      this.$.appChooser.removeAllBookmarks();
+    });
+  },
+
   /**
    * Elements can override onRouteChange to handle route changes.
    * Overrides function in behavior.
@@ -23,11 +36,17 @@ Polymer({
    */
   onRouteChange: function(route, step) {
     if (`step-${step}` == this.id) {
+      this.finalized_ = false;
       nux.BookmarkProxyImpl.getInstance().isBookmarkBarShown().then(
           bookmarkBarShown => {
             this.$.appChooser.bookmarkBarWasShown = bookmarkBarShown;
           });
       this.$.appChooser.populateAllBookmarks();
+    } else {
+      if (this.finalized_)
+        return;
+      // TODO(hcarmona): Add metrics?
+      this.$.appChooser.removeAllBookmarks();
     }
   },
 
@@ -41,6 +60,7 @@ Polymer({
   /** @private */
   onGetStartedClicked_: function() {
     // TODO(hcarmona): Add metrics.
+    this.finalized_ = true;
     welcome.navigateToNextStep();
   },
 });