Issue 399313007: Mojo: Convert assert()s under mojo/public/cpp/bindings/... to MOJO_DCHECK()s.

Issue 399313007: Mojo: Convert assert()s under mojo/public/cpp/bindings/... to MOJO_DCHECK()s. (Closed)

Created:
6 years, 5 months ago by viettrungluu

Modified:
6 years, 5 months ago

Reviewers:
yzshen1

CC:
chromium-reviews, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, darin (slow to review), ben+mojo_chromium.org

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

Project:
chromium

Visibility:
Public.

More Reviews

Description

Mojo: Convert assert()s under mojo/public/cpp/bindings/... to MOJO_DCHECK()s. R=yzshen@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=284243

Patch Set 1 #

Total comments: 1

Created: 6 years, 5 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+67 lines, -71 lines)			Patch
M	mojo/public/cpp/bindings/lib/array_internal.h	View	2 chunks	+3 lines, -2 lines	0 comments	Download
M	mojo/public/cpp/bindings/lib/bindings_serialization.cc	View	4 chunks	+4 lines, -5 lines	0 comments	Download
M	mojo/public/cpp/bindings/lib/bounds_checker.cc	View	2 chunks	+4 lines, -5 lines	1 comment	Download
M	mojo/public/cpp/bindings/lib/connector.cc	View	3 chunks	+4 lines, -4 lines	0 comments	Download
M	mojo/public/cpp/bindings/lib/filter_chain.cc	View	2 chunks	+4 lines, -4 lines	0 comments	Download
M	mojo/public/cpp/bindings/lib/fixed_buffer.cc	View	2 chunks	+2 lines, -3 lines	0 comments	Download
M	mojo/public/cpp/bindings/lib/interface_impl_internal.h	View	4 chunks	+4 lines, -3 lines	0 comments	Download
M	mojo/public/cpp/bindings/lib/interface_ptr_internal.h	View	6 chunks	+10 lines, -11 lines	0 comments	Download
M	mojo/public/cpp/bindings/lib/message.cc	View	2 chunks	+4 lines, -3 lines	0 comments	Download
M	mojo/public/cpp/bindings/lib/message_queue.cc	View	3 chunks	+4 lines, -6 lines	0 comments	Download
M	mojo/public/cpp/bindings/lib/router.cc	View	4 chunks	+6 lines, -4 lines	0 comments	Download
M	mojo/public/cpp/bindings/lib/validation_errors.cc	View	2 chunks	+2 lines, -4 lines	0 comments	Download
M	mojo/public/cpp/bindings/message.h	View	3 chunks	+4 lines, -5 lines	0 comments	Download
M	mojo/public/cpp/bindings/string.h	View	3 chunks	+3 lines, -5 lines	0 comments	Download
M	mojo/public/cpp/bindings/struct_ptr.h	View	4 chunks	+9 lines, -7 lines	0 comments	Download

Messages

Total messages: 7 (0 generated)

Expand Messages | Collapse Messages

yzshen1

LGTM with one nit. https://codereview.chromium.org/399313007/diff/1/mojo/public/cpp/bindings/lib/bounds_checker.cc File mojo/public/cpp/bindings/lib/bounds_checker.cc (right): https://codereview.chromium.org/399313007/diff/1/mojo/public/cpp/bindings/lib/bounds_checker.cc#newcode24 mojo/public/cpp/bindings/lib/bounds_checker.cc:24: MOJO_DCHECK(false) << "Not reached"; It ...

6 years, 5 months ago (2014-07-18 20:56:45 UTC) #2

viettrungluu

On 2014/07/18 20:56:45, yzshen1 wrote: > LGTM with one nit. > > https://codereview.chromium.org/399313007/diff/1/mojo/public/cpp/bindings/lib/bounds_checker.cc > File ...

6 years, 5 months ago (2014-07-18 21:19:57 UTC) #3

viettrungluu

The CQ bit was unchecked by viettrungluu@chromium.org

6 years, 5 months ago (2014-07-18 21:20:13 UTC) #5

yzshen1

It still seems a little nicer to use NOTREACHED than using MOJO_DCHECK(false) << "Not reached" ...

6 years, 5 months ago (2014-07-18 21:29:45 UTC) #6

viettrungluu

6 years, 5 months ago (2014-07-18 23:07:59 UTC) #7

Message was sent while issue was closed.

Committed patchset #1 manually as r284243 (presubmit successful).

Expand Messages | Collapse Messages