[Cronet] Enable Brotli

[Cronet] Enable Brotli 

- removes the compile time flag to disable brotli when building Cronet.
- adds an API to enable Brotli on CronetEngine.
- adds three basic java tests in BrotliTest.java

BUG=710633
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Review-Url: https://codereview.chromium.org/2805053005
Cr-Commit-Position: refs/heads/master@{#465216}
Committed: https://chromium.googlesource.com/chromium/src/+/186d2bf1b6ff14929c79d6d4a8b310ba639dfbf9

[xunjieli, 2017-04-07 01:41:12]

Description was changed from

==========
[Cronet] temp


BUG=
==========

to

==========
[Cronet] temp


BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

[xunjieli, 2017-04-11 21:52:34]

Description was changed from

==========
[Cronet] temp


BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

to

==========
[Cronet] Enable Brotli 


BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

[xunjieli, 2017-04-12 14:28:58]

Description was changed from

==========
[Cronet] Enable Brotli 


BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

to

==========
[Cronet] Enable Brotli 


BUG=710633
==========

[xunjieli, 2017-04-12 14:29:42]

Patchset #1 (id:1) has been deleted

[xunjieli, 2017-04-12 14:29:48]

Patchset #1 (id:20001) has been deleted

[xunjieli, 2017-04-12 14:33:02]

Description was changed from

==========
[Cronet] Enable Brotli 


BUG=710633
==========

to

==========
[Cronet] Enable Brotli 

- removes the compile time flag to disable brotli when building Cronet.
- adds a API to enable Brotli on CronetEngine.
- adds three basic java tests in BrotliTest.java

BUG=710633
==========

[xunjieli, 2017-04-12 14:33:03]

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

[xunjieli, 2017-04-12 14:34:00]

Description was changed from

==========
[Cronet] Enable Brotli 

- removes the compile time flag to disable brotli when building Cronet.
- adds a API to enable Brotli on CronetEngine.
- adds three basic java tests in BrotliTest.java

BUG=710633
==========

to

==========
[Cronet] Enable Brotli 

- removes the compile time flag to disable brotli when building Cronet.
- adds an API to enable Brotli on CronetEngine.
- adds three basic java tests in BrotliTest.java

BUG=710633
==========

[xunjieli, 2017-04-12 14:34:21]

Misha: PTAL. Thank you.

[mef, 2017-04-12 17:45:49]

Looks pretty good.

https://codereview.chromium.org/2805053005/diff/40001/components/cronet/andro...
File components/cronet/android/api/src/org/chromium/net/CronetEngine.java
(right):

https://codereview.chromium.org/2805053005/diff/40001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/CronetEngine.java:181: public
Builder enableBrotli(boolean value) {
Addition of public API requires to bump API version:

components/cronet/tools/update_api.py --api_jar
components/cronet/android/cronet_api.jar

https://codereview.chromium.org/2805053005/diff/40001/components/cronet/andro...
File
components/cronet/android/api/src/org/chromium/net/ICronetEngineBuilder.java
(right):

https://codereview.chromium.org/2805053005/diff/40001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/ICronetEngineBuilder.java:30:
public abstract ICronetEngineBuilder enableBrotli(boolean value);
This method should have default 'do nothing' implementation to cover the case of
old implementation, which does not have it:

   public ICronetEngineBuilder enableBrotli(boolean value) {
      return this;
   }

https://codereview.chromium.org/2805053005/diff/40001/components/cronet/andro...
File
components/cronet/android/test/javatests/src/org/chromium/net/BrotliTest.java
(right):

https://codereview.chromium.org/2805053005/diff/40001/components/cronet/andro...
components/cronet/android/test/javatests/src/org/chromium/net/BrotliTest.java:61:
assertFalse(callback.mResponseAsString.contains("accept-encoding: gzip, deflate,
br"));
Should this response exclude 'br'?

https://codereview.chromium.org/2805053005/diff/40001/components/cronet/andro...
components/cronet/android/test/javatests/src/org/chromium/net/BrotliTest.java:76:
assertEquals(200, callback.mResponseInfo.getHttpStatusCode());
Should this also verify 'quicFox' response?

[xunjieli, 2017-04-12 21:17:56]

Description was changed from

==========
[Cronet] Enable Brotli 

- removes the compile time flag to disable brotli when building Cronet.
- adds an API to enable Brotli on CronetEngine.
- adds three basic java tests in BrotliTest.java

BUG=710633
==========

to

==========
[Cronet] Enable Brotli 

- removes the compile time flag to disable brotli when building Cronet.
- adds an API to enable Brotli on CronetEngine.
- adds three basic java tests in BrotliTest.java

BUG=710633
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

[xunjieli, 2017-04-13 21:26:15]

Thanks! PTAL.

https://codereview.chromium.org/2805053005/diff/40001/components/cronet/andro...
File components/cronet/android/api/src/org/chromium/net/CronetEngine.java
(right):

https://codereview.chromium.org/2805053005/diff/40001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/CronetEngine.java:181: public
Builder enableBrotli(boolean value) {
On 2017/04/12 17:45:49, mef wrote:
> Addition of public API requires to bump API version:
> 
> components/cronet/tools/update_api.py --api_jar
> components/cronet/android/cronet_api.jar

Done.

https://codereview.chromium.org/2805053005/diff/40001/components/cronet/andro...
File
components/cronet/android/api/src/org/chromium/net/ICronetEngineBuilder.java
(right):

https://codereview.chromium.org/2805053005/diff/40001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/ICronetEngineBuilder.java:30:
public abstract ICronetEngineBuilder enableBrotli(boolean value);
On 2017/04/12 17:45:49, mef wrote:
> This method should have default 'do nothing' implementation to cover the case
of
> old implementation, which does not have it:
> 
>    public ICronetEngineBuilder enableBrotli(boolean value) {
>       return this;
>    }

Done.

https://codereview.chromium.org/2805053005/diff/40001/components/cronet/andro...
File
components/cronet/android/test/javatests/src/org/chromium/net/BrotliTest.java
(right):

https://codereview.chromium.org/2805053005/diff/40001/components/cronet/andro...
components/cronet/android/test/javatests/src/org/chromium/net/BrotliTest.java:61:
assertFalse(callback.mResponseAsString.contains("accept-encoding: gzip, deflate,
br"));
On 2017/04/12 17:45:49, mef wrote:
> Should this response exclude 'br'?

Done. I used assertFalse(). But I agree this is a bit misleading. I have changed
it to
assertFalse(callback.mResponseAsString.contains("br"));

https://codereview.chromium.org/2805053005/diff/40001/components/cronet/andro...
components/cronet/android/test/javatests/src/org/chromium/net/BrotliTest.java:76:
assertEquals(200, callback.mResponseInfo.getHttpStatusCode());
On 2017/04/12 17:45:49, mef wrote:
> Should this also verify 'quicFox' response?

Done. Sorry, I thought I did add the line, but looks like I didn't.

[mef, 2017-04-14 13:42:06]

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

https://codereview.chromium.org/2805053005/diff/40001/components/cronet/andro...
components/cronet/android/test/src/org/chromium/net/Http2TestHandler.java:188:
0x65, 0x20, 0x6c, 0x61, 0x7a, 0x79, 0x20, 0x64, 0x6f, 0x67, 0x03};
I'm curious why 0x03 ->0x00?

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

https://codereview.chromium.org/2805053005/diff/120001/components/cronet/andr...
components/cronet/android/test/javatests/src/org/chromium/net/BrotliTest.java:77:
String expectedResponse = "The quick brown fox jumps over the lazy dog\0";
I'm a bit curious about \0 here. 
Is it intentional to verify that 0 is not treated as string terminator?

[xunjieli, 2017-04-14 14:16:58]

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

https://codereview.chromium.org/2805053005/diff/120001/components/cronet/andr...
components/cronet/android/test/javatests/src/org/chromium/net/BrotliTest.java:77:
String expectedResponse = "The quick brown fox jumps over the lazy dog\0";
On 2017/04/14 13:42:04, mef wrote:
> I'm a bit curious about \0 here. 
> Is it intentional to verify that 0 is not treated as string terminator?

It used to be a EndOfText charater (0x03). I replaced it with 0x00 because I am
not sure how to represent 0x03 in java string. 
I can get rid of the terminator all together? WDYT?

The 0x03 came from xxd -i this file:
https://github.com/google/brotli/blob/master/tests/testdata/quickfox.compressed.

[mef, 2017-04-14 17:02:07]

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

https://codereview.chromium.org/2805053005/diff/120001/components/cronet/andr...
components/cronet/android/test/javatests/src/org/chromium/net/BrotliTest.java:77:
String expectedResponse = "The quick brown fox jumps over the lazy dog\0";
On 2017/04/14 14:16:58, xunjieli wrote:
> On 2017/04/14 13:42:04, mef wrote:
> > I'm a bit curious about \0 here. 
> > Is it intentional to verify that 0 is not treated as string terminator?
> 
> It used to be a EndOfText charater (0x03). I replaced it with 0x00 because I
am
> not sure how to represent 0x03 in java string. 
> I can get rid of the terminator all together? WDYT?
> 
> The 0x03 came from xxd -i this file:
>
https://github.com/google/brotli/blob/master/tests/testdata/quickfox.compressed.
> 

Thanks for explanation! 
Looking at uncompressed
https://raw.githubusercontent.com/google/brotli/master/tests/testdata/quickfox
there shouldn't be any weird character at the end of uncompressed string, so
this seems odd.

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

https://codereview.chromium.org/2805053005/diff/140001/components/cronet/andr...
components/cronet/android/test/src/org/chromium/net/Http2TestHandler.java:184:
byte[] quickfoxCompressed = {0x0b, 0x15, -1, 0x54, 0x68, 0x65, 0x20, 0x71, 0x75,
0x69,
The third byte in curl
https://raw.githubusercontent.com/google/brotli/master/tests/testdata/quickfo...
not -1 as in this response. is this expected?.

[xunjieli, 2017-04-14 17:25:25]

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

https://codereview.chromium.org/2805053005/diff/140001/components/cronet/andr...
components/cronet/android/test/src/org/chromium/net/Http2TestHandler.java:184:
byte[] quickfoxCompressed = {0x0b, 0x15, -1, 0x54, 0x68, 0x65, 0x20, 0x71, 0x75,
0x69,
On 2017/04/14 17:02:07, mef wrote:
> The third byte in curl
>
https://raw.githubusercontent.com/google/brotli/master/tests/testdata/quickfo...
> not -1 as in this response. is this expected?.

The third byte is 0x80, but Java byte is signed so I used -1 to represent 0x80.

[mef, 2017-04-14 17:40:59]

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

https://codereview.chromium.org/2805053005/diff/140001/components/cronet/andr...
components/cronet/android/test/src/org/chromium/net/Http2TestHandler.java:184:
byte[] quickfoxCompressed = {0x0b, 0x15, -1, 0x54, 0x68, 0x65, 0x20, 0x71, 0x75,
0x69,
On 2017/04/14 17:25:25, xunjieli wrote:
> On 2017/04/14 17:02:07, mef wrote:
> > The third byte in curl
> >
>
https://raw.githubusercontent.com/google/brotli/master/tests/testdata/quickfo...
> > not -1 as in this response. is this expected?.
> 
> The third byte is 0x80, but Java byte is signed so I used -1 to represent
0x80.

I see, that might be the problem as -1 is 0xFF, not 0x80. I think 0x80 would be
-0x80.

[xunjieli, 2017-04-14 18:24:15]

Thanks!

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

https://codereview.chromium.org/2805053005/diff/140001/components/cronet/andr...
components/cronet/android/test/src/org/chromium/net/Http2TestHandler.java:184:
byte[] quickfoxCompressed = {0x0b, 0x15, -1, 0x54, 0x68, 0x65, 0x20, 0x71, 0x75,
0x69,
On 2017/04/14 17:40:59, mef wrote:
> On 2017/04/14 17:25:25, xunjieli wrote:
> > On 2017/04/14 17:02:07, mef wrote:
> > > The third byte in curl
> > >
> >
>
https://raw.githubusercontent.com/google/brotli/master/tests/testdata/quickfo...
> > > not -1 as in this response. is this expected?.
> > 
> > The third byte is 0x80, but Java byte is signed so I used -1 to represent
> 0x80.
> 
> I see, that might be the problem as -1 is 0xFF, not 0x80. I think 0x80 would
be
> -0x80. 

Done. sorry for the very stupid error. Not sure what I was thinking...

[mef, 2017-04-14 18:34:58]

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

https://codereview.chromium.org/2805053005/diff/140001/components/cronet/andr...
components/cronet/android/test/src/org/chromium/net/Http2TestHandler.java:184:
byte[] quickfoxCompressed = {0x0b, 0x15, -1, 0x54, 0x68, 0x65, 0x20, 0x71, 0x75,
0x69,
On 2017/04/14 18:24:15, xunjieli wrote:
> On 2017/04/14 17:40:59, mef wrote:
> > On 2017/04/14 17:25:25, xunjieli wrote:
> > > On 2017/04/14 17:02:07, mef wrote:
> > > > The third byte in curl
> > > >
> > >
> >
>
https://raw.githubusercontent.com/google/brotli/master/tests/testdata/quickfo...
> > > > not -1 as in this response. is this expected?.
> > > 
> > > The third byte is 0x80, but Java byte is signed so I used -1 to represent
> > 0x80.
> > 
> > I see, that might be the problem as -1 is 0xFF, not 0x80. I think 0x80 would
> be
> > -0x80. 
> 
> Done. sorry for the very stupid error. Not sure what I was thinking...

No problem, but original test data has the '0x03' last byte. 
Can you try to restore it to see whether it still works?
I hope that fixing 0x80 would fix decompressed expectations.

[xunjieli, 2017-04-14 18:47:37]

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

https://codereview.chromium.org/2805053005/diff/140001/components/cronet/andr...
components/cronet/android/test/src/org/chromium/net/Http2TestHandler.java:184:
byte[] quickfoxCompressed = {0x0b, 0x15, -1, 0x54, 0x68, 0x65, 0x20, 0x71, 0x75,
0x69,
On 2017/04/14 18:34:58, mef wrote:
> On 2017/04/14 18:24:15, xunjieli wrote:
> > On 2017/04/14 17:40:59, mef wrote:
> > > On 2017/04/14 17:25:25, xunjieli wrote:
> > > > On 2017/04/14 17:02:07, mef wrote:
> > > > > The third byte in curl
> > > > >
> > > >
> > >
> >
>
https://raw.githubusercontent.com/google/brotli/master/tests/testdata/quickfo...
> > > > > not -1 as in this response. is this expected?.
> > > > 
> > > > The third byte is 0x80, but Java byte is signed so I used -1 to
represent
> > > 0x80.
> > > 
> > > I see, that might be the problem as -1 is 0xFF, not 0x80. I think 0x80
would
> > be
> > > -0x80. 
> > 
> > Done. sorry for the very stupid error. Not sure what I was thinking...
> 
> No problem, but original test data has the '0x03' last byte. 
> Can you try to restore it to see whether it still works?
> I hope that fixing 0x80 would fix decompressed expectations.

Done. You are right! That fixed it. Thanks for catching this bug.

[mef, 2017-04-14 20:11:24]

On 2017/04/14 18:47:37, xunjieli wrote:
>
https://codereview.chromium.org/2805053005/diff/140001/components/cronet/andr...
> File components/cronet/android/test/src/org/chromium/net/Http2TestHandler.java
> (right):
> 
>
https://codereview.chromium.org/2805053005/diff/140001/components/cronet/andr...
> components/cronet/android/test/src/org/chromium/net/Http2TestHandler.java:184:
> byte[] quickfoxCompressed = {0x0b, 0x15, -1, 0x54, 0x68, 0x65, 0x20, 0x71,
0x75,
> 0x69,
> On 2017/04/14 18:34:58, mef wrote:
> > On 2017/04/14 18:24:15, xunjieli wrote:
> > > On 2017/04/14 17:40:59, mef wrote:
> > > > On 2017/04/14 17:25:25, xunjieli wrote:
> > > > > On 2017/04/14 17:02:07, mef wrote:
> > > > > > The third byte in curl
> > > > > >
> > > > >
> > > >
> > >
> >
>
https://raw.githubusercontent.com/google/brotli/master/tests/testdata/quickfo...
> > > > > > not -1 as in this response. is this expected?.
> > > > > 
> > > > > The third byte is 0x80, but Java byte is signed so I used -1 to
> represent
> > > > 0x80.
> > > > 
> > > > I see, that might be the problem as -1 is 0xFF, not 0x80. I think 0x80
> would
> > > be
> > > > -0x80. 
> > > 
> > > Done. sorry for the very stupid error. Not sure what I was thinking...
> > 
> > No problem, but original test data has the '0x03' last byte. 
> > Can you try to restore it to see whether it still works?
> > I hope that fixing 0x80 would fix decompressed expectations.
> 
> Done. You are right! That fixed it. Thanks for catching this bug.

Great! LGTM.

[xunjieli, 2017-04-17 14:47:33]

The CQ bit was checked by xunjieli@chromium.org

[commit-bot: I haz the power, 2017-04-17 14:47:40]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-17 14:54:39]

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

[commit-bot: I haz the power, 2017-04-17 14:54:40]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[xunjieli, 2017-04-17 15:06:04]

Description was changed from

==========
[Cronet] Enable Brotli 

- removes the compile time flag to disable brotli when building Cronet.
- adds an API to enable Brotli on CronetEngine.
- adds three basic java tests in BrotliTest.java

BUG=710633
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

to

==========
[Cronet] Enable Brotli 

- removes the compile time flag to disable brotli when building Cronet.
- adds an API to enable Brotli on CronetEngine.
- adds three basic java tests in BrotliTest.java

BUG=710633
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

[xunjieli, 2017-04-17 15:06:04]

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

[xunjieli, 2017-04-17 15:06:33]

+jbudorick
John, PTAL at tools/mb/mb_config.pyl. Thanks!

[jbudorick, 2017-04-17 15:16:54]

lgtm, though we'll also need to remove the flag from here:
https://codesearch.chromium.org/chromium/build/scripts/slave/recipe_modules/c...

[xunjieli, 2017-04-17 15:40:20]

On 2017/04/17 15:16:54, jbudorick wrote:
> lgtm, though we'll also need to remove the flag from here:
>
https://codesearch.chromium.org/chromium/build/scripts/slave/recipe_modules/c...

Thanks a lot!  I will wait until the other CL takes effect so this one won't
cause any test breakages.

[jbudorick, 2017-04-17 15:41:47]

On 2017/04/17 15:40:20, xunjieli wrote:
> On 2017/04/17 15:16:54, jbudorick wrote:
> > lgtm, though we'll also need to remove the flag from here:
> >
>
https://codesearch.chromium.org/chromium/build/scripts/slave/recipe_modules/c...
> 
> Thanks a lot!  I will wait until the other CL takes effect so this one won't
> cause any test breakages.

I think the other one will only affect downstream, while this one will only
affect upstream. Without looking at the rest of the CL, we might have some
broken downstream bots until this one lands.

[xunjieli, 2017-04-17 15:48:17]

On 2017/04/17 15:41:47, jbudorick wrote:
> On 2017/04/17 15:40:20, xunjieli wrote:
> > On 2017/04/17 15:16:54, jbudorick wrote:
> > > lgtm, though we'll also need to remove the flag from here:
> > >
> >
>
https://codesearch.chromium.org/chromium/build/scripts/slave/recipe_modules/c...
> > 
> > Thanks a lot!  I will wait until the other CL takes effect so this one won't
> > cause any test breakages.
> 
> I think the other one will only affect downstream, while this one will only
> affect upstream. Without looking at the rest of the CL, we might have some
> broken downstream bots until this one lands.

Acknowledged. Removing "disable_brotli_filter=true" doesn't do anything other
than compiling some additional code in the binary.
This Cl adds some Java tests which depend on the additional code being included.
I think the downstream bots should be fine with the flag being removed. I will
draft a CL to flip the flag on the internal buildbots now.

[jbudorick, 2017-04-17 15:49:39]

On 2017/04/17 15:48:17, xunjieli wrote:
> On 2017/04/17 15:41:47, jbudorick wrote:
> > On 2017/04/17 15:40:20, xunjieli wrote:
> > > On 2017/04/17 15:16:54, jbudorick wrote:
> > > > lgtm, though we'll also need to remove the flag from here:
> > > >
> > >
> >
>
https://codesearch.chromium.org/chromium/build/scripts/slave/recipe_modules/c...
> > > 
> > > Thanks a lot!  I will wait until the other CL takes effect so this one
won't
> > > cause any test breakages.
> > 
> > I think the other one will only affect downstream, while this one will only
> > affect upstream. Without looking at the rest of the CL, we might have some
> > broken downstream bots until this one lands.
> 
> Acknowledged. Removing "disable_brotli_filter=true" doesn't do anything other
> than compiling some additional code in the binary.

ah, ok, sgtm.

> This Cl adds some Java tests which depend on the additional code being
included.
> I think the downstream bots should be fine with the flag being removed. I will
> draft a CL to flip the flag on the internal buildbots now.

I think the CL you just landed will do that once it rolls.

[xunjieli, 2017-04-17 15:55:13]

On 2017/04/17 15:49:39, jbudorick wrote:
> On 2017/04/17 15:48:17, xunjieli wrote:
> > On 2017/04/17 15:41:47, jbudorick wrote:
> > > On 2017/04/17 15:40:20, xunjieli wrote:
> > > > On 2017/04/17 15:16:54, jbudorick wrote:
> > > > > lgtm, though we'll also need to remove the flag from here:
> > > > >
> > > >
> > >
> >
>
https://codesearch.chromium.org/chromium/build/scripts/slave/recipe_modules/c...
> > > > 
> > > > Thanks a lot!  I will wait until the other CL takes effect so this one
> won't
> > > > cause any test breakages.
> > > 
> > > I think the other one will only affect downstream, while this one will
only
> > > affect upstream. Without looking at the rest of the CL, we might have some
> > > broken downstream bots until this one lands.
> > 
> > Acknowledged. Removing "disable_brotli_filter=true" doesn't do anything
other
> > than compiling some additional code in the binary.
> 
> ah, ok, sgtm.
> 
> > This Cl adds some Java tests which depend on the additional code being
> included.
> > I think the downstream bots should be fine with the flag being removed. I
will
> > draft a CL to flip the flag on the internal buildbots now.
> 
> I think the CL you just landed will do that once it rolls.

Ah, that makes sense! There are so many changes in infra and it's hard to keep
track. It's a good thing that we need to do few things now :)

[xunjieli, 2017-04-18 13:40:16]

The CQ bit was checked by xunjieli@chromium.org

[commit-bot: I haz the power, 2017-04-18 13:40:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-18 13:45:51]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1492522816685460,
"parent_rev": "ebc379c0d84a4c0447bcc95317c85ffee5c9025e", "commit_rev":
"186d2bf1b6ff14929c79d6d4a8b310ba639dfbf9"}

[commit-bot: I haz the power, 2017-04-18 13:46:03]

Description was changed from

==========
[Cronet] Enable Brotli 

- removes the compile time flag to disable brotli when building Cronet.
- adds an API to enable Brotli on CronetEngine.
- adds three basic java tests in BrotliTest.java

BUG=710633
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

to

==========
[Cronet] Enable Brotli 

- removes the compile time flag to disable brotli when building Cronet.
- adds an API to enable Brotli on CronetEngine.
- adds three basic java tests in BrotliTest.java

BUG=710633
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Review-Url: https://codereview.chromium.org/2805053005
Cr-Commit-Position: refs/heads/master@{#465216}
Committed:
https://chromium.googlesource.com/chromium/src/+/186d2bf1b6ff14929c79d6d4a8b3...
==========

[commit-bot: I haz the power, 2017-04-18 13:46:05]

Committed patchset #8 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/186d2bf1b6ff14929c79d6d4a8b3...