Annotated functions/classes more cleanly kept

Annotated functions/classes more cleanly kept

This has no effect on the Proguarded .dex

BUG=619937
Committed: https://crrev.com/e3fa0e21d8b8819ba19b68ec1445d7ebf301214f
Cr-Commit-Position: refs/heads/master@{#406355}

[smaier, 2016-07-14 15:29:55]

Description was changed from

==========
Annotations more cleanly kept

This has no effect on the Proguarded .dex

BUG=619937
==========

to

==========
Annotated functions/classes more cleanly kept

This has no effect on the Proguarded .dex

BUG=619937
==========

[smaier, 2016-07-14 15:29:55]

smaier@chromium.org changed reviewers:
+ agrieve@chromium.org, wnwen@chromium.org

[smaier, 2016-07-14 15:30:00]

The CQ bit was checked by smaier@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-14 15:30:14]

Dry run: 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, 2016-07-14 16:18:52]

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

[commit-bot: I haz the power, 2016-07-14 16:18:52]

Dry run: This issue passed the CQ dry run.

[agrieve, 2016-07-15 14:44:34]

just a small nit.

Might be worth also considering moving these annotation-specific rules to their
own .flags file to be used by both webview and Chrome

https://codereview.chromium.org/2150533005/diff/1/chrome/android/java/proguar...
File chrome/android/java/proguard.flags (right):

https://codereview.chromium.org/2150533005/diff/1/chrome/android/java/proguar...
chrome/android/java/proguard.flags:18: -keep
@org.chromium.base.annotations.UsedByReflection class
com.google.android.apps.chrome.**
I think we're fine to leave the right-hand-side of this as "class *". I don't
know why we'd not have code in our namespace, but if we did, I'd want the
annotation to still work.

I think we should probably do the same for the -kepclasseswithmembers below as
well.  Now that we've being more specific on the annotation class, we can relax
the right-hand side (and have fewer lines, yay!)

[smaier, 2016-07-15 21:00:57]

smaier@chromium.org changed reviewers:
+ torne@chromium.org, yfriedman@chromium.org

[smaier, 2016-07-15 21:00:58]

torne@chromium.org: Please review changes in android_webview/

yfriedman@chromium.org: Please review changes in chrome/android/

[Torne, 2016-07-18 12:39:18]

WebView LGTM here. Thanks for doing this, this is much cleaner and nicer!

[Yaron, 2016-07-18 13:41:21]

https://codereview.chromium.org/2150533005/diff/40001/base/base_proguard_conf...
File base/base_proguard_config.flags (right):

https://codereview.chromium.org/2150533005/diff/40001/base/base_proguard_conf...
base/base_proguard_config.flags:1: # Keep line number information, useful for
stack traces.
put this in base/android?

https://codereview.chromium.org/2150533005/diff/40001/chrome/android/webapk/s...
File chrome/android/webapk/shell_apk/BUILD.gn (right):

https://codereview.chromium.org/2150533005/diff/40001/chrome/android/webapk/s...
chrome/android/webapk/shell_apk/BUILD.gn:91:
"//base/base_proguard_config.flags",
This is a bit awkward as webapk shell doesn't depend on base. But the proguard
definitions there aren't strictly about "base" either.

[agrieve, 2016-07-18 15:53:16]

https://codereview.chromium.org/2150533005/diff/40001/chrome/android/webapk/s...
File chrome/android/webapk/shell_apk/BUILD.gn (right):

https://codereview.chromium.org/2150533005/diff/40001/chrome/android/webapk/s...
chrome/android/webapk/shell_apk/BUILD.gn:91:
"//base/base_proguard_config.flags",
On 2016/07/18 13:41:21, Yaron wrote:
> This is a bit awkward as webapk shell doesn't depend on base. But the proguard
> definitions there aren't strictly about "base" either.

//build/android would be another place that makes sense if we wanted to split
them up even further (into base & non-base things)

[smaier, 2016-07-18 17:28:50]

https://codereview.chromium.org/2150533005/diff/40001/base/base_proguard_conf...
File base/base_proguard_config.flags (right):

https://codereview.chromium.org/2150533005/diff/40001/base/base_proguard_conf...
base/base_proguard_config.flags:1: # Keep line number information, useful for
stack traces.
On 2016/07/18 13:41:21, Yaron wrote:
> put this in base/android?

