[Android] Add a java version of the test server spawner.

[Android] Add a java version of the test server spawner.

BUG=448626
Committed: https://crrev.com/e1799e662af4e87b1e6d501e3511628f8579174d
Cr-Commit-Position: refs/heads/master@{#313532}

[jbudorick, 2015-01-23 00:14:08]

jbudorick@chromium.org changed reviewers:
+ klundberg@chromium.org, phajdan.jr@chromium.org, tedchoc@chromium.org

[jbudorick, 2015-01-23 00:14:09]

klundberg: build/android/
tedchoc: chrome/android/ and testing/android/
phajdan: net/test/android/

ptal

https://codereview.chromium.org/867073002/diff/1/net/test/android/javatests/s...
File net/test/android/javatests/src/org/chromium/net/test/BaseTestServer.java
(right):

https://codereview.chromium.org/867073002/diff/1/net/test/android/javatests/s...
net/test/android/javatests/src/org/chromium/net/test/BaseTestServer.java:1: //
Copyright 2015 The Chromium Authors. All rights reserved.
I'm not sure if this belongs in //net or //base (along with BaseTcpTestServer
and BaseHttpTestServer). Thoughts welcome.

[jbudorick, 2015-01-23 01:02:56]

jbudorick@chromium.org changed reviewers:
+ cjhopman@chromium.org

[jbudorick, 2015-01-23 01:02:56]

+cjhopman for gyp/gn

[Paweł Hajdan Jr., 2015-01-23 10:44:50]

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
File
net/test/android/javatests/src/org/chromium/net/test/BaseHttpTestServer.java
(right):

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/BaseHttpTestServer.java:64:
conn.close();
Shouldn't this be in a "finally" clause?

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
File net/test/android/javatests/src/org/chromium/net/test/BaseTcpTestServer.java
(right):

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/BaseTcpTestServer.java:23:
mServerSocket.setSoTimeout(5000);
Can we avoid a hardcoded timeout here? Please consider having a solution similar
to TestTimeouts on the C++ side: centralized, and controlled with command-line
flags.

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
File net/test/android/javatests/src/org/chromium/net/test/TestServerBuilder.java
(right):

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/TestServerBuilder.java:52:
private String mCertAndKeyFile;
It'd be useful to comment each variable.

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/TestServerBuilder.java:114:
// Needs to support all options provided by testserver_base.py and testserver.py
Why does it need to? I'm not sure if keeping the two in sync is viable.

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
File net/test/android/javatests/src/org/chromium/net/test/TestServerSpawner.java
(right):

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/TestServerSpawner.java:67:
Log.i(TAG, "Received GET /kill request.");
Can we avoid supporting killing the server this way? Why not just kill the
process?

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/TestServerSpawner.java:82:
entity = new StringEntity("ready");
Is the purpose of this to wait for the server to be ready? This'd suggest a
polling loop on the client side, which is not optimal. Please consider a way (if
possible) to use waiting for an event instead. It'd be much more reliable.

This comment is based on *actual* problems with the python http server. Just
trying to avoid repeating some known problems in this new code.

[klundberg, 2015-01-23 15:04:11]

LGTM for the build/android pieces

[jbudorick, 2015-01-23 19:25:15]

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
File
net/test/android/javatests/src/org/chromium/net/test/BaseHttpTestServer.java
(right):

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/BaseHttpTestServer.java:64:
conn.close();
On 2015/01/23 10:44:49, Paweł Hajdan Jr. wrote:
> Shouldn't this be in a "finally" clause?

Done.

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
File net/test/android/javatests/src/org/chromium/net/test/BaseTcpTestServer.java
(right):

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/BaseTcpTestServer.java:23:
mServerSocket.setSoTimeout(5000);
On 2015/01/23 10:44:49, Paweł Hajdan Jr. wrote:
> Can we avoid a hardcoded timeout here? Please consider having a solution
similar
> to TestTimeouts on the C++ side: centralized, and controlled with command-line
> flags.

