mojo: Utility class to parse .data file.

Committed: https://crrev.com/6a55ddd7152aff785deecbde163d5df7a843f9cc
Cr-Commit-Position: refs/heads/master@{#292588}

[darin (slow to review), 2014-08-27 15:55:56]

darin@chromium.org changed reviewers:
+ darin@chromium.org

[darin (slow to review), 2014-08-27 15:55:56]

https://codereview.chromium.org/508033004/diff/1/mojo/android/javatests/valid...
File mojo/android/javatests/validation_test_util.cc (right):

https://codereview.chromium.org/508033004/diff/1/mojo/android/javatests/valid...
mojo/android/javatests/validation_test_util.cc:27: if
(!mojo::test::ParseValidationTestInput(
nit: "mojo::" prefix isn't necessary since you are in the mojo namespace
already.

[qsr, 2014-08-27 16:02:25]

https://chromiumcodereview.appspot.com/508033004/diff/1/mojo/android/javatest...
File mojo/android/javatests/validation_test_util.cc (right):

https://chromiumcodereview.appspot.com/508033004/diff/1/mojo/android/javatest...
mojo/android/javatests/validation_test_util.cc:27: if
(!mojo::test::ParseValidationTestInput(
On 2014/08/27 15:55:56, darin wrote:
> nit: "mojo::" prefix isn't necessary since you are in the mojo namespace
> already.

Done.

[yzshen1, 2014-08-27 17:19:25]

LGTM with nits.

(But I wouldn't call myself a good Java reviewer. :))

https://chromiumcodereview.appspot.com/508033004/diff/20001/mojo/android/java...
File
mojo/android/javatests/src/org/chromium/mojo/bindings/ValidationTestUtilTest.java
(right):

https://chromiumcodereview.appspot.com/508033004/diff/20001/mojo/android/java...
mojo/android/javatests/src/org/chromium/mojo/bindings/ValidationTestUtilTest.java:23:
boolean expectedResult,
Do we need to align these parameters with the one on the previous line?
(But I don't know Java style guide well.)

https://chromiumcodereview.appspot.com/508033004/diff/20001/mojo/public/cpp/b...
File mojo/public/cpp/bindings/tests/BUILD.gn (right):

https://chromiumcodereview.appspot.com/508033004/diff/20001/mojo/public/cpp/b...
mojo/public/cpp/bindings/tests/BUILD.gn:36:
source_set("mojo_public_bindings_test_utils") {
nit: the GN style guide mentions:

"Prefer to do this by giving a target a short (possibly non-unique) name that
makes writing dependencies clearer."
....
"For example, it looks much better to write a dependency as
"//mojo/public/bindings" rather than "//mojo/public/bindings:mojo_bindings". So
in the file //mojo/public/bindings/BUILD.gn:"

[qsr, 2014-08-28 08:32:48]

qsr@chromium.org changed reviewers:
+ ppi@chromium.org

[qsr, 2014-08-28 08:32:48]

+ppi

https://chromiumcodereview.appspot.com/508033004/diff/20001/mojo/android/java...
File
mojo/android/javatests/src/org/chromium/mojo/bindings/ValidationTestUtilTest.java
(right):

https://chromiumcodereview.appspot.com/508033004/diff/20001/mojo/android/java...
mojo/android/javatests/src/org/chromium/mojo/bindings/ValidationTestUtilTest.java:23:
boolean expectedResult,
On 2014/08/27 17:19:25, yzshen1 wrote:
> Do we need to align these parameters with the one on the previous line?
> (But I don't know Java style guide well.)

Horizontal alignment is never required:
https://google-styleguide.googlecode.com/svn/trunk/javaguide.html#s4.6.3-hori...

In my case, I'm using an auto-formatter, and to help future changed, I prefer to
follow what it does.

https://chromiumcodereview.appspot.com/508033004/diff/20001/mojo/public/cpp/b...
File mojo/public/cpp/bindings/tests/BUILD.gn (right):

https://chromiumcodereview.appspot.com/508033004/diff/20001/mojo/public/cpp/b...
mojo/public/cpp/bindings/tests/BUILD.gn:36:
source_set("mojo_public_bindings_test_utils") {
On 2014/08/27 17:19:25, yzshen1 wrote:
> nit: the GN style guide mentions:
> 
> "Prefer to do this by giving a target a short (possibly non-unique) name that
> makes writing dependencies clearer."
> ....
> "For example, it looks much better to write a dependency as
> "//mojo/public/bindings" rather than "//mojo/public/bindings:mojo_bindings".
So
> in the file //mojo/public/bindings/BUILD.gn:"

 I'm following the style of the previous target. I'll push it as it, and will do
a followup CL to change both names.

[ppi, 2014-08-28 11:42:42]

Thanks, lgtm % nits.

https://chromiumcodereview.appspot.com/508033004/diff/20001/mojo/android/java...
File
mojo/android/javatests/src/org/chromium/mojo/bindings/ValidationTestUtil.java
(right):

https://chromiumcodereview.appspot.com/508033004/diff/20001/mojo/android/java...
mojo/android/javatests/src/org/chromium/mojo/bindings/ValidationTestUtil.java:40:
super();
What is this for?

https://chromiumcodereview.appspot.com/508033004/diff/20001/mojo/android/java...
mojo/android/javatests/src/org/chromium/mojo/bindings/ValidationTestUtil.java:50:
public static Data parseData(String dataAsString) {
What is the format of the data? Is this serialized mojo message format, or the
validation tests input format defined in validation_test_input_parser.h? If the
latter, please refer to this file in the class comment.

https://chromiumcodereview.appspot.com/508033004/diff/20001/mojo/android/java...
File
mojo/android/javatests/src/org/chromium/mojo/bindings/ValidationTestUtilTest.java
(right):

https://chromiumcodereview.appspot.com/508033004/diff/20001/mojo/android/java...
mojo/android/javatests/src/org/chromium/mojo/bindings/ValidationTestUtilTest.java:23:
boolean expectedResult,
nit: expectedResults -> isResultExpected

[qsr, 2014-08-28 14:44:17]

https://chromiumcodereview.appspot.com/508033004/diff/20001/mojo/android/java...
File
mojo/android/javatests/src/org/chromium/mojo/bindings/ValidationTestUtil.java
(right):

https://chromiumcodereview.appspot.com/508033004/diff/20001/mojo/android/java...
mojo/android/javatests/src/org/chromium/mojo/bindings/ValidationTestUtil.java:40:
super();
On 2014/08/28 11:42:41, ppi wrote:
> What is this for?

 Removed.

https://chromiumcodereview.appspot.com/508033004/diff/20001/mojo/android/java...
mojo/android/javatests/src/org/chromium/mojo/bindings/ValidationTestUtil.java:50:
public static Data parseData(String dataAsString) {
On 2014/08/28 11:42:41, ppi wrote:
> What is the format of the data? Is this serialized mojo message format, or the
> validation tests input format defined in validation_test_input_parser.h? If
the
> latter, please refer to this file in the class comment.

Done.

https://chromiumcodereview.appspot.com/508033004/diff/20001/mojo/android/java...
File
mojo/android/javatests/src/org/chromium/mojo/bindings/ValidationTestUtilTest.java
(right):

https://chromiumcodereview.appspot.com/508033004/diff/20001/mojo/android/java...
mojo/android/javatests/src/org/chromium/mojo/bindings/ValidationTestUtilTest.java:23:
boolean expectedResult,
On 2014/08/28 11:42:41, ppi wrote:
> nit: expectedResults -> isResultExpected

Changed to isInputValid

[qsr, 2014-08-28 14:44:21]

The CQ bit was checked by qsr@chromium.org

[commit-bot: I haz the power, 2014-08-28 14:45:32]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/qsr@chromium.org/508033004/40001

[commit-bot: I haz the power, 2014-08-28 19:39:13]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)

[commit-bot: I haz the power, 2014-08-28 20:01:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-28 20:01:51]

Failed to commit the patch.

[qsr, 2014-08-29 07:58:40]

Committed patchset #3 (id:40001) to pending queue manually as f0bf875 (presubmit
successful).

[commit-bot: I haz the power, 2014-09-10 03:06:54]

Patchset 3 (id:??) landed as
https://crrev.com/6a55ddd7152aff785deecbde163d5df7a843f9cc
Cr-Commit-Position: refs/heads/master@{#292588}