[Sync] Ensure that CONFIGURE_CLEAN types get unapplied before configuration

[Sync] Ensure that CONFIGURE_CLEAN types get unapplied before configuration

The issue is that in SyncBackendHostImpl::ConfigureDataTypes types in
clean_first_types are excluded from types_to_purge and therefore are not being
unapplied as part of  PurgeDisabledTypes call.

This affects purgiong types with persistence error as well as catchup phase of
enabling passphrase encryption.

I'm adjusted logic to include these types in unapply_types and added test for
this scenario.

BUG=675036
R=zea@chromium.org

Committed: https://crrev.com/483a408c43aa945fcafef315fb55ed6d3a00c663
Cr-Commit-Position: refs/heads/master@{#439297}

[pavely, 2016-12-16 20:23:17]

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

[commit-bot: I haz the power, 2016-12-16 20:24:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Nicolas Zea, 2016-12-16 22:08:42]

https://codereview.chromium.org/2580033002/diff/1/components/sync/driver/glue...
File components/sync/driver/glue/sync_backend_host_impl.cc (right):

https://codereview.chromium.org/2580033002/diff/1/components/sync/driver/glue...
components/sync/driver/glue/sync_backend_host_impl.cc:285: ModelTypeSet::All(),
Difference(current_types, clean_first_types));
I think it might be simpler to understand if we just insert clean_first_types
into types_to_purge?

https://codereview.chromium.org/2580033002/diff/1/components/sync/driver/glue...
File components/sync/driver/glue/sync_backend_host_impl_unittest.cc (right):

https://codereview.chromium.org/2580033002/diff/1/components/sync/driver/glue...
components/sync/driver/glue/sync_backend_host_impl_unittest.cc:848:
EXPECT_TRUE(fake_manager_->GetAndResetCleanedTypes().HasAll(clean_types));
It seems like this test isn't really verifying that types_to_unapply is part of
types_to_purge, right? Is there a way to incorporate that?

[commit-bot: I haz the power, 2016-12-16 22:35:42]

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

[commit-bot: I haz the power, 2016-12-16 22:35:42]

Dry run: This issue passed the CQ dry run.

[pavely, 2016-12-16 23:57:37]

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

[pavely, 2016-12-16 23:57:46]

https://codereview.chromium.org/2580033002/diff/1/components/sync/driver/glue...
File components/sync/driver/glue/sync_backend_host_impl.cc (right):

https://codereview.chromium.org/2580033002/diff/1/components/sync/driver/glue...
components/sync/driver/glue/sync_backend_host_impl.cc:285: ModelTypeSet::All(),
Difference(current_types, clean_first_types));
On 2016/12/16 22:08:42, Nicolas Zea wrote:
> I think it might be simpler to understand if we just insert clean_first_types
> into types_to_purge?

Done.

https://codereview.chromium.org/2580033002/diff/1/components/sync/driver/glue...
File components/sync/driver/glue/sync_backend_host_impl_unittest.cc (right):

https://codereview.chromium.org/2580033002/diff/1/components/sync/driver/glue...
components/sync/driver/glue/sync_backend_host_impl_unittest.cc:848:
EXPECT_TRUE(fake_manager_->GetAndResetCleanedTypes().HasAll(clean_types));
On 2016/12/16 22:08:42, Nicolas Zea wrote:
> It seems like this test isn't really verifying that types_to_unapply is part
of
> types_to_purge, right? Is there a way to incorporate that?

Done.

[commit-bot: I haz the power, 2016-12-16 23:58:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Nicolas Zea, 2016-12-17 00:18:55]

lgtm

https://codereview.chromium.org/2580033002/diff/20001/components/sync/engine/...
File components/sync/engine/fake_sync_manager.h (right):

https://codereview.chromium.org/2580033002/diff/20001/components/sync/engine/...
components/sync/engine/fake_sync_manager.h:50: // types.
nit: may as well call out "since the last call to GetAndReset.."

[pavely, 2016-12-17 01:20:40]

The CQ bit was checked by pavely@chromium.org

[pavely, 2016-12-17 01:20:40]

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/2580033002/#ps40001
(title: "Adjusted comment")

[commit-bot: I haz the power, 2016-12-17 01:21:08]

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, 2016-12-17 02:37:07]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1481937640043920,
"parent_rev": "166f01c098d67e504b936fb4ea1d0437380005d2", "commit_rev":
"4150b0dc1e6e2305690bc83dfffda9cc0f7e6577"}

[commit-bot: I haz the power, 2016-12-17 02:37:37]

Description was changed from

==========
[Sync] Ensure that CONFIGURE_CLEAN types get unapplied before configuration

The issue is that in SyncBackendHostImpl::ConfigureDataTypes types in
clean_first_types are excluded from types_to_purge and therefore are not being
unapplied as part of  PurgeDisabledTypes call.

This affects purgiong types with persistence error as well as catchup phase of
enabling passphrase encryption.

I'm adjusted logic to include these types in unapply_types and added test for
this scenario.

BUG=675036
R=zea@chromium.org
==========

to

==========
[Sync] Ensure that CONFIGURE_CLEAN types get unapplied before configuration

The issue is that in SyncBackendHostImpl::ConfigureDataTypes types in
clean_first_types are excluded from types_to_purge and therefore are not being
unapplied as part of  PurgeDisabledTypes call.

This affects purgiong types with persistence error as well as catchup phase of
enabling passphrase encryption.

I'm adjusted logic to include these types in unapply_types and added test for
this scenario.

BUG=675036
R=zea@chromium.org

Review-Url: https://codereview.chromium.org/2580033002
==========

[commit-bot: I haz the power, 2016-12-17 02:37:37]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-12-17 02:40:16]

Description was changed from

==========
[Sync] Ensure that CONFIGURE_CLEAN types get unapplied before configuration

The issue is that in SyncBackendHostImpl::ConfigureDataTypes types in
clean_first_types are excluded from types_to_purge and therefore are not being
unapplied as part of  PurgeDisabledTypes call.

This affects purgiong types with persistence error as well as catchup phase of
enabling passphrase encryption.

I'm adjusted logic to include these types in unapply_types and added test for
this scenario.

BUG=675036
R=zea@chromium.org

Review-Url: https://codereview.chromium.org/2580033002
==========

to

==========
[Sync] Ensure that CONFIGURE_CLEAN types get unapplied before configuration

The issue is that in SyncBackendHostImpl::ConfigureDataTypes types in
clean_first_types are excluded from types_to_purge and therefore are not being
unapplied as part of  PurgeDisabledTypes call.

This affects purgiong types with persistence error as well as catchup phase of
enabling passphrase encryption.

I'm adjusted logic to include these types in unapply_types and added test for
this scenario.

BUG=675036
R=zea@chromium.org

Committed: https://crrev.com/483a408c43aa945fcafef315fb55ed6d3a00c663
Cr-Commit-Position: refs/heads/master@{#439297}
==========

[commit-bot: I haz the power, 2016-12-17 02:40:17]

Patchset 3 (id:??) landed as
https://crrev.com/483a408c43aa945fcafef315fb55ed6d3a00c663
Cr-Commit-Position: refs/heads/master@{#439297}