Done.

[Yaron, 2016-07-18 17:43:13]

lgtm

[agrieve, 2016-07-18 18:02:21]

lgtm % 2 things

https://codereview.chromium.org/2150533005/diff/60001/base/android/base_progu...
File base/android/base_proguard_config.flags (right):

https://codereview.chromium.org/2150533005/diff/60001/base/android/base_progu...
base/android/base_proguard_config.flags:10: -keep @interface
android.webkit.JavascriptInterface
I don't think we need a keep for this since it's a part of android framework and
not webview apk.

https://codereview.chromium.org/2150533005/diff/60001/chrome/android/java/pro...
File chrome/android/java/proguard.flags (right):

https://codereview.chromium.org/2150533005/diff/60001/chrome/android/java/pro...
chrome/android/java/proguard.flags:12:
-keepclasseswithmembers,includedescriptorclasses class
com.google.android.apps.chrome.** {
I think it might be better to have the version of this within base/ just apply
to all classes rather than scope them at all. Renaming never works with JNI, so
there shouldn't never be a case where in any library you'd want obfuscation on a
native method.

[smaier, 2016-07-18 18:50:09]

https://codereview.chromium.org/2150533005/diff/60001/base/android/base_progu...
File base/android/base_proguard_config.flags (right):

https://codereview.chromium.org/2150533005/diff/60001/base/android/base_progu...
base/android/base_proguard_config.flags:10: -keep @interface
android.webkit.JavascriptInterface
On 2016/07/18 18:02:21, agrieve wrote:
> I don't think we need a keep for this since it's a part of android framework
and
> not webview apk.

Done.

https://codereview.chromium.org/2150533005/diff/60001/chrome/android/java/pro...
File chrome/android/java/proguard.flags (right):

https://codereview.chromium.org/2150533005/diff/60001/chrome/android/java/pro...
chrome/android/java/proguard.flags:12:
-keepclasseswithmembers,includedescriptorclasses class
com.google.android.apps.chrome.** {
On 2016/07/18 18:02:21, agrieve wrote:
> I think it might be better to have the version of this within base/ just apply
> to all classes rather than scope them at all. Renaming never works with JNI,
so
> there shouldn't never be a case where in any library you'd want obfuscation on
a
> native method.

Done.

[smaier, 2016-07-18 18:50:59]

The CQ bit was checked by smaier@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-18 18:52:01]

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

[smaier, 2016-07-18 20:07:16]

The CQ bit was checked by smaier@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-18 20:07:50]

Dry run: 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, 2016-07-18 21:27:57]

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

[commit-bot: I haz the power, 2016-07-18 21:27:58]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[smaier, 2016-07-19 18:35:28]

The CQ bit was checked by smaier@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-19 18:36:25]

Dry run: 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, 2016-07-19 19:43:54]

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

[commit-bot: I haz the power, 2016-07-19 19:43:55]

Dry run: This issue passed the CQ dry run.

[smaier, 2016-07-19 19:48:01]

The CQ bit was checked by smaier@chromium.org

[smaier, 2016-07-19 19:48:02]

The patchset sent to the CQ was uploaded after l-g-t-m from torne@chromium.org,
yfriedman@chromium.org, agrieve@chromium.org
Link to the patchset: https://codereview.chromium.org/2150533005/#ps140001
(title: "keeping all native functions")

[commit-bot: I haz the power, 2016-07-19 19:48:34]

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, 2016-07-19 19:53:41]

Description was changed from

==========
Annotated functions/classes more cleanly kept

This has no effect on the Proguarded .dex

BUG=619937
==========

to

==========
Annotated functions/classes more cleanly kept

This has no effect on the Proguarded .dex

BUG=619937
==========

[commit-bot: I haz the power, 2016-07-19 19:53:42]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-07-19 19:53:50]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-19 19:54:55]

Description was changed from

==========
Annotated functions/classes more cleanly kept

This has no effect on the Proguarded .dex

BUG=619937
==========

to

==========
Annotated functions/classes more cleanly kept

This has no effect on the Proguarded .dex

BUG=619937

Committed: https://crrev.com/e3fa0e21d8b8819ba19b68ec1445d7ebf301214f
Cr-Commit-Position: refs/heads/master@{#406355}
==========

[commit-bot: I haz the power, 2016-07-19 19:54:56]

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