Build NetError.java to use in java side tests by preprocessing net_errors.cc

Build NetError.java to use in java side tests by preprocessing net_errors.cc

Java side tests will be using a map to convert these error codes to platform specific ones.
This generates a java file which can be used to refer to the errorcode. The included test
just does a trivial check whether the java file can be included and used.

BUG=137967
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=155796

[Yaron, 2012-09-07 01:05:44]

http://codereview.chromium.org/10912136/diff/1/net/android/javatests/src/org/...
File net/android/javatests/src/org/chromium/net/NetErrorsTest.java (right):

http://codereview.chromium.org/10912136/diff/1/net/android/javatests/src/org/...
net/android/javatests/src/org/chromium/net/NetErrorsTest.java:6: * Test suite
for Android's default ProxySelector implementation. The purpose of these tests
This whole comment seems misplaced

http://codereview.chromium.org/10912136/diff/1/net/android/javatests/src/org/...
net/android/javatests/src/org/chromium/net/NetErrorsTest.java:34:
@Feature({"Android-WebView"})
This isn't true.

http://codereview.chromium.org/10912136/diff/1/net/net.gyp
File net/net.gyp (right):

http://codereview.chromium.org/10912136/diff/1/net/net.gyp#newcode2076
net/net.gyp:2076: 'net_errors_java',
This should be added to export_dependent_settings

http://codereview.chromium.org/10912136/diff/1/net/net.gyp#newcode2093
net/net.gyp:2093: 'net_errors_java',
This should be pulled transitively through net_java.

http://codereview.chromium.org/10912136/diff/1/net/net.gyp#newcode2102
net/net.gyp:2102: 'target_name': 'net_errors_java',
This should use a new gyp template (like build/java.gypi) and parameterized
accordingly because presumably will be used elsewhere

http://codereview.chromium.org/10912136/diff/1/net/net.gyp#newcode2116
net/net.gyp:2116: '<(SHARED_INTERMEDIATE_DIR)/net/template/NetError.java',
I don't see where this java file gets compiled or included in the jar/apk?

[joth, 2012-09-07 21:45:46]

http://codereview.chromium.org/10912136/diff/1/net/android/java/net_errors_ja...
File net/android/java/net_errors_java.template (right):

http://codereview.chromium.org/10912136/diff/1/net/android/java/net_errors_ja...
net/android/java/net_errors_java.template:6: // DO NOT EDIT!
note this comment doesn't make it through to the generated .java file as the gcc
-E preprocessor step strips comments. So either remove it from here, or we can
work out how to get it to leave the comments in. (man gcc did mention how, but
said it had some surprising effects and I didn't get chance to test if they
effect us)

http://codereview.chromium.org/10912136/diff/1/net/android/javatests/src/org/...
File net/android/javatests/src/org/chromium/net/NetErrorsTest.java (right):

http://codereview.chromium.org/10912136/diff/1/net/android/javatests/src/org/...
net/android/javatests/src/org/chromium/net/NetErrorsTest.java:26: private static
int FAILED_ERROR = -2;
comment these are copied from net_error_list.h and need to be manually kept in
sync

http://codereview.chromium.org/10912136/diff/1/net/net.gyp
File net/net.gyp (right):

http://codereview.chromium.org/10912136/diff/1/net/net.gyp#newcode2102
net/net.gyp:2102: 'target_name': 'net_errors_java',
On 2012/09/07 01:05:44, Yaron wrote:
> This should use a new gyp template (like build/java.gypi) and parameterized
> accordingly because presumably will be used elsewhere

right now net_error_list.h is the only known example of this style of constant
list. I agree we're likely to want to add more in future, but I'd just put a
comment here indicating this should be extracted as gypi if/when another user
for it is needed.
(at the same point we may want to remove the checked in .template file and have
a  more cleverer script generate that guy on the fly too, before running the
pre-processor over it)

http://codereview.chromium.org/10912136/diff/1/net/net.gyp#newcode2116
net/net.gyp:2116: '<(SHARED_INTERMEDIATE_DIR)/net/template/NetError.java',
On 2012/09/07 01:05:44, Yaron wrote:
> I don't see where this java file gets compiled or included in the jar/apk?

the 'additional_src_dirs' statement above shoves this into any dependent code's
build

[Yusuf, 2012-09-07 22:43:56]

