[Profile creation] Don't push LOAD_SIGNIN to history

This CL fixes a bug with the back navigation in profile creation flow
that triggers after the user chooses to sign in and then tries to
navigate back to the main picker screen.

The problem was with pushing LOAD_SIGNIN state on the history stack that
was not removed when coming back from that step.

This CL thus avoids pushing this state to history by directly triggering
the switch in the native UI layout.

This CL also fixes an unrelated minor issue that once call site to
record page visits got accidentally removed.

Bug: 1126913
Change-Id: I871473652cf6f632b5687f6b550f63d94f420766
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2623944
Commit-Queue: Monica Basta <msalama@chromium.org>
Reviewed-by: Monica Basta <msalama@chromium.org>
Auto-Submit: Jan Krcal <jkrcal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843508}

chrome/browser/resources/signin/profile_picker/navigation_behavior.js

@@ -80,13 +80,15 @@ if (!history.state || !history.state.route || !history.state.step) {
           {route: Routes.MAIN, step: computeStep(Routes.MAIN), isFirst: true},
           '', '/');
   }
-  recordNavigation();
+  recordPageVisited(history.state.step);
 }
 
-
-function recordNavigation() {
+/**
+ * @param {string} step
+ */
+export function recordPageVisited(step) {
   let page = /** @type {!Pages} */ (Pages.MAIN_VIEW);
-  switch (history.state.step) {
+  switch (step) {
     case 'mainView':
       page = Pages.MAIN_VIEW;
       break;
@@ -116,6 +118,7 @@ const routeObservers = new Set();
 function notifyObservers() {
   const route = /** @type {!Routes} */ (history.state.route);
   const step = history.state.step;
+  recordPageVisited(step);
   routeObservers.forEach(observer => {
     (/** @type {{onRouteChange: Function}} */ (observer))
         .onRouteChange(route, step);

chrome/browser/resources/signin/profile_picker/profile_creation_flow/profile_type_choice.js

@@ -11,8 +11,8 @@ import {FocusOutlineManager} from 'chrome://resources/js/cr/ui/focus_outline_man
 import {WebUIListenerBehavior} from 'chrome://resources/js/web_ui_listener_behavior.m.js';
 import {html, Polymer} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 
-import {AutogeneratedThemeColorInfo, ManageProfilesBrowserProxyImpl} from '../manage_profiles_browser_proxy.js';
-import {navigateToPreviousRoute, navigateToStep, ProfileCreationSteps, Routes} from '../navigation_behavior.js';
+import {AutogeneratedThemeColorInfo, ManageProfilesBrowserProxy, ManageProfilesBrowserProxyImpl} from '../manage_profiles_browser_proxy.js';
+import {navigateToPreviousRoute, navigateToStep, ProfileCreationSteps, recordPageVisited, Routes} from '../navigation_behavior.js';
 
 Polymer({
   is: 'profile-type-choice',
@@ -37,13 +37,18 @@ Polymer({
     },
   },
 
+  /** @private {?ManageProfilesBrowserProxy} */
+  manageProfilesBrowserProxy_: null,
+
   /** @override */
   ready() {
+    this.manageProfilesBrowserProxy_ =
+        ManageProfilesBrowserProxyImpl.getInstance();
     this.addWebUIListener(
         'load-signin-finished',
         success => this.handleLoadSigninFinished_(success));
     FocusOutlineManager.forDocument(document);
-    ManageProfilesBrowserProxyImpl.getInstance().recordSignInPromoImpression();
+    this.manageProfilesBrowserProxy_.recordSignInPromoImpression();
   },
 
   /** @private */
@@ -56,7 +61,12 @@ Polymer({
   onSignInClick_() {
     assert(!this.loadSigninInProgess_);
     this.loadSigninInProgess_ = true;
-    navigateToStep(Routes.NEW_PROFILE, ProfileCreationSteps.LOAD_SIGNIN);
+
+    // Explicitly record the page visit as this step is not pushed to the
+    // history stack.
+    recordPageVisited(ProfileCreationSteps.LOAD_SIGNIN);
+    this.manageProfilesBrowserProxy_.loadSignInProfileCreationFlow(
+        this.profileThemeInfo.color);
   },
 
   /** @private */

chrome/browser/resources/signin/profile_picker/profile_picker_app.js

@@ -82,17 +82,9 @@ Polymer({
       return;
     }
 
-    if (step == ProfileCreationSteps.LOAD_SIGNIN) {
-      assert(
-          route == Routes.NEW_PROFILE,
-          'LOAD_SIGNIN step must be a part of NEW_PROFILE route');
-      assert(
-          this.currentRoute_ == Routes.NEW_PROFILE,
-          'NEW_PROFILE route must have been already initialized');
-      this.manageProfilesBrowserProxy_.loadSignInProfileCreationFlow(
-          this.newProfileThemeInfo.color);
-      return;
-    }
+    assert(
+        step !== ProfileCreationSteps.LOAD_SIGNIN,
+        'LOAD_SIGNIN should not appear in navigation (only used for metrics)');
 
     const setStep = () => {
       this.$.viewManager.switchView(step, 'fade-in', 'no-animation');