I'm open to not hardcoding the timeout here, but this just times out the call to
ServerSocket.accept() so we don't wait there forever. It doesn't cause the tests
to time out.

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
File net/test/android/javatests/src/org/chromium/net/test/BaseTestServer.java
(right):

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/BaseTestServer.java:21:
accept();
Continuing from BaseTcpTestServer: hitting the socket timeout in
BaseTcpTestServer just lets us check if the server should be stopped. If not, we
try to accept again.

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
File net/test/android/javatests/src/org/chromium/net/test/TestServerBuilder.java
(right):

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/TestServerBuilder.java:52:
private String mCertAndKeyFile;
On 2015/01/23 10:44:50, Paweł Hajdan Jr. wrote:
> It'd be useful to comment each variable.

Done.

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/TestServerBuilder.java:114:
// Needs to support all options provided by testserver_base.py and testserver.py
On 2015/01/23 10:44:49, Paweł Hajdan Jr. wrote:
> Why does it need to? I'm not sure if keeping the two in sync is viable.

I guess it doesn't have to (and indeed, there are some we can't support), so I
removed the comment.

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
File net/test/android/javatests/src/org/chromium/net/test/TestServerSpawner.java
(right):

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/TestServerSpawner.java:67:
Log.i(TAG, "Received GET /kill request.");
On 2015/01/23 10:44:50, Paweł Hajdan Jr. wrote:
> Can we avoid supporting killing the server this way? Why not just kill the
> process?

see below

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/TestServerSpawner.java:82:
entity = new StringEntity("ready");
On 2015/01/23 10:44:50, Paweł Hajdan Jr. wrote:
> Is the purpose of this to wait for the server to be ready? This'd suggest a
> polling loop on the client side, which is not optimal. Please consider a way
(if
> possible) to use waiting for an event instead. It'd be much more reliable.
> 
> This comment is based on *actual* problems with the python http server. Just
> trying to avoid repeating some known problems in this new code.

(leaving the code as-is given pending discussion)

This is what the SpawnerCommunicator is expecting. I had been planning to leave
the client side (i.e. SpawnerCommunicator and RemoteTestServer) alone while
reimplementing the server side in java, as the existing server implementation
has to keep working in the meantime. In that scenario, I'd certainly be open to
changing the client side after the existing server implementation is no longer
used in Android tests.

wdyt?

[Ted C, 2015-01-26 22:10:11]

https://codereview.chromium.org/867073002/diff/80001/net/test/android/javates...
File
net/test/android/javatests/src/org/chromium/net/test/BaseHttpTestServer.java
(right):

