Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2158743002:
Register a pref to control migration status (Closed)
|
Created:
4 years, 5 months ago by lshang Modified:
4 years, 4 months ago CC:
chromium-reviews, markusheintz_, msramek+watch_chromium.org, raymes+watch_chromium.org, chrome-apps-syd-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@change_scoping_type Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionRegister a pref to control migration status
Domain scoped content settings are migrated to be origin scoped, but the
migration process is expected to run only once. In this CL, a pref is registered
to control this, which has three states:
1. not migrated, which means the migration process has not been started yet.
2. migrated in HostContentSettingsMap(HCSM), which will not do migration again
in HCSM, but can do in PrefModelAssociator.
3. migrated in PrefModelAssociator, which means the migration is all finished
and won't get called again.
BUG=604612
Committed: https://crrev.com/e8d505658c6c05c87bc2f9adfb0a8f27b8ba94f8
Cr-Commit-Position: refs/heads/master@{#408064}
Patch Set 1 #Patch Set 2 : add some comments #
Total comments: 12
Patch Set 3 : add test #Patch Set 4 : add some comments to test #
Total comments: 8
Patch Set 5 : address review comments #
Total comments: 12
Patch Set 6 : minor change #
Total comments: 6
Patch Set 7 : address review comments #
Dependent Patchsets: Messages
Total messages: 45 (31 generated)
Description was changed from ========== Register a pref to control migration status add pref BUG= ========== to ========== Register a pref to control migration status Domain scoped content settings are migrated to be origin scoped, but the migration process is expected to run only once. In this CL, a pref is registered to control this, which has three states: 1. not migrated, which means the migration process has not been started yet. 2. migrated in HostContentSettingsMap(HCSM), which will not do migration again in HCSM, but can do in PrefModelAssociator. 3. migrated in PrefModelAssociator, which means the migration is all finished and won't get called again. BUG=604612 ==========
Description was changed from ========== Register a pref to control migration status Domain scoped content settings are migrated to be origin scoped, but the migration process is expected to run only once. In this CL, a pref is registered to control this, which has three states: 1. not migrated, which means the migration process has not been started yet. 2. migrated in HostContentSettingsMap(HCSM), which will not do migration again in HCSM, but can do in PrefModelAssociator. 3. migrated in PrefModelAssociator, which means the migration is all finished and won't get called again. BUG=604612 ========== to ========== Register a pref to control migration status Domain scoped content settings are migrated to be origin scoped, but the migration process is expected to run only once. In this CL, a pref is registered to control this, which has three states: 1. not migrated, which means the migration process has not been started yet. 2. migrated in HostContentSettingsMap(HCSM), which will not do migration again in HCSM, but can do in PrefModelAssociator. 3. migrated in PrefModelAssociator, which means the migration is all finished and won't get called again. BUG=604612 ==========
Description was changed from ========== Register a pref to control migration status Domain scoped content settings are migrated to be origin scoped, but the migration process is expected to run only once. In this CL, a pref is registered to control this, which has three states: 1. not migrated, which means the migration process has not been started yet. 2. migrated in HostContentSettingsMap(HCSM), which will not do migration again in HCSM, but can do in PrefModelAssociator. 3. migrated in PrefModelAssociator, which means the migration is all finished and won't get called again. BUG=604612 ========== to ========== Register a pref to control migration status Domain scoped content settings are migrated to be origin scoped, but the migration process is expected to run only once. In this CL, a pref is registered to control this, which has three states: 1. not migrated, which means the migration process has not been started yet. 2. migrated in HostContentSettingsMap(HCSM), which will not do migration again in HCSM, but can do in PrefModelAssociator. 3. migrated in PrefModelAssociator, which means the migration is all finished and won't get called again. BUG=604612 ==========
Description was changed from ========== Register a pref to control migration status Domain scoped content settings are migrated to be origin scoped, but the migration process is expected to run only once. In this CL, a pref is registered to control this, which has three states: 1. not migrated, which means the migration process has not been started yet. 2. migrated in HostContentSettingsMap(HCSM), which will not do migration again in HCSM, but can do in PrefModelAssociator. 3. migrated in PrefModelAssociator, which means the migration is all finished and won't get called again. BUG=604612 ========== to ========== Register a pref to control migration status Domain scoped content settings are migrated to be origin scoped, but the migration process is expected to run only once. In this CL, a pref is registered to control this, which has three states: 1. not migrated, which means the migration process has not been started yet. 2. migrated in HostContentSettingsMap(HCSM), which will not do migration again in HCSM, but can do in PrefModelAssociator. 3. migrated in PrefModelAssociator, which means the migration is all finished and won't get called again. BUG=604612 ==========
Description was changed from ========== Register a pref to control migration status Domain scoped content settings are migrated to be origin scoped, but the migration process is expected to run only once. In this CL, a pref is registered to control this, which has three states: 1. not migrated, which means the migration process has not been started yet. 2. migrated in HostContentSettingsMap(HCSM), which will not do migration again in HCSM, but can do in PrefModelAssociator. 3. migrated in PrefModelAssociator, which means the migration is all finished and won't get called again. BUG=604612 ========== to ========== Register a pref to control migration status Domain scoped content settings are migrated to be origin scoped, but the migration process is expected to run only once. In this CL, a pref is registered to control this, which has three states: 1. not migrated, which means the migration process has not been started yet. 2. migrated in HostContentSettingsMap(HCSM), which will not do migration again in HCSM, but can do in PrefModelAssociator. 3. migrated in PrefModelAssociator, which means the migration is all finished and won't get called again. BUG=604612 ==========
lshang@chromium.org changed reviewers: + raymes@chromium.org
PTAL thanks!
Thanks Liu! A couple of thoughts: 1) Could we land this before we land https://codereview.chromium.org/2075103002/ ? 2) Could we add a test for this to host_content_settings_map_unittest? https://codereview.chromium.org/2158743002/diff/20001/chrome/browser/content_... File chrome/browser/content_settings/host_content_settings_map_factory.cc (right): https://codereview.chromium.org/2158743002/diff/20001/chrome/browser/content_... chrome/browser/content_settings/host_content_settings_map_factory.cc:87: content_settings::DONE_IN_HCSM_AND_SYNC); Instead of checking/modifying the pref here, I think we should instead pass a bool into MigrateDomainScopedSettings indicating whether it is being called before sync or after sync. https://codereview.chromium.org/2158743002/diff/20001/components/content_sett... File components/content_settings/core/browser/host_content_settings_map.h (right): https://codereview.chromium.org/2158743002/diff/20001/components/content_sett... components/content_settings/core/browser/host_content_settings_map.h:43: // Enum describing status of migration process. This is to ensure that migration nit: Enum describing the status of domain to origin migration of content settings. Migration will be done twice: once upon construction of the HostContentSettingsMap (before syncing any content settings) and once after sync has finished. We always migrate before sync to ensure that settings will get migrated even if a user doesn't have sync enabled. We migrate after sync to ensure that any sync'd settings will be migrated. Once these events have occurred, we won't perform migration again. https://codereview.chromium.org/2158743002/diff/20001/components/content_sett... components/content_settings/core/browser/host_content_settings_map.h:45: enum ContentSettingMigrationStatus { We should be more specific about what migration this is for (there are many things to migrate :). We could call it DomainToOriginMigrationStatus. https://codereview.chromium.org/2158743002/diff/20001/components/content_sett... components/content_settings/core/browser/host_content_settings_map.h:49: DONE_IN_HCSM, I would just differentiate MIGRATED_BEFORE_SYNC and MIGRATED_AFTER_SYNC https://codereview.chromium.org/2158743002/diff/20001/components/content_sett... components/content_settings/core/browser/host_content_settings_map.h:54: }; We could move this into the .cc file
https://codereview.chromium.org/2158743002/diff/20001/chrome/browser/content_... File chrome/browser/content_settings/host_content_settings_map_factory.cc (right): https://codereview.chromium.org/2158743002/diff/20001/chrome/browser/content_... chrome/browser/content_settings/host_content_settings_map_factory.cc:87: content_settings::DONE_IN_HCSM_AND_SYNC); On 2016/07/19 01:48:15, raymes wrote: > Instead of checking/modifying the pref here, I think we should instead pass a > bool into MigrateDomainScopedSettings indicating whether it is being called > before sync or after sync. We all need to run the migration code either before sync or after sync, each for once, so I don't see the function of the bool passing in here? https://codereview.chromium.org/2158743002/diff/20001/components/content_sett... File components/content_settings/core/browser/host_content_settings_map.h (right): https://codereview.chromium.org/2158743002/diff/20001/components/content_sett... components/content_settings/core/browser/host_content_settings_map.h:49: DONE_IN_HCSM, On 2016/07/19 01:48:15, raymes wrote: > I would just differentiate MIGRATED_BEFORE_SYNC and MIGRATED_AFTER_SYNC I think we need to have three states here: - haven't migrated, so that we can run it in HCSM constructor - MIGRATED_BEFORE_SYNC, which means we have run it in HCSM, so we will not do it again on the next chrome start, but can run it in sync - MIGRATED_AFTER_SYNC, which means we have run it in sync, so the migration code won't get run again at anywhere if we omit the first state, and user didn't enable sync, then we will never know if we should run the migration in HCSM on browser start. Does my point seem reasonable? I'm happy to discuss more in person:-)
Patchset #3 (id:40001) has been deleted
Patchset #3 (id:60001) has been deleted
The CQ bit was checked by lshang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by lshang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Thanks Raymes! https://codereview.chromium.org/2158743002/diff/20001/chrome/browser/content_... File chrome/browser/content_settings/host_content_settings_map_factory.cc (right): https://codereview.chromium.org/2158743002/diff/20001/chrome/browser/content_... chrome/browser/content_settings/host_content_settings_map_factory.cc:87: content_settings::DONE_IN_HCSM_AND_SYNC); On 2016/07/19 01:48:15, raymes wrote: > Instead of checking/modifying the pref here, I think we should instead pass a > bool into MigrateDomainScopedSettings indicating whether it is being called > before sync or after sync. Done. https://codereview.chromium.org/2158743002/diff/20001/components/content_sett... File components/content_settings/core/browser/host_content_settings_map.h (right): https://codereview.chromium.org/2158743002/diff/20001/components/content_sett... components/content_settings/core/browser/host_content_settings_map.h:43: // Enum describing status of migration process. This is to ensure that migration On 2016/07/19 01:48:15, raymes wrote: > nit: Enum describing the status of domain to origin migration of content > settings. Migration will be done twice: once upon construction of the > HostContentSettingsMap (before syncing any content settings) and once after sync > has finished. We always migrate before sync to ensure that settings will get > migrated even if a user doesn't have sync enabled. We migrate after sync to > ensure that any sync'd settings will be migrated. Once these events have > occurred, we won't perform migration again. Done. https://codereview.chromium.org/2158743002/diff/20001/components/content_sett... components/content_settings/core/browser/host_content_settings_map.h:45: enum ContentSettingMigrationStatus { On 2016/07/19 01:48:15, raymes wrote: > We should be more specific about what migration this is for (there are many > things to migrate :). We could call it DomainToOriginMigrationStatus. Done. https://codereview.chromium.org/2158743002/diff/20001/components/content_sett... components/content_settings/core/browser/host_content_settings_map.h:49: DONE_IN_HCSM, On 2016/07/19 01:48:15, raymes wrote: > I would just differentiate MIGRATED_BEFORE_SYNC and MIGRATED_AFTER_SYNC Done. https://codereview.chromium.org/2158743002/diff/20001/components/content_sett... components/content_settings/core/browser/host_content_settings_map.h:54: }; On 2016/07/19 01:48:15, raymes wrote: > We could move this into the .cc file Done.
On 2016/07/19 01:48:15, raymes wrote: > Thanks Liu! > > A couple of thoughts: > 1) Could we land this before we land https://codereview.chromium.org/2075103002/ > ? Yep, we can land this CL first. I've removed the dependency to the other CL. > 2) Could we add a test for this to host_content_settings_map_unittest? > Test added.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Thanks Liu! https://codereview.chromium.org/2158743002/diff/100001/chrome/browser/content... File chrome/browser/content_settings/host_content_settings_map_unittest.cc (right): https://codereview.chromium.org/2158743002/diff/100001/chrome/browser/content... chrome/browser/content_settings/host_content_settings_map_unittest.cc:1480: EXPECT_EQ(1, prefs->GetInteger(prefs::kDomainToOriginMigrationStatus)); Rather than checking the value of the pref (which is more of an implementation detail), could we instead add some content settings and check whether migration happens, e.g. -Add an old setting -Migrate before sync. -Check migration -Add another old setting -Migrate before sync -Check no migration happened -Migrate after sync -Check migration -Add another old setting -Migrate after sync -Check no migration https://codereview.chromium.org/2158743002/diff/100001/components/content_set... File components/content_settings/core/browser/host_content_settings_map.cc (right): https://codereview.chromium.org/2158743002/diff/100001/components/content_set... components/content_settings/core/browser/host_content_settings_map.cc:526: if (status == NOT_MIGRATED && after_sync) This should never happen, right? I think this should be a DCHECK instead. https://codereview.chromium.org/2158743002/diff/100001/components/content_set... File components/content_settings/core/browser/host_content_settings_map.h (right): https://codereview.chromium.org/2158743002/diff/100001/components/content_set... components/content_settings/core/browser/host_content_settings_map.h:322: void MigrateDomainScopedSettings(bool after_sync); nit: Please add a comment for |after_sync| https://codereview.chromium.org/2158743002/diff/100001/components/content_set... File components/content_settings/core/common/pref_names.cc (right): https://codereview.chromium.org/2158743002/diff/100001/components/content_set... components/content_settings/core/common/pref_names.cc:22: // scoped settings. nit: Integer that indicates the status of migrating domain scoped setting to origin scoped settings.
https://codereview.chromium.org/2158743002/diff/100001/chrome/browser/content... File chrome/browser/content_settings/host_content_settings_map_unittest.cc (right): https://codereview.chromium.org/2158743002/diff/100001/chrome/browser/content... chrome/browser/content_settings/host_content_settings_map_unittest.cc:1480: EXPECT_EQ(1, prefs->GetInteger(prefs::kDomainToOriginMigrationStatus)); On 2016/07/21 00:58:54, raymes wrote: > Rather than checking the value of the pref (which is more of an implementation > detail), could we instead add some content settings and check whether migration > happens, e.g. > > -Add an old setting > -Migrate before sync. > -Check migration > -Add another old setting > -Migrate before sync > -Check no migration happened > -Migrate after sync > -Check migration > -Add another old setting > -Migrate after sync > -Check no migration Done. https://codereview.chromium.org/2158743002/diff/100001/components/content_set... File components/content_settings/core/browser/host_content_settings_map.cc (right): https://codereview.chromium.org/2158743002/diff/100001/components/content_set... components/content_settings/core/browser/host_content_settings_map.cc:526: if (status == NOT_MIGRATED && after_sync) On 2016/07/21 00:58:54, raymes wrote: > This should never happen, right? I think this should be a DCHECK instead. Done. https://codereview.chromium.org/2158743002/diff/100001/components/content_set... File components/content_settings/core/browser/host_content_settings_map.h (right): https://codereview.chromium.org/2158743002/diff/100001/components/content_set... components/content_settings/core/browser/host_content_settings_map.h:322: void MigrateDomainScopedSettings(bool after_sync); On 2016/07/21 00:58:54, raymes wrote: > nit: Please add a comment for |after_sync| Done. https://codereview.chromium.org/2158743002/diff/100001/components/content_set... File components/content_settings/core/common/pref_names.cc (right): https://codereview.chromium.org/2158743002/diff/100001/components/content_set... components/content_settings/core/common/pref_names.cc:22: // scoped settings. On 2016/07/21 00:58:54, raymes wrote: > nit: Integer that indicates the status of migrating domain scoped setting to > origin scoped settings. Done.
The CQ bit was checked by lshang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2158743002/diff/120001/chrome/browser/content... File chrome/browser/content_settings/host_content_settings_map_unittest.cc (right): https://codereview.chromium.org/2158743002/diff/120001/chrome/browser/content... chrome/browser/content_settings/host_content_settings_map_unittest.cc:1467: TEST_F(HostContentSettingsMapTest, DomainToOriginMigrationStatus) { nit: add a comment explaining what this test does. // Ensure that migration only happens once upon construction of the HCSM and once after syncing (even when these events occur multiple times). https://codereview.chromium.org/2158743002/diff/120001/chrome/browser/content... chrome/browser/content_settings/host_content_settings_map_unittest.cc:1482: http_host, http_host, CONTENT_SETTINGS_TYPE_COOKIES, std::string())); nit: I don't think we need this. https://codereview.chromium.org/2158743002/diff/120001/components/content_set... File components/content_settings/core/browser/host_content_settings_map.cc (right): https://codereview.chromium.org/2158743002/diff/120001/components/content_set... components/content_settings/core/browser/host_content_settings_map.cc:73: // Have done migration both in HostContentSettingsMap construction and sync nit: and after sync is finished. No migration will happen after this point. https://codereview.chromium.org/2158743002/diff/120001/components/content_set... components/content_settings/core/browser/host_content_settings_map.cc:595: DomainToOriginMigrationStatus::MIGRATED_BEFORE_SYNC); nit: no need for the DomainToOriginMigrationStatus prefix https://codereview.chromium.org/2158743002/diff/120001/components/content_set... File components/content_settings/core/browser/host_content_settings_map.h (right): https://codereview.chromium.org/2158743002/diff/120001/components/content_set... components/content_settings/core/browser/host_content_settings_map.h:320: // |after_sync| is false means the migration is done upon construction of the nit: put this on the previous line or in a new paragraph (with an extra blank comment line above) https://codereview.chromium.org/2158743002/diff/120001/components/content_set... components/content_settings/core/browser/host_content_settings_map.h:320: // |after_sync| is false means the migration is done upon construction of the nit: |after_sync| will be false when called upon construction of this object and true when called by the sync layer after sync is completed.
The CQ bit was checked by lshang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lshang@chromium.org changed reviewers: + msramek@chromium.org
Also +msramek@ for review. PTAL thanks! https://codereview.chromium.org/2158743002/diff/120001/chrome/browser/content... File chrome/browser/content_settings/host_content_settings_map_unittest.cc (right): https://codereview.chromium.org/2158743002/diff/120001/chrome/browser/content... chrome/browser/content_settings/host_content_settings_map_unittest.cc:1467: TEST_F(HostContentSettingsMapTest, DomainToOriginMigrationStatus) { On 2016/07/25 03:56:38, raymes wrote: > nit: add a comment explaining what this test does. > // Ensure that migration only happens once upon construction of the HCSM and > once after syncing (even when these events occur multiple times). Done. https://codereview.chromium.org/2158743002/diff/120001/chrome/browser/content... chrome/browser/content_settings/host_content_settings_map_unittest.cc:1482: http_host, http_host, CONTENT_SETTINGS_TYPE_COOKIES, std::string())); On 2016/07/25 03:56:38, raymes wrote: > nit: I don't think we need this. Done. https://codereview.chromium.org/2158743002/diff/120001/components/content_set... File components/content_settings/core/browser/host_content_settings_map.cc (right): https://codereview.chromium.org/2158743002/diff/120001/components/content_set... components/content_settings/core/browser/host_content_settings_map.cc:73: // Have done migration both in HostContentSettingsMap construction and sync On 2016/07/25 03:56:38, raymes wrote: > nit: and after sync is finished. No migration will happen after this point. Done. https://codereview.chromium.org/2158743002/diff/120001/components/content_set... components/content_settings/core/browser/host_content_settings_map.cc:595: DomainToOriginMigrationStatus::MIGRATED_BEFORE_SYNC); On 2016/07/25 03:56:38, raymes wrote: > nit: no need for the DomainToOriginMigrationStatus prefix Done. https://codereview.chromium.org/2158743002/diff/120001/components/content_set... File components/content_settings/core/browser/host_content_settings_map.h (right): https://codereview.chromium.org/2158743002/diff/120001/components/content_set... components/content_settings/core/browser/host_content_settings_map.h:320: // |after_sync| is false means the migration is done upon construction of the On 2016/07/25 03:56:38, raymes wrote: > nit: |after_sync| will be false when called upon construction of this object and > true when called by the sync layer after sync is completed. Done. https://codereview.chromium.org/2158743002/diff/120001/components/content_set... components/content_settings/core/browser/host_content_settings_map.h:320: // |after_sync| is false means the migration is done upon construction of the On 2016/07/25 03:56:38, raymes wrote: > nit: put this on the previous line or in a new paragraph (with an extra blank > comment line above) Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
LGTM with optional nits. https://codereview.chromium.org/2158743002/diff/140001/components/content_set... File components/content_settings/core/browser/host_content_settings_map.cc (right): https://codereview.chromium.org/2158743002/diff/140001/components/content_set... components/content_settings/core/browser/host_content_settings_map.cc:520: int status = prefs_->GetInteger(prefs::kDomainToOriginMigrationStatus); nit: DomainToOriginMigrationStatus status = static_cast ... https://codereview.chromium.org/2158743002/diff/140001/components/content_set... File components/content_settings/core/browser/host_content_settings_map.h (right): https://codereview.chromium.org/2158743002/diff/140001/components/content_set... components/content_settings/core/browser/host_content_settings_map.h:283: friend class HostContentSettingsMapTest_DomainToOriginMigrationStatus_Test; nit: Can we change this and the below to FRIEND_TEST_ALL_PREFIXES ? It's a bit more readable and it makes it easier to disable the tests if ever needed. https://codereview.chromium.org/2158743002/diff/140001/components/content_set... File components/content_settings/core/common/pref_names.cc (right): https://codereview.chromium.org/2158743002/diff/140001/components/content_set... components/content_settings/core/common/pref_names.cc:21: // Integer that indicates the status of migrating domain scoped setting to nit: settings
The CQ bit was checked by lshang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Thanks Martin! https://codereview.chromium.org/2158743002/diff/140001/components/content_set... File components/content_settings/core/browser/host_content_settings_map.cc (right): https://codereview.chromium.org/2158743002/diff/140001/components/content_set... components/content_settings/core/browser/host_content_settings_map.cc:520: int status = prefs_->GetInteger(prefs::kDomainToOriginMigrationStatus); On 2016/07/26 19:21:02, msramek wrote: > nit: DomainToOriginMigrationStatus status = static_cast ... Done. https://codereview.chromium.org/2158743002/diff/140001/components/content_set... File components/content_settings/core/browser/host_content_settings_map.h (right): https://codereview.chromium.org/2158743002/diff/140001/components/content_set... components/content_settings/core/browser/host_content_settings_map.h:283: friend class HostContentSettingsMapTest_DomainToOriginMigrationStatus_Test; On 2016/07/26 19:21:02, msramek wrote: > nit: Can we change this and the below to FRIEND_TEST_ALL_PREFIXES ? It's a bit > more readable and it makes it easier to disable the tests if ever needed. Done. https://codereview.chromium.org/2158743002/diff/140001/components/content_set... File components/content_settings/core/common/pref_names.cc (right): https://codereview.chromium.org/2158743002/diff/140001/components/content_set... components/content_settings/core/common/pref_names.cc:21: // Integer that indicates the status of migrating domain scoped setting to On 2016/07/26 19:21:02, msramek wrote: > nit: settings Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by lshang@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from raymes@chromium.org, msramek@chromium.org Link to the patchset: https://codereview.chromium.org/2158743002/#ps160001 (title: "address review comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Register a pref to control migration status Domain scoped content settings are migrated to be origin scoped, but the migration process is expected to run only once. In this CL, a pref is registered to control this, which has three states: 1. not migrated, which means the migration process has not been started yet. 2. migrated in HostContentSettingsMap(HCSM), which will not do migration again in HCSM, but can do in PrefModelAssociator. 3. migrated in PrefModelAssociator, which means the migration is all finished and won't get called again. BUG=604612 ========== to ========== Register a pref to control migration status Domain scoped content settings are migrated to be origin scoped, but the migration process is expected to run only once. In this CL, a pref is registered to control this, which has three states: 1. not migrated, which means the migration process has not been started yet. 2. migrated in HostContentSettingsMap(HCSM), which will not do migration again in HCSM, but can do in PrefModelAssociator. 3. migrated in PrefModelAssociator, which means the migration is all finished and won't get called again. BUG=604612 ==========
Message was sent while issue was closed.
Committed patchset #7 (id:160001)
Message was sent while issue was closed.
Description was changed from ========== Register a pref to control migration status Domain scoped content settings are migrated to be origin scoped, but the migration process is expected to run only once. In this CL, a pref is registered to control this, which has three states: 1. not migrated, which means the migration process has not been started yet. 2. migrated in HostContentSettingsMap(HCSM), which will not do migration again in HCSM, but can do in PrefModelAssociator. 3. migrated in PrefModelAssociator, which means the migration is all finished and won't get called again. BUG=604612 ========== to ========== Register a pref to control migration status Domain scoped content settings are migrated to be origin scoped, but the migration process is expected to run only once. In this CL, a pref is registered to control this, which has three states: 1. not migrated, which means the migration process has not been started yet. 2. migrated in HostContentSettingsMap(HCSM), which will not do migration again in HCSM, but can do in PrefModelAssociator. 3. migrated in PrefModelAssociator, which means the migration is all finished and won't get called again. BUG=604612 Committed: https://crrev.com/e8d505658c6c05c87bc2f9adfb0a8f27b8ba94f8 Cr-Commit-Position: refs/heads/master@{#408064} ==========
Message was sent while issue was closed.
Patchset 7 (id:??) landed as https://crrev.com/e8d505658c6c05c87bc2f9adfb0a8f27b8ba94f8 Cr-Commit-Position: refs/heads/master@{#408064} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
