Bug 1106593: Apply Proguard to bundled Fennec libraries. r=nalexander draft
authorChris Kitching <chriskitching@linux.com>
Fri, 05 Dec 2014 11:50:48 -0800
changeset 231264 877254882ad87984a27021929e6213a0937aa1c9
parent 231263 e2759f8d7232e1856d25f73a79b6bacaa85ca2de
child 504909 dc931df8afc381877abffd0dd4f4b7dc4cc4f51e
push id202
push userchriskitching@linux.com
push dateTue, 09 Dec 2014 20:01:51 +0000
reviewersnalexander
bugs1106593
milestone37.0a1
Bug 1106593: Apply Proguard to bundled Fennec libraries. r=nalexander
mobile/android/base/Makefile.in
mobile/android/config/proguard.cfg
mobile/android/config/proguard/play-services-keeps.cfg
mobile/android/config/proguard/proguard.cfg
mobile/android/config/proguard/strip-libs.cfg
--- a/mobile/android/base/Makefile.in
+++ b/mobile/android/base/Makefile.in
@@ -55,16 +55,20 @@ GARBAGE += \
   javah.out \
   jni-stubs.inc \
   GeneratedJNIWrappers.cpp \
   GeneratedJNIWrappers.h \
   $(NULL)
 
 GARBAGE_DIRS += classes db jars res sync services generated
 
+# The bootclasspath is functionally identical to the classpath, but allows the classes given
+# to redefine classes in core packages, such as java.lang. android.jar is here as it provides
+# Android's definition of the Java Standard Library. The compatabiliy lib here tweaks a few
+# of the core classes to paint over changes in behaviour between versions.
 JAVA_BOOTCLASSPATH = \
     $(ANDROID_SDK)/android.jar \
     $(ANDROID_COMPAT_LIB) \
     $(NULL)
 
 JAVA_BOOTCLASSPATH := $(subst $(NULL) ,:,$(strip $(JAVA_BOOTCLASSPATH)))
 
 # If native devices are enabled, add Google Play Services and some of the v7 compat libraries
@@ -73,16 +77,30 @@ ifdef MOZ_NATIVE_DEVICES
         $(GOOGLE_PLAY_SERVICES_LIB) \
         $(ANDROID_MEDIAROUTER_LIB) \
         $(ANDROID_APPCOMPAT_LIB) \
         $(NULL)
 endif
 
 JAVA_CLASSPATH := $(subst $(NULL) ,:,$(strip $(JAVA_CLASSPATH)))
 
+# Library jars that we're bundling: these are subject to Proguard before inclusion
+# into classes.dex.
+java_bundled_libs = \
+    $(ANDROID_COMPAT_LIB) \
+    $(GOOGLE_PLAY_SERVICES_LIB) \
+    $(ANDROID_MEDIAROUTER_LIB) \
+    $(ANDROID_APPCOMPAT_LIB) \
+    $(NULL)
+
+java_bundled_libs := $(subst $(NULL) ,:,$(strip $(java_bundled_libs)))
+
+# All the jars we're compiling from source. (not to be confused with
+# java_bundled_libs, which holds the jars which we're including as
+# binaries).
 ALL_JARS = \
   constants.jar \
   gecko-R.jar \
   gecko-browser.jar \
   gecko-mozglue.jar \
   gecko-thirdparty.jar \
   gecko-util.jar \
   sync-thirdparty.jar \
@@ -97,77 +115,101 @@ ALL_JARS += search-activity.jar
 endif
 
 ifdef MOZ_ANDROID_MLS_STUMBLER
 extra_packages += org.mozilla.mozstumbler
 ALL_JARS += ../stumbler/stumbler.jar
 generated/org/mozilla/mozstumbler/R.java: .aapt.deps ;
 endif
 
+# The list of jars in Java classpath notation (colon-separated).
+all_jars_classpath = $(subst $(NULL) ,:,$(strip $(ALL_JARS)))
+
 include $(topsrcdir)/config/config.mk
 
 # Note that we're going to set up a dependency directly between embed_android.dex and the java files
 # Instead of on the .class files, since more than one .class file might be produced per .java file
 # Sync dependencies are provided in a single jar. Sync classes themselves are delivered as source,
 # because Android resource classes must be compiled together in order to avoid overlapping resource
 # indices.
 
 library_jars = \
