From 1f6d161d4d33f2d31d456bc701965b472370cdf6 Mon Sep 17 00:00:00 2001 From: olly Date: Mon, 29 Jan 2018 04:50:15 -0800 Subject: [PATCH] Fix proguard configurations 1. When we try and load something via reflection and find the class, always throw rather than failing silently if we subsequently fail to instantiate an instance. This is indicative of a broken proguard setup, and failing silently makes it hard to spot. 2. Add library/core proguard configuration to ensure extension renderer constructors that we access via reflection are kept. 3. Add demos/main proguard configuration to ensure ImaAdsLoader constructor that we access via reflection is kept. 4. Added IMA proguard file to hopefully fix #3723, although I wasn't actually able to reproduce the issue. Issue: #3723 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=183648187 --- demos/main/build.gradle | 5 +- demos/main/proguard-rules.txt | 6 ++ .../exoplayer2/demo/PlayerActivity.java | 77 +++++++++-------- demos/main/src/main/proguard-rules.txt | 6 ++ extensions/ima/proguard-rules.txt | 6 ++ extensions/ima/src/main/proguard-rules.txt | 6 ++ library/core/build.gradle | 1 + library/core/proguard-rules.txt | 25 ++++++ .../exoplayer2/DefaultRenderersFactory.java | 85 +++++++++++++------ .../extractor/DefaultExtractorsFactory.java | 12 ++- .../upstream/DefaultDataSource.java | 17 ++-- 11 files changed, 171 insertions(+), 75 deletions(-) create mode 100644 demos/main/proguard-rules.txt create mode 100644 demos/main/src/main/proguard-rules.txt create mode 100644 extensions/ima/proguard-rules.txt create mode 100644 extensions/ima/src/main/proguard-rules.txt create mode 100644 library/core/proguard-rules.txt diff --git a/demos/main/build.gradle b/demos/main/build.gradle index adad8f0e58..f637e39ce4 100644 --- a/demos/main/build.gradle +++ b/demos/main/build.gradle @@ -27,7 +27,10 @@ android { release { shrinkResources true minifyEnabled true - proguardFiles getDefaultProguardFile('proguard-android.txt') + proguardFiles = [ + "proguard-rules.txt", + getDefaultProguardFile('proguard-android.txt') + ] } debug { jniDebuggable = true diff --git a/demos/main/proguard-rules.txt b/demos/main/proguard-rules.txt new file mode 100644 index 0000000000..de5e0a4203 --- /dev/null +++ b/demos/main/proguard-rules.txt @@ -0,0 +1,6 @@ +# Proguard rules specific to the main demo app. + +# Constructor accessed via reflection in PlayerActivity +-keepclassmembers class com.google.android.exoplayer2.ext.ima.ImaAdsLoader { + (android.content.Context, android.net.Uri); +} diff --git a/demos/main/src/main/java/com/google/android/exoplayer2/demo/PlayerActivity.java b/demos/main/src/main/java/com/google/android/exoplayer2/demo/PlayerActivity.java index a426832108..e91feaa291 100644 --- a/demos/main/src/main/java/com/google/android/exoplayer2/demo/PlayerActivity.java +++ b/demos/main/src/main/java/com/google/android/exoplayer2/demo/PlayerActivity.java @@ -16,7 +16,6 @@ package com.google.android.exoplayer2.demo; import android.app.Activity; -import android.content.Context; import android.content.Intent; import android.content.pm.PackageManager; import android.net.Uri; @@ -76,6 +75,7 @@ import com.google.android.exoplayer2.upstream.DefaultBandwidthMeter; import com.google.android.exoplayer2.upstream.HttpDataSource; import com.google.android.exoplayer2.util.EventLogger; import com.google.android.exoplayer2.util.Util; +import java.lang.reflect.Constructor; import java.net.CookieHandler; import java.net.CookieManager; import java.net.CookiePolicy; @@ -353,9 +353,10 @@ public class PlayerActivity extends Activity releaseAdsLoader(); loadedAdTagUri = adTagUri; } - try { - mediaSource = createAdsMediaSource(mediaSource, Uri.parse(adTagUriString)); - } catch (Exception e) { + MediaSource adsMediaSource = createAdsMediaSource(mediaSource, Uri.parse(adTagUriString)); + if (adsMediaSource != null) { + mediaSource = adsMediaSource; + } else { showToast(R.string.ima_not_loaded); } } else { @@ -463,39 +464,47 @@ public class PlayerActivity extends Activity .buildHttpDataSourceFactory(useBandwidthMeter ? BANDWIDTH_METER : null); } - /** - * Returns an ads media source, reusing the ads loader if one exists. - * - * @throws Exception Thrown if it was not possible to create an ads media source, for example, due - * to a missing dependency. - */ - private MediaSource createAdsMediaSource(MediaSource mediaSource, Uri adTagUri) throws Exception { + /** Returns an ads media source, reusing the ads loader if one exists. */ + private @Nullable MediaSource createAdsMediaSource(MediaSource mediaSource, Uri adTagUri) { // Load the extension source using reflection so the demo app doesn't have to depend on it. // The ads loader is reused for multiple playbacks, so that ad playback can resume. - Class loaderClass = Class.forName("com.google.android.exoplayer2.ext.ima.ImaAdsLoader"); - if (adsLoader == null) { - adsLoader = (AdsLoader) loaderClass.getConstructor(Context.class, Uri.class) - .newInstance(this, adTagUri); - adUiViewGroup = new FrameLayout(this); - // The demo app has a non-null overlay frame layout. - playerView.getOverlayFrameLayout().addView(adUiViewGroup); - } - AdsMediaSource.MediaSourceFactory adMediaSourceFactory = - new AdsMediaSource.MediaSourceFactory() { - @Override - public MediaSource createMediaSource( - Uri uri, @Nullable Handler handler, @Nullable MediaSourceEventListener listener) { - return PlayerActivity.this.buildMediaSource( - uri, /* overrideExtension= */ null, handler, listener); - } + try { + Class loaderClass = Class.forName("com.google.android.exoplayer2.ext.ima.ImaAdsLoader"); + if (adsLoader == null) { + // Full class names used so the LINT.IfChange rule triggers should any of the classes move. + // LINT.IfChange + Constructor loaderConstructor = + loaderClass + .asSubclass(AdsLoader.class) + .getConstructor(android.content.Context.class, android.net.Uri.class); + // LINT.ThenChange(../../../../../../../../proguard-rules.txt) + adsLoader = loaderConstructor.newInstance(this, adTagUri); + adUiViewGroup = new FrameLayout(this); + // The demo app has a non-null overlay frame layout. + playerView.getOverlayFrameLayout().addView(adUiViewGroup); + } + AdsMediaSource.MediaSourceFactory adMediaSourceFactory = + new AdsMediaSource.MediaSourceFactory() { + @Override + public MediaSource createMediaSource( + Uri uri, @Nullable Handler handler, @Nullable MediaSourceEventListener listener) { + return PlayerActivity.this.buildMediaSource( + uri, /* overrideExtension= */ null, handler, listener); + } - @Override - public int[] getSupportedTypes() { - return new int[] {C.TYPE_DASH, C.TYPE_SS, C.TYPE_HLS, C.TYPE_OTHER}; - } - }; - return new AdsMediaSource( - mediaSource, adMediaSourceFactory, adsLoader, adUiViewGroup, mainHandler, eventLogger); + @Override + public int[] getSupportedTypes() { + return new int[] {C.TYPE_DASH, C.TYPE_SS, C.TYPE_HLS, C.TYPE_OTHER}; + } + }; + return new AdsMediaSource( + mediaSource, adMediaSourceFactory, adsLoader, adUiViewGroup, mainHandler, eventLogger); + } catch (ClassNotFoundException e) { + // IMA extension not loaded. + return null; + } catch (Exception e) { + throw new RuntimeException(e); + } } private void releaseAdsLoader() { diff --git a/demos/main/src/main/proguard-rules.txt b/demos/main/src/main/proguard-rules.txt new file mode 100644 index 0000000000..de5e0a4203 --- /dev/null +++ b/demos/main/src/main/proguard-rules.txt @@ -0,0 +1,6 @@ +# Proguard rules specific to the main demo app. + +# Constructor accessed via reflection in PlayerActivity +-keepclassmembers class com.google.android.exoplayer2.ext.ima.ImaAdsLoader { + (android.content.Context, android.net.Uri); +} diff --git a/extensions/ima/proguard-rules.txt b/extensions/ima/proguard-rules.txt new file mode 100644 index 0000000000..feef3daf7a --- /dev/null +++ b/extensions/ima/proguard-rules.txt @@ -0,0 +1,6 @@ +# Proguard rules specific to the IMA extension. + +-keep class com.google.ads.interactivemedia.** { *; } +-keep interface com.google.ads.interactivemedia.** { *; } +-keep class com.google.obf.** { *; } +-keep interface com.google.obf.** { *; } diff --git a/extensions/ima/src/main/proguard-rules.txt b/extensions/ima/src/main/proguard-rules.txt new file mode 100644 index 0000000000..feef3daf7a --- /dev/null +++ b/extensions/ima/src/main/proguard-rules.txt @@ -0,0 +1,6 @@ +# Proguard rules specific to the IMA extension. + +-keep class com.google.ads.interactivemedia.** { *; } +-keep interface com.google.ads.interactivemedia.** { *; } +-keep class com.google.obf.** { *; } +-keep interface com.google.obf.** { *; } diff --git a/library/core/build.gradle b/library/core/build.gradle index 6d7394371d..a87e11065f 100644 --- a/library/core/build.gradle +++ b/library/core/build.gradle @@ -21,6 +21,7 @@ android { defaultConfig { minSdkVersion project.ext.minSdkVersion targetSdkVersion project.ext.targetSdkVersion + consumerProguardFiles 'proguard-rules.txt' } // Workaround to prevent circular dependency on project :testutils. diff --git a/library/core/proguard-rules.txt b/library/core/proguard-rules.txt new file mode 100644 index 0000000000..9eefd64d62 --- /dev/null +++ b/library/core/proguard-rules.txt @@ -0,0 +1,25 @@ +# Proguard rules specific to the core module. + +# Constructors accessed via reflection in DefaultRenderersFactory +-keepclassmembers class com.google.android.exoplayer2.ext.vp9.LibvpxVideoRenderer { + (boolean, long, android.os.Handler, com.google.android.exoplayer2.video.VideoRendererEventListener, int); +} +-keepclassmembers class com.google.android.exoplayer2.ext.opus.LibopusAudioRenderer { + (android.os.Handler, com.google.android.exoplayer2.audio.AudioRendererEventListener, com.google.android.exoplayer2.audio.AudioProcessor[]); +} +-keepclassmembers class com.google.android.exoplayer2.ext.flac.LibflacAudioRenderer { + (android.os.Handler, com.google.android.exoplayer2.audio.AudioRendererEventListener, com.google.android.exoplayer2.audio.AudioProcessor[]); +} +-keepclassmembers class com.google.android.exoplayer2.ext.ffmpeg.FfmpegAudioRenderer { + (android.os.Handler, com.google.android.exoplayer2.audio.AudioRendererEventListener, com.google.android.exoplayer2.audio.AudioProcessor[]); +} + +# Constructors accessed via reflection in DefaultExtractorsFactory +-keepclassmembers class com.google.android.exoplayer2.ext.flac.FlacExtractor { + (); +} + +# Constructors accessed via reflection in DefaultDataSource +-keepclassmembers class com.google.android.exoplayer2.ext.rtmp.RtmpDataSource { + (); +} diff --git a/library/core/src/main/java/com/google/android/exoplayer2/DefaultRenderersFactory.java b/library/core/src/main/java/com/google/android/exoplayer2/DefaultRenderersFactory.java index 2272306117..16074108b1 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/DefaultRenderersFactory.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/DefaultRenderersFactory.java @@ -185,18 +185,32 @@ public class DefaultRenderersFactory implements RenderersFactory { } try { - Class clazz = - Class.forName("com.google.android.exoplayer2.ext.vp9.LibvpxVideoRenderer"); - Constructor constructor = clazz.getConstructor(boolean.class, long.class, Handler.class, - VideoRendererEventListener.class, int.class); - Renderer renderer = (Renderer) constructor.newInstance(true, allowedVideoJoiningTimeMs, - eventHandler, eventListener, MAX_DROPPED_VIDEO_FRAME_COUNT_TO_NOTIFY); + // Full class names used for constructor args so the LINT rule triggers if any of them move. + // LINT.IfChange + Class clazz = Class.forName("com.google.android.exoplayer2.ext.vp9.LibvpxVideoRenderer"); + Constructor constructor = + clazz.getConstructor( + boolean.class, + long.class, + android.os.Handler.class, + com.google.android.exoplayer2.video.VideoRendererEventListener.class, + int.class); + // LINT.ThenChange(../../../../../../../proguard-rules.txt) + Renderer renderer = + (Renderer) + constructor.newInstance( + true, + allowedVideoJoiningTimeMs, + eventHandler, + eventListener, + MAX_DROPPED_VIDEO_FRAME_COUNT_TO_NOTIFY); out.add(extensionRendererIndex++, renderer); Log.i(TAG, "Loaded LibvpxVideoRenderer."); } catch (ClassNotFoundException e) { // Expected if the app was built without the extension. } catch (Exception e) { - throw new RuntimeException(e); + // The extension is present, but instantiation failed. + throw new RuntimeException("Error instantiating VP9 extension", e); } } @@ -230,48 +244,67 @@ public class DefaultRenderersFactory implements RenderersFactory { } try { - Class clazz = - Class.forName("com.google.android.exoplayer2.ext.opus.LibopusAudioRenderer"); - Constructor constructor = clazz.getConstructor(Handler.class, - AudioRendererEventListener.class, AudioProcessor[].class); - Renderer renderer = (Renderer) constructor.newInstance(eventHandler, eventListener, - audioProcessors); + // Full class names used for constructor args so the LINT rule triggers if any of them move. + // LINT.IfChange + Class clazz = Class.forName("com.google.android.exoplayer2.ext.opus.LibopusAudioRenderer"); + Constructor constructor = + clazz.getConstructor( + android.os.Handler.class, + com.google.android.exoplayer2.audio.AudioRendererEventListener.class, + com.google.android.exoplayer2.audio.AudioProcessor[].class); + // LINT.ThenChange(../../../../../../../proguard-rules.txt) + Renderer renderer = + (Renderer) constructor.newInstance(eventHandler, eventListener, audioProcessors); out.add(extensionRendererIndex++, renderer); Log.i(TAG, "Loaded LibopusAudioRenderer."); } catch (ClassNotFoundException e) { // Expected if the app was built without the extension. } catch (Exception e) { - throw new RuntimeException(e); + // The extension is present, but instantiation failed. + throw new RuntimeException("Error instantiating Opus extension", e); } try { - Class clazz = - Class.forName("com.google.android.exoplayer2.ext.flac.LibflacAudioRenderer"); - Constructor constructor = clazz.getConstructor(Handler.class, - AudioRendererEventListener.class, AudioProcessor[].class); - Renderer renderer = (Renderer) constructor.newInstance(eventHandler, eventListener, - audioProcessors); + // Full class names used for constructor args so the LINT rule triggers if any of them move. + // LINT.IfChange + Class clazz = Class.forName("com.google.android.exoplayer2.ext.flac.LibflacAudioRenderer"); + Constructor constructor = + clazz.getConstructor( + android.os.Handler.class, + com.google.android.exoplayer2.audio.AudioRendererEventListener.class, + com.google.android.exoplayer2.audio.AudioProcessor[].class); + // LINT.ThenChange(../../../../../../../proguard-rules.txt) + Renderer renderer = + (Renderer) constructor.newInstance(eventHandler, eventListener, audioProcessors); out.add(extensionRendererIndex++, renderer); Log.i(TAG, "Loaded LibflacAudioRenderer."); } catch (ClassNotFoundException e) { // Expected if the app was built without the extension. } catch (Exception e) { - throw new RuntimeException(e); + // The extension is present, but instantiation failed. + throw new RuntimeException("Error instantiating FLAC extension", e); } try { + // Full class names used for constructor args so the LINT rule triggers if any of them move. + // LINT.IfChange Class clazz = Class.forName("com.google.android.exoplayer2.ext.ffmpeg.FfmpegAudioRenderer"); - Constructor constructor = clazz.getConstructor(Handler.class, - AudioRendererEventListener.class, AudioProcessor[].class); - Renderer renderer = (Renderer) constructor.newInstance(eventHandler, eventListener, - audioProcessors); + Constructor constructor = + clazz.getConstructor( + android.os.Handler.class, + com.google.android.exoplayer2.audio.AudioRendererEventListener.class, + com.google.android.exoplayer2.audio.AudioProcessor[].class); + // LINT.ThenChange(../../../../../../../proguard-rules.txt) + Renderer renderer = + (Renderer) constructor.newInstance(eventHandler, eventListener, audioProcessors); out.add(extensionRendererIndex++, renderer); Log.i(TAG, "Loaded FfmpegAudioRenderer."); } catch (ClassNotFoundException e) { // Expected if the app was built without the extension. } catch (Exception e) { - throw new RuntimeException(e); + // The extension is present, but instantiation failed. + throw new RuntimeException("Error instantiating FFmpeg extension", e); } } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/extractor/DefaultExtractorsFactory.java b/library/core/src/main/java/com/google/android/exoplayer2/extractor/DefaultExtractorsFactory.java index 87165e7a9b..b85ecba3a4 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/extractor/DefaultExtractorsFactory.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/extractor/DefaultExtractorsFactory.java @@ -55,13 +55,17 @@ public final class DefaultExtractorsFactory implements ExtractorsFactory { static { Constructor flacExtractorConstructor = null; try { + // LINT.IfChange flacExtractorConstructor = Class.forName("com.google.android.exoplayer2.ext.flac.FlacExtractor") - .asSubclass(Extractor.class).getConstructor(); + .asSubclass(Extractor.class) + .getConstructor(); + // LINT.ThenChange(../../../../../../../../proguard-rules.txt) } catch (ClassNotFoundException e) { - // Extractor not found. - } catch (NoSuchMethodException e) { - // Constructor not found. + // Expected if the app was built without the FLAC extension. + } catch (Exception e) { + // The FLAC extension is present, but instantiation failed. + throw new RuntimeException("Error instantiating FLAC extension", e); } FLAC_EXTRACTOR_CONSTRUCTOR = flacExtractorConstructor; } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultDataSource.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultDataSource.java index 0542b6ce8c..b5469db72e 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultDataSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultDataSource.java @@ -21,7 +21,6 @@ import android.util.Log; import com.google.android.exoplayer2.util.Assertions; import com.google.android.exoplayer2.util.Util; import java.io.IOException; -import java.lang.reflect.InvocationTargetException; /** * A {@link DataSource} that supports multiple URI schemes. The supported schemes are: @@ -193,18 +192,16 @@ public final class DefaultDataSource implements DataSource { private DataSource getRtmpDataSource() { if (rtmpDataSource == null) { try { + // LINT.IfChange Class clazz = Class.forName("com.google.android.exoplayer2.ext.rtmp.RtmpDataSource"); - rtmpDataSource = (DataSource) clazz.getDeclaredConstructor().newInstance(); + rtmpDataSource = (DataSource) clazz.getConstructor().newInstance(); + // LINT.ThenChange(../../../../../../../../proguard-rules.txt) } catch (ClassNotFoundException e) { + // Expected if the app was built without the RTMP extension. Log.w(TAG, "Attempting to play RTMP stream without depending on the RTMP extension"); - } catch (InstantiationException e) { - Log.e(TAG, "Error instantiating RtmpDataSource", e); - } catch (IllegalAccessException e) { - Log.e(TAG, "Error instantiating RtmpDataSource", e); - } catch (NoSuchMethodException e) { - Log.e(TAG, "Error instantiating RtmpDataSource", e); - } catch (InvocationTargetException e) { - Log.e(TAG, "Error instantiating RtmpDataSource", e); + } catch (Exception e) { + // The RTMP extension is present, but instantiation failed. + throw new RuntimeException("Error instantiating RTMP extension", e); } if (rtmpDataSource == null) { rtmpDataSource = baseDataSource;