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

Issue 494943004: Mojo C++ bindings: better log messages for some validation errors at the receiver side. (Closed)

Created:
6 years, 4 months ago by yzshen1
Modified:
6 years, 3 months ago
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
Project:
chromium
Visibility:
Public.

Description

Mojo C++ bindings: better log messages for some validation errors at the receiver side. The CL provides more information in the log messages for the following errors: - Fixed-size array has wrong number of elements. - non-nullable field is set to null/invalid handle. The reason to have better log messages for these errors is that they are triggered when users (of the bindings) provide invalid arguments. The users probably want to find out what they have done wrong. The other validation errors are rarer, caused by bugs in the bindings or malicious senders. Providing better log messages for them is of slightly lower priority. BUG=324170 TEST=None Committed: https://crrev.com/90017b5f151ca42ffd7a14cfc2eaa08370da9fae Cr-Commit-Position: refs/heads/master@{#291704}

Patch Set 1 #

Patch Set 2 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+28 lines, -9 lines) Patch
M mojo/public/cpp/bindings/lib/array_internal.h View 1 3 chunks +15 lines, -3 lines 0 comments Download
M mojo/public/cpp/bindings/lib/validation_errors.h View 1 chunk +2 lines, -1 line 0 comments Download
M mojo/public/cpp/bindings/lib/validation_errors.cc View 1 1 chunk +7 lines, -3 lines 0 comments Download
M mojo/public/tools/bindings/generators/cpp_templates/struct_macros.tmpl View 2 chunks +4 lines, -2 lines 0 comments Download

Messages

Total messages: 16 (0 generated)
yzshen1
Hi, Darin. Would you please take a look? Thanks! This CL depends on https://codereview.chromium.org/473793004/
6 years, 4 months ago (2014-08-21 07:17:59 UTC) #1
darin (slow to review)
LGTM
6 years, 4 months ago (2014-08-21 23:56:52 UTC) #2
yzshen1
The CQ bit was checked by yzshen@chromium.org
6 years, 4 months ago (2014-08-22 00:07:14 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yzshen@chromium.org/494943004/20001
6 years, 4 months ago (2014-08-22 00:09:14 UTC) #4
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: android_dbg_tests_recipe on tryserver.chromium.linux ...
6 years, 4 months ago (2014-08-22 01:10:06 UTC) #5
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 4 months ago (2014-08-22 02:06:32 UTC) #6
commit-bot: I haz the power
Try jobs failed on following builders: android_dbg_tests_recipe on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tests_recipe/builds/1187)
6 years, 4 months ago (2014-08-22 02:06:33 UTC) #7
yzshen1
The CQ bit was checked by yzshen@chromium.org
6 years, 4 months ago (2014-08-22 02:46:24 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yzshen@chromium.org/494943004/20001
6 years, 4 months ago (2014-08-22 02:47:07 UTC) #9
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: android_dbg_tests_recipe on tryserver.chromium.linux ...
6 years, 4 months ago (2014-08-22 03:41:07 UTC) #10
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 4 months ago (2014-08-22 04:36:03 UTC) #11
commit-bot: I haz the power
Try jobs failed on following builders: android_dbg_tests_recipe on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tests_recipe/builds/1249)
6 years, 4 months ago (2014-08-22 04:36:04 UTC) #12
yzshen1
The CQ bit was checked by yzshen@chromium.org
6 years, 3 months ago (2014-08-25 16:33:35 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yzshen@chromium.org/494943004/20001
6 years, 3 months ago (2014-08-25 16:34:32 UTC) #14
commit-bot: I haz the power
Committed patchset #2 (20001) as 87bda7783c7ab69ad05024ba98a7aae4c4a6821c
6 years, 3 months ago (2014-08-25 17:28:28 UTC) #15
commit-bot: I haz the power
6 years, 3 months ago (2014-09-10 02:35:19 UTC) #16
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/90017b5f151ca42ffd7a14cfc2eaa08370da9fae
Cr-Commit-Position: refs/heads/master@{#291704}

Powered by Google App Engine
This is Rietveld 408576698