Issue 6209001: Change DCHECK in BookmarkBarGtk to DLOG and return.

Issue 6209001: Change DCHECK in BookmarkBarGtk to DLOG and return. (Closed)

Created:
9 years, 11 months ago by Tessa MacDuff

Modified:
9 years, 7 months ago

Reviewers:
Paweł Hajdan Jr., Elliot Glaysher, Evan Stade

CC:
chromium-reviews, Aaron Boodman, Erik does not do reviews, pam+watch_chromium.org, Paweł Hajdan Jr.

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

Description

Change DCHECK in BookmarkBarGtk to DLOG and return. If the BookmarkBarNode isn't loaded (yet) then we shouldn't be creating any bookmark buttons. This fixes the crashes reported in ExtensionInstallUIBrowserTest. BUG=44548 TEST=browser_tests, unit_tests, trybots Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=72197

Patch Set 1 #

Patch Set 2 : remove new test #

Patch Set 3 : improve Observe() check, revert CreateAllBookmarkButtons to DCHECK #

Total comments: 2

Patch Set 4 : remove comment #

Patch Set 5 : merge #

Created: 9 years, 11 months ago

Download [raw] [tar.bz2]

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

Messages

Total messages: 6 (0 generated)

Expand Messages | Collapse Messages

Evan Stade

+erg, original author I believe I'd recommend that you leave the DCHECK alone. Change the ...

9 years, 11 months ago (2011-01-21 01:55:16 UTC) #2

Paweł Hajdan Jr.

Thank you for fixing flakiness, just a tiny nit. No need to wait for another ...

9 years, 11 months ago (2011-01-21 09:13:44 UTC) #4

Tessa MacDuff

9 years, 11 months ago (2011-01-21 21:33:02 UTC) #6

http://codereview.chromium.org/6209001/diff/6002/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_install_ui_browsertest.cc (right):

http://codereview.chromium.org/6209001/diff/6002/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_install_ui_browsertest.cc:36: // Crashy,
http://crbug.com/44548.
On 2011/01/21 09:13:44, Paweł Hajdan Jr. wrote:
> nit: Please also remove the comment.

Done.

Expand Messages | Collapse Messages