Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(473)

Issue 7981002: Fix crash by preventing extensions from changing their IDs. (Closed)

Created:
9 years, 3 months ago by jstritar
Modified:
9 years, 2 months ago
CC:
chromium-reviews, Aaron Boodman, Erik does not do reviews, mihaip+watch_chromium.org
Visibility:
Public.

Description

Fix crash by preventing extensions from changing their IDs. I can trigger the CHECK in 96466 by manually changing the 'key' field in the copy of an extension's manifest stored in the Preferences. With this change, the ExtensionService will stop loading the extension and instead print an error message if an extension's ID changes. This should not happen under normal conditions, so I'm not positive this is causing the crashes seen on real machines. BUG=96466 TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=102939

Patch Set 1 #

Patch Set 2 : . #

Patch Set 3 : do not add extension, rather than CHECKing #

Patch Set 4 : update comment #

Total comments: 2

Patch Set 5 : add UMA action #

Unified diffs Side-by-side diffs Delta from patch set Stats (+13 lines, -0 lines) Patch
M chrome/browser/extensions/extension_service.cc View 1 2 3 4 1 chunk +10 lines, -0 lines 0 comments Download
M chrome/common/extensions/extension_constants.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/extensions/extension_constants.cc View 1 2 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
jstritar
Hi Antony, Could you take a look at this? I updated this CL to use ...
9 years, 2 months ago (2011-09-26 18:28:31 UTC) #1
asargent_no_longer_on_chrome
lgtm http://codereview.chromium.org/7981002/diff/3002/chrome/browser/extensions/extension_service.cc File chrome/browser/extensions/extension_service.cc (right): http://codereview.chromium.org/7981002/diff/3002/chrome/browser/extensions/extension_service.cc#newcode1437 chrome/browser/extensions/extension_service.cc:1437: extension = NULL; It might be good to ...
9 years, 2 months ago (2011-09-26 22:08:04 UTC) #2
jstritar
http://codereview.chromium.org/7981002/diff/3002/chrome/browser/extensions/extension_service.cc File chrome/browser/extensions/extension_service.cc (right): http://codereview.chromium.org/7981002/diff/3002/chrome/browser/extensions/extension_service.cc#newcode1437 chrome/browser/extensions/extension_service.cc:1437: extension = NULL; On 2011/09/26 22:08:04, Antony Sargent wrote: ...
9 years, 2 months ago (2011-09-26 22:11:03 UTC) #3
asargent_no_longer_on_chrome
Your call - either way seems fine to me. On Mon, Sep 26, 2011 at ...
9 years, 2 months ago (2011-09-26 22:25:11 UTC) #4
jstritar
Okay- for now I added a UserAction that we can replace when adding more histograms.
9 years, 2 months ago (2011-09-27 02:10:24 UTC) #5
commit-bot: I haz the power
CQ is trying tha patch. Follow status at https://chromium-status.appspot.com/cq/jstritar@chromium.org/7981002/8001
9 years, 2 months ago (2011-09-27 14:29:14 UTC) #6
commit-bot: I haz the power
9 years, 2 months ago (2011-09-27 16:22:37 UTC) #7
Change committed as 102939

Powered by Google App Engine
This is Rietveld 408576698