Delete the PreferenceMACs on profile deletion.

Delete the PreferenceMACs on profile deletion.

On Windows, the tracked preference logic writes some data to the
registry. It is currently not cleared when the profile is deleted.

Alternatives considered: extending ProfileAttributesStorage::Observer 
and listening for OnProfileWasRemoved or OnProfileWillBeRemoved. However, 
the tracked pref code is componentized to avoid any reference to Chrome.

Known issue: the PreferenceMACs in the registry will not be deleted if
Chrome crashes during profile deletion. There's logic in
chrome_browser_main which cleans up the profile folders when this happens
(https://cs.chromium.org/chromium/src/chrome/browser/chrome_browser_main.cc?dr=CSs&l=1279)
However, I couldn't find a clean way to clean up the registry when the
profile no longer exists.

BUG=723834
Review-Url: https://codereview.chromium.org/2905903002
Cr-Commit-Position: refs/heads/master@{#494955}
Committed: https://chromium.googlesource.com/chromium/src/+/45e347284efb236e79bc3a1ff4c68640dafe9b3d

[proberge, 2017-05-25 19:46:49]

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

[commit-bot: I haz the power, 2017-05-25 19:47:33]

Dry run: CQ is trying da patch.

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

[proberge, 2017-05-25 19:49:08]

Description was changed from

==========
Delete the PreferenceMACs on profile deletion.

On Windows, the tracked preference logic writes some data to the
registry. It is currently not cleared when the profile is deleted.

Alternatives considered: observing ProfileInfoCacheObserver's
OnProfileWasRemoved or OnProfileWillBeRemoved. However, the tracked pref
code is componentized to avoid any reference to Chrome.

Known issue: the PreferenceMACs in the registry will not be deleted if
Chrome crashes during profile deletion. There's logic in
chrome_browser_main which cleans up the profile folders when this happens
(https://cs.chromium.org/chromium/src/chrome/browser/chrome_browser_main.cc?dr...)
However, I couldn't find a clean way to clean up the registry when the
profile no longer exists.

BUG=723834
==========

to

==========
Delete the PreferenceMACs on profile deletion.

On Windows, the tracked preference logic writes some data to the
registry. It is currently not cleared when the profile is deleted.

Alternatives considered: extending ProfileAttributesStorage::Observer 
and listening for OnProfileWasRemoved or OnProfileWillBeRemoved. However, 
the tracked pref code is componentized to avoid any reference to Chrome.

Known issue: the PreferenceMACs in the registry will not be deleted if
Chrome crashes during profile deletion. There's logic in
chrome_browser_main which cleans up the profile folders when this happens
(https://cs.chromium.org/chromium/src/chrome/browser/chrome_browser_main.cc?dr...)
However, I couldn't find a clean way to clean up the registry when the
profile no longer exists.

BUG=723834
==========

[proberge, 2017-05-25 19:49:08]

proberge@chromium.org changed reviewers:
+ gab@chromium.org

[proberge, 2017-05-25 19:50:19]

Hi Gab, PTAL. 

Any suggestions on where to put a unit test for this logic?

[commit-bot: I haz the power, 2017-05-25 21:28:36]

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

[commit-bot: I haz the power, 2017-05-25 21:28:37]

Dry run: This issue passed the CQ dry run.

[gab, 2017-05-26 18:15:31]

On 2017/05/25 19:50:19, proberge wrote:
> Hi Gab, PTAL. 
> 
> Any suggestions on where to put a unit test for this logic?

Re. "There's logic in
chrome_browser_main which cleans up the profile folders when this happens
(https://cs.chromium.org/chromium/src/chrome/browser/chrome_browser_main.cc?dr...)
However, I couldn't find a clean way to clean up the registry when the
profile no longer exists."

How about cleaning up the registry as part of NukeProfileFromDisk()?

Does regular profile deletion also go through that code path?

If so we can put all the logic (and tests) there?

[proberge, 2017-06-05 14:45:52]

Added an alternate solution that uses NukeProfileFromDisk. PTAL and let me know
which of the two you prefer.

[gab, 2017-06-05 17:28:57]

Looking at this further, I don't think we have a choice but to go with the
plumbing solution (#1) as the layering violation will simply not work when prefs
are running as a Mojo service.

I looked at #1 again and identified what I didn't like about it : adding
"profile" knowledge to prefs. But I have suggestions below to avoid that.

Thanks,
Gab

https://codereview.chromium.org/2905903002/diff/1/chrome/browser/profiles/pro...
File chrome/browser/profiles/profile_manager.cc (right):

https://codereview.chromium.org/2905903002/diff/1/chrome/browser/profiles/pro...
chrome/browser/profiles/profile_manager.cc:1496:
profile->GetPrefs()->CleanupForProfileDeletion();
Why is this inside if
(ProfileSyncServiceFactory::GetInstance()->HasProfileSyncService() { ... } ?

https://codereview.chromium.org/2905903002/diff/1/components/prefs/pref_servi...
File components/prefs/pref_service.h (right):

https://codereview.chromium.org/2905903002/diff/1/components/prefs/pref_servi...
components/prefs/pref_service.h:312: void CleanupForProfileDeletion();
// Invoked when the store is deleted from disk. Allows this PrefService 
// to tangentially cleanup data it may have saved outside the store.
void OnStoreDeletionFromDisk();

(everywhere)


1) components/prefs isn't aware of "profiles"
2) although we can hint at that in the comment, the method name itself shouldn't
enforce that this signal be used strictly for cleanup

[proberge, 2017-06-05 21:22:11]

https://codereview.chromium.org/2905903002/diff/1/chrome/browser/profiles/pro...
File chrome/browser/profiles/profile_manager.cc (right):

https://codereview.chromium.org/2905903002/diff/1/chrome/browser/profiles/pro...
chrome/browser/profiles/profile_manager.cc:1496:
profile->GetPrefs()->CleanupForProfileDeletion();
On 2017/06/05 17:28:57, gab wrote:
> Why is this inside if
> (ProfileSyncServiceFactory::GetInstance()->HasProfileSyncService() { ... } ?

My bad, I thought I put it under if (profile) {}.

https://codereview.chromium.org/2905903002/diff/1/components/prefs/pref_servi...
File components/prefs/pref_service.h (right):

https://codereview.chromium.org/2905903002/diff/1/components/prefs/pref_servi...
components/prefs/pref_service.h:312: void CleanupForProfileDeletion();
On 2017/06/05 17:28:57, gab wrote:
> // Invoked when the store is deleted from disk. Allows this PrefService 
> // to tangentially cleanup data it may have saved outside the store.
> void OnStoreDeletionFromDisk();
> 
> (everywhere)
> 
> 
> 1) components/prefs isn't aware of "profiles"
> 2) although we can hint at that in the comment, the method name itself
shouldn't
> enforce that this signal be used strictly for cleanup

Done.

[gab, 2017-06-06 14:48:26]

https://codereview.chromium.org/2905903002/diff/40001/chrome/browser/profiles...
File chrome/browser/profiles/profile_manager.cc (right):

https://codereview.chromium.org/2905903002/diff/40001/chrome/browser/profiles...
chrome/browser/profiles/profile_manager.cc:1517: // It is safe to delete a not
yet loaded Profile from disk.
The logic will go through here if the user deletes a profile from
chrome://settings without loading it. We need to support that use case too.

[proberge, 2017-06-07 19:09:46]

Patchset #4 (id:60001) has been deleted

[proberge, 2017-06-07 19:19:00]

Patchset #5 (id:100001) has been deleted

[proberge, 2017-06-07 19:22:50]

https://codereview.chromium.org/2905903002/diff/40001/chrome/browser/profiles...
File chrome/browser/profiles/profile_manager.cc (right):

https://codereview.chromium.org/2905903002/diff/40001/chrome/browser/profiles...
chrome/browser/profiles/profile_manager.cc:1517: // It is safe to delete a not
yet loaded Profile from disk.
On 2017/06/06 14:48:26, gab wrote:
> The logic will go through here if the user deletes a profile from
> chrome://settings without loading it. We need to support that use case too.

Done. Note that this involves added complexity to try to load the profile in
order to call OnStoreDeletionFromDisk. Clearing the registry will still fail if
the profile can't be loaded. 

Loading the the profile might be working against the todo for
https://bugs.chromium.org/p/chromium/issues/detail?id=88586.

In practice, I couldn't get this branch to be hit when removing a profile on
Windows. When trying to remove a profile, the UI displays some stats (This will
permanently delete N items from your device.) which require loading the profile.

[gab, 2017-06-08 13:12:55]

gab@chromium.org changed reviewers:
+ skuhne@chromium.org

[gab, 2017-06-08 13:13:21]

On 2017/06/07 19:22:50, proberge wrote:
>
https://codereview.chromium.org/2905903002/diff/40001/chrome/browser/profiles...
> File chrome/browser/profiles/profile_manager.cc (right):
> 
>
https://codereview.chromium.org/2905903002/diff/40001/chrome/browser/profiles...
> chrome/browser/profiles/profile_manager.cc:1517: // It is safe to delete a not
> yet loaded Profile from disk.
> On 2017/06/06 14:48:26, gab wrote:
> > The logic will go through here if the user deletes a profile from
> > chrome://settings without loading it. We need to support that use case too.
> 
> Done. Note that this involves added complexity to try to load the profile in
> order to call OnStoreDeletionFromDisk. Clearing the registry will still fail
if
> the profile can't be loaded. 
> 
> Loading the the profile might be working against the todo for
> https://bugs.chromium.org/p/chromium/issues/detail?id=88586.
> 
> In practice, I couldn't get this branch to be hit when removing a profile on
> Windows. When trying to remove a profile, the UI displays some stats (This
will
> permanently delete N items from your device.) which require loading the
profile.

+skuhne@ as owner of profiles/ to comment/review this part

[gab, 2017-06-08 16:14:28]

Other than that, prefs lgtm when profiles/ owners are happy with approach there.

https://codereview.chromium.org/2905903002/diff/120001/components/prefs/persi...
File components/prefs/persistent_pref_store.h (right):

https://codereview.chromium.org/2905903002/diff/120001/components/prefs/persi...
components/prefs/persistent_pref_store.h:76: // Cleans data stored outside of
the profile directory.
Don't mention "profile".

re-use formulation similar to PrefService.

https://codereview.chromium.org/2905903002/diff/120001/components/prefs/pref_...
File components/prefs/pref_filter.h (right):

https://codereview.chromium.org/2905903002/diff/120001/components/prefs/pref_...
components/prefs/pref_filter.h:62: // Cleans data stored outside of the profile
directory.
ditto

https://codereview.chromium.org/2905903002/diff/120001/components/prefs/testi...
File components/prefs/testing_pref_store.cc (right):

https://codereview.chromium.org/2905903002/diff/120001/components/prefs/testi...
components/prefs/testing_pref_store.cc:183: NOTIMPLEMENTED();
That results in a log warning, in this case I think the right impl is actually
an empty body, so remove NOTIMPLEMENTED()

https://codereview.chromium.org/2905903002/diff/120001/services/preferences/t...
File services/preferences/tracked/pref_hash_filter.cc (right):

https://codereview.chromium.org/2905903002/diff/120001/services/preferences/t...
services/preferences/tracked/pref_hash_filter.cc:208: // The PrefStore will
attempt to flush the preferences of a deleted profile.
avoid "profile"

[proberge, 2017-06-08 18:33:00]

https://codereview.chromium.org/2905903002/diff/120001/components/prefs/persi...
File components/prefs/persistent_pref_store.h (right):

https://codereview.chromium.org/2905903002/diff/120001/components/prefs/persi...
components/prefs/persistent_pref_store.h:76: // Cleans data stored outside of
the profile directory.
On 2017/06/08 16:14:28, gab wrote:
> Don't mention "profile".
> 
> re-use formulation similar to PrefService.

Done.

https://codereview.chromium.org/2905903002/diff/120001/components/prefs/pref_...
File components/prefs/pref_filter.h (right):

https://codereview.chromium.org/2905903002/diff/120001/components/prefs/pref_...
components/prefs/pref_filter.h:62: // Cleans data stored outside of the profile
directory.
On 2017/06/08 16:14:28, gab wrote:
> ditto

Done.

https://codereview.chromium.org/2905903002/diff/120001/components/prefs/testi...
File components/prefs/testing_pref_store.cc (right):

https://codereview.chromium.org/2905903002/diff/120001/components/prefs/testi...
components/prefs/testing_pref_store.cc:183: NOTIMPLEMENTED();
On 2017/06/08 16:14:28, gab wrote:
> That results in a log warning, in this case I think the right impl is actually
> an empty body, so remove NOTIMPLEMENTED()

Done.

https://codereview.chromium.org/2905903002/diff/120001/services/preferences/t...
File services/preferences/tracked/pref_hash_filter.cc (right):

https://codereview.chromium.org/2905903002/diff/120001/services/preferences/t...
services/preferences/tracked/pref_hash_filter.cc:208: // The PrefStore will
attempt to flush the preferences of a deleted profile.
On 2017/06/08 16:14:28, gab wrote:
> avoid "profile"

Done.

[gab, 2017-06-20 21:38:27]

On 2017/06/08 13:13:21, gab wrote:
> On 2017/06/07 19:22:50, proberge wrote:
> >
>
https://codereview.chromium.org/2905903002/diff/40001/chrome/browser/profiles...
> > File chrome/browser/profiles/profile_manager.cc (right):
> > 
> >
>
https://codereview.chromium.org/2905903002/diff/40001/chrome/browser/profiles...
> > chrome/browser/profiles/profile_manager.cc:1517: // It is safe to delete a
not
> > yet loaded Profile from disk.
> > On 2017/06/06 14:48:26, gab wrote:
> > > The logic will go through here if the user deletes a profile from
> > > chrome://settings without loading it. We need to support that use case
too.
> > 
> > Done. Note that this involves added complexity to try to load the profile in
> > order to call OnStoreDeletionFromDisk. Clearing the registry will still fail
> if
> > the profile can't be loaded. 
> > 
> > Loading the the profile might be working against the todo for
> > https://bugs.chromium.org/p/chromium/issues/detail?id=88586.
> > 
> > In practice, I couldn't get this branch to be hit when removing a profile on
> > Windows. When trying to remove a profile, the UI displays some stats (This
> will
> > permanently delete N items from your device.) which require loading the
> profile.
> 
> +skuhne@ as owner of profiles/ to comment/review this part

ping skuhne@ ^^^

[Mr4D (OOO till 08-26), 2017-06-20 22:57:21]

lgtm for Profiles.

[proberge, 2017-06-21 19:10:28]

The CQ bit was checked by proberge@chromium.org

[proberge, 2017-06-21 19:10:28]

The patchset sent to the CQ was uploaded after l-g-t-m from gab@chromium.org
Link to the patchset: https://codereview.chromium.org/2905903002/#ps140001
(title: "Address review comments on #5")

[commit-bot: I haz the power, 2017-06-21 19:10:51]

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-06-21 19:21:06]

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

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

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[proberge, 2017-06-21 19:23:20]

proberge@chromium.org changed reviewers:
+ sammc@chromium.org

[proberge, 2017-06-21 19:23:42]

++samc for services/preferences approval

[Sam McNally, 2017-06-21 23:39:46]

lgtm

[proberge, 2017-06-22 14:07:48]

Patchset #7 (id:160001) has been deleted

[proberge, 2017-06-22 14:11:50]

The CQ bit was checked by proberge@chromium.org

[proberge, 2017-06-22 14:11:51]

The patchset sent to the CQ was uploaded after l-g-t-m from sammc@chromium.org,
gab@chromium.org, skuhne@chromium.org
Link to the patchset: https://codereview.chromium.org/2905903002/#ps200001
(title: "Make services_unittests compile")

[commit-bot: I haz the power, 2017-06-22 14:12:03]

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-06-22 14:18:54]

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

[commit-bot: I haz the power, 2017-06-22 14:18:56]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[proberge, 2017-06-22 14:22:42]

proberge@chromium.org changed reviewers:
+ kenrb@chromium.org

[proberge, 2017-06-22 14:22:43]

++kenrb for .mojom file changes approval

[kenrb, 2017-06-22 14:57:12]

mojom lgtm

[proberge, 2017-06-29 18:44:04]

@gab PTAL

[gab, 2017-07-05 23:44:31]

On 2017/06/29 18:44:04, proberge wrote:
> @gab PTAL

Not sure what specifically you wanted me to look at again? Already gave l.g.t.m.
above and this hasn't changed much since?

[proberge, 2017-07-06 00:23:15]

On 2017/07/05 23:44:31, gab (in-out til july17) wrote:
> On 2017/06/29 18:44:04, proberge wrote:
> > @gab PTAL
> 
> Not sure what specifically you wanted me to look at again? Already gave
l.g.t.m.
> above and this hasn't changed much since?

Sorry for not explaining further a week ago:

I moved one line (MarkProfileDirectoryForDeletion) in patchset 9 to fix a
MacOS-only test
which was broken by my change. See:
(https://cs.chromium.org/chromium/src/chrome/browser/profiles/profile_manager_...)

The test destroys two profiles in quick succession. With the new logic to
attempt loading an unloaded
profile before loading it, the test failed as one of the deleted profiles was
chosen as the active profile
instead of a third profile.
I *think* the fix makes sense but would like an additional pair of eyes on it.

[gab, 2017-07-06 00:33:12]

On 2017/07/06 00:23:15, proberge wrote:
> On 2017/07/05 23:44:31, gab (in-out til july17) wrote:
> > On 2017/06/29 18:44:04, proberge wrote:
> > > @gab PTAL
> > 
> > Not sure what specifically you wanted me to look at again? Already gave
> l.g.t.m.
> > above and this hasn't changed much since?
> 
> Sorry for not explaining further a week ago:

Sorry for the delay, had missed the ping per thinking I was done with this
review.. 

> 
> I moved one line (MarkProfileDirectoryForDeletion) in patchset 9 to fix a
> MacOS-only test
> which was broken by my change. See:
>
(https://cs.chromium.org/chromium/src/chrome/browser/profiles/profile_manager_...)
> 
> The test destroys two profiles in quick succession. With the new logic to
> attempt loading an unloaded
> profile before loading it, the test failed as one of the deleted profiles was
> chosen as the active profile
> instead of a third profile.
> I *think* the fix makes sense but would like an additional pair of eyes on it.

This is something for profiles/ owners to say, I don't know, as I said above
this looks good to me if profiles/ owners are happy with the call's position.

[proberge, 2017-07-06 13:50:41]

On 2017/07/06 00:33:12, gab (in-out til july17) wrote:
> On 2017/07/06 00:23:15, proberge wrote:
> > On 2017/07/05 23:44:31, gab (in-out til july17) wrote:
> > > On 2017/06/29 18:44:04, proberge wrote:
> > > > @gab PTAL
> > > 
> > > Not sure what specifically you wanted me to look at again? Already gave
> > l.g.t.m.
> > > above and this hasn't changed much since?
> > 
> > Sorry for not explaining further a week ago:
> 
> Sorry for the delay, had missed the ping per thinking I was done with this
> review.. 
> 
> > 
> > I moved one line (MarkProfileDirectoryForDeletion) in patchset 9 to fix a
> > MacOS-only test
> > which was broken by my change. See:
> >
>
(https://cs.chromium.org/chromium/src/chrome/browser/profiles/profile_manager_...)
> > 
> > The test destroys two profiles in quick succession. With the new logic to
> > attempt loading an unloaded
> > profile before loading it, the test failed as one of the deleted profiles
was
> > chosen as the active profile
> > instead of a third profile.
> > I *think* the fix makes sense but would like an additional pair of eyes on
it.
> 
> This is something for profiles/ owners to say, I don't know, as I said above
> this looks good to me if profiles/ owners are happy with the call's position.

Sounds good. Let's wait until skuhne is back from vacation to take another look.

[Mr4D (OOO till 08-26), 2017-08-16 14:38:55]

https://codereview.chromium.org/2905903002/diff/220001/chrome/browser/profile...
File chrome/browser/profiles/profile_manager.cc (right):

https://codereview.chromium.org/2905903002/diff/220001/chrome/browser/profile...
chrome/browser/profiles/profile_manager.cc:1509: // Clean-up pref data that
won't be cleaned up by NukeProfileFromDisk.
Looking at this code path there is no one calling NukeProfileFromDisk if a
profile exists (only in the else case below). Is the comment correct?

https://codereview.chromium.org/2905903002/diff/220001/chrome/browser/profile...
chrome/browser/profiles/profile_manager.cc:1530: // Attempt to load the profile
before deleting it.
Can you please add some more information here towards why this is necessary?

[proberge, 2017-08-16 19:40:35]

Patchset #10 (id:240001) has been deleted

[proberge, 2017-08-16 19:49:55]

Thanks Mr4D!

https://codereview.chromium.org/2905903002/diff/220001/chrome/browser/profile...
File chrome/browser/profiles/profile_manager.cc (right):

https://codereview.chromium.org/2905903002/diff/220001/chrome/browser/profile...
chrome/browser/profiles/profile_manager.cc:1509: // Clean-up pref data that
won't be cleaned up by NukeProfileFromDisk.
On 2017/08/16 14:38:55, Mr4D wrote:
> Looking at this code path there is no one calling NukeProfileFromDisk if a
> profile exists (only in the else case below). Is the comment correct?

NukeProfileFromDisk should be called for the profile on the next Chrome startup
during CleanUpDeletedProfiles. I'll update the comment to be clearer.

https://codereview.chromium.org/2905903002/diff/220001/chrome/browser/profile...
chrome/browser/profiles/profile_manager.cc:1530: // Attempt to load the profile
before deleting it.
On 2017/08/16 14:38:55, Mr4D wrote:
> Can you please add some more information here towards why this is necessary?

Done.

[proberge, 2017-08-16 19:50:01]

The CQ bit was checked by proberge@chromium.org

[proberge, 2017-08-16 19:50:01]

The patchset sent to the CQ was uploaded after l-g-t-m from kenrb@chromium.org,
sammc@chromium.org, gab@chromium.org, skuhne@chromium.org
Link to the patchset: https://codereview.chromium.org/2905903002/#ps280001
(title: "Update comments")

[commit-bot: I haz the power, 2017-08-16 19:50:10]

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-08-16 21:24:08]

CQ is committing da patch.

Bot data: {"patchset_id": 280001, "attempt_start_ts": 1502913001431740,
"parent_rev": "32998680778ff8071eb3dd6f8f91a02c046acbc6", "commit_rev":
"609a8275e909d415805c4251055913b202f8ca12"}

[commit-bot: I haz the power, 2017-08-16 21:24:31]

CQ is committing da patch.

Bot data: {"patchset_id": 280001, "attempt_start_ts": 1502913001431740,
"parent_rev": "4ba596be0a7d9dc2d762c42cb057b9e1ec776504", "commit_rev":
"45e347284efb236e79bc3a1ff4c68640dafe9b3d"}

[commit-bot: I haz the power, 2017-08-16 21:24:55]

Description was changed from

==========
Delete the PreferenceMACs on profile deletion.

On Windows, the tracked preference logic writes some data to the
registry. It is currently not cleared when the profile is deleted.

Alternatives considered: extending ProfileAttributesStorage::Observer 
and listening for OnProfileWasRemoved or OnProfileWillBeRemoved. However, 
the tracked pref code is componentized to avoid any reference to Chrome.

Known issue: the PreferenceMACs in the registry will not be deleted if
Chrome crashes during profile deletion. There's logic in
chrome_browser_main which cleans up the profile folders when this happens
(https://cs.chromium.org/chromium/src/chrome/browser/chrome_browser_main.cc?dr...)
However, I couldn't find a clean way to clean up the registry when the
profile no longer exists.

BUG=723834
==========

to

==========
Delete the PreferenceMACs on profile deletion.

On Windows, the tracked preference logic writes some data to the
registry. It is currently not cleared when the profile is deleted.

Alternatives considered: extending ProfileAttributesStorage::Observer 
and listening for OnProfileWasRemoved or OnProfileWillBeRemoved. However, 
the tracked pref code is componentized to avoid any reference to Chrome.

Known issue: the PreferenceMACs in the registry will not be deleted if
Chrome crashes during profile deletion. There's logic in
chrome_browser_main which cleans up the profile folders when this happens
(https://cs.chromium.org/chromium/src/chrome/browser/chrome_browser_main.cc?dr...)
However, I couldn't find a clean way to clean up the registry when the
profile no longer exists.

BUG=723834

Review-Url: https://codereview.chromium.org/2905903002
Cr-Commit-Position: refs/heads/master@{#494955}
Committed:
https://chromium.googlesource.com/chromium/src/+/45e347284efb236e79bc3a1ff4c6...
==========

[commit-bot: I haz the power, 2017-08-16 21:24:56]

Committed patchset #11 (id:280001) as
https://chromium.googlesource.com/chromium/src/+/45e347284efb236e79bc3a1ff4c6...