Issue 6539002: Begun the DCHECK() cleanup. Starting off with /src/chrome/a* and /src/chrome/...

Issue 6539002: Begun the DCHECK() cleanup. Starting off with /src/chrome/a* and /src/chrome/... (Closed)

Created:
9 years, 10 months ago by KushalP

Modified:
9 years, 7 months ago

Reviewers:
Robert Sesek, Peter Kasting

CC:
chromium-reviews, michaeln, Ilya Sherman, Erik does not do reviews, Aaron Boodman, pam+watch_chromium.org, Paweł Hajdan Jr., James Hawkins, dhollowa

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

Visibility:
Public.

Description

Begin the DCHECK() cleanup. Starting off with /src/chrome/a* and /src/chrome/browser/a* BUG=58409 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=75314

Patch Set 7 : '' #

Total comments: 1

Created: 9 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+48 lines, -47 lines)			Patch
M	AUTHORS	View	1 2 3 4 5	1 chunk	+1 line, -0 lines	0 comments	Download
M	chrome/app/breakpad_mac.mm	View	1 2 3 4 5	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/app/scoped_ole_initializer.h	View	1 2 3 4 5	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/about_flags.cc	View	1 2 3 4 5	3 chunks	+3 lines, -3 lines	0 comments	Download
M	chrome/browser/alternate_nav_url_fetcher.cc	View	1 2 3 4 5	2 chunks	+2 lines, -2 lines	0 comments	Download
M	chrome/browser/app_controller_mac.mm	View	1 2 3 4 5	3 chunks	+3 lines, -3 lines	0 comments	Download
M	chrome/browser/appcache/appcache_dispatcher_host.cc	View	1 2 3 4 5	1 chunk	+3 lines, -3 lines	0 comments	Download
M	chrome/browser/appcache/appcache_frontend_proxy.cc	View	1 2 3 4 5	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/autocomplete/autocomplete_edit_view_win.cc	View	1 2 3 4 5	5 chunks	+5 lines, -5 lines	1 comment	Download
M	chrome/browser/autocomplete/autocomplete_popup_view_mac.mm	View	1 2 3 4 5	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/autocomplete/history_provider.cc	View	1 2 3 4 5	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/autocomplete_history_manager.cc	View	1 2 3 4 5	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/autofill/autofill_cc_infobar_win.cc	View	1 2 3 4 5	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/autofill/autofill_dialog_controller_mac.mm	View	1 2 3 4 5	2 chunks	+4 lines, -4 lines	0 comments	Download
M	chrome/browser/autofill/autofill_manager.cc	View	1 2 3 4 5	4 chunks	+6 lines, -6 lines	0 comments	Download
M	chrome/browser/autofill/contact_info.cc	View	1 2 3 4 5	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/autofill/select_control_handler.cc	View	1 2 3 4 5	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/automation/automation_provider.cc	View	1 2 3 4 5	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/automation/automation_resource_message_filter.cc	View	1 2 3 4 5	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/automation/automation_resource_tracker.cc	View	1 2 3 4 5	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/automation/testing_automation_provider.cc	View	1 2 3 4 5	4 chunks	+4 lines, -4 lines	0 comments	Download
M	chrome/browser/automation/ui_controls_linux.cc	View	1 2 3 4 5	2 chunks	+2 lines, -2 lines	0 comments	Download
M	chrome/browser/automation/ui_controls_win.cc	View	1 2 3 4 5	2 chunks	+2 lines, -2 lines	0 comments	Download
M	chrome/browser/automation/url_request_automation_job.cc	View	1 2 3 4 5	1 chunk	+1 line, -1 line	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

Robert Sesek

http://codereview.chromium.org/6539002/diff/6015/chrome/browser/autocomplete/history_url_provider.cc File chrome/browser/autocomplete/history_url_provider.cc (right): http://codereview.chromium.org/6539002/diff/6015/chrome/browser/autocomplete/history_url_provider.cc#newcode745 chrome/browser/autocomplete/history_url_provider.cc:745: DCHECK(first == matches->end()) << DCHECK_NE? http://codereview.chromium.org/6539002/diff/6015/chrome/browser/autofill/autofill_dialog_controller_mac.mm File chrome/browser/autofill/autofill_dialog_controller_mac.mm (right): ...

9 years, 10 months ago (2011-02-17 17:17:31 UTC) #1

KushalP

I've made your requested changes and pushed(?) the latest changelist (4) through to codereview. http://codereview.chromium.org/6539002/diff/6015/chrome/browser/autocomplete/history_url_provider.cc ...

9 years, 10 months ago (2011-02-17 17:38:10 UTC) #2

Robert Sesek

LGTM. I'll run this through the trybots and land it if they're green.

9 years, 10 months ago (2011-02-17 17:44:59 UTC) #3

Robert Sesek

http://codereview.chromium.org/6539002/diff/4055/chrome/browser/autocomplete/autocomplete_edit_view_win.cc File chrome/browser/autocomplete/autocomplete_edit_view_win.cc (right): http://codereview.chromium.org/6539002/diff/4055/chrome/browser/autocomplete/autocomplete_edit_view_win.cc#newcode360 chrome/browser/autocomplete/autocomplete_edit_view_win.cc:360: ~PaintPatcher() { DCHECK_EQ(refcount_, 0); } This needs to be ...

9 years, 10 months ago (2011-02-17 19:57:02 UTC) #4

Peter Kasting

9 years, 10 months ago (2011-02-22 23:31:40 UTC) #5

This change should not have landed as-is.  The order for DCHECK_EQ() and
DCHECK_NE() is "expected, actual" but this patch gets that backward.  The
practical effect of this is that DCHECK failures will print very strange error
messages.  (Note that DCHECK_GT/GE/LT/LE() are different; for "a > 3" you do
"DCHECK_GT(a, 3)" and not "DCHECK_LT(3, a)".)

Can one of you either fix the places this patch got wrong, or else revert the
patch until a fixed version is ready?

Expand Messages | Collapse Messages