Start server thread in new TestWebServer.start() method rather than constructor

Start server thread in new TestWebServer.start() method rather than constructor

BUG=413790
Committed: https://crrev.com/c28e68dc1302e50b713fc8d96faa574d67ddc79a
Cr-Commit-Position: refs/heads/master@{#297753}

[paulmiller, 2014-09-12 18:47:17]

paulmiller@chromium.org changed reviewers:
+ benm@chromium.org, boliu@chromium.org, digit@chromium.org, hush@chromium.org

[paulmiller, 2014-09-12 18:49:03]

Please review. Added Bo for android_webview/, Ben for content/, and David for
net/.

[boliu, 2014-09-12 21:19:05]

On 2014/09/12 18:49:03, paulmiller wrote:
> Please review. Added Bo for android_webview/, Ben for content/, and David for
> net/.

Personally I think this is not worth the churn. I suggested in the bug to just
make TestWebServer final instead. But I guess that's really up to the net owner.
So I'll approve if net owner is happy.

[hush (inactive), 2014-09-13 00:36:25]

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

https://codereview.chromium.org/568873002/diff/1/net/test/android/javatests/s...
net/test/android/javatests/src/org/chromium/net/test/util/TestWebServer.java:124:
public void start() {
can you change the constructor to be private and move what's in the constructor
right now into start(), and let start() take a boolean param? And make start
static and return a TestWebServer instance.
In this way, the user of this API can just replace 
new TestWebServer(true)
with 
TestWebServer.start(true)

This matches what newt@ did in https://codereview.chromium.org/522943003/ and
also eliminates the possibly of the user calling TestWebServer constructor but
not calling start().

[paulmiller, 2014-09-16 01:10:02]

updated

[paulmiller, 2014-09-18 22:15:44]

ping. I think I addressed Hui's comment in TestWebServer.java. still hoping to
hear from David for TransitionTest.java.

[hush (inactive), 2014-09-18 22:18:10]

hush@chromium.org changed reviewers:
+ yfriedman@chromium.org

[hush (inactive), 2014-09-18 22:27:13]

On 2014/09/18 22:15:44, paulmiller wrote:
> ping. I think I addressed Hui's comment in TestWebServer.java. still hoping to
> hear from David for TransitionTest.java.

Yaron: for net/test/ when you are back from vacation

[Yaron, 2014-09-24 22:01:23]

On 2014/09/18 22:27:13, hush wrote:
> On 2014/09/18 22:15:44, paulmiller wrote:
> > ping. I think I addressed Hui's comment in TestWebServer.java. still hoping
to
> > hear from David for TransitionTest.java.
> 
> Yaron: for net/test/ when you are back from vacation

net/ lgtm. Note this is also referenced in the internal repo. Please upload a CL
and get it approved prior to landing this.

[boliu, 2014-09-24 22:02:53]

aw lgtm

Internal repo == clank?

[boliu, 2014-09-24 22:04:05]

On 2014/09/24 22:02:53, boliu wrote:
> aw lgtm
> 
> Internal repo == clank?

Ok, yeah. Welcome to clank Paul :) Even Hui hasn't landed any changes in clank I
think.

[benm (inactive), 2014-10-01 18:14:48]

lgtm

[paulmiller, 2014-10-02 00:13:51]

The CQ bit was checked by paulmiller@chromium.org

[commit-bot: I haz the power, 2014-10-02 00:14:32]

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

[commit-bot: I haz the power, 2014-10-02 00:59:41]

Committed patchset #4 (id:60001) as 85f349f71dca23b71fe5ead7e2c3dc0db09ac7d8

[commit-bot: I haz the power, 2014-10-02 01:00:27]

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