-    $(JAVA_CLASSPATH) \
-    $(JAVA_BOOTCLASSPATH) \
+    $(ANDROID_SDK)/android.jar \
     $(NULL)
 
 library_jars := $(subst $(NULL) ,:,$(strip $(library_jars)))
 
 classes.dex: .proguard.deps
 	$(REPORT_BUILD)
-	$(DX) --dex --output=classes.dex jars-proguarded $(subst :, ,$(ANDROID_COMPAT_LIB):$(JAVA_CLASSPATH))
+	$(DX) --dex --output=classes.dex jars-proguarded
 
 ifdef MOZ_DISABLE_PROGUARD
   PROGUARD_PASSES=0
 else
   ifdef MOZ_DEBUG
     PROGUARD_PASSES=1
   else
     ifndef MOZILLA_OFFICIAL
       PROGUARD_PASSES=1
     else
       PROGUARD_PASSES=6
     endif
   endif
 endif
 
+proguard_config_dir=$(topsrcdir)/mobile/android/config/proguard
+
 # This stanza ensures that the set of GeckoView classes does not depend on too
 # much of Fennec, where "too much" is defined as the set of potentially
 # non-GeckoView classes that GeckoView already depended on at a certain point in
 # time.  The idea is to set a high-water mark that is not to be crossed.
 classycle_jar := $(topsrcdir)/mobile/android/build/classycle/classycle-1.4.1.jar
 .geckoview.deps: geckoview.ddf $(classycle_jar) $(ALL_JARS)
 	java -cp $(classycle_jar) \
 		classycle.dependency.DependencyChecker \
 		-mergeInnerClasses \
 		-dependencies=@$< \
 		$(ALL_JARS)
 	@$(TOUCH) $@
 
+# First, we delete debugging information from libraries. Having line-number information for libraries
+# for which we lack the source isn't useful, so this saves us a bit of space. Importantly, Proguard
+# has a bug causing it to sometimes corrupt this information if present (which it does for some of the
+# included libraries). This corruption prevents dex from completing, so we need to get rid of it.
+# This prevents us from seeing line numbers in stack traces for stack frames inside libraries. I doubt
+# anyone cares.
+# This step can occur much earlier than the main Proguard pass: it needs only gecko-R.jar to have been
+# compiled (as that's where the library R.java files end up), but it does block the main Proguard pass.
+.bundled.proguard.deps: gecko-R.jar $(proguard_config_dir)/strip-libs.cfg
+	$(REPORT_BUILD)
+	@$(TOUCH) $@
+	java \
+    -Xmx512m -Xms128m \
+    -jar $(ANDROID_SDK_ROOT)/tools/proguard/lib/proguard.jar \
+    @$(proguard_config_dir)/strip-libs.cfg \
+    -injars $(subst ::,:,$(java_bundled_libs))\
+    -outjars bundled-jars-nodebug \
+    -libraryjars $(library_jars):gecko-R.jar
+
 # We touch the target file before invoking Proguard so that Proguard's
 # outputs are fresher than the target, preventing a subsequent
 # invocation from thinking Proguard's outputs are stale.  This is safe
 # because Make removes the target file if any recipe command fails.
-.proguard.deps: .geckoview.deps $(ALL_JARS) $(topsrcdir)/mobile/android/config/proguard.cfg
+.proguard.deps: .geckoview.deps .bundled.proguard.deps $(ALL_JARS) $(proguard_config_dir)/proguard.cfg
 	$(REPORT_BUILD)
 	@$(TOUCH) $@
 	java \
-		-Xmx512m -Xms128m \
-		-jar $(ANDROID_SDK_ROOT)/tools/proguard/lib/proguard.jar \
-		@$(topsrcdir)/mobile/android/config/proguard.cfg \
-		-optimizationpasses $(PROGUARD_PASSES) \
-		-injars $(subst ::,:,$(subst $(NULL) ,:,$(strip $(ALL_JARS)))) \
-		-outjars jars-proguarded \
-		-libraryjars $(library_jars)
+    -Xmx512m -Xms128m \
+    -jar $(ANDROID_SDK_ROOT)/tools/proguard/lib/proguard.jar \
+    @$(proguard_config_dir)/proguard.cfg \
+    -optimizationpasses $(PROGUARD_PASSES) \
+    -injars $(subst ::,:,$(all_jars_classpath)):bundled-jars-nodebug \
+    -outjars jars-proguarded \
+    -libraryjars $(library_jars)
+
 
 CLASSES_WITH_JNI= \
     org.mozilla.gecko.ANRReporter \
     org.mozilla.gecko.GeckoAppShell \
     org.mozilla.gecko.GeckoJavaSampler \
     org.mozilla.gecko.gfx.NativePanZoomController \
     org.mozilla.gecko.util.NativeJSContainer \
     org.mozilla.gecko.util.NativeJSObject \
