Commit e8c6f424 authored by Scott Chen's avatar Scott Chen Committed by Commit Bot

Settings: back navigation on sync setup shouldn't sign users out.

As of the time of this CL (M66), window's unload event doesn't not fire
when the user navigates away with the back button. To make the behavior
consistent between tab-closinng/reload/back, we instead listen to the
beforeunload event.

Bug: 806912
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I319683f14284e4d6884aa22c84c78fea08ac78ef
Reviewed-on: https://chromium-review.googlesource.com/965251
Commit-Queue: Scott Chen <scottchen@chromium.org>
Reviewed-by: default avatarTommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543909}
parent 2839be2c
...@@ -107,9 +107,13 @@ Polymer({ ...@@ -107,9 +107,13 @@ Polymer({
* The unload callback is needed because the sign-in flow needs to know * The unload callback is needed because the sign-in flow needs to know
* if the user has closed the tab with the sync settings. This property is * if the user has closed the tab with the sync settings. This property is
* non-null if the user is currently navigated on the sync settings route. * non-null if the user is currently navigated on the sync settings route.
*
* TODO(scottchen): We had to change from unload to beforeunload due to
* crbug.com/501292. Change back to unload once it's fixed.
*
* @private {?Function} * @private {?Function}
*/ */
unloadCallback_: null, beforeunloadCallback_: null,
/** /**
* Whether the user decided to abort sync. * Whether the user decided to abort sync.
...@@ -138,9 +142,9 @@ Polymer({ ...@@ -138,9 +142,9 @@ Polymer({
if (settings.getCurrentRoute() == settings.routes.SYNC) if (settings.getCurrentRoute() == settings.routes.SYNC)
this.onNavigateAwayFromPage_(); this.onNavigateAwayFromPage_();
if (this.unloadCallback_) { if (this.beforeunloadCallback_) {
window.removeEventListener('unload', this.unloadCallback_); window.removeEventListener('beforeunload', this.beforeunloadCallback_);
this.unloadCallback_ = null; this.beforeunloadCallback_ = null;
} }
}, },
...@@ -165,7 +169,7 @@ Polymer({ ...@@ -165,7 +169,7 @@ Polymer({
onNavigateToPage_: function() { onNavigateToPage_: function() {
assert(settings.getCurrentRoute() == settings.routes.SYNC); assert(settings.getCurrentRoute() == settings.routes.SYNC);
if (this.unloadCallback_) if (this.beforeunloadCallback_)
return; return;
// Display loading page until the settings have been retrieved. // Display loading page until the settings have been retrieved.
...@@ -173,13 +177,13 @@ Polymer({ ...@@ -173,13 +177,13 @@ Polymer({
this.browserProxy_.didNavigateToSyncPage(); this.browserProxy_.didNavigateToSyncPage();
this.unloadCallback_ = this.onNavigateAwayFromPage_.bind(this); this.beforeunloadCallback_ = this.onNavigateAwayFromPage_.bind(this);
window.addEventListener('unload', this.unloadCallback_); window.addEventListener('beforeunload', this.beforeunloadCallback_);
}, },
/** @private */ /** @private */
onNavigateAwayFromPage_: function() { onNavigateAwayFromPage_: function() {
if (!this.unloadCallback_) if (!this.beforeunloadCallback_)
return; return;
// Reset the status to CONFIGURE such that the searching algorithm can // Reset the status to CONFIGURE such that the searching algorithm can
...@@ -189,8 +193,8 @@ Polymer({ ...@@ -189,8 +193,8 @@ Polymer({
this.browserProxy_.didNavigateAwayFromSyncPage(this.didAbort_); this.browserProxy_.didNavigateAwayFromSyncPage(this.didAbort_);
this.didAbort_ = false; this.didAbort_ = false;
window.removeEventListener('unload', this.unloadCallback_); window.removeEventListener('beforeunload', this.beforeunloadCallback_);
this.unloadCallback_ = null; this.beforeunloadCallback_ = null;
}, },
/** /**
......
...@@ -207,7 +207,8 @@ PeopleHandler::~PeopleHandler() { ...@@ -207,7 +207,8 @@ PeopleHandler::~PeopleHandler() {
if (!web_ui()) if (!web_ui())
return; return;
// This case is hit when the user performs a back navigation. // Note that if the user left the sync page by closing the tab, refresh,
// or via the back navigation, it would first go through OnDidClosePage().
CloseSyncSetup(); CloseSyncSetup();
} }
...@@ -730,7 +731,6 @@ void PeopleHandler::CloseSyncSetup() { ...@@ -730,7 +731,6 @@ void PeopleHandler::CloseSyncSetup() {
#if !defined(OS_CHROMEOS) #if !defined(OS_CHROMEOS)
// Sign out the user on desktop Chrome if they click cancel during // Sign out the user on desktop Chrome if they click cancel during
// initial setup. // initial setup.
// TODO(rsimha): Revisit this for M30. See http://crbug.com/252049.
if (sync_service->IsFirstSetupInProgress()) { if (sync_service->IsFirstSetupInProgress()) {
SigninManagerFactory::GetForProfile(profile_) SigninManagerFactory::GetForProfile(profile_)
->SignOut(signin_metrics::ABORT_SIGNIN, ->SignOut(signin_metrics::ABORT_SIGNIN,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment