[sync] Prevent flash of error icon while loading syncSetup page

[sync] Prevent flash of error icon while loading syncSetup page

Profile sync service relies on the lifetime of
SyncSetupInProgressHandles to determine if setup is in progress.
Currently, none of these handles are alive in the period between the
closure of the sync confirmation dialog and the loading of the sync
setup page. This causes profile sync service to think that setup is both
incomplete and not in progress, which surfaces an error icon on the
avatar.

This CL causes PeopleHandler to acquire its SyncSetupInProgressHandle
earlier if it detects that it's navigating directly to the syncSetup
page.

R=
BUG=738789
Review-Url: https://codereview.chromium.org/2971353002
Cr-Commit-Position: refs/heads/master@{#487236}
Committed: https://chromium.googlesource.com/chromium/src/+/a7580295ea5bc9ae2c4c8c8d02821d56ecdc2af0

[Patrick Noland, 2017-07-07 18:08:37]

The CQ bit was checked by pnoland@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-07 18:08:58]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-07 19:07:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-07-07 19:08:00]

Dry run: This issue passed the CQ dry run.

[Patrick Noland, 2017-07-07 19:12:32]

Description was changed from

==========
[sync] Prevent flash of error icon while loading syncSetup page

Profile sync service relies on the lifetime of
SyncSetupInProgressHandles to determine if setup is in progress.
Currently, none of these handles are alive in the period between the
closure of the sync confirmation dialog and the loading of the sync
setup page. This causes profile sync service to think that setup is both
incomplete and not in progress, which surfaces an error icon on the
avatar.

This CL causes PeopleHandler to acquire its SyncSetupInProgressHandle
earlier if it detects that it's navigating directly to the syncSetup
page.

R=
BUG=738789
==========

to

==========
[sync] Prevent flash of error icon while loading syncSetup page

Profile sync service relies on the lifetime of
SyncSetupInProgressHandles to determine if setup is in progress.
Currently, none of these handles are alive in the period between the
closure of the sync confirmation dialog and the loading of the sync
setup page. This causes profile sync service to think that setup is both
incomplete and not in progress, which surfaces an error icon on the
avatar.

This CL causes PeopleHandler to acquire its SyncSetupInProgressHandle
earlier if it detects that it's navigating directly to the syncSetup
page.

R=
BUG=738789
==========

[Patrick Noland, 2017-07-07 19:12:32]

pnoland@chromium.org changed reviewers:
+ skym@chromium.org, tommycli@chromium.org

[Patrick Noland, 2017-07-07 19:13:25]

Sky, Tommy, PTAL

[skym, 2017-07-07 19:29:51]

lgtm

[tommycli, 2017-07-17 16:44:23]

lgtm with nit

https://codereview.chromium.org/2971353002/diff/1/chrome/browser/ui/webui/set...
File chrome/browser/ui/webui/settings/people_handler.cc (right):

https://codereview.chromium.org/2971353002/diff/1/chrome/browser/ui/webui/set...
chrome/browser/ui/webui/settings/people_handler.cc:697: void
PeopleHandler::MaybeAcquireSyncBlocker() {
I've been trained to not use MaybeFoo() generally, since then the reader
naturally asks "Maybe? Why maybe? under what conditions?"

In this case, maybe you can rename to: InitailizeSyncBlocker -- if the caller
truly does not need to know the details within...

[Patrick Noland, 2017-07-17 17:52:19]

The CQ bit was checked by pnoland@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-17 17:52:35]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-17 19:10:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-07-17 19:10:56]

Dry run: This issue passed the CQ dry run.

[Patrick Noland, 2017-07-17 20:38:52]

The CQ bit was checked by pnoland@chromium.org

[Patrick Noland, 2017-07-17 20:38:53]

The patchset sent to the CQ was uploaded after l-g-t-m from
tommycli@chromium.org, skym@chromium.org
Link to the patchset: https://codereview.chromium.org/2971353002/#ps20001
(title: "rename MaybeAcquireSyncBlocker")

[Patrick Noland, 2017-07-17 20:39:12]

https://codereview.chromium.org/2971353002/diff/1/chrome/browser/ui/webui/set...
File chrome/browser/ui/webui/settings/people_handler.cc (right):

https://codereview.chromium.org/2971353002/diff/1/chrome/browser/ui/webui/set...
chrome/browser/ui/webui/settings/people_handler.cc:697: void
PeopleHandler::MaybeAcquireSyncBlocker() {
On 2017/07/17 16:44:22, tommycli wrote:
> I've been trained to not use MaybeFoo() generally, since then the reader
> naturally asks "Maybe? Why maybe? under what conditions?"
> 
> In this case, maybe you can rename to: InitailizeSyncBlocker -- if the caller
> truly does not need to know the details within...

Done.

[commit-bot: I haz the power, 2017-07-17 20:39:12]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-17 20:48:53]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1500323932806310,
"parent_rev": "254fa14030667b2a7b707e975a23437d9faca45d", "commit_rev":
"a7580295ea5bc9ae2c4c8c8d02821d56ecdc2af0"}

[commit-bot: I haz the power, 2017-07-17 20:49:08]

Description was changed from

==========
[sync] Prevent flash of error icon while loading syncSetup page

Profile sync service relies on the lifetime of
SyncSetupInProgressHandles to determine if setup is in progress.
Currently, none of these handles are alive in the period between the
closure of the sync confirmation dialog and the loading of the sync
setup page. This causes profile sync service to think that setup is both
incomplete and not in progress, which surfaces an error icon on the
avatar.

This CL causes PeopleHandler to acquire its SyncSetupInProgressHandle
earlier if it detects that it's navigating directly to the syncSetup
page.

R=
BUG=738789
==========

to

==========
[sync] Prevent flash of error icon while loading syncSetup page

Profile sync service relies on the lifetime of
SyncSetupInProgressHandles to determine if setup is in progress.
Currently, none of these handles are alive in the period between the
closure of the sync confirmation dialog and the loading of the sync
setup page. This causes profile sync service to think that setup is both
incomplete and not in progress, which surfaces an error icon on the
avatar.

This CL causes PeopleHandler to acquire its SyncSetupInProgressHandle
earlier if it detects that it's navigating directly to the syncSetup
page.

R=
BUG=738789

Review-Url: https://codereview.chromium.org/2971353002
Cr-Commit-Position: refs/heads/master@{#487236}
Committed:
https://chromium.googlesource.com/chromium/src/+/a7580295ea5bc9ae2c4c8c8d0282...
==========

[commit-bot: I haz the power, 2017-07-17 20:49:10]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/a7580295ea5bc9ae2c4c8c8d0282...