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

Issue 366015: Fix theme install crasher.... (Closed)

Created:
11 years, 1 month ago by Miranda Callahan
Modified:
9 years, 7 months ago
Reviewers:
tony
CC:
chromium-reviews_googlegroups.com, Aaron Boodman, Erik does not do reviews, pam+watch_chromium.org, ben+cc_chromium.org
Visibility:
Public.

Description

Fix theme install crasher. BUG=26704 TEST= change themes. browser does not crash. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=31036

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -1 line) Patch
M chrome/browser/extensions/theme_installed_infobar_delegate.cc View 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 3 (0 generated)
Miranda Callahan
11 years, 1 month ago (2009-11-04 22:27:44 UTC) #1
tony
LGTM. Maybe on Linux we don't create the invisible ok button?
11 years, 1 month ago (2009-11-04 22:30:39 UTC) #2
Miranda Callahan
11 years, 1 month ago (2009-11-04 22:40:04 UTC) #3
On 2009/11/04 22:30:39, tony wrote:
> LGTM.  Maybe on Linux we don't create the invisible ok button?

Perhaps -- it's code in the Views::Layout that wants to do things with the
button and crashes it.  The ConfirmInfoBar layout seems to go back and forth
between assuming there is always an ok_button, and checking to see whether it
exists or not.  I tried to impose strict checking hygiene here, but it was
insufficient -- the assumed OK button is in pretty deep.  But for now, just
allowing the invisible button does no real harm, and takes us back to where we
were before the regression.

Powered by Google App Engine
This is Rietveld 408576698