[Cronet] Get Cronet performance test running again

[Cronet] Get Cronet performance test running again

1. Enable HTTPS for QUIC
  a. Insert a MockCertVerifier to pretend our cert is in root store
  b. Insert HostResolver rules to pretend test.example.com resolves
     to the the IP address of the USB reverse-tether interface
  c. Switch from using libcronet.so to libcronet_test.so as it
     contains several utilities we need (e.g. MockCertVerifier).
     This required making a Cronet testing Java target.
  d. Pass private key and cert to quic_server
  e. Copy cert to device
2. Call PathUtils.setPrivateDataDirectorySuffix() to avoid crash
3. Modify proguard options to avoid crbug.com/488192
4. Make Telemetry happy again
  a. Put basic header in config file
  b. Specify default_chrome_root
5. Avoid Theme.NoDisplay as it fails with Android Marshmallow

Committed: https://crrev.com/3721400b7fd14c4d9579c7d99eb936c59622d707
Cr-Commit-Position: refs/heads/master@{#372684}

[pauljensen, 2015-12-16 18:43:43]

Description was changed from

==========
[Cronet] Get Cronet performance test running again
==========

to

==========
[Cronet] Get Cronet performance test running again

1. Enable HTTPS for QUIC
  a. Insert a MockCertVerifier to pretend our cert is in root store
  b. Insert HostResolver rules to pretend test.example.com resolves
     to the the IP address of the USB reverse-tether interface
  c. Switch from using libcronet.so to libcronet_test.so as it
     contains several utilities we need (e.g. MockCertVerifier).
     This required breaking libcronet_test.so into two pieces, so
     it wasn't dependent on Java tests (which cronet_perf_test_apk
     doesn't contain).
  d. Pass private key and cert to quic_server
  e. Copy cert to device
2. Call PathUtils.setPrivateDataDirectorySuffix() to avoid crash
3. Modify proguard options to avoid crbug.com/488192
4. Make Telemetry happy again
  a. Put basic header in config file
  b. Specify default_chrome_root
5. Avoid Theme.NoDisplay as it fails with Android Marshmallow
==========

[pauljensen, 2015-12-17 18:38:25]

pauljensen@chromium.org changed reviewers:
+ mef@chromium.org

[pauljensen, 2015-12-17 18:38:25]

Misha, would you like to review as you reviewed cronet_perf_test originally?

[xunjieli, 2015-12-17 21:42:22]

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

[xunjieli, 2015-12-17 21:42:23]

Just driving by. This is great!

https://codereview.chromium.org/1536433002/diff/20001/components/cronet.gypi
File components/cronet.gypi (right):

https://codereview.chromium.org/1536433002/diff/20001/components/cronet.gypi#...
components/cronet.gypi:396:
'cronet/android/test/cronet_url_request_context_config_test.h',
Should these three files be in components/cronet/android/test/javatests/ ?

It's a bit clear that these files are from javatests if they are placed along
side the javatests/src.

https://codereview.chromium.org/1536433002/diff/20001/components/cronet/andro...
File components/cronet/android/test/cronet_javatest_jni.cc (right):

https://codereview.chromium.org/1536433002/diff/20001/components/cronet/andro...
components/cronet/android/test/cronet_javatest_jni.cc:16:
sCronetTestsRegisteredMethods[] = {
Should this be in the anonymous namespace? (like the other two existing *_jni.cc
files)

https://codereview.chromium.org/1536433002/diff/20001/components/cronet/andro...
File components/cronet/android/test/native_test_server.cc (right):

https://codereview.chromium.org/1536433002/diff/20001/components/cronet/andro...
components/cronet/android/test/native_test_server.cc:172: const std::string&
destination) {
nit: document what |destination| is.

[xunjieli, 2015-12-17 21:42:41]

xunjieli@chromium.org changed reviewers:
- xunjieli@chromium.org

[pkotwicz, 2015-12-18 00:32:27]

Can you please mirror the GYP changes in GN?

[pauljensen, 2015-12-18 13:14:41]

On 2015/12/18 00:32:27, pkotwicz wrote:
> Can you please mirror the GYP changes in GN?

AFAIK there is no cronet.gn

[pauljensen, 2015-12-18 14:26:08]

https://codereview.chromium.org/1536433002/diff/20001/components/cronet.gypi
File components/cronet.gypi (right):

https://codereview.chromium.org/1536433002/diff/20001/components/cronet.gypi#...
components/cronet.gypi:396:
'cronet/android/test/cronet_url_request_context_config_test.h',
On 2015/12/17 21:42:23, xunjieli wrote:
> Should these three files be in components/cronet/android/test/javatests/ ?
> 
> It's a bit clear that these files are from javatests if they are placed along
> side the javatests/src.

Done.

https://codereview.chromium.org/1536433002/diff/20001/components/cronet/andro...
File components/cronet/android/test/cronet_javatest_jni.cc (right):

https://codereview.chromium.org/1536433002/diff/20001/components/cronet/andro...
components/cronet/android/test/cronet_javatest_jni.cc:16:
sCronetTestsRegisteredMethods[] = {
On 2015/12/17 21:42:23, xunjieli wrote:
> Should this be in the anonymous namespace? (like the other two existing
*_jni.cc
> files)

I don't think so.  Coding style guide says "Place a function's variables in the
narrowest scope possible" thus I think it's preferable to be function scoped
rather than file scoped.

https://codereview.chromium.org/1536433002/diff/20001/components/cronet/andro...
File components/cronet/android/test/native_test_server.cc (right):

https://codereview.chromium.org/1536433002/diff/20001/components/cronet/andro...
components/cronet/android/test/native_test_server.cc:172: const std::string&
destination) {
On 2015/12/17 21:42:23, xunjieli wrote:
> nit: document what |destination| is.

Done.

[xunjieli, 2015-12-18 19:15:36]

On 2015/12/18 13:14:41, pauljensen wrote:
> On 2015/12/18 00:32:27, pkotwicz wrote:
> > Can you please mirror the GYP changes in GN?
> 
> AFAIK there is no cronet.gn

Looks like we do!! (https://codereview.chromium.org/1483843002) Thanks,
pkotwitcz. This is great!

[pauljensen, 2015-12-18 19:28:52]

On 2015/12/18 19:15:36, xunjieli (OOO1223-0118) wrote:
> On 2015/12/18 13:14:41, pauljensen wrote:
> > On 2015/12/18 00:32:27, pkotwicz wrote:
> > > Can you please mirror the GYP changes in GN?
> > 
> > AFAIK there is no cronet.gn
> 
> Looks like we do!! (https://codereview.chromium.org/1483843002) Thanks,
> pkotwitcz. This is great!

I don't see cronet_perf_test_apk in GN. Looks like it still remains to be
converted to GN (it's in the gyp_remaining target of gn_migration.gypi).

[mef, 2015-12-29 17:27:45]

Sorry for delayed answer. I think this looks pretty good, but I think/hope that
we can simplify a couple of things.

https://codereview.chromium.org/1536433002/diff/40001/components/cronet.gypi
File components/cronet.gypi (left):

https://codereview.chromium.org/1536433002/diff/40001/components/cronet.gypi#...
components/cronet.gypi:327:
'cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.java',
I'm not sure I completely understand the need for this split.

https://codereview.chromium.org/1536433002/diff/40001/components/cronet/andro...
File components/cronet/android/proguard.cfg (right):

https://codereview.chromium.org/1536433002/diff/40001/components/cronet/andro...
components/cronet/android/proguard.cfg:43: -dontwarn org.apache.http.**
can this be done in separate proguard.cfg? We include this proguard.cfg in
cronet_package, so reference to org.apache code seems inappropriate.

https://codereview.chromium.org/1536433002/diff/40001/components/cronet/andro...
File
components/cronet/android/test/javaperftests/src/org/chromium/net/CronetPerfTestActivity.java
(right):

https://codereview.chromium.org/1536433002/diff/40001/components/cronet/andro...
components/cronet/android/test/javaperftests/src/org/chromium/net/CronetPerfTestActivity.java:202:
NativeTestServer.registerHostResolverProc(
registerHostResolverProc doesn't really have anything to do with
NativeTestServer apart from historical location.
I wonder whether it belongs to CronetTestUtil or somesuch?

https://codereview.chromium.org/1536433002/diff/40001/components/cronet/andro...
File
components/cronet/android/test/javatests/cronet_url_request_context_config_test.cc
(right):

https://codereview.chromium.org/1536433002/diff/40001/components/cronet/andro...
components/cronet/android/test/javatests/cronet_url_request_context_config_test.cc:21:
static void VerifyUrlRequestContextConfig(
I think that if this method did take expected data as parameters and didn't have
to hardcode them, then it wouldn't have to move into separate native library.

https://codereview.chromium.org/1536433002/diff/40001/components/cronet/andro...
File components/cronet/android/test/native_test_server.cc (right):

https://codereview.chromium.org/1536433002/diff/40001/components/cronet/andro...
components/cronet/android/test/native_test_server.cc:231:
URLRequestContextAdapter* context_adapter =
Does it make sense to call context_adapter->GetURLRequestContext() here and post
RegisterHostResolverProcHelper() call in both legacy and non-legacy case?

[pauljensen, 2016-01-21 03:51:09]

Description was changed from

==========
[Cronet] Get Cronet performance test running again

1. Enable HTTPS for QUIC
  a. Insert a MockCertVerifier to pretend our cert is in root store
  b. Insert HostResolver rules to pretend test.example.com resolves
     to the the IP address of the USB reverse-tether interface
  c. Switch from using libcronet.so to libcronet_test.so as it
     contains several utilities we need (e.g. MockCertVerifier).
     This required breaking libcronet_test.so into two pieces, so
     it wasn't dependent on Java tests (which cronet_perf_test_apk
     doesn't contain).
  d. Pass private key and cert to quic_server
  e. Copy cert to device
2. Call PathUtils.setPrivateDataDirectorySuffix() to avoid crash
3. Modify proguard options to avoid crbug.com/488192
4. Make Telemetry happy again
  a. Put basic header in config file
  b. Specify default_chrome_root
5. Avoid Theme.NoDisplay as it fails with Android Marshmallow
==========

to

==========
[Cronet] Get Cronet performance test running again

1. Enable HTTPS for QUIC
  a. Insert a MockCertVerifier to pretend our cert is in root store
  b. Insert HostResolver rules to pretend test.example.com resolves
     to the the IP address of the USB reverse-tether interface
  c. Switch from using libcronet.so to libcronet_test.so as it
     contains several utilities we need (e.g. MockCertVerifier).
     This required making a Cronet testing Java target.
  d. Pass private key and cert to quic_server
  e. Copy cert to device
2. Call PathUtils.setPrivateDataDirectorySuffix() to avoid crash
3. Modify proguard options to avoid crbug.com/488192
4. Make Telemetry happy again
  a. Put basic header in config file
  b. Specify default_chrome_root
5. Avoid Theme.NoDisplay as it fails with Android Marshmallow
==========

[pauljensen, 2016-01-21 03:51:50]

PTAL Misha, thank you!

https://codereview.chromium.org/1536433002/diff/40001/components/cronet.gypi
File components/cronet.gypi (left):

https://codereview.chromium.org/1536433002/diff/40001/components/cronet.gypi#...
components/cronet.gypi:327:
'cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.java',
On 2015/12/29 17:27:44, mef wrote:
> I'm not sure I completely understand the need for this split.

I reworked it so as to avoid the split by simply making a separate Java test
target.

https://codereview.chromium.org/1536433002/diff/40001/components/cronet/andro...
File components/cronet/android/proguard.cfg (right):

https://codereview.chromium.org/1536433002/diff/40001/components/cronet/andro...
components/cronet/android/proguard.cfg:43: -dontwarn org.apache.http.**
On 2015/12/29 17:27:44, mef wrote:
> can this be done in separate proguard.cfg? We include this proguard.cfg in
> cronet_package, so reference to org.apache code seems inappropriate.

Done.

https://codereview.chromium.org/1536433002/diff/40001/components/cronet/andro...
File
components/cronet/android/test/javaperftests/src/org/chromium/net/CronetPerfTestActivity.java
(right):

https://codereview.chromium.org/1536433002/diff/40001/components/cronet/andro...
components/cronet/android/test/javaperftests/src/org/chromium/net/CronetPerfTestActivity.java:202:
NativeTestServer.registerHostResolverProc(
On 2015/12/29 17:27:45, mef wrote:
> registerHostResolverProc doesn't really have anything to do with
> NativeTestServer apart from historical location.
> I wonder whether it belongs to CronetTestUtil or somesuch?

Done.

https://codereview.chromium.org/1536433002/diff/40001/components/cronet/andro...
File
components/cronet/android/test/javatests/cronet_url_request_context_config_test.cc
(right):

https://codereview.chromium.org/1536433002/diff/40001/components/cronet/andro...
components/cronet/android/test/javatests/cronet_url_request_context_config_test.cc:21:
static void VerifyUrlRequestContextConfig(
On 2015/12/29 17:27:45, mef wrote:
> I think that if this method did take expected data as parameters and didn't
have
> to hardcode them, then it wouldn't have to move into separate native library.

I reverted the moving it to a separate native library.

https://codereview.chromium.org/1536433002/diff/40001/components/cronet/andro...
File components/cronet/android/test/native_test_server.cc (right):

https://codereview.chromium.org/1536433002/diff/40001/components/cronet/andro...
components/cronet/android/test/native_test_server.cc:231:
URLRequestContextAdapter* context_adapter =
On 2015/12/29 17:27:45, mef wrote:
> Does it make sense to call context_adapter->GetURLRequestContext() here and
post
> RegisterHostResolverProcHelper() call in both legacy and non-legacy case?

GetURLRequestContext() can only be called from network thread, so we can't do it
here.

[mef, 2016-01-22 16:21:58]

Looks pretty good!

https://codereview.chromium.org/1536433002/diff/120001/components/cronet/andr...
File
components/cronet/android/test/javaperftests/src/org/chromium/net/CronetPerfTestActivity.java
(right):

https://codereview.chromium.org/1536433002/diff/120001/components/cronet/andr...
components/cronet/android/test/javaperftests/src/org/chromium/net/CronetPerfTestActivity.java:201:
JSONObject quicParams = new JSONObject().put("host_whitelist", host);
nice touch!

https://codereview.chromium.org/1536433002/diff/120001/components/cronet/andr...
components/cronet/android/test/javaperftests/src/org/chromium/net/CronetPerfTestActivity.java:210:
CronetTestUtil.registerHostResolverProc(
Can we change CronetTestUtil.registerHostResolverProc to take CronetEngine and
do magic getUrlRequestContextAdapter() dance internally?

https://codereview.chromium.org/1536433002/diff/120001/components/cronet/andr...
File components/cronet/android/test/native_test_server.h (right):

https://codereview.chromium.org/1536433002/diff/120001/components/cronet/andr...
components/cronet/android/test/native_test_server.h:12: const char
kFakeSdchDomain[] = "fake.sdch.domain";
not sure whether these belong here or in cronet_test_util.h.

https://codereview.chromium.org/1536433002/diff/120001/components/cronet/andr...
File components/cronet/android/test/src/org/chromium/net/CronetTestUtil.java
(right):

https://codereview.chromium.org/1536433002/diff/120001/components/cronet/andr...
components/cronet/android/test/src/org/chromium/net/CronetTestUtil.java:20: *
Registers customized DNS mapping for {@link NativeTestServer}.
It is not only for NativeTestServer.

https://codereview.chromium.org/1536433002/diff/120001/components/cronet/andr...
components/cronet/android/test/src/org/chromium/net/CronetTestUtil.java:27:
public static void registerHostResolverProc(
could it be better to have 2 separate registerHostResolverProc methods, one of
which takes CronetEngine, and another takes legacy object?

[pauljensen, 2016-01-25 02:47:48]

PTAL, thank you!

https://codereview.chromium.org/1536433002/diff/120001/components/cronet/andr...
File
components/cronet/android/test/javaperftests/src/org/chromium/net/CronetPerfTestActivity.java
(right):

https://codereview.chromium.org/1536433002/diff/120001/components/cronet/andr...
components/cronet/android/test/javaperftests/src/org/chromium/net/CronetPerfTestActivity.java:201:
JSONObject quicParams = new JSONObject().put("host_whitelist", host);
On 2016/01/22 16:21:57, mef wrote:
> nice touch!

Acknowledged.

https://codereview.chromium.org/1536433002/diff/120001/components/cronet/andr...
components/cronet/android/test/javaperftests/src/org/chromium/net/CronetPerfTestActivity.java:210:
CronetTestUtil.registerHostResolverProc(
On 2016/01/22 16:21:57, mef wrote:
> Can we change CronetTestUtil.registerHostResolverProc to take CronetEngine and
> do magic getUrlRequestContextAdapter() dance internally?

Done.

https://codereview.chromium.org/1536433002/diff/120001/components/cronet/andr...
File components/cronet/android/test/native_test_server.h (right):

https://codereview.chromium.org/1536433002/diff/120001/components/cronet/andr...
components/cronet/android/test/native_test_server.h:12: const char
kFakeSdchDomain[] = "fake.sdch.domain";
On 2016/01/22 16:21:57, mef wrote:
> not sure whether these belong here or in cronet_test_util.h.

I think cronet_test_util.h is better.  Moved.

https://codereview.chromium.org/1536433002/diff/120001/components/cronet/andr...
File components/cronet/android/test/src/org/chromium/net/CronetTestUtil.java
(right):

https://codereview.chromium.org/1536433002/diff/120001/components/cronet/andr...
components/cronet/android/test/src/org/chromium/net/CronetTestUtil.java:20: *
Registers customized DNS mapping for {@link NativeTestServer}.
On 2016/01/22 16:21:57, mef wrote:
> It is not only for NativeTestServer.

Done.

https://codereview.chromium.org/1536433002/diff/120001/components/cronet/andr...
components/cronet/android/test/src/org/chromium/net/CronetTestUtil.java:27:
public static void registerHostResolverProc(
On 2016/01/22 16:21:57, mef wrote:
> could it be better to have 2 separate registerHostResolverProc methods, one of
> which takes CronetEngine, and another takes legacy object?

Done.

[mef, 2016-01-26 21:45:04]

https://codereview.chromium.org/1536433002/diff/140001/components/cronet/andr...
File components/cronet/android/test/cronet_test_util.h (right):

https://codereview.chromium.org/1536433002/diff/140001/components/cronet/andr...
components/cronet/android/test/cronet_test_util.h:12: const char
kFakeSdchDomain[] = "fake.sdch.domain";
nit: I think normally header file declares "extern const char" and source file
contains actual definiton.

Not very important for thest code, but still.

https://codereview.chromium.org/1536433002/diff/140001/components/cronet/andr...
File
components/cronet/android/test/javatests/src/org/chromium/net/CronetTestBase.java
(right):

https://codereview.chromium.org/1536433002/diff/140001/components/cronet/andr...
components/cronet/android/test/javatests/src/org/chromium/net/CronetTestBase.java:167:
CronetTestUtil.registerHostResolverProc(framework.mRequestFactory, "127.0.0.1");
nit: maybe make a const?

https://codereview.chromium.org/1536433002/diff/140001/components/cronet/andr...
File components/cronet/android/test/src/org/chromium/net/CronetTestUtil.java
(right):

https://codereview.chromium.org/1536433002/diff/140001/components/cronet/andr...
components/cronet/android/test/src/org/chromium/net/CronetTestUtil.java:20: *
Registers customized DNS mapping for testing host names used by test servers.
I think we should specify fake host names as static string constants in this
class, otherwise it is unclear what host names are mapped when you
registerHostResolverProc.

https://codereview.chromium.org/1536433002/diff/140001/components/cronet/andr...
components/cronet/android/test/src/org/chromium/net/CronetTestUtil.java:22: *
@param isLegacyAPI {@code true} if this context adapter is a part of the
obsolete @param.

https://codereview.chromium.org/1536433002/diff/140001/components/cronet/andr...
components/cronet/android/test/src/org/chromium/net/CronetTestUtil.java:37: *
@param isLegacyAPI {@code true} if this context adapter is a part of the
obsolete @param.

[pauljensen, 2016-01-29 17:10:09]

PTAL.  I added GN support too :)

https://codereview.chromium.org/1536433002/diff/140001/components/cronet/andr...
File components/cronet/android/test/cronet_test_util.h (right):

https://codereview.chromium.org/1536433002/diff/140001/components/cronet/andr...
components/cronet/android/test/cronet_test_util.h:12: const char
kFakeSdchDomain[] = "fake.sdch.domain";
On 2016/01/26 21:45:03, mef wrote:
> nit: I think normally header file declares "extern const char" and source file
> contains actual definiton.
> 
> Not very important for thest code, but still.

Done.

https://codereview.chromium.org/1536433002/diff/140001/components/cronet/andr...
File
components/cronet/android/test/javatests/src/org/chromium/net/CronetTestBase.java
(right):

https://codereview.chromium.org/1536433002/diff/140001/components/cronet/andr...
components/cronet/android/test/javatests/src/org/chromium/net/CronetTestBase.java:167:
CronetTestUtil.registerHostResolverProc(framework.mRequestFactory, "127.0.0.1");
On 2016/01/26 21:45:03, mef wrote:
> nit: maybe make a const?

Done.

https://codereview.chromium.org/1536433002/diff/140001/components/cronet/andr...
File components/cronet/android/test/src/org/chromium/net/CronetTestUtil.java
(right):

https://codereview.chromium.org/1536433002/diff/140001/components/cronet/andr...
components/cronet/android/test/src/org/chromium/net/CronetTestUtil.java:20: *
Registers customized DNS mapping for testing host names used by test servers.
On 2016/01/26 21:45:03, mef wrote:
> I think we should specify fake host names as static string constants in this
> class, otherwise it is unclear what host names are mapped when you
> registerHostResolverProc.

They can be accessed via other APIs; I've listed them here.  I also combined
quic_test_server.cc#kServerHost to use the new
cronet_test_util.h#kFakeQuicDomain

https://codereview.chromium.org/1536433002/diff/140001/components/cronet/andr...
components/cronet/android/test/src/org/chromium/net/CronetTestUtil.java:22: *
@param isLegacyAPI {@code true} if this context adapter is a part of the
On 2016/01/26 21:45:03, mef wrote:
> obsolete @param.

Done.

https://codereview.chromium.org/1536433002/diff/140001/components/cronet/andr...
components/cronet/android/test/src/org/chromium/net/CronetTestUtil.java:37: *
@param isLegacyAPI {@code true} if this context adapter is a part of the
On 2016/01/26 21:45:03, mef wrote:
> obsolete @param.

Done.

[mef, 2016-01-29 17:21:15]

looks good, could you re-sync to origin/master, before I do the last pass?

https://codereview.chromium.org/1536433002/diff/180001/components/cronet/andr...
File components/cronet/android/test/cronet_test_util.cc (right):

https://codereview.chromium.org/1536433002/diff/180001/components/cronet/andr...
components/cronet/android/test/cronet_test_util.cc:21: // the file served (
nit: weird line wrapping. cl format?

[pauljensen, 2016-01-29 19:17:11]

I've synced to the ToT and all seems well.  Builds, runs.

https://codereview.chromium.org/1536433002/diff/180001/components/cronet/andr...
File components/cronet/android/test/cronet_test_util.cc (right):

https://codereview.chromium.org/1536433002/diff/180001/components/cronet/andr...
components/cronet/android/test/cronet_test_util.cc:21: // the file served (
On 2016/01/29 17:21:14, mef wrote:
> nit: weird line wrapping. cl format?

I dunno, I just moved this line; I didn't edit it.  It seems like the best spot
to wrap the line.

[mef, 2016-01-29 22:53:05]

lgtm

[pauljensen, 2016-01-30 02:21:49]

The CQ bit was checked by pauljensen@chromium.org

[commit-bot: I haz the power, 2016-01-30 02:22:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1536433002/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1536433002/200001

[commit-bot: I haz the power, 2016-01-30 02:25:34]

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

[commit-bot: I haz the power, 2016-01-30 02:25:35]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[pauljensen, 2016-01-30 03:37:16]

The CQ bit was checked by pauljensen@chromium.org

[pauljensen, 2016-01-30 03:37:16]

The patchset sent to the CQ was uploaded after l-g-t-m from mef@chromium.org
Link to the patchset: https://codereview.chromium.org/1536433002/#ps240001
(title: "fix sync conflict")

[commit-bot: I haz the power, 2016-01-30 03:37:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1536433002/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1536433002/240001

[commit-bot: I haz the power, 2016-01-30 04:41:59]

Description was changed from

==========
[Cronet] Get Cronet performance test running again

1. Enable HTTPS for QUIC
  a. Insert a MockCertVerifier to pretend our cert is in root store
  b. Insert HostResolver rules to pretend test.example.com resolves
     to the the IP address of the USB reverse-tether interface
  c. Switch from using libcronet.so to libcronet_test.so as it
     contains several utilities we need (e.g. MockCertVerifier).
     This required making a Cronet testing Java target.
  d. Pass private key and cert to quic_server
  e. Copy cert to device
2. Call PathUtils.setPrivateDataDirectorySuffix() to avoid crash
3. Modify proguard options to avoid crbug.com/488192
4. Make Telemetry happy again
  a. Put basic header in config file
  b. Specify default_chrome_root
5. Avoid Theme.NoDisplay as it fails with Android Marshmallow
==========

to

==========
[Cronet] Get Cronet performance test running again

1. Enable HTTPS for QUIC
  a. Insert a MockCertVerifier to pretend our cert is in root store
  b. Insert HostResolver rules to pretend test.example.com resolves
     to the the IP address of the USB reverse-tether interface
  c. Switch from using libcronet.so to libcronet_test.so as it
     contains several utilities we need (e.g. MockCertVerifier).
     This required making a Cronet testing Java target.
  d. Pass private key and cert to quic_server
  e. Copy cert to device
2. Call PathUtils.setPrivateDataDirectorySuffix() to avoid crash
3. Modify proguard options to avoid crbug.com/488192
4. Make Telemetry happy again
  a. Put basic header in config file
  b. Specify default_chrome_root
5. Avoid Theme.NoDisplay as it fails with Android Marshmallow
==========

[commit-bot: I haz the power, 2016-01-30 04:42:00]

Committed patchset #13 (id:240001)

[commit-bot: I haz the power, 2016-01-30 04:42:53]

Description was changed from

==========
[Cronet] Get Cronet performance test running again

1. Enable HTTPS for QUIC
  a. Insert a MockCertVerifier to pretend our cert is in root store
  b. Insert HostResolver rules to pretend test.example.com resolves
     to the the IP address of the USB reverse-tether interface
  c. Switch from using libcronet.so to libcronet_test.so as it
     contains several utilities we need (e.g. MockCertVerifier).
     This required making a Cronet testing Java target.
  d. Pass private key and cert to quic_server
  e. Copy cert to device
2. Call PathUtils.setPrivateDataDirectorySuffix() to avoid crash
3. Modify proguard options to avoid crbug.com/488192
4. Make Telemetry happy again
  a. Put basic header in config file
  b. Specify default_chrome_root
5. Avoid Theme.NoDisplay as it fails with Android Marshmallow
==========

to

==========
[Cronet] Get Cronet performance test running again

1. Enable HTTPS for QUIC
  a. Insert a MockCertVerifier to pretend our cert is in root store
  b. Insert HostResolver rules to pretend test.example.com resolves
     to the the IP address of the USB reverse-tether interface
  c. Switch from using libcronet.so to libcronet_test.so as it
     contains several utilities we need (e.g. MockCertVerifier).
     This required making a Cronet testing Java target.
  d. Pass private key and cert to quic_server
  e. Copy cert to device
2. Call PathUtils.setPrivateDataDirectorySuffix() to avoid crash
3. Modify proguard options to avoid crbug.com/488192
4. Make Telemetry happy again
  a. Put basic header in config file
  b. Specify default_chrome_root
5. Avoid Theme.NoDisplay as it fails with Android Marshmallow

Committed: https://crrev.com/58f1ff598b4f96f9645574564f296f0e428f37ff
Cr-Commit-Position: refs/heads/master@{#372532}
==========

[commit-bot: I haz the power, 2016-01-30 04:42:53]

Patchset 13 (id:??) landed as
https://crrev.com/58f1ff598b4f96f9645574564f296f0e428f37ff
Cr-Commit-Position: refs/heads/master@{#372532}

[jbroman, 2016-01-30 05:31:38]

A revert of this CL (patchset #13 id:240001) has been created in
https://codereview.chromium.org/1650773003/ by jbroman@chromium.org.

The reason for reverting is: Broke Android (failed in proguard obfuscation on
cronet_perf_test_apk):
https://build.chromium.org/p/chromium/builders/Android/builds/51325.

[pauljensen, 2016-01-31 02:45:09]

Description was changed from

==========
[Cronet] Get Cronet performance test running again

1. Enable HTTPS for QUIC
  a. Insert a MockCertVerifier to pretend our cert is in root store
  b. Insert HostResolver rules to pretend test.example.com resolves
     to the the IP address of the USB reverse-tether interface
  c. Switch from using libcronet.so to libcronet_test.so as it
     contains several utilities we need (e.g. MockCertVerifier).
     This required making a Cronet testing Java target.
  d. Pass private key and cert to quic_server
  e. Copy cert to device
2. Call PathUtils.setPrivateDataDirectorySuffix() to avoid crash
3. Modify proguard options to avoid crbug.com/488192
4. Make Telemetry happy again
  a. Put basic header in config file
  b. Specify default_chrome_root
5. Avoid Theme.NoDisplay as it fails with Android Marshmallow

Committed: https://crrev.com/58f1ff598b4f96f9645574564f296f0e428f37ff
Cr-Commit-Position: refs/heads/master@{#372532}
==========

to

==========
[Cronet] Get Cronet performance test running again

1. Enable HTTPS for QUIC
  a. Insert a MockCertVerifier to pretend our cert is in root store
  b. Insert HostResolver rules to pretend test.example.com resolves
     to the the IP address of the USB reverse-tether interface
  c. Switch from using libcronet.so to libcronet_test.so as it
     contains several utilities we need (e.g. MockCertVerifier).
     This required making a Cronet testing Java target.
  d. Pass private key and cert to quic_server
  e. Copy cert to device
2. Call PathUtils.setPrivateDataDirectorySuffix() to avoid crash
3. Modify proguard options to avoid crbug.com/488192
4. Make Telemetry happy again
  a. Put basic header in config file
  b. Specify default_chrome_root
5. Avoid Theme.NoDisplay as it fails with Android Marshmallow
==========

[pauljensen, 2016-01-31 02:48:34]

Misha, PTAL. I added a proguard ignore for Netty

[mef, 2016-01-31 21:12:24]

On 2016/01/31 02:48:34, pauljensen wrote:
> Misha, PTAL. I added a proguard ignore for Netty

lgtm, I'm just curious why do we need to proguard perf test app? 
For better test coverage?
The sample app uses libcronet.so instead of libcronet_tests.so, so it would
provide better approximation of real-world usage.

[pauljensen, 2016-02-01 16:05:09]

On 2016/01/31 21:12:24, mef wrote:
> On 2016/01/31 02:48:34, pauljensen wrote:
> > Misha, PTAL. I added a proguard ignore for Netty
> 
> lgtm, I'm just curious why do we need to proguard perf test app? 
> For better test coverage?
> The sample app uses libcronet.so instead of libcronet_tests.so, so it would
> provide better approximation of real-world usage.

I'd like the perf test to be as close to reality as possible so we can catch
issues/regressions a real app might encounter.  I think proguard is an important
piece of this as I could imagine it having performance effects.

[pauljensen, 2016-02-01 16:05:23]

The CQ bit was checked by pauljensen@chromium.org

[commit-bot: I haz the power, 2016-02-01 16:07:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1536433002/260001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1536433002/260001

[mef, 2016-02-01 16:14:34]

On 2016/02/01 16:05:09, pauljensen wrote:
> On 2016/01/31 21:12:24, mef wrote:
> > On 2016/01/31 02:48:34, pauljensen wrote:
> > > Misha, PTAL. I added a proguard ignore for Netty
> > 
> > lgtm, I'm just curious why do we need to proguard perf test app? 
> > For better test coverage?
> > The sample app uses libcronet.so instead of libcronet_tests.so, so it would
> > provide better approximation of real-world usage.
> 
> I'd like the perf test to be as close to reality as possible so we can catch
> issues/regressions a real app might encounter.  I think proguard is an
important
> piece of this as I could imagine it having performance effects.

sgtm, thanks!

[commit-bot: I haz the power, 2016-02-01 17:23:19]

Description was changed from

==========
[Cronet] Get Cronet performance test running again

1. Enable HTTPS for QUIC
  a. Insert a MockCertVerifier to pretend our cert is in root store
  b. Insert HostResolver rules to pretend test.example.com resolves
     to the the IP address of the USB reverse-tether interface
  c. Switch from using libcronet.so to libcronet_test.so as it
     contains several utilities we need (e.g. MockCertVerifier).
     This required making a Cronet testing Java target.
  d. Pass private key and cert to quic_server
  e. Copy cert to device
2. Call PathUtils.setPrivateDataDirectorySuffix() to avoid crash
3. Modify proguard options to avoid crbug.com/488192
4. Make Telemetry happy again
  a. Put basic header in config file
  b. Specify default_chrome_root
5. Avoid Theme.NoDisplay as it fails with Android Marshmallow
==========

to

==========
[Cronet] Get Cronet performance test running again

1. Enable HTTPS for QUIC
  a. Insert a MockCertVerifier to pretend our cert is in root store
  b. Insert HostResolver rules to pretend test.example.com resolves
     to the the IP address of the USB reverse-tether interface
  c. Switch from using libcronet.so to libcronet_test.so as it
     contains several utilities we need (e.g. MockCertVerifier).
     This required making a Cronet testing Java target.
  d. Pass private key and cert to quic_server
  e. Copy cert to device
2. Call PathUtils.setPrivateDataDirectorySuffix() to avoid crash
3. Modify proguard options to avoid crbug.com/488192
4. Make Telemetry happy again
  a. Put basic header in config file
  b. Specify default_chrome_root
5. Avoid Theme.NoDisplay as it fails with Android Marshmallow
==========

[commit-bot: I haz the power, 2016-02-01 17:23:20]

Committed patchset #14 (id:260001)

[commit-bot: I haz the power, 2016-02-01 17:24:10]

Description was changed from

==========
[Cronet] Get Cronet performance test running again

1. Enable HTTPS for QUIC
  a. Insert a MockCertVerifier to pretend our cert is in root store
  b. Insert HostResolver rules to pretend test.example.com resolves
     to the the IP address of the USB reverse-tether interface
  c. Switch from using libcronet.so to libcronet_test.so as it
     contains several utilities we need (e.g. MockCertVerifier).
     This required making a Cronet testing Java target.
  d. Pass private key and cert to quic_server
  e. Copy cert to device
2. Call PathUtils.setPrivateDataDirectorySuffix() to avoid crash
3. Modify proguard options to avoid crbug.com/488192
4. Make Telemetry happy again
  a. Put basic header in config file
  b. Specify default_chrome_root
5. Avoid Theme.NoDisplay as it fails with Android Marshmallow
==========

to

==========
[Cronet] Get Cronet performance test running again

1. Enable HTTPS for QUIC
  a. Insert a MockCertVerifier to pretend our cert is in root store
  b. Insert HostResolver rules to pretend test.example.com resolves
     to the the IP address of the USB reverse-tether interface
  c. Switch from using libcronet.so to libcronet_test.so as it
     contains several utilities we need (e.g. MockCertVerifier).
     This required making a Cronet testing Java target.
  d. Pass private key and cert to quic_server
  e. Copy cert to device
2. Call PathUtils.setPrivateDataDirectorySuffix() to avoid crash
3. Modify proguard options to avoid crbug.com/488192
4. Make Telemetry happy again
  a. Put basic header in config file
  b. Specify default_chrome_root
5. Avoid Theme.NoDisplay as it fails with Android Marshmallow

Committed: https://crrev.com/3721400b7fd14c4d9579c7d99eb936c59622d707
Cr-Commit-Position: refs/heads/master@{#372684}
==========

[commit-bot: I haz the power, 2016-02-01 17:24:11]

Patchset 14 (id:??) landed as
https://crrev.com/3721400b7fd14c4d9579c7d99eb936c59622d707
Cr-Commit-Position: refs/heads/master@{#372684}