willchan@ for net/net.gyp changes

[Yaron, 2012-09-07 22:49:06]

lgtm % nits

http://codereview.chromium.org/10912136/diff/1/net/net.gyp
File net/net.gyp (right):

http://codereview.chromium.org/10912136/diff/1/net/net.gyp#newcode2116
net/net.gyp:2116: '<(SHARED_INTERMEDIATE_DIR)/net/template/NetError.java',
On 2012/09/07 21:45:46, joth wrote:
> On 2012/09/07 01:05:44, Yaron wrote:
> > I don't see where this java file gets compiled or included in the jar/apk?
> 
> the 'additional_src_dirs' statement above shoves this into any dependent
code's
> build

Right, thanks!

http://codereview.chromium.org/10912136/diff/5001/net/android/java/net_errors...
File net/android/java/net_errors_java.template (right):

http://codereview.chromium.org/10912136/diff/5001/net/android/java/net_errors...
net/android/java/net_errors_java.template:1: package org.chromium.net;
Add chromium copyright.

http://codereview.chromium.org/10912136/diff/5001/net/android/javatests/src/o...
File net/android/javatests/src/org/chromium/net/NetErrorsTest.java (right):

http://codereview.chromium.org/10912136/diff/5001/net/android/javatests/src/o...
net/android/javatests/src/org/chromium/net/NetErrorsTest.java:29:
@Feature({"TabContents"})
Doesn't seem like a "TabContents" test. For lack of a better one how about
"Main"? Or start a new "Net" one.

http://codereview.chromium.org/10912136/diff/5001/net/net.gyp
File net/net.gyp (right):

http://codereview.chromium.org/10912136/diff/5001/net/net.gyp#newcode2079
net/net.gyp:2079: 'net_errors_java',
Sorry if I was unclear. This needs to be in both "dependencies" and
"export_dependent_settings"

[willchan no longer on Chromium, 2012-09-07 22:50:12]

I don't know what's going on here with all this java goop, but nothing looks
terribly wrong to me. So, LTGM. Please alert me if there's something in
particular I should pay attention to.

http://codereview.chromium.org/10912136/diff/5001/net/net.gyp
File net/net.gyp (right):

http://codereview.chromium.org/10912136/diff/5001/net/net.gyp#newcode2102
net/net.gyp:2102: # This should be extracted to a gypi file and parametrized if
we have more use cases of
80 cols
s/parametrized/parameterized/

[cjhopman, 2012-09-07 23:03:19]

http://codereview.chromium.org/10912136/diff/5001/net/net.gyp
File net/net.gyp (right):

http://codereview.chromium.org/10912136/diff/5001/net/net.gyp#newcode2079
net/net.gyp:2079: 'net_errors_java',
I believe that this should only be in dependencies. If the settings are
exported, then a target that depends on net would end up compiling the generated
.java file into its .jar and we would have duplicate definitions. Instead, by
just having it in dependencies, the generated java file will be included in the
net_java jar, and then dependents of net_java will have access to it through the
jar.

[commit-bot: I haz the power, 2012-09-08 00:40:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yusufo@chromium.org/10912136/2003

[commit-bot: I haz the power, 2012-09-08 00:40:32]

Presubmit check for 10912136-2003 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    net

[Yusuf, 2012-09-08 00:43:14]

willchan@ it was just for adding the new target that creates this java file from
a native file using the preprocessor, so we have the constants on both native
and Java sides. If it looks good to you, do you mind giving another LGTM.
Somehow it didn't seem to catch the last one.

[willchan no longer on Chromium, 2012-09-08 00:43:52]

lgtm

[commit-bot: I haz the power, 2012-09-10 17:17:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yusufo@chromium.org/10912136/2003

[commit-bot: I haz the power, 2012-09-10 17:17:38]

Failed to apply patch for net/net.gyp:
While running patch -p1 --forward --force;
patching file net/net.gyp
Hunk #1 FAILED at 2097.
Hunk #2 FAILED at 2113.
Hunk #3 succeeded at 2136 (offset 16 lines).
2 out of 3 hunks FAILED -- saving rejects to file net/net.gyp.rej

[commit-bot: I haz the power, 2012-09-10 17:56:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yusufo@chromium.org/10912136/10001

[commit-bot: I haz the power, 2012-09-10 19:22:51]

Change committed as 155796