Handle changes in extension ID generation logic better.

Handle changes in extension ID generation logic better.

Repurpose CleanupBadExtensionKeys (which deals with a long-fixed problem) to
remap extension prefs if the generated ID (based on the persisted path) does not
match the persisted ID.

BUG=75945
TEST=ExtensionPrefsIdChange
R=asargent@chromium.org

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=80466

[asargent_no_longer_on_chrome, 2011-04-04 23:46:15]

mostly lg with a few nits

http://codereview.chromium.org/6677147/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extension_prefs.cc (right):

http://codereview.chromium.org/6677147/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_prefs.cc:160:
DCHECK(!dictionary->HasKey(computed_id));
Are you positive it's not possible for someone's prefs to get into this state
where there is already an entry under the correct key? Would it make sense to
just delete the value under the incorrect key and leave whatever *is* there
under the correct one if this actually happened?

http://codereview.chromium.org/6677147/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_prefs.cc:170: Value* extension_prefs;
nit: initialize to NULL

http://codereview.chromium.org/6677147/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_prefs.cc:172: i->first, &extension_prefs));
You should probably either change this to a CHECK, or handle the failure
gracefully (otherwise you'll assign the garbage/NULL in extension_prefs into the
dictionary).

http://codereview.chromium.org/6677147/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extension_prefs_unittest.cc (right):

http://codereview.chromium.org/6677147/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_prefs_unittest.cc:626: fprintf(stderr,
"generated extension id: %s\n", extension_->id().c_str());
nit: remove this debugging output, or switch to using VLOG

http://codereview.chromium.org/6677147/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_prefs_unittest.cc:641:
prefs()->SetExtensionState(extension_.get(), Extension::DISABLED);
maybe add a check here that you get back the extension correctly by the expected
id?

[Mihai Parparita -not on Chrome, 2011-04-05 00:22:47]

http://codereview.chromium.org/6677147/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extension_prefs.cc (right):

http://codereview.chromium.org/6677147/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_prefs.cc:160:
DCHECK(!dictionary->HasKey(computed_id));
On 2011/04/04 23:46:15, Antony Sargent wrote:
> Are you positive it's not possible for someone's prefs to get into this state
> where there is already an entry under the correct key? Would it make sense to
> just delete the value under the incorrect key and leave whatever *is* there
> under the correct one if this actually happened?

Just deleting the value under the incorrect key seems safer, switched to that.

http://codereview.chromium.org/6677147/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_prefs.cc:170: Value* extension_prefs;
On 2011/04/04 23:46:15, Antony Sargent wrote:
> nit: initialize to NULL

Done.

http://codereview.chromium.org/6677147/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_prefs.cc:172: i->first, &extension_prefs));
On 2011/04/04 23:46:15, Antony Sargent wrote:
> You should probably either change this to a CHECK, or handle the failure
> gracefully (otherwise you'll assign the garbage/NULL in extension_prefs into
the
> dictionary).

Changed to CHECK.

http://codereview.chromium.org/6677147/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extension_prefs_unittest.cc (right):

http://codereview.chromium.org/6677147/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_prefs_unittest.cc:626: fprintf(stderr,
"generated extension id: %s\n", extension_->id().c_str());
On 2011/04/04 23:46:15, Antony Sargent wrote:
> nit: remove this debugging output, or switch to using VLOG 

Oops, removed.

http://codereview.chromium.org/6677147/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_prefs_unittest.cc:641:
prefs()->SetExtensionState(extension_.get(), Extension::DISABLED);
On 2011/04/04 23:46:15, Antony Sargent wrote:
> maybe add a check here that you get back the extension correctly by the
expected
> id?

Added a check using ExtensionPrefs::GetExtensions

[asargent_no_longer_on_chrome, 2011-04-05 03:36:14]

lgtm. Nice fix.