From a1ef41e59d48c36790c0d087cf942eeba367de77 Mon Sep 17 00:00:00 2001 From: Collin Jackson Date: Mon, 26 Aug 2019 08:42:55 -0700 Subject: [PATCH 1/2] [firebase_admob] Fixes memory leak by removing static reference for Activities --- .../firebaseadmob/FirebaseAdMobPlugin.java | 22 +++++++++++++------ .../plugins/firebaseadmob/MobileAd.java | 16 +++++++++----- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/packages/firebase_admob/android/src/main/java/io/flutter/plugins/firebaseadmob/FirebaseAdMobPlugin.java b/packages/firebase_admob/android/src/main/java/io/flutter/plugins/firebaseadmob/FirebaseAdMobPlugin.java index 62e6fe4befeb..826cf71ce93b 100644 --- a/packages/firebase_admob/android/src/main/java/io/flutter/plugins/firebaseadmob/FirebaseAdMobPlugin.java +++ b/packages/firebase_admob/android/src/main/java/io/flutter/plugins/firebaseadmob/FirebaseAdMobPlugin.java @@ -20,6 +20,7 @@ public class FirebaseAdMobPlugin implements MethodCallHandler { private final Registrar registrar; private final MethodChannel channel; + private final MobileAdRegistrar adRegistrar; RewardedVideoAdWrapper rewardedWrapper; @@ -31,12 +32,17 @@ public static void registerWith(Registrar registrar) { } final MethodChannel channel = new MethodChannel(registrar.messenger(), "plugins.flutter.io/firebase_admob"); - channel.setMethodCallHandler(new FirebaseAdMobPlugin(registrar, channel)); + + final MobileAdRegistrar adRegistry = new MobileAdRegistrar(); + + channel.setMethodCallHandler(new FirebaseAdMobPlugin(registrar, channel, adRegistry)); } - private FirebaseAdMobPlugin(Registrar registrar, MethodChannel channel) { + private FirebaseAdMobPlugin( + Registrar registrar, MethodChannel channel, MobileAdRegistrar adRegistrar) { this.registrar = registrar; this.channel = channel; + this.adRegistrar = adRegistrar; FirebaseApp.initializeApp(registrar.context()); rewardedWrapper = new RewardedVideoAdWrapper(registrar.activity(), channel); } @@ -84,7 +90,7 @@ private void callLoadBannerAd( adSize = new AdSize(width, height); } - MobileAd.Banner banner = MobileAd.createBanner(id, adSize, activity, channel); + MobileAd.Banner banner = adRegistrar.createBanner(id, adSize, activity, channel); if (banner.status != MobileAd.Status.CREATED) { if (banner.status == MobileAd.Status.FAILED) @@ -143,7 +149,7 @@ private void callLoadRewardedVideoAd(MethodCall call, Result result) { } private void callShowAd(int id, MethodCall call, Result result) { - MobileAd ad = MobileAd.getAdForId(id); + MobileAd ad = adRegistrar.getAdForId(id); if (ad == null) { result.error("ad_not_loaded", "show failed, the specified ad was not loaded id=" + id, null); return; @@ -164,7 +170,8 @@ private void callShowAd(int id, MethodCall call, Result result) { } private void callIsAdLoaded(int id, MethodCall call, Result result) { - MobileAd ad = MobileAd.getAdForId(id); + MobileAd ad = adRegistrar.getAdForId(id); + if (ad == null) { result.error("no_ad_for_id", "isAdLoaded failed, no add exists for id=" + id, null); return; @@ -182,7 +189,8 @@ private void callShowRewardedVideoAd(MethodCall call, Result result) { } private void callDisposeAd(int id, MethodCall call, Result result) { - MobileAd ad = MobileAd.getAdForId(id); + MobileAd ad = adRegistrar.getAdForId(id); + if (ad == null) { result.error("no_ad_for_id", "dispose failed, no add exists for id=" + id, null); return; @@ -212,7 +220,7 @@ public void onMethodCall(MethodCall call, Result result) { callLoadBannerAd(id, activity, channel, call, result); break; case "loadInterstitialAd": - callLoadInterstitialAd(MobileAd.createInterstitial(id, activity, channel), call, result); + callLoadInterstitialAd(adRegistrar.createInterstitial(id, activity, channel), call, result); break; case "loadRewardedVideoAd": callLoadRewardedVideoAd(call, result); diff --git a/packages/firebase_admob/android/src/main/java/io/flutter/plugins/firebaseadmob/MobileAd.java b/packages/firebase_admob/android/src/main/java/io/flutter/plugins/firebaseadmob/MobileAd.java index a13573194c94..5d6f2e70cbe1 100644 --- a/packages/firebase_admob/android/src/main/java/io/flutter/plugins/firebaseadmob/MobileAd.java +++ b/packages/firebase_admob/android/src/main/java/io/flutter/plugins/firebaseadmob/MobileAd.java @@ -6,7 +6,6 @@ import android.app.Activity; import android.util.Log; -import android.util.SparseArray; import android.view.Gravity; import android.view.View; import android.view.ViewGroup; @@ -73,7 +72,7 @@ Status getStatus() { abstract void show(); void dispose() { - allAds.remove(id); + adRegistrar.unregister(id); } private Map argumentsMap(Object... args) { @@ -127,8 +126,13 @@ static class Banner extends MobileAd { private AdView adView; private AdSize adSize; - private Banner(Integer id, AdSize adSize, Activity activity, MethodChannel channel) { - super(id, activity, channel); + Banner( + MobileAdRegistrar adRegistrar, + Integer id, + AdSize adSize, + Activity activity, + MethodChannel channel) { + super(adRegistrar, id, activity, channel); this.adSize = adSize; } @@ -195,8 +199,8 @@ void dispose() { static class Interstitial extends MobileAd { private InterstitialAd interstitial = null; - private Interstitial(int id, Activity activity, MethodChannel channel) { - super(id, activity, channel); + Interstitial(MobileAdRegistrar adRegistrar, int id, Activity activity, MethodChannel channel) { + super(adRegistrar, id, activity, channel); } @Override From 749c421671b2f4fc9af6b24f313bb4fbc74d899d Mon Sep 17 00:00:00 2001 From: Collin Jackson Date: Mon, 26 Aug 2019 08:48:00 -0700 Subject: [PATCH 2/2] Manually merge rejected hunks --- packages/firebase_admob/CHANGELOG.md | 4 ++ .../plugins/firebaseadmob/MobileAd.java | 26 +++------- .../firebaseadmob/MobileAdRegistrar.java | 48 +++++++++++++++++++ packages/firebase_admob/pubspec.yaml | 2 +- 4 files changed, 60 insertions(+), 20 deletions(-) create mode 100644 packages/firebase_admob/android/src/main/java/io/flutter/plugins/firebaseadmob/MobileAdRegistrar.java diff --git a/packages/firebase_admob/CHANGELOG.md b/packages/firebase_admob/CHANGELOG.md index 7a2b5bf14eac..5c01284d5605 100644 --- a/packages/firebase_admob/CHANGELOG.md +++ b/packages/firebase_admob/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.9.0+6 + +* Fix Android memory leak. + ## 0.9.0+5 * Update documentation to reflect new repository location. diff --git a/packages/firebase_admob/android/src/main/java/io/flutter/plugins/firebaseadmob/MobileAd.java b/packages/firebase_admob/android/src/main/java/io/flutter/plugins/firebaseadmob/MobileAd.java index 5d6f2e70cbe1..115472f43873 100644 --- a/packages/firebase_admob/android/src/main/java/io/flutter/plugins/firebaseadmob/MobileAd.java +++ b/packages/firebase_admob/android/src/main/java/io/flutter/plugins/firebaseadmob/MobileAd.java @@ -20,15 +20,15 @@ abstract class MobileAd extends AdListener { private static final String TAG = "flutter"; - private static SparseArray allAds = new SparseArray(); + private final MethodChannel channel; + private final MobileAdRegistrar adRegistrar; final Activity activity; - final MethodChannel channel; final int id; - Status status; double anchorOffset; double horizontalCenterOffset; int anchorType; + Status status; enum Status { CREATED, @@ -38,7 +38,8 @@ enum Status { LOADED, } - private MobileAd(int id, Activity activity, MethodChannel channel) { + private MobileAd( + MobileAdRegistrar adRegistrar, int id, Activity activity, MethodChannel channel) { this.id = id; this.activity = activity; this.channel = channel; @@ -46,21 +47,8 @@ private MobileAd(int id, Activity activity, MethodChannel channel) { this.anchorOffset = 0.0; this.horizontalCenterOffset = 0.0; this.anchorType = Gravity.BOTTOM; - allAds.put(id, this); - } - - static Banner createBanner(Integer id, AdSize adSize, Activity activity, MethodChannel channel) { - MobileAd ad = getAdForId(id); - return (ad != null) ? (Banner) ad : new Banner(id, adSize, activity, channel); - } - - static Interstitial createInterstitial(Integer id, Activity activity, MethodChannel channel) { - MobileAd ad = getAdForId(id); - return (ad != null) ? (Interstitial) ad : new Interstitial(id, activity, channel); - } - - static MobileAd getAdForId(Integer id) { - return allAds.get(id); + this.adRegistrar = adRegistrar; + adRegistrar.register(id, this); } Status getStatus() { diff --git a/packages/firebase_admob/android/src/main/java/io/flutter/plugins/firebaseadmob/MobileAdRegistrar.java b/packages/firebase_admob/android/src/main/java/io/flutter/plugins/firebaseadmob/MobileAdRegistrar.java new file mode 100644 index 000000000000..f2c2dfff8736 --- /dev/null +++ b/packages/firebase_admob/android/src/main/java/io/flutter/plugins/firebaseadmob/MobileAdRegistrar.java @@ -0,0 +1,48 @@ +package io.flutter.plugins.firebaseadmob; + +import android.app.Activity; +import android.util.SparseArray; +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; +import com.google.android.gms.ads.AdSize; +import io.flutter.plugin.common.MethodChannel; + +/* Class that acts as Factory and registry for MobileAds. */ +class MobileAdRegistrar { + private SparseArray allAds = new SparseArray<>(); + + /* Returns a MobileAd if it is registered in this Registrar. */ + @Nullable + MobileAd getAdForId(Integer id) { + return allAds.get(id); + } + + /* Registers a new MobileAd with the given id. */ + void register(int id, MobileAd ad) { + allAds.put(id, ad); + } + + /* Returns an already registered Banner with the given id or creates a new Banner. */ + @NonNull + MobileAd.Banner createBanner( + Integer id, AdSize adSize, Activity activity, MethodChannel channel) { + MobileAd ad = getAdForId(id); + return (ad != null) + ? (MobileAd.Banner) ad + : new MobileAd.Banner(this, id, adSize, activity, channel); + } + + /* Returns an already registered Interstitial with the given id or creates a new Interstitial. */ + @NonNull + MobileAd.Interstitial createInterstitial(Integer id, Activity activity, MethodChannel channel) { + MobileAd ad = getAdForId(id); + return (ad != null) + ? (MobileAd.Interstitial) ad + : new MobileAd.Interstitial(this, id, activity, channel); + } + + /* Unregisters a MobileAd with the given id. */ + void unregister(Integer id) { + allAds.remove(id); + } +} diff --git a/packages/firebase_admob/pubspec.yaml b/packages/firebase_admob/pubspec.yaml index db7647bebbbe..0748194ca909 100644 --- a/packages/firebase_admob/pubspec.yaml +++ b/packages/firebase_admob/pubspec.yaml @@ -3,7 +3,7 @@ description: Flutter plugin for Firebase AdMob, supporting banner, interstitial (full-screen), and rewarded video ads author: Flutter Team homepage: https://github.com/FirebaseExtended/flutterfire/tree/master/packages/firebase_admob -version: 0.9.0+5 +version: 0.9.0+6 flutter: plugin: