[Sync] Don't set sync requested when setting setup completed.

[Sync] Don't set sync requested when setting setup completed.

These flags are independent.

BUG=577385
Committed: https://crrev.com/76b7de45b4592d5a09d2cfe8cdc1dde30ad67dc7
Cr-Commit-Position: refs/heads/master@{#371417}

[maxbogue, 2016-01-14 18:12:47]

maxbogue@chromium.org changed reviewers:
+ zea@chromium.org

[maxbogue, 2016-01-14 18:12:48]

Nicolas, PTAL!

[Nicolas Zea, 2016-01-15 21:21:19]

https://codereview.chromium.org/1586563004/diff/1/components/sync_driver/sync...
File components/sync_driver/sync_prefs.cc (left):

https://codereview.chromium.org/1586563004/diff/1/components/sync_driver/sync...
components/sync_driver/sync_prefs.cc:132: SetSyncRequested(true);
Does it make sense to DCHECK that sync was already requested? It seems like that
should be the case right?

[maxbogue, 2016-01-20 18:31:55]

https://codereview.chromium.org/1586563004/diff/1/components/sync_driver/sync...
File components/sync_driver/sync_prefs.cc (left):

https://codereview.chromium.org/1586563004/diff/1/components/sync_driver/sync...
components/sync_driver/sync_prefs.cc:132: SetSyncRequested(true);
On 2016/01/15 21:21:19, Nicolas Zea wrote:
> Does it make sense to DCHECK that sync was already requested? It seems like
that
> should be the case right?

Not really any reason to. I think it will be easier to not have to worry about
that in some startup circumstances, and it wouldn't really help anything.

[Nicolas Zea, 2016-01-20 20:49:31]

LGTM. Would be good to do a quick audit of all callsites that set
SyncSetupCompleted to true and make sure they weren't relying on this logic
though (they shouldn't be, but we should verify)

[maxbogue, 2016-01-26 00:38:35]

On 2016/01/20 20:49:31, Nicolas Zea wrote:
> LGTM. Would be good to do a quick audit of all callsites that set
> SyncSetupCompleted to true and make sure they weren't relying on this logic
> though (they shouldn't be, but we should verify)

I've added a RequestStart for the one place I thought might need it. It's
difficult to parse because the previous logic wouldn't have actually started
sync when it was called; it would just have set the flag so the next time
something else (signin, etc) tried to trigger it, it would have worked.

[maxbogue, 2016-01-26 00:38:40]

The CQ bit was checked by maxbogue@chromium.org

[maxbogue, 2016-01-26 00:38:41]

The patchset sent to the CQ was uploaded after l-g-t-m from zea@chromium.org
Link to the patchset: https://codereview.chromium.org/1586563004/#ps20001
(title: "RequestStart in OneClickSigninSyncStarter.")

[commit-bot: I haz the power, 2016-01-26 00:40:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1586563004/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1586563004/20001

[commit-bot: I haz the power, 2016-01-26 01:56:53]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-26 01:58:14]

Description was changed from

==========
[Sync] Don't set sync requested when setting setup completed.

These flags are independent.

BUG=577385
==========

to

==========
[Sync] Don't set sync requested when setting setup completed.

These flags are independent.

BUG=577385

Committed: https://crrev.com/76b7de45b4592d5a09d2cfe8cdc1dde30ad67dc7
Cr-Commit-Position: refs/heads/master@{#371417}
==========

[commit-bot: I haz the power, 2016-01-26 01:58:15]

Patchset 2 (id:??) landed as
https://crrev.com/76b7de45b4592d5a09d2cfe8cdc1dde30ad67dc7
Cr-Commit-Position: refs/heads/master@{#371417}