https://codereview.chromium.org/867073002/diff/80001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/BaseHttpTestServer.java:67:
protected HttpParams getConnectionParams() {
all non-private methods should have javadoc

https://codereview.chromium.org/867073002/diff/80001/net/test/android/javates...
File net/test/android/javatests/src/org/chromium/net/test/BaseTcpTestServer.java
(right):

https://codereview.chromium.org/867073002/diff/80001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/BaseTcpTestServer.java:15:
public abstract class BaseTcpTestServer extends BaseTestServer {
Are there any usages of this class or BaseTestServer directly?

Seems like more indirection than needed if we don't see usages of these in the
near future (and could be abstracted if an when we do need them).

https://codereview.chromium.org/867073002/diff/80001/net/test/android/javates...
File net/test/android/javatests/src/org/chromium/net/test/TestServerBuilder.java
(right):

https://codereview.chromium.org/867073002/diff/80001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/TestServerBuilder.java:21:
class TestServerBuilder {
javadoc for what the intended purpose of this class is.

https://codereview.chromium.org/867073002/diff/80001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/TestServerBuilder.java:182:
mCertAndKeyFile = null;
null, 0, and false are the defaults in java.

And since they're non-final (maybe they should be final), I would inline the
default at declaration time.

https://codereview.chromium.org/867073002/diff/80001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/TestServerBuilder.java:197:
mSslBulkCiphers = new Vector<String>(SSL_BULK_CIPHERS);
ArrayList is way more common than vector.

https://codereview.chromium.org/867073002/diff/80001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/TestServerBuilder.java:224:
case "basic-auth-proxy":
switching on strings is only applicable in KK+, so if this supposed to run all
the way back to ICS then this won't work.

https://codereview.chromium.org/867073002/diff/80001/net/test/android/javates...
File net/test/android/javatests/src/org/chromium/net/test/TestServerSpawner.java
(right):

https://codereview.chromium.org/867073002/diff/80001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/TestServerSpawner.java:98:
private static final int ARBITRARY_MAX_JSON_SIZE = 65536;
put this up with the other statics.

[cjhopman, 2015-01-26 22:12:10]

lgtm

[cjhopman, 2015-01-26 22:12:10]

lgtm

lgtm

[jbudorick, 2015-01-27 00:27:56]

https://codereview.chromium.org/867073002/diff/80001/net/test/android/javates...
File
net/test/android/javatests/src/org/chromium/net/test/BaseHttpTestServer.java
(right):

https://codereview.chromium.org/867073002/diff/80001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/BaseHttpTestServer.java:67:
protected HttpParams getConnectionParams() {
On 2015/01/26 22:10:10, Ted C wrote:
> all non-private methods should have javadoc

Done.

https://codereview.chromium.org/867073002/diff/80001/net/test/android/javates...
File net/test/android/javatests/src/org/chromium/net/test/BaseTcpTestServer.java
(right):

https://codereview.chromium.org/867073002/diff/80001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/BaseTcpTestServer.java:15:
public abstract class BaseTcpTestServer extends BaseTestServer {
On 2015/01/26 22:10:10, Ted C wrote:
> Are there any usages of this class or BaseTestServer directly?
> 
> Seems like more indirection than needed if we don't see usages of these in the
> near future (and could be abstracted if an when we do need them).

I'm planning to implement at least the TCP_ECHO and FTP servers directly on top
of this one, and I'll be implementing the UDP_ECHO server on top of
BaseTestServer. I'll be working on those CLs immediately after this one lands.

https://codereview.chromium.org/867073002/diff/80001/net/test/android/javates...
File net/test/android/javatests/src/org/chromium/net/test/TestServerBuilder.java
(right):

https://codereview.chromium.org/867073002/diff/80001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/TestServerBuilder.java:21:
class TestServerBuilder {
On 2015/01/26 22:10:10, Ted C wrote:
> javadoc for what the intended purpose of this class is.

Done.

https://codereview.chromium.org/867073002/diff/80001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/TestServerBuilder.java:182:
mCertAndKeyFile = null;
On 2015/01/26 22:10:10, Ted C wrote:
> null, 0, and false are the defaults in java.
> 

(my old c++03 initializer list habits die hard)

> And since they're non-final (maybe they should be final), I would inline the
> default at declaration time.

Done.

https://codereview.chromium.org/867073002/diff/80001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/TestServerBuilder.java:197:
mSslBulkCiphers = new Vector<String>(SSL_BULK_CIPHERS);
On 2015/01/26 22:10:10, Ted C wrote:
> ArrayList is way more common than vector.

Switched to ArrayList.

https://codereview.chromium.org/867073002/diff/80001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/TestServerBuilder.java:224:
case "basic-auth-proxy":
On 2015/01/26 22:10:11, Ted C wrote:
> switching on strings is only applicable in KK+, so if this supposed to run all
> the way back to ICS then this won't work.

It's certainly supposed to run pre-KK. Switched.

https://codereview.chromium.org/867073002/diff/80001/net/test/android/javates...
File net/test/android/javatests/src/org/chromium/net/test/TestServerSpawner.java
(right):

https://codereview.chromium.org/867073002/diff/80001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/TestServerSpawner.java:98:
private static final int ARBITRARY_MAX_JSON_SIZE = 65536;
On 2015/01/26 22:10:11, Ted C wrote:
> put this up with the other statics.

Done.

[cjhopman, 2015-01-27 01:18:40]

https://codereview.chromium.org/867073002/diff/80001/net/test/android/javates...
File net/test/android/javatests/src/org/chromium/net/test/TestServerBuilder.java
(right):

https://codereview.chromium.org/867073002/diff/80001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/TestServerBuilder.java:224:
case "basic-auth-proxy":
On 2015/01/27 00:27:56, jbudorick wrote:
> On 2015/01/26 22:10:11, Ted C wrote:
> > switching on strings is only applicable in KK+, so if this supposed to run
all
> > the way back to ICS then this won't work.
> 
> It's certainly supposed to run pre-KK. Switched.

string switch actually works fine pre-kk. The main java 7 feature that is kk+
only is try-with-resource. See
http://tools.android.com/tech-docs/new-build-system/user-guide#TOC-Using-sour...

[Paweł Hajdan Jr., 2015-01-27 15:10:50]

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
File net/test/android/javatests/src/org/chromium/net/test/BaseTcpTestServer.java
(right):

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/BaseTcpTestServer.java:23:
mServerSocket.setSoTimeout(5000);
On 2015/01/23 19:25:14, jbudorick wrote:
> On 2015/01/23 10:44:49, Paweł Hajdan Jr. wrote:
> > Can we avoid a hardcoded timeout here? Please consider having a solution
> similar
> > to TestTimeouts on the C++ side: centralized, and controlled with
command-line
> > flags.
> 
> I'm open to not hardcoding the timeout here, but this just times out the call
to
> ServerSocket.accept() so we don't wait there forever. It doesn't cause the
tests
> to time out.

Yes, let's avoid the hardcoded timeout.

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
File net/test/android/javatests/src/org/chromium/net/test/TestServerBuilder.java
(right):

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/TestServerBuilder.java:114:
// Needs to support all options provided by testserver_base.py and testserver.py
On 2015/01/23 19:25:15, jbudorick wrote:
> On 2015/01/23 10:44:49, Paweł Hajdan Jr. wrote:
> > Why does it need to? I'm not sure if keeping the two in sync is viable.
> 
> I guess it doesn't have to (and indeed, there are some we can't support), so I
> removed the comment.

Thanks! Are all of the options needed? They can always be added later, but once
they're checked in removal is harder.

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
File net/test/android/javatests/src/org/chromium/net/test/TestServerSpawner.java
(right):

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/TestServerSpawner.java:82:
entity = new StringEntity("ready");
On 2015/01/23 19:25:15, jbudorick wrote:
> On 2015/01/23 10:44:50, Paweł Hajdan Jr. wrote:
> > Is the purpose of this to wait for the server to be ready? This'd suggest a
> > polling loop on the client side, which is not optimal. Please consider a way
> (if
> > possible) to use waiting for an event instead. It'd be much more reliable.
> > 
> > This comment is based on *actual* problems with the python http server. Just
> > trying to avoid repeating some known problems in this new code.
> 
> (leaving the code as-is given pending discussion)
> 
> This is what the SpawnerCommunicator is expecting. I had been planning to
leave
> the client side (i.e. SpawnerCommunicator and RemoteTestServer) alone while
> reimplementing the server side in java, as the existing server implementation
> has to keep working in the meantime. In that scenario, I'd certainly be open
to
> changing the client side after the existing server implementation is no longer
> used in Android tests.
> 
> wdyt?

Sounds good, how about comments and TODOs?

[Ted C, 2015-01-27 18:26:22]

lgtm

https://codereview.chromium.org/867073002/diff/120001/net/test/android/javate...
File net/test/android/javatests/src/org/chromium/net/test/TestServerBuilder.java
(right):

https://codereview.chromium.org/867073002/diff/120001/net/test/android/javate...
net/test/android/javatests/src/org/chromium/net/test/TestServerBuilder.java:223:
if ("basic-auth-proxy".equals(key)) {
sorry for the churn...didn't know string switch was valid...learned something
new today.

[jbudorick, 2015-01-28 00:01:11]

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
File net/test/android/javatests/src/org/chromium/net/test/BaseTcpTestServer.java
(right):

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/BaseTcpTestServer.java:23:
mServerSocket.setSoTimeout(5000);
On 2015/01/27 15:10:50, Paweł Hajdan Jr. wrote:
> On 2015/01/23 19:25:14, jbudorick wrote:
> > On 2015/01/23 10:44:49, Paweł Hajdan Jr. wrote:
> > > Can we avoid a hardcoded timeout here? Please consider having a solution
> > similar
> > > to TestTimeouts on the C++ side: centralized, and controlled with
> command-line
> > > flags.
> > 
> > I'm open to not hardcoding the timeout here, but this just times out the
call
> to
> > ServerSocket.accept() so we don't wait there forever. It doesn't cause the
> tests
> > to time out.
> 
> Yes, let's avoid the hardcoded timeout.

Done.

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
File net/test/android/javatests/src/org/chromium/net/test/TestServerBuilder.java
(right):

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/TestServerBuilder.java:114:
// Needs to support all options provided by testserver_base.py and testserver.py
On 2015/01/27 15:10:50, Paweł Hajdan Jr. wrote:
> On 2015/01/23 19:25:15, jbudorick wrote:
> > On 2015/01/23 10:44:49, Paweł Hajdan Jr. wrote:
> > > Why does it need to? I'm not sure if keeping the two in sync is viable.
> > 
> > I guess it doesn't have to (and indeed, there are some we can't support), so
I
> > removed the comment.
> 
> Thanks! Are all of the options needed? They can always be added later, but
once
> they're checked in removal is harder.

I ran the four test suites for which we currently use the test server
(unit_tests, net_unittests, content_unittests, and content_browsertests) and
removed all options that were not used. I also removed all corresponding members
that appear to be noops in their default state.

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
File net/test/android/javatests/src/org/chromium/net/test/TestServerSpawner.java
(right):

https://codereview.chromium.org/867073002/diff/60001/net/test/android/javates...
net/test/android/javatests/src/org/chromium/net/test/TestServerSpawner.java:82:
entity = new StringEntity("ready");
On 2015/01/27 15:10:50, Paweł Hajdan Jr. wrote:
> On 2015/01/23 19:25:15, jbudorick wrote:
> > On 2015/01/23 10:44:50, Paweł Hajdan Jr. wrote:
> > > Is the purpose of this to wait for the server to be ready? This'd suggest
a
> > > polling loop on the client side, which is not optimal. Please consider a
way
> > (if
> > > possible) to use waiting for an event instead. It'd be much more reliable.
> > > 
> > > This comment is based on *actual* problems with the python http server.
Just
> > > trying to avoid repeating some known problems in this new code.
> > 
> > (leaving the code as-is given pending discussion)
> > 
> > This is what the SpawnerCommunicator is expecting. I had been planning to
> leave
> > the client side (i.e. SpawnerCommunicator and RemoteTestServer) alone while
> > reimplementing the server side in java, as the existing server
implementation
> > has to keep working in the meantime. In that scenario, I'd certainly be open
> to
> > changing the client side after the existing server implementation is no
longer
> > used in Android tests.
> > 
> > wdyt?
> 
> Sounds good, how about comments and TODOs?

Done.

https://codereview.chromium.org/867073002/diff/120001/net/test/android/javate...
File net/test/android/javatests/src/org/chromium/net/test/TestServerBuilder.java
(right):

https://codereview.chromium.org/867073002/diff/120001/net/test/android/javate...
net/test/android/javatests/src/org/chromium/net/test/TestServerBuilder.java:223:
if ("basic-auth-proxy".equals(key)) {
On 2015/01/27 18:26:22, Ted C wrote:
> sorry for the churn...didn't know string switch was valid...learned something
> new today.

np. switched back

[Paweł Hajdan Jr., 2015-01-28 13:18:56]

LGTM, thanks!

[jbudorick, 2015-01-28 14:04:56]

The CQ bit was checked by jbudorick@chromium.org

[commit-bot: I haz the power, 2015-01-28 14:05:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/867073002/140001

[commit-bot: I haz the power, 2015-01-28 14:59:43]

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

[commit-bot: I haz the power, 2015-01-28 14:59:44]

Try jobs failed on following builders:
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[jbudorick, 2015-01-28 15:42:54]

The CQ bit was checked by jbudorick@chromium.org

[commit-bot: I haz the power, 2015-01-28 15:43:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/867073002/160001

[commit-bot: I haz the power, 2015-01-28 17:10:58]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2015-01-28 17:12:21]

Patchset 9 (id:??) landed as
https://crrev.com/e1799e662af4e87b1e6d501e3511628f8579174d
Cr-Commit-Position: refs/heads/master@{#313532}

[ppi, 2015-02-26 15:50:03]

https://codereview.chromium.org/867073002/diff/160001/build/config/android/ru...
File build/config/android/rules.gni (right):

https://codereview.chromium.org/867073002/diff/160001/build/config/android/ru...
build/config/android/rules.gni:1493: "//net/android:net_java_test_support",
Please don't. /build should not depend on /net. Test apks that need that
dependency can add it themselves.

[jbudorick, 2015-02-26 16:20:02]

On 2015/02/26 15:50:03, ppi wrote:
>
https://codereview.chromium.org/867073002/diff/160001/build/config/android/ru...
> File build/config/android/rules.gni (right):
> 
>
https://codereview.chromium.org/867073002/diff/160001/build/config/android/ru...
> build/config/android/rules.gni:1493: "//net/android:net_java_test_support",
> Please don't. /build should not depend on /net. Test apks that need that
> dependency can add it themselves.

https://code.google.com/p/chromium/issues/detail?id=462239