new file mode 100644
--- /dev/null
+++ b/mobile/android/config/proguard/play-services-keeps.cfg
@@ -0,0 +1,19 @@
+# Rules to prevent Google Play Services from exploding
+# (From http://developer.android.com/google/play-services/setup.html#Proguard
+# With the reference to "Object" changed so it'll actually *work*...)
+-keep class * extends java.util.ListResourceBundle {
+    protected java.lang.Object[][] getContents();
+}
+
+-keep public class com.google.android.gms.common.internal.safeparcel.SafeParcelable {
+    public static final *** NULL;
+}
+
+-keepnames @com.google.android.gms.common.annotation.KeepName class *
+-keepclassmembernames class * {
+    @com.google.android.gms.common.annotation.KeepName *;
+}
+
+-keepnames class * implements android.os.Parcelable {
+    public static final ** CREATOR;
+}
rename from mobile/android/config/proguard.cfg
rename to mobile/android/config/proguard/proguard.cfg
--- a/mobile/android/config/proguard.cfg
+++ b/mobile/android/config/proguard/proguard.cfg
@@ -105,16 +105,21 @@
 
 #
 # Mozilla-specific rules
 #
 # Merging classes can generate dex warnings about anonymous inner classes.
 -optimizations !class/merging/horizontal
 -optimizations !class/merging/vertical
 
+# This optimisation causes corrupt bytecode if we run more than two passes.
+# Testing shows that running the extra passes of everything else saves us
+# more than this optimisation does, so bye bye!
+-optimizations !code/allocation/variable
+
 # Keep miscellaneous targets.
 
 # Keep the annotation.
 -keep @interface org.mozilla.gecko.mozglue.JNITarget
 
 # Keep classes tagged with the annotation.
 -keep @org.mozilla.gecko.mozglue.JNITarget class *
 
@@ -202,8 +207,14 @@
     *;
 }
 
 # Disable obfuscation because it makes exception stack traces more difficult to read.
 -dontobfuscate
 
 # Suppress warnings about missing descriptor classes.
 #-dontnote **,!ch.boye.**,!org.mozilla.gecko.sync.**
+
+-include "play-services-keeps.cfg"
+
+# Don't print spurious warnings from the support library.
+# See: http://stackoverflow.com/questions/22441366/note-android-support-v4-text-icucompatics-cant-find-dynamically-referenced-cl
+-dontnote android.support.**
new file mode 100644
--- /dev/null
+++ b/mobile/android/config/proguard/strip-libs.cfg
@@ -0,0 +1,40 @@
+# Proguard step for stripping debug information.
+#
+# This is useful to work around a bug in the way Proguard handles debug information: it
+# sometimes corrupts it. Classes with corrupt debug information cannot be dexed, but
+# classes with *no* debug information can be. There's no way to configure Proguard to
+# delete debug information on a per-class basis, so we need this special extra step for
+# stripping debug information only from those classes for which the Proguard bug is
+# encountered.
+#
+# Currently, this pass is applied to all bundled library jars for which we are not
+# compiling the source. This is slightly more than is strictly necessary to work around
+# the Proguard bug, but such debug information is of negligible value and stripping it
+# too slightly simplifies the makefile and saves us a handful of kilobytes of binary size.
+#
+# Configuring Proguard to do nothing except strip metadata is done by having it run only
+# the obfuscation pass, but with a configuration that prevents it from renaming any classes.
+# It then attempts to delete class metadata, so we further configure it not to do so for
+# anything except the problematic debug information.
+
+# Run only the obfuscator.
+-dontoptimize
+-dontshrink
+-dontpreverify
+-verbose
+
+# Don't rename anything.
+-keeppackagenames
+
+# Seriously, don't rename anything.
+-keep class *
+-keepclassmembers class * {
+    *;
+}
+
+# Don't delete other useful metadata.
+-keepattributes Exceptions,InnerClasses,Signature,Deprecated,*Annotation*,EnclosingMethod
+
+# Don't print spurious warnings from the support library.
+# See: http://stackoverflow.com/questions/22441366/note-android-support-v4-text-icucompatics-cant-find-dynamically-referenced-cl
+-dontnote android.support.**