googleads / googleads-mobile-flutter

A Flutter plugin for the Google Mobile Ads SDK
Apache License 2.0
344 stars 285 forks source link

AdWidget is already in the widget tree #36

Closed SarbudeenDeveloper closed 3 years ago

SarbudeenDeveloper commented 3 years ago

This error occur This AdWidget is already in the Widget tree I/flutter ( 7541): If you placed this AdWidget in a list, make sure you create a new instance in the builder function I/flutter ( 7541): with a unique ad object.

Code Looks like: ListView( children: [ AdWidget(ad: BannerAd(//Parameters)..load() as BannerAd), AdWidget(ad: BannerAd(//Parameters)..load() as BannerAd), AdWidget(ad: BannerAd(//Parameters)..load() as BannerAd), ] )

In the first time ad loads properly but when i scroll to bottom again scroll to top error occurs

figengungor commented 3 years ago

I also have similar issue, you can test this with codelab by increasing sample items. https://github.com/googlecodelabs/admob-inline-ads-in-flutter/issues/3

d-apps commented 3 years ago

I had this issue, I solved returning new instances of BannerAd objects, example below.

There are other ways to do it for example creating another stateful class like in the package's example folder.

static Future<Widget> getBannerWidget(
      {@required BuildContext context,
        AdSize adSize,
      }) async{

    BannerAd bannerAd = BannerAd(
      adUnitId: getBannerAdUnitId(),
      size: adSize ?? AdSize.smartBanner,
      request: _adRequest,
      listener: AdListener(
        // Called when an ad is successfully received.
        onAdLoaded: (Ad ad) {

          if(_debug){
            print('Ad loaded.');
          }

        },
        // Called when an ad request failed.
        onAdFailedToLoad: (Ad ad, LoadAdError error) {

          if(_debug){
            print('Ad failed to load: $error');
            //_bannerAd.dispose();
          }

        },
        // Called when an ad opens an overlay that covers the screen.
        onAdOpened: (Ad ad) {

          if(_debug){
            print('Ad opened.');
          }

        },
        // Called when an ad removes an overlay that covers the screen.
        onAdClosed: (Ad ad) {

          if(_debug){
            print('Ad closed.');
          }

        },
        // Called when an ad is in the process of leaving the application.
        onApplicationExit: (Ad ad) {

          if(_debug){
            print('Left application.');
          }

        },
      ),
    );

    await bannerAd.load();

    return Container(
        child: AdWidget(ad: bannerAd),
        constraints: BoxConstraints(
          maxHeight: 90,
          maxWidth: MediaQuery.of(context).size.width,
          minHeight: 32,
          minWidth: MediaQuery.of(context).size.width,
        ),
    );

  }

In the screen:

FutureBuilder<Widget>(
                future: Ads.getBannerWidget(
                    context: context,
                    adSize: AdSize.leaderboard
                ),
                builder: (_, snapshot){

                  if(!snapshot.hasData){

                    return Text("Loading Ad...");

                  } else {

                    return Container(
                      height: 90,
                      width: MediaQuery.of(context).size.width,
                      child: snapshot.data,
                    );

                  }

                },
              ),
mileusna commented 3 years ago

While I was using _google_mobileads closed beta I had no isssue with this, but when I switched to public _google_mobileads package I started to get this error.

It can be handeled with StatefulWidget, but it ruins UX and probably loweres the CTR of the ads since the ads will be constantly reloaded while you are scrolling your page up and down (widget will be removed from widget tree when not visible anymore, then it will be recreated when you scroll up again).

I'm looking for some other ways to implement this to avoid constant banner reloading on scroll and avoiding this error on the same time, but it really sucks!

Ads should not be implemented on this way. Imagine AdSense on webpage constantly reloading while you are scrolling the page.

d-apps commented 3 years ago

While I was using _google_mobileads closed beta I had no isssue with this, but when I switched to public _google_mobileads package I started to get this error.

It can be handeled with StatfulWidget, but it ruins UX and probably loweres the CTR of the ads since the ads will be constantly reloaded while you are scrolling your page up and down (widget will be removed from widget tree when not visible anymore, then it will be recreated when you scrool up again).

I'm looking for some other ways to implement this to avoid constant banner reloading on scroll and avoiding this error on the same time, but it really sucks!

Ads should not be implemented on this way. Imagine AdSense on webpage constantly reloading while you are scrolling the page.

You are right, if you find some way to handle this please share with me, if we could find some way like: using StatelessWidget or at least some way to make the BannerAd not reload while scrolling the screen is gonna be great, I try to do some other tests.

If it still doesn't work I guess we will have to use NativeAd instead, the configuration is not that friendly, did you try NativeAd already?

mileusna commented 3 years ago

If it still doesn't work I guess we will have to use NativeAd instead, the configuration is not that friendly, did you try NativeAd already?

Let me save you some time on implementing Native ads for handling this issue. :) They behave on the same way like banner ads in the scroll lists, which is even more anoyying. Native ads should blend into your app, but with constant reloading they loose their point.

d-apps commented 3 years ago

If it still doesn't work I guess we will have to use NativeAd instead, the configuration is not that friendly, did you try NativeAd already?

Let me save you some time on implementing Native ads for handling this issue. :) They behave on the same way like banner ads in the scroll lists, which is even more anoyying. Native ads should blend into your app, but with constant reloading they loose their point.

I found a workaround, keep the StatefulWidget alive even if we scroll adding AutomaticKeepAliveClientMixin to my StatefulWifget class.

I added this mixin and now it's not rebuilding my Ad Widget.


class _BannerAdWidgetState extends State<BannerAdWidget> with AutomaticKeepAliveClientMixin

  @override
  Widget build(BuildContext context) {

    super.build(context);

  @override
  bool get wantKeepAlive => true;
mileusna commented 3 years ago

I found a workaround, keep the StatefulWidget alive even if we scroll adding AutomaticKeepAliveClientMixin to my StatefulWifget class.

Thank you, It looks like it will do the trick. I still have to test more, but it looks promising. As I said I already implemented ads with _google_mobileads closed beta package, but now this changes in public package turns my app upside down all of a sudden.

d-apps commented 3 years ago

I found a workaround, keep the StatefulWidget alive even if we scroll adding AutomaticKeepAliveClientMixin to my StatefulWifget class.

Thank you, It looks like it will do the trick. I still have to test more, but it looks promising. As I said I already implemented ads with _google_mobileads closed beta package, but now this changes in public package turns my app upside down all of a sudden.

It might need some optimizations about memory usage but after some tests in one of my apps, scrolling a lot, the app didn't crash and ads were updating correctly, not rebuilding the widget, so far so good.

jjliu15 commented 3 years ago

@blasten This seems to be an unintended consequence of https://github.com/googleads/googleads-mobile-flutter/pull/9.

At least for banner and native ads, it does make sense that we might want to reshow an ad, for example if it's in a scrolling view. From a policy standpoint, only interstitial and rewarded ads cannot be shown multiple times.

What do you think of reverting https://github.com/googleads/googleads-mobile-flutter/pull/9 partially, so that it only applies to interstitial/rewarded?

mileusna commented 3 years ago

What do you think of reverting #9 partially, so that it only applies to interstitial/rewarded?

Lets not forget PublisherBannerAd, I guess they perform the same although I didn't test this issue with them.

It is OK to prevent adding same ad twice to widget tree, but this current bahavior with single ad in scrolling view/SliverList etc. doesn't make sense.

blasten commented 3 years ago

@jjliu15 The current logic uses the id generated below to track the platform views. A platform view (android.view.View) cannot be placed in two places at the same time. https://github.com/googleads/googleads-mobile-flutter/blob/e338904bad92a80e2bef90bc6a003c05292a9702/packages/google_mobile_ads/lib/src/ad_instance_manager.dart#L129-L130

@mileusna do you have an example?

jjliu15 commented 3 years ago

@blasten FYI this should be addressed by https://github.com/googleads/googleads-mobile-flutter/pull/37

mileusna commented 3 years ago

@blasten This is not the issue with the same ad placed in two places. This is an issue with single ad placed into scrolling view, loaded only once, placed in one spot. Scrolling the page up and down will cause Flutter error "This AdWidget is already in the Widget tree I/flutter ( 7541): If you placed this AdWidget in a list, make sure you create a new instance in the builder function I/flutter ( 7541): with a unique ad object."

But if you do that, (create a new instance in the builder of the banner/native ad each time when widget is build), then you get crappy UX with ads constantly loading and flickering while you scroll the page. This will probably cause low CTR as well, since when you scroll back, banner spot is usually empty since the new ad is loading on each widget rebuilt, etc.

This issue did not exist in closed beta which I used and it worked as expected (at least as I expected), it is something recently changed that cause this error and force us to use less convinent methods to achive the same behavior.

mileusna commented 3 years ago

@mileusna do you have an example?

@blasten

I will try to illustrate the problem, I can't copy/paste working code from my app since it is even more complexed.

With ads, you want them to be fast, so our goal it to load ad only once, as soon as possible, and then you put that ad into widget tree and don't reload that ad while the user is on the screen, scrolling up and down through the page.

import 'package:flutter/material.dart';
import 'package:flutter/cupertino.dart';
import 'package:google_mobile_ads/google_mobile_ads.dart';

class Screen extends StatefulWidget {
  Screen();

  @override
  ScreenState createState() => ScreenState();
}

class ScreenState extends State {

  NativeAd nativeAd;  // our ad as property

  ScreenState();

  @override
  void didChangeDependencies() {
    // load ad only once ASAP
    nativeAd = NativeAd(
        adUnitId: "XXXXX",
        factoryId: "xxxx"
        // other params
        // ...
        );
    nativeAd.load();
    super.didChangeDependencies();
  }

  @override
  Widget build(BuildContext context) {
    return CustomScrollView(slivers: <Widget>[
      SliverList(
          delegate: SliverChildListDelegate(
        <Widget>[
          // components
          // ...
          // ...
          // put the ad we have just created into scroll view
          // and don't reload ad on each widget rebuild / user scroll
          AdWidget(ad: nativeAd)
          // ...
          // ...
        ],
      ))
    ]);
  }

  @override
  void dispose() {
    nativeAd.dispose();
    super.dispose();
  }
}

So as you can see, we have one ad in one place, but it will still cause "AdWidget is already in the widget tree" error.

drachim-dev commented 3 years ago

I still get the error This AdWidget is already in the Widget tree when using CustomScrollView with SliverList. Error appears when showing or hiding items. I adapted the ReusableInlineExample as follows:

bool _showDismissable = true;

@override
Widget build(BuildContext context) {
  return Scaffold(
      body: CustomScrollView(
    slivers: [
      SliverList(
          delegate: SliverChildListDelegate(
        [
          Text(Constants.placeholderText, style: TextStyle(fontSize: 24)),
          if (_showDismissable)
            ListTile(
              title: Text('Dismiss me'),
              trailing: IconButton(
                  icon: Icon(Icons.clear),
                  onPressed: () {
                    setState(() {
                      _showDismissable = false;
                    });
                  }),
            ),
          if (_nativeAdIsLoaded && _nativeAd != null)
            Container(width: 250, height: 350, child: AdWidget(ad: _nativeAd)),
        ],
      ))
    ],
  ));
}

As soon as the ListTile is hidden, it will cause This AdWidget is already in the Widget tree error. Using SingleChildScrollView with Column works whereas CustomScrollView and SliverList doesn't.

@blasten Shall I create a new issue or will you reopen this?

mileusna commented 3 years ago

@drachim-dev I use CustomScrollView and SliverList and everything is working fine with 0.11.0+3 (small example above).

I can't find problem in your code, but make sure that you are not adding same ad in the widget tree twice. Adding single ad on multiple places is not allowed and then this error will occure for a reason.

drachim-dev commented 3 years ago

@mileusna The problem occurs when a widget gets removed or added to the SliverList after the ad was already loaded. See _showDismissable in my example above.

Seems to be fixed with 0.11.0+4 and by using Completer as shown in the example!

vietstone-ng commented 3 years ago

Hi, I encounter this error for every 'hot reload'. Does anyone encounter the same problem?

AbdurRehman-coder commented 2 years ago

I have this error, I don't know how to solve it AdWidget requires Ad.load to be called before AdWidget is inserted into the tree. I don't know where I am doing the mistake. `class AdMobTest extends StatefulWidget { const AdMobTest({Key? key}) : super(key: key);

@override _AdMobTestState createState() => _AdMobTestState(); }

class _AdMobTestState extends State { BannerAd? _bannerAd; bool _isBannerLoaded = false;

@override void initState() async{ // TODO: implement initState super.initState(); bannerAd = BannerAd( adUnitId: 'ca-app-pub-3940256099942544/6300978111', size: AdSize.largeBanner, request: AdRequest(), listener: // COMPLETE: Create a BannerAd instance BannerAdListener( onAdLoaded: () { setState(() { _isBannerLoaded = true; }); }, onAdFailedToLoad: (ad, error) { // Releases an ad resource when it fails to load ad.dispose();

        print('Ad load failed...: (code=${error.code} message=${error.message})');
      },
    ),
);
await _bannerAd!.load();

}

final BannerAdListener listener = BannerAdListener( // Called when an ad is successfully received. onAdLoaded: (Ad ad) => print('Ad successfully loaded.'), // Called when an ad request failed. onAdFailedToLoad: (ad, loadAdError) { // Gets the domain from which the error came. String domain = loadAdError.domain; print('domain: $domain');

    // Gets the error code. See
    // https://developers.google.com/android/reference/com/google/android/gms/ads/AdRequest
    // and https://developers.google.com/admob/ios/api/reference/Enums/GADErrorCode
    // for a list of possible codes.
    int code = loadAdError.code;
    print('code error: $code');

    // A log friendly string summarizing the error.
    String message = loadAdError.message;
     print('message error: $message');
    // Get response information, which may include results of mediation requests.
    ResponseInfo? responseInfo = loadAdError.responseInfo;
    print('response info error: $responseInfo');
  },
// onAdFailedToLoad: (Ad ad, LoadAdError error) {
//   // Dispose the ad here to free resources.
//   ad.dispose();
//   print('Ads failed to load: $error');
// },
// Called when an ad opens an overlay that covers the screen.
onAdOpened: (Ad ad) => print('Ad opened.'),
// Called when an ad removes an overlay that covers the screen.
onAdClosed: (Ad ad) => print('Ad closed.'),
// Called when an impression occurs on the ad.
onAdImpression: (Ad ad) => print('Ad impression.'),

); @override void dispose() { // TODO: implement dispose super.dispose(); _bannerAd?.dispose(); } @override Widget build(BuildContext context) { return Scaffold( appBar: AppBar( title: Text('AdMob Test'), ), body: Container( width: MediaQuery.of(context).size.width, height: MediaQuery.of(context).size.height, child: Padding( padding: const EdgeInsets.all(8.0), child: Center( child: Column( children: [ // if (_isBannerLoaded == true) Container( color: Colors.green, width: _bannerAd?.size.width.toDouble(), height: _bannerAd?.size.height.toDouble(), child: Center( child: AdWidget(ad: _bannerAd!), ), ), Text('Testing'), ], ), ), ), ), ); } }`

lisuizhe commented 2 years ago

For encountered same problem with me: this problem is not fixed completely, with google_mobile_ads: ^1.0.1. I have my AdWidget with BannerAd in the bottom of scrollable containers. When first time open the screen and update the scrollable container by adding or removing an item(hence the position of AdWidget will be changed), I get the same " AdWidget is already in the widget tree" error. My guessing is that the AdWidget is once removed and recreated by the scrollable container(i.e. ListView), and in some timing there are two AdWidget with same BannerAd object in the widget tree. So one possible approach is lazy loading the AdWidget to give time for previous one to be removed

In my case, I successfully workaround this by wrapping the AdWidget in below DelayedLoad widget

class DelayedLoad extends StatelessWidget {
  final Widget child;
  final Duration delay;

  DelayedLoad({
    this.delay = const Duration(milliseconds: 500),
    required this.child,
  });

  @override
  Widget build(BuildContext context) {
    return FutureBuilder(
      future: Future.delayed(delay, () => true),
      builder: (context, snapshot) {
        return snapshot.hasData ? child : Container();
      },
    );
  }
}
Xoshbin commented 2 years ago

I had the same issue, with adding a key to the adWidget, now the error appears for a millisecond, and then the ad loads. AdWidget(key: UniqueKey(),ad: myNative)

PedroContipelli commented 2 months ago

I can confirm this only happens for me when the ListView re-renders in a way such that my BannerAdWrapper() changes index within the List (i.e. from another item loading in above it). I was able to fix it with the code below, by ensuring the BannerAd only loads in if all the elements before it have already loaded in.

// Inside ListView <Widget>[]
if (_interstitialAd != null)
  InterstitialAdWrapper(_interstitialAd),
if (_interstitialAd != null && _bannerAd != null)
  BannerAdWrapper(_bannerAd)