Bug fix - Fix crash reusing banner and native ad objects on Android (#64)

jjliu15 · web-flow · commit 025af72a3cfc · 2021-03-09T16:06:58.000-08:00
* Only destroy native and banner ads from Flutter dispose().

* Add a page to the example app which only uses one ad object for native and banner ads
diff --git a/packages/google_mobile_ads/CHANGELOG.md b/packages/google_mobile_ads/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 0.11.0+3
+
+* Fixes an [Android crash](https://github.com/googleads/googleads-mobile-flutter/issues/46) when reusing Native and Banner Ad objects
+
 ## 0.11.0+2
 
 * Set min Android version to `19`.
diff --git a/packages/google_mobile_ads/android/src/main/java/io/flutter/plugins/googlemobileads/AdInstanceManager.java b/packages/google_mobile_ads/android/src/main/java/io/flutter/plugins/googlemobileads/AdInstanceManager.java
@@ -69,6 +69,13 @@ void trackAd(@NonNull FlutterAd ad, int adId) {
   }
 
   void disposeAd(int adId) {
+    if (!ads.containsKey(adId)) {
+      return;
+    }
+    Object adObject = ads.get(adId);
+    if (adObject instanceof FlutterDestroyableAd) {
+      ((FlutterDestroyableAd) adObject).destroy();
+    }
     ads.remove(adId);
   }
 
diff --git a/packages/google_mobile_ads/android/src/main/java/io/flutter/plugins/googlemobileads/Constants.java b/packages/google_mobile_ads/android/src/main/java/io/flutter/plugins/googlemobileads/Constants.java
@@ -17,5 +17,5 @@
 /** Constants used in the plugin. */
 public class Constants {
   /** Version request agent. Should be bumped alongside plugin versions. */
-  public static final String REQUEST_AGENT_PREFIX_VERSIONED = "Flutter-GMA-0.11.0+2";
+  public static final String REQUEST_AGENT_PREFIX_VERSIONED = "Flutter-GMA-0.11.0+3";
 }
diff --git a/packages/google_mobile_ads/android/src/main/java/io/flutter/plugins/googlemobileads/FlutterBannerAd.java b/packages/google_mobile_ads/android/src/main/java/io/flutter/plugins/googlemobileads/FlutterBannerAd.java
@@ -20,7 +20,7 @@
 import com.google.android.gms.ads.AdView;
 import io.flutter.plugin.platform.PlatformView;
 
-class FlutterBannerAd extends FlutterAd implements PlatformView {
+class FlutterBannerAd extends FlutterAd implements PlatformView, FlutterDestroyableAd {
   @NonNull private final AdInstanceManager manager;
   @NonNull private final String adUnitId;
   @NonNull private final FlutterAdSize size;
@@ -97,6 +97,13 @@ public View getView() {
 
   @Override
   public void dispose() {
+    // Do nothing. Cleanup is handled in destroy() below, which is triggered from dispose() being
+    // called on the flutter ad object. This is allows for reuse of the  ad view, for example
+    // in a scrolling list view.
+  }
+
+  @Override
+  public void destroy() {
     if (view != null) {
       view.destroy();
       view = null;
diff --git a/packages/google_mobile_ads/android/src/main/java/io/flutter/plugins/googlemobileads/FlutterDestroyableAd.java b/packages/google_mobile_ads/android/src/main/java/io/flutter/plugins/googlemobileads/FlutterDestroyableAd.java
@@ -0,0 +1,8 @@
+package io.flutter.plugins.googlemobileads;
+
+/** Interface for a destroyable ad. */
+public interface FlutterDestroyableAd {
+
+  /** Destroy any ads and free up references. */
+  void destroy();
+}
diff --git a/packages/google_mobile_ads/android/src/main/java/io/flutter/plugins/googlemobileads/FlutterNativeAd.java b/packages/google_mobile_ads/android/src/main/java/io/flutter/plugins/googlemobileads/FlutterNativeAd.java
@@ -27,7 +27,7 @@
 import io.flutter.plugins.googlemobileads.GoogleMobileAdsPlugin.NativeAdFactory;
 import java.util.Map;
 
-class FlutterNativeAd extends FlutterAd implements PlatformView {
+class FlutterNativeAd extends FlutterAd implements PlatformView, FlutterDestroyableAd {
   private static final String TAG = "FlutterNativeAd";
 
   @NonNull private final AdInstanceManager manager;
@@ -141,6 +141,13 @@ public View getView() {
 
   @Override
   public void dispose() {
+    // Do nothing. Cleanup is handled in destroy() below, which is triggered from dispose() being
+    // called on the flutter ad object. This is allows for reuse of the ad view, for example
+    // in a scrolling list view.
+  }
+
+  @Override
+  public void destroy() {
     if (ad != null) {
       ad.destroy();
       ad = null;
diff --git a/packages/google_mobile_ads/android/src/main/java/io/flutter/plugins/googlemobileads/FlutterPublisherBannerAd.java b/packages/google_mobile_ads/android/src/main/java/io/flutter/plugins/googlemobileads/FlutterPublisherBannerAd.java
@@ -27,7 +27,7 @@
  * Wrapper around {@link com.google.android.gms.ads.doubleclick.PublisherAdView} for the Google
  * Mobile Ads Plugin.
  */
-class FlutterPublisherBannerAd extends FlutterAd implements PlatformView {
+class FlutterPublisherBannerAd extends FlutterAd implements PlatformView, FlutterDestroyableAd {
   @NonNull private final AdInstanceManager manager;
   @NonNull private final String adUnitId;
   @NonNull private final List<FlutterAdSize> sizes;
@@ -126,10 +126,16 @@ public View getView() {
 
   @Override
   public void dispose() {
+    // Do nothing. Cleanup is handled in destroy() below, which is triggered from dispose() being
+    // called on the flutter ad object. This is allows for reuse of the ad view, for example
+    // in a scrolling list view.
+  }
+
+  @Override
+  public void destroy() {
     if (view != null) {
       view.destroy();
       view = null;
     }
-    // Do nothing.
   }
 }
diff --git a/packages/google_mobile_ads/android/src/test/java/io/flutter/plugins/googlemobileads/GoogleMobileAdsTest.java b/packages/google_mobile_ads/android/src/test/java/io/flutter/plugins/googlemobileads/GoogleMobileAdsTest.java
@@ -46,6 +46,7 @@
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
 import org.mockito.ArgumentMatchers;
+import org.mockito.Mockito;
 
 public class GoogleMobileAdsTest {
   private AdInstanceManager testManager;
@@ -229,22 +230,41 @@ public void showRewardedAd() {
   }
 
   @Test
-  public void disposeAd() {
-    final FlutterBannerAd bannerAd =
-        new FlutterBannerAd.Builder()
-            .setManager(testManager)
-            .setAdUnitId("testId")
-            .setSize(new FlutterAdSize(1, 2))
-            .setRequest(request)
-            .build();
+  public void disposeAd_banner() {
+    FlutterBannerAd bannerAd = Mockito.mock(FlutterBannerAd.class);
     testManager.trackAd(bannerAd, 2);
     assertNotNull(testManager.adForId(2));
     assertNotNull(testManager.adIdFor(bannerAd));
     testManager.disposeAd(2);
+    verify(bannerAd).destroy();
     assertNull(testManager.adForId(2));
     assertNull(testManager.adIdFor(bannerAd));
   }
 
+  @Test
+  public void disposeAd_publisherBanner() {
+    FlutterPublisherBannerAd publisherBannerAd = Mockito.mock(FlutterPublisherBannerAd.class);
+    testManager.trackAd(publisherBannerAd, 2);
+    assertNotNull(testManager.adForId(2));
+    assertNotNull(testManager.adIdFor(publisherBannerAd));
+    testManager.disposeAd(2);
+    verify(publisherBannerAd).destroy();
+    assertNull(testManager.adForId(2));
+    assertNull(testManager.adIdFor(publisherBannerAd));
+  }
+
+  @Test
+  public void disposeAd_native() {
+    FlutterNativeAd flutterNativeAd = Mockito.mock(FlutterNativeAd.class);
+    testManager.trackAd(flutterNativeAd, 2);
+    assertNotNull(testManager.adForId(2));
+    assertNotNull(testManager.adIdFor(flutterNativeAd));
+    testManager.disposeAd(2);
+    verify(flutterNativeAd).destroy();
+    assertNull(testManager.adForId(2));
+    assertNull(testManager.adIdFor(flutterNativeAd));
+  }
+
   @Test
   public void adMessageCodec_encodeFlutterAdSize() {
     final AdMessageCodec codec = new AdMessageCodec();
diff --git a/packages/google_mobile_ads/example/lib/constants.dart b/packages/google_mobile_ads/example/lib/constants.dart
@@ -0,0 +1,14 @@
+/// Constants used in the example app.
+class Constants {
+  /// Placeholder text.
+  static const String placeholderText =
+      'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod'
+      ' tempor incididunt ut labore et dolore magna aliqua. Faucibus purus in'
+      ' massa tempor. Quis enim lobortis scelerisque fermentum dui faucibus'
+      ' in. Nibh praesent tristique magna sit amet purus gravida quis.'
+      ' Magna sit amet purus gravida quis blandit turpis cursus in. Sed'
+      ' adipiscing diam donec adipiscing tristique. Urna porttitor rhoncus'
+      ' dolor purus non enim praesent. Pellentesque habitant morbi tristique'
+      ' senectus et netus. Risus ultricies tristique nulla aliquet enim tortor'
+      ' at.';
+}
diff --git a/packages/google_mobile_ads/example/lib/main.dart b/packages/google_mobile_ads/example/lib/main.dart
@@ -19,6 +19,9 @@ import 'dart:async';
 import 'package:flutter/material.dart';
 import 'package:google_mobile_ads/google_mobile_ads.dart';
 
+import 'constants.dart';
+import 'reusable_inline_example.dart';
+
 // You can also test with your own ad unit IDs by registering your device as a
 // test device. Check the logs for your device's ID value.
 const String testDevice = 'YOUR_DEVICE_ID';
@@ -36,28 +39,6 @@ class _MyAppState extends State<MyApp> {
     nonPersonalizedAds: true,
   );
 
-  final List<String> _article = <String>[
-    'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod'
-        ' tempor incididunt ut labore et dolore magna aliqua. Faucibus purus in'
-        ' massa tempor. Quis enim lobortis scelerisque fermentum dui faucibus'
-        ' in. Nibh praesent tristique magna sit amet purus gravida quis.'
-        ' Magna sit amet purus gravida quis blandit turpis cursus in. Sed'
-        ' adipiscing diam donec adipiscing tristique. Urna porttitor rhoncus'
-        ' dolor purus non enim praesent. Pellentesque habitant morbi tristique'
-        ' senectus et netus. Risus ultricies tristique nulla aliquet enim tortor'
-        ' at.',
-    'Eget dolor morbi non arcu. Nec sagittis aliquam malesuada bibendum. Nec'
-        ' feugiat nisl pretium fusce id velit ut. Volutpat commodo sed egestas'
-        ' egestas. Ultrices neque ornare aenean euismod elementum. Consequat'
-        ' semper viverra nam libero justo laoreet sit amet cursus. Fusce ut'
-        ' placerat orci nulla pellentesque dignissim. Justo donec enim diam'
-        ' vulputate ut pharetra sit amet.',
-    'Et malesuada fames ac turpis egestas maecenas. Augue ut lectus arcu'
-        ' bibendum at varius. Mauris rhoncus aenean vel elit scelerisque mauris'
-        ' pellentesque pulvinar. Faucibus a pellentesque sit amet porttitor'
-        ' eget dolor.'
-  ];
-
   InterstitialAd _interstitialAd;
   bool _interstitialReady = false;
 
@@ -148,69 +129,84 @@ class _MyAppState extends State<MyApp> {
   @override
   Widget build(BuildContext context) {
     return MaterialApp(
-      home: Scaffold(
-        appBar: AppBar(
-          title: const Text('AdMob Plugin example app'),
-          actions: <Widget>[
-            PopupMenuButton<String>(
-              onSelected: (String result) {
-                assert(result == 'InterstitialAd' || result == 'RewardedAd');
-                switch (result) {
-                  case 'InterstitialAd':
-                    if (!_interstitialReady) return;
-                    _interstitialAd.show();
-                    _interstitialReady = false;
-                    _interstitialAd = null;
-                    break;
-                  case 'RewardedAd':
-                    if (!_rewardedReady) return;
-                    _rewardedAd.show();
-                    _rewardedReady = false;
-                    _rewardedAd = null;
+      home: Builder(
+        builder: (context) => Scaffold(
+          appBar: AppBar(
+            title: const Text('AdMob Plugin example app'),
+            actions: <Widget>[
+              PopupMenuButton<String>(
+                onSelected: (String result) {
+                  switch (result) {
+                    case 'InterstitialAd':
+                      if (!_interstitialReady) return;
+                      _interstitialAd.show();
+                      _interstitialReady = false;
+                      _interstitialAd = null;
+                      break;
+                    case 'RewardedAd':
+                      if (!_rewardedReady) return;
+                      _rewardedAd.show();
+                      _rewardedReady = false;
+                      _rewardedAd = null;
+                      break;
+                    case 'ReusableInlineExample':
+                      Navigator.push(
+                        context,
+                        MaterialPageRoute(
+                            builder: (context) => ReusableInlineExample()),
+                      );
+                      break;
+                    default:
+                      throw AssertionError('unexpected button: ${result}');
+                  }
+                },
+                itemBuilder: (BuildContext context) => <PopupMenuEntry<String>>[
+                  PopupMenuItem<String>(
+                    child: Text('$InterstitialAd'),
+                    value: '$InterstitialAd',
+                  ),
+                  PopupMenuItem<String>(
+                    child: Text('$RewardedAd'),
+                    value: '$RewardedAd',
+                  ),
+                  PopupMenuItem<String>(
+                    child: Text('Reusable Inline Ads Object Example'),
+                    value: 'ReusableInlineExample',
+                  ),
+                ],
+              ),
+            ],
+          ),
+          body: Padding(
+            padding: const EdgeInsets.symmetric(horizontal: 16.0),
+            child: ListView.separated(
+              cacheExtent: 500,
+              itemCount: 100,
+              separatorBuilder: (BuildContext context, int index) {
+                return Container(
+                  height: 40,
+                );
+              },
+              itemBuilder: (BuildContext context, int index) {
+                if (index % 2 == 0) {
+                  return Text(
+                    Constants.placeholderText,
+                    style: TextStyle(fontSize: 24),
+                  );
                 }
+
+                final int adIndex = (index / 2).floor();
+                Widget adWidget;
+                if (adIndex % 3 == 0) {
+                  adWidget = BannerAdWidget(AdSize.banner);
+                } else if (adIndex % 3 == 1) {
+                  adWidget = PublisherBannerAdWidget(AdSize.largeBanner);
+                } else {
+                  adWidget = NativeAdWidget();
+                }
+                return Center(child: adWidget);
               },
-              itemBuilder: (BuildContext context) => <PopupMenuEntry<String>>[
-                PopupMenuItem<String>(
-                  child: Text('$InterstitialAd'),
-                  value: '$InterstitialAd',
-                ),
-                PopupMenuItem<String>(
-                  child: Text('$RewardedAd'),
-                  value: '$RewardedAd',
-                ),
-              ],
             ),
-          ],
-        ),
-        body: Padding(
-          padding: const EdgeInsets.symmetric(horizontal: 16.0),
-          child: ListView.separated(
-            cacheExtent: 500,
-            itemCount: 100,
-            separatorBuilder: (BuildContext context, int index) {
-              return Container(
-                height: 40,
-              );
-            },
-            itemBuilder: (BuildContext context, int index) {
-              if (index % 2 == 0) {
-                return Text(
-                  _article[(index / 2).ceil() % 3],
-                  style: TextStyle(fontSize: 24),
-                );
-              }
-
-              final int adIndex = (index / 2).floor();
-              Widget adWidget;
-              if (adIndex % 3 == 0) {
-                adWidget = BannerAdWidget(AdSize.banner);
-              } else if (adIndex % 3 == 1) {
-                adWidget = PublisherBannerAdWidget(AdSize.largeBanner);
-              } else {
-                adWidget = NativeAdWidget();
-              }
-              return Center(child: adWidget);
-            },
           ),
         ),
       ),
diff --git a/packages/google_mobile_ads/example/lib/reusable_inline_example.dart b/packages/google_mobile_ads/example/lib/reusable_inline_example.dart
diff --git a/packages/google_mobile_ads/ios/Classes/FLTConstants.h b/packages/google_mobile_ads/ios/Classes/FLTConstants.h
diff --git a/packages/google_mobile_ads/pubspec.yaml b/packages/google_mobile_ads/pubspec.yaml

Original file line number	Diff line number	Diff line change
`@@ -69,6 +69,13 @@ void trackAd(@NonNull FlutterAd ad, int adId) {`
`69`	`69`	`}`
`70`	`70`
`71`	`71`	`void disposeAd(int adId) {`
	`72`	`+ if (!ads.containsKey(adId)) {`
	`73`	`+ return;`
	`74`	`+ }`
	`75`	`+ Object adObject = ads.get(adId);`
	`76`	`+ if (adObject instanceof FlutterDestroyableAd) {`
	`77`	`+ ((FlutterDestroyableAd) adObject).destroy();`
	`78`	`+ }`
`72`	`79`	`ads.remove(adId);`
`73`	`80`	`}`
`74`	`81`
Original file line number	Diff line number	Diff line change
`@@ -17,5 +17,5 @@`
`17`	`17`	`/** Constants used in the plugin. */`
`18`	`18`	`public class Constants {`
`19`	`19`	`/** Version request agent. Should be bumped alongside plugin versions. */`
`20`		`- public static final String REQUEST_AGENT_PREFIX_VERSIONED = "Flutter-GMA-0.11.0+2";`
	`20`	`+ public static final String REQUEST_AGENT_PREFIX_VERSIONED = "Flutter-GMA-0.11.0+3";`
`21`	`21`	`}`