Make sure that the browser will always restore settings page instead of sign out page after user si…

chrome/browser/resources/settings/people_page/people_page.js

 // Copyright 2015 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.
 
 /**
  * @fileoverview
  * 'settings-people-page' is the settings page containing sign-in settings.
  */
 Polymer({
   is: 'settings-people-page',
@@ skipping 213 matching lines @@
 
   /** @private */
   onSigninTap_: function() {
     this.syncBrowserProxy_.startSignIn();
   },
 
   /** @private */
   onDisconnectClosed_: function() {
     if (settings.getCurrentRoute() == settings.Route.SIGN_OUT)
       settings.navigateToPreviousRoute();
+    this.dispatchEvent(new Event('signout-dialog-closed'));

[sky, 2016/11/11 00:32:17]

I'm not a good reviewer for the changes to this file. Please find another
reviewer.

[Dan Beam, 2016/11/11 01:56:46]

this.fire('signout-dialog-closed');

[zmin, 2016/11/11 22:54:01]

Done.

   },
 
   /** @private */
   onDisconnectTap_: function() {
     settings.navigateTo(settings.Route.SIGN_OUT);
   },
 
   /** @private */
   onDisconnectCancel_: function() {
     this.$.disconnectDialog.close();
   },
 
   /** @private */
   onDisconnectConfirm_: function() {
     var deleteProfile = !!this.syncStatus.domain ||
         (this.$.deleteProfile && this.$.deleteProfile.checked);
-    this.syncBrowserProxy_.signOut(deleteProfile);
+    // Trigger the sign out event after the navigateToPreviousRoute().
+    // So that the navigation to the setting page could be finished before the
+    // sign out if navigateToPreviousRoute() returns synchronously even the
+    // browser is closed after the sign out. Otherwise, the navigation will be
+    // finshed during session restore if the browser is closed before the async
+    // callback executed.
+    listenOnce(this, 'signout-dialog-closed', function callback() {

[Dan Beam, 2016/11/11 01:56:46]

no need to name this "callback", just:

listenOnce(this, 'signout-dialog-closed', function() {
  this.syncBrowserProxy_.signOut(deleteProfile);
}.bind(this));

also, is there any reason not just listen for this type of event all the time?
i.e.

Polymer({

  ...

  listeners: {
    'signout-dialog-closed': 'onSignoutDialogClosed_',
  },

  /** @private */
  onSignoutDialogClosed_: function() {
    this.syncBrowserProxy_.signOut(deleteProfile);
  },
});

[zmin, 2016/11/11 22:54:01]

Remove callback and, no it can't listen this even all the time. Because not all
sign out dialog closing ends up with the sign out. User cancel the sign out will
also close the sign out dialog.

+      this.syncBrowserProxy_.signOut(deleteProfile);
+    }.bind(this));
 
     this.$.disconnectDialog.close();
   },
 
   /** @private */
   onSyncTap_: function() {
     assert(this.syncStatus.signedIn);
     assert(this.syncStatus.syncSystemEnabled);
 
     if (!this.isSyncStatusActionable_(this.syncStatus))
@@ skipping 149 matching lines @@
 
   /**
    * @param {!settings.SyncStatus} syncStatus
    * @return {boolean} Whether to show the "Sign in to Chrome" button.
    * @private
    */
   showSignin_: function(syncStatus) {
     return !!syncStatus.signinAllowed && !syncStatus.signedIn;
   },
 });