Use proguard.cfg in cronet sample apk.

Use proguard.cfg in release build of Cronet Sample apk.
Update proguard.cfg to reflect that @UsedByReflection was moved to base/.
Add --release option to cr_cronet.py tool.

Notes:
- Chromium debug build does not apply proguard at all.
- Chromium instrumentation test does not allow custom proguard config (but it uses proguard config from instrumented app, which is nice).

BUG=390267
Committed: https://crrev.com/24b4b89d638187b95fde8fa3f629cc5d533b64fe
Cr-Commit-Position: refs/heads/master@{#294671}

[mef, 2014-08-27 22:18:22]

mef@chromium.org changed reviewers:
+ clm@google.com, mmenke@chromium.org, xunjieli@chromium.org

[mef, 2014-08-27 22:22:28]

Hi,

please take a look and comment, as I'm admittedly confused by proguard in
general and its use in chromium in particular.

Couple of notes :

- Chromium debug build does not apply proguard at all.
- Chromium instrumentation test does not allow custom proguard config (but it
uses proguard config from instrumented app, which is nice).
- For some reason junit cannot find tests unless they are explicitly marked as
keepers.

thanks,
-m

[xunjieli, 2014-08-28 15:35:04]

Sorry I might have missed the conversation. Why do we need to use proguard for
the sample apk?

https://codereview.chromium.org/512953002/diff/1/components/cronet/tools/cr_c...
File components/cronet/tools/cr_cronet.py (right):

https://codereview.chromium.org/512953002/diff/1/components/cronet/tools/cr_c...
components/cronet/tools/cr_cronet.py:35: out = 'out/Debug'
nit: can this variable be sth like "out_dir" ?

[mef, 2014-08-28 15:39:35]

On 2014/08/28 15:35:04, xunjieli1 wrote:
> Sorry I might have missed the conversation. Why do we need to use proguard for
> the sample apk?
It is to proactively find subtle errors induced by proguard misconfiguration
prior to delivering update to consumers.
Basically, we've only discovered that proguard removed unused method
getAllHeaders via manual QA of consumer app.

> 
>
https://codereview.chromium.org/512953002/diff/1/components/cronet/tools/cr_c...
> File components/cronet/tools/cr_cronet.py (right):
> 
>
https://codereview.chromium.org/512953002/diff/1/components/cronet/tools/cr_c...
> components/cronet/tools/cr_cronet.py:35: out = 'out/Debug'
> nit: can this variable be sth like "out_dir" ?
Will do.

[mmenke, 2014-08-28 17:26:36]

https://codereview.chromium.org/512953002/diff/1/components/cronet/android/sa...
File components/cronet/android/sample/javatests/proguard.cfg (right):

https://codereview.chromium.org/512953002/diff/1/components/cronet/android/sa...
components/cronet/android/sample/javatests/proguard.cfg:7: -keep class
org.chromium.base.library_loader.LibraryLoaderHelper {
Hrm...These are the ones you said you didn't understand the need for, right?

https://codereview.chromium.org/512953002/diff/1/components/cronet/android/sa...
components/cronet/android/sample/javatests/proguard.cfg:45: # Keep Test classes
Can we just keep everything?

https://codereview.chromium.org/512953002/diff/1/components/cronet/tools/cr_c...
File components/cronet/tools/cr_cronet.py (right):

https://codereview.chromium.org/512953002/diff/1/components/cronet/tools/cr_c...
components/cronet/tools/cr_cronet.py:49: '--apk=CronetSample.apk')
Should these args have leading spaces?  release_arg does, anyways, but so does
the path of the python file...

[mef, 2014-08-31 21:34:39]

Thanks, PTAL.

https://codereview.chromium.org/512953002/diff/1/components/cronet/android/sa...
File components/cronet/android/sample/javatests/proguard.cfg (right):

https://codereview.chromium.org/512953002/diff/1/components/cronet/android/sa...
components/cronet/android/sample/javatests/proguard.cfg:7: -keep class
org.chromium.base.library_loader.LibraryLoaderHelper {
On 2014/08/28 17:26:36, mmenke wrote:
> Hrm...These are the ones you said you didn't understand the need for, right?

Well, I understand that they have to be kept, so test app could access them
directly, but I think it weakens the proguard test.

https://codereview.chromium.org/512953002/diff/1/components/cronet/android/sa...
components/cronet/android/sample/javatests/proguard.cfg:45: # Keep Test classes
On 2014/08/28 17:26:36, mmenke wrote:
> Can we just keep everything?

Done.

https://codereview.chromium.org/512953002/diff/1/components/cronet/tools/cr_c...
File components/cronet/tools/cr_cronet.py (right):

https://codereview.chromium.org/512953002/diff/1/components/cronet/tools/cr_c...
components/cronet/tools/cr_cronet.py:35: out = 'out/Debug'
On 2014/08/28 15:35:04, xunjieli1 wrote:
> nit: can this variable be sth like "out_dir" ? 

Done.

https://codereview.chromium.org/512953002/diff/1/components/cronet/tools/cr_c...
components/cronet/tools/cr_cronet.py:49: '--apk=CronetSample.apk')
On 2014/08/28 17:26:36, mmenke wrote:
> Should these args have leading spaces?  release_arg does, anyways, but so does
> the path of the python file...

Done.

[mmenke, 2014-09-02 15:04:30]

LGTM

[Charles, 2014-09-02 16:52:19]

https://codereview.chromium.org/512953002/diff/20001/components/cronet/androi...
File components/cronet/android/sample/javatests/proguard.cfg (right):

https://codereview.chromium.org/512953002/diff/20001/components/cronet/androi...
components/cronet/android/sample/javatests/proguard.cfg:9: -keep class
org.chromium.base.CommandLine {
If you're keeping all these classes here, adding proguard to your tests is
unlikely to actually test anything.

[xunjieli, 2014-09-02 16:53:17]

Sorry one more question.

https://codereview.chromium.org/512953002/diff/20001/components/cronet/androi...
File components/cronet/android/sample/javatests/proguard.cfg (right):

https://codereview.chromium.org/512953002/diff/20001/components/cronet/androi...
components/cronet/android/sample/javatests/proguard.cfg:4: -keep class
org.chromium.base.library_loader.LibraryLoaderHelper {
It looks like the sample is only using
https://code.google.com/p/chromium/codesearch#chromium/src/components/cronet/...
, and not the LibraryLoader from base. 

Why do we need to keep this class?

[mef, 2014-09-12 20:49:24]

Now that CronetTest is separated from CronetSample I've updated CronetSample to
use Proguard in release.

https://codereview.chromium.org/512953002/diff/20001/components/cronet/androi...
File components/cronet/android/sample/javatests/proguard.cfg (right):

https://codereview.chromium.org/512953002/diff/20001/components/cronet/androi...
components/cronet/android/sample/javatests/proguard.cfg:4: -keep class
org.chromium.base.library_loader.LibraryLoaderHelper {
On 2014/09/02 16:53:16, xunjieli1 wrote:
> It looks like the sample is only using
>
https://code.google.com/p/chromium/codesearch#chromium/src/components/cronet/...
> , and not the LibraryLoader from base. 
> 
> Why do we need to keep this class?
base_javatests include org.chromium.base.LibraryLoaderHelperTest, which uses
this class. We do need to clean that at some point.

https://codereview.chromium.org/512953002/diff/20001/components/cronet/androi...
components/cronet/android/sample/javatests/proguard.cfg:9: -keep class
org.chromium.base.CommandLine {
On 2014/09/02 16:52:19, Charles wrote:
> If you're keeping all these classes here, adding proguard to your tests is
> unlikely to actually test anything.
Good point. I think it makes sense to build CronetSample.apk with cronet
proguard.txt only to find proguard issues. We will move tests to CronetTest.apk
and that will not use proguard.

[mef, 2014-09-12 20:51:18]

The CQ bit was checked by mef@chromium.org

[commit-bot: I haz the power, 2014-09-12 20:56:39]

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

[commit-bot: I haz the power, 2014-09-12 21:58:05]

Committed patchset #4 (id:60001) as f5d38942512c370ddd7b14d84a51e1faeb8820cd

[commit-bot: I haz the power, 2014-09-12 22:03:16]

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