[Sync] Calculate active types and track catch up in DTMI not PSS.

[Sync] Calculate active types and track catch up in DTMI not PSS.

This change is in preparation for factoring encryption-related logic out
of ProfileSyncService. It moves the calculation of active data types
from PSS to DTM (which makes sense since it's where we know which types
are active).

It also removes redundant tracking of the catch_up_configure_in_progress_
state from PSS, since this is already tracked in DTMI. Instead it's
passed as a boolean in the ConfigureResult object.

BUG=685337
Review-Url: https://codereview.chromium.org/2649133012
Cr-Commit-Position: refs/heads/master@{#446433}
Committed: https://chromium.googlesource.com/chromium/src/+/80b39191c5d7cbe798092d33e15f2118d658dbf1

[maxbogue, 2017-01-25 20:23:47]

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

[commit-bot: I haz the power, 2017-01-25 20:24:34]

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-01-25 20:46:39]

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

[commit-bot: I haz the power, 2017-01-25 20:46:40]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[maxbogue, 2017-01-26 01:57:15]

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

[commit-bot: I haz the power, 2017-01-26 01:58:17]

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-01-26 03:35:23]

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

[commit-bot: I haz the power, 2017-01-26 03:35:24]

Dry run: This issue passed the CQ dry run.

[maxbogue, 2017-01-26 03:39:09]

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

[maxbogue, 2017-01-26 03:39:09]

Pavel, PTAL!

[pavely, 2017-01-26 18:02:51]

https://codereview.chromium.org/2649133012/diff/20001/components/browser_sync...
File components/browser_sync/profile_sync_service.h (left):

https://codereview.chromium.org/2649133012/diff/20001/components/browser_sync...
components/browser_sync/profile_sync_service.h:944: bool
catch_up_configure_in_progress_;
I don't think we should move this field to DTM. Having this field in PSS allows
to trace logic without reviewing implementation of DTMI. By moving it to DTM you
force reader to grok details of DTM to understand logic in PSS. Does presence of
this field in PSS hinders your encryption related refactoring?

[maxbogue, 2017-01-26 18:41:39]

https://codereview.chromium.org/2649133012/diff/20001/components/browser_sync...
File components/browser_sync/profile_sync_service.h (left):

https://codereview.chromium.org/2649133012/diff/20001/components/browser_sync...
components/browser_sync/profile_sync_service.h:944: bool
catch_up_configure_in_progress_;
On 2017/01/26 18:02:51, pavely wrote:
> I don't think we should move this field to DTM. Having this field in PSS
allows
> to trace logic without reviewing implementation of DTMI. By moving it to DTM
you
> force reader to grok details of DTM to understand logic in PSS. Does presence
of
> this field in PSS hinders your encryption related refactoring?

It does, because one function I want to move out of PSS triggers a catch up
configure and currently sets the PSS flag. I don't think it's particularly
complicated to understand that the "was_catch_up_configure" field on a
ConfigureResult will be true if the configure was a catch up configure, is it?
There's really no reason we should be storing "we're doing a catch up configure"
in two different places...

[pavely, 2017-01-26 20:07:06]

lgtm

[maxbogue, 2017-01-26 20:37:35]

The CQ bit was checked by maxbogue@chromium.org

[commit-bot: I haz the power, 2017-01-26 20:38:13]

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-01-26 20:43:36]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1485463055975030,
"parent_rev": "672f4a2e91db3968619c5b46d6c466270584fdb6", "commit_rev":
"80b39191c5d7cbe798092d33e15f2118d658dbf1"}

[commit-bot: I haz the power, 2017-01-26 20:44:08]

Description was changed from

==========
[Sync] Calculate active types and track catch up in DTMI not PSS.

This change is in preparation for factoring encryption-related logic out
of ProfileSyncService. It moves the calculation of active data types
from PSS to DTM (which makes sense since it's where we know which types
are active).

It also removes redundant tracking of the catch_up_configure_in_progress_
state from PSS, since this is already tracked in DTMI. Instead it's
passed as a boolean in the ConfigureResult object.

BUG=685337
==========

to

==========
[Sync] Calculate active types and track catch up in DTMI not PSS.

This change is in preparation for factoring encryption-related logic out
of ProfileSyncService. It moves the calculation of active data types
from PSS to DTM (which makes sense since it's where we know which types
are active).

It also removes redundant tracking of the catch_up_configure_in_progress_
state from PSS, since this is already tracked in DTMI. Instead it's
passed as a boolean in the ConfigureResult object.

BUG=685337

Review-Url: https://codereview.chromium.org/2649133012
Cr-Commit-Position: refs/heads/master@{#446433}
Committed:
https://chromium.googlesource.com/chromium/src/+/80b39191c5d7cbe798092d33e15f...
==========

[commit-bot: I haz the power, 2017-01-26 20:44:09]

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