mobfox / MobFox-iOS-SDK

Apache License 2.0
15 stars 12 forks source link

CustomEvent WebView created for every request #54

Closed andre-arsenault closed 6 years ago

andre-arsenault commented 6 years ago

Issue

A separate WebView is created for every banner ad request made through the CustomEvent interface, and previous banners are never deallocated. This results in a significant resource leakage.

All of your example code in this repository for CustomEvent integrations shows the allocation of a new native banner object for the appropriate SDK whenever -requestAdWithSize is called. This could easily be fixed by only creating a new instance of the appropriate native banner object if one has not already been created.

Unfortunately, it appears that this function is always invoked on a new instance of the CustomEvent adapter object, and old adapters are never deallocated. This prevents the suggested fix above from being effective.

This issue occurs whether you use MobFoxAd's refresh member to refresh the ads, or you call -loadAd on your own app-driven schedule.

See log below (looking carefully at the logged values for self. Conversant is used as an example here, but it applies to all CustomEvent adapters. I can also confirm via logging that the adapter object's -dealloc method is never called.

2017-11-10 12:58:18.237607+0000 LifeTotal[451:32555] dbg: ### Conversant: loadAd ###
2017-11-10 12:58:18.238034+0000 LifeTotal[451:32555] dbg: ### Conversant: params: {
    "o_iosadvid" = "MILZIRMW-Z0W4-MCB4-GHKP-C0KU0WRXFRYJ";
    "viewcontroller_parent" = "<MobFoxPlugin: 0x1550f030>";
}
2017-11-10 12:58:18.238108+0000 LifeTotal[451:32555] dbg: ### Conversant: networkID: 675e31fc-3465-11e4-b31e-15f4fced7073
2017-11-10 12:58:18.238301+0000 LifeTotal[451:32555] dbg: ### Conversant: self: <MobFoxCustomEventConversant: 0xa841360>
2017-11-10 12:58:18.267337+0000 LifeTotal[451:32555] dbg: ### CONVERSANT: >>> BANNER CONVERSANT: greystripeBannerDisplayViewController <<<
2017-11-10 12:58:19.391843+0000 LifeTotal[451:32555] dbg: ### CONVERSANT: >>> BANNER CONVERSANT: greystripeAdFetchSucceeded <<<
2017-11-10 12:58:19.419095+0000 LifeTotal[451:32555] dbg: ### CONVERSANT: >>> BANNER CONVERSANT: greystripeBannerDisplayViewController <<<
2017-11-10 12:58:46.759071+0000 LifeTotal[451:32555] dbg: ### Conversant: loadAd ###
2017-11-10 12:58:46.759368+0000 LifeTotal[451:32555] dbg: ### Conversant: params: {
    "o_iosadvid" = "MILZIRMW-Z0W4-MCB4-GHKP-C0KU0WRXFRYJ";
    "viewcontroller_parent" = "<MobFoxPlugin: 0x1550f030>";
}
2017-11-10 12:58:46.759474+0000 LifeTotal[451:32555] dbg: ### Conversant: networkID: 675e31fc-3465-11e4-b31e-15f4fced7073
2017-11-10 12:58:46.759649+0000 LifeTotal[451:32555] dbg: ### Conversant: self: <MobFoxCustomEventConversant: 0xa83eff0>
2017-11-10 12:58:46.772341+0000 LifeTotal[451:32555] dbg: ### CONVERSANT: >>> BANNER CONVERSANT: greystripeBannerDisplayViewController <<<
2017-11-10 12:58:47.081902+0000 LifeTotal[451:32555] dbg: ### CONVERSANT: >>> BANNER CONVERSANT: greystripeAdFetchSucceeded <<<
2017-11-10 12:58:47.105777+0000 LifeTotal[451:32555] dbg: ### CONVERSANT: >>> BANNER CONVERSANT: greystripeBannerDisplayViewController <<<
2017-11-10 12:59:16.676881+0000 LifeTotal[451:32555] dbg: ### Conversant: loadAd ###
2017-11-10 12:59:16.677143+0000 LifeTotal[451:32555] dbg: ### Conversant: params: {
    "o_iosadvid" = "MILZIRMW-Z0W4-MCB4-GHKP-C0KU0WRXFRYJ";
    "viewcontroller_parent" = "<MobFoxPlugin: 0x1550f030>";
}
2017-11-10 12:59:16.677241+0000 LifeTotal[451:32555] dbg: ### Conversant: networkID: 675e31fc-3465-11e4-b31e-15f4fced7073
2017-11-10 12:59:16.677430+0000 LifeTotal[451:32555] dbg: ### Conversant: self: <MobFoxCustomEventConversant: 0xa85f480>
2017-11-10 12:59:16.689603+0000 LifeTotal[451:32555] dbg: ### CONVERSANT: >>> BANNER CONVERSANT: greystripeBannerDisplayViewController <<<
2017-11-10 12:59:17.158990+0000 LifeTotal[451:32555] dbg: ### CONVERSANT: >>> BANNER CONVERSANT: greystripeAdFetchSucceeded <<<
2017-11-10 12:59:17.182076+0000 LifeTotal[451:32555] dbg: ### CONVERSANT: >>> BANNER CONVERSANT: greystripeBannerDisplayViewController <<<
2017-11-10 12:59:47.720919+0000 LifeTotal[451:32555] dbg: ### Conversant: loadAd ###
2017-11-10 12:59:47.721223+0000 LifeTotal[451:32555] dbg: ### Conversant: params: {
    "o_iosadvid" = "MILZIRMW-Z0W4-MCB4-GHKP-C0KU0WRXFRYJ";
    "viewcontroller_parent" = "<MobFoxPlugin: 0x1550f030>";
}
2017-11-10 12:59:47.721337+0000 LifeTotal[451:32555] dbg: ### Conversant: networkID: 675e31fc-3465-11e4-b31e-15f4fced7073
2017-11-10 12:59:47.721448+0000 LifeTotal[451:32555] dbg: ### Conversant: self: <MobFoxCustomEventConversant: 0xa86d1e0>
2017-11-10 12:59:47.733622+0000 LifeTotal[451:32555] dbg: ### CONVERSANT: >>> BANNER CONVERSANT: greystripeBannerDisplayViewController <<<
2017-11-10 12:59:48.657814+0000 LifeTotal[451:32555] dbg: ### CONVERSANT: >>> BANNER CONVERSANT: greystripeAdFetchSucceeded <<<
2017-11-10 12:59:48.681832+0000 LifeTotal[451:32555] dbg: ### CONVERSANT: >>> BANNER CONVERSANT: greystripeBannerDisplayViewController <<<
2017-11-10 13:00:18.021610+0000 LifeTotal[451:32555] dbg: ### Conversant: loadAd ###
2017-11-10 13:00:18.022016+0000 LifeTotal[451:32555] dbg: ### Conversant: params: {
    "o_iosadvid" = "MILZIRMW-Z0W4-MCB4-GHKP-C0KU0WRXFRYJ";
    "viewcontroller_parent" = "<MobFoxPlugin: 0x1550f030>";
}
2017-11-10 13:00:18.022122+0000 LifeTotal[451:32555] dbg: ### Conversant: networkID: 675e31fc-3465-11e4-b31e-15f4fced7073
2017-11-10 13:00:18.022236+0000 LifeTotal[451:32555] dbg: ### Conversant: self: <MobFoxCustomEventConversant: 0xab617a0>
2017-11-10 13:00:18.034493+0000 LifeTotal[451:32555] dbg: ### CONVERSANT: >>> BANNER CONVERSANT: greystripeBannerDisplayViewController <<<
2017-11-10 13:00:18.378252+0000 LifeTotal[451:32555] dbg: ### CONVERSANT: >>> BANNER CONVERSANT: greystripeAdFetchSucceeded <<<
2017-11-10 13:00:18.397842+0000 LifeTotal[451:32555] dbg: ### CONVERSANT: >>> BANNER CONVERSANT: greystripeBannerDisplayViewController <<<
2017-11-10 13:00:47.762452+0000 LifeTotal[451:32555] dbg: ### Conversant: loadAd ###
2017-11-10 13:00:47.762729+0000 LifeTotal[451:32555] dbg: ### Conversant: params: {
    "o_iosadvid" = "MILZIRMW-Z0W4-MCB4-GHKP-C0KU0WRXFRYJ";
    "viewcontroller_parent" = "<MobFoxPlugin: 0x1550f030>";
}
2017-11-10 13:00:47.762802+0000 LifeTotal[451:32555] dbg: ### Conversant: networkID: 675e31fc-3465-11e4-b31e-15f4fced7073
2017-11-10 13:00:47.762913+0000 LifeTotal[451:32555] dbg: ### Conversant: self: <MobFoxCustomEventConversant: 0xab21410>
2017-11-10 13:00:47.774833+0000 LifeTotal[451:32555] dbg: ### CONVERSANT: >>> BANNER CONVERSANT: greystripeBannerDisplayViewController <<<
2017-11-10 13:00:48.128856+0000 LifeTotal[451:32555] dbg: ### CONVERSANT: >>> BANNER CONVERSANT: greystripeAdFetchSucceeded <<<
2017-11-10 13:00:48.148750+0000 LifeTotal[451:32555] dbg: ### CONVERSANT: >>> BANNER CONVERSANT: greystripeBannerDisplayViewController <<<
2017-11-10 13:01:16.847072+0000 LifeTotal[451:32555] dbg: ### Conversant: loadAd ###
2017-11-10 13:01:16.847312+0000 LifeTotal[451:32555] dbg: ### Conversant: params: {
    "o_iosadvid" = "MILZIRMW-Z0W4-MCB4-GHKP-C0KU0WRXFRYJ";
    "viewcontroller_parent" = "<MobFoxPlugin: 0x1550f030>";
}
2017-11-10 13:01:16.847411+0000 LifeTotal[451:32555] dbg: ### Conversant: networkID: 675e31fc-3465-11e4-b31e-15f4fced7073
2017-11-10 13:01:16.847523+0000 LifeTotal[451:32555] dbg: ### Conversant: self: <MobFoxCustomEventConversant: 0xab6d100>
2017-11-10 13:01:16.861593+0000 LifeTotal[451:32555] dbg: ### CONVERSANT: >>> BANNER CONVERSANT: greystripeBannerDisplayViewController <<<
2017-11-10 13:01:17.556266+0000 LifeTotal[451:32555] dbg: ### CONVERSANT: >>> BANNER CONVERSANT: greystripeAdFetchSucceeded <<<
2017-11-10 13:01:17.580462+0000 LifeTotal[451:32555] dbg: ### CONVERSANT: >>> BANNER CONVERSANT: greystripeBannerDisplayViewController <<<

The end result is an ever-growing stack of web views, one for each ad request. This can be seen using Safari (or Chrome, on Android) dev tools:

stacking custom event webviews

Expected Behavior

Use Case

Our app is a single-page hybrid app with a refreshing banner at the the bottom of the page. It is always in view, and we want it to refresh every so often. The app is intended to run for extended periods of time while the users play a physical card game.

andre-arsenault commented 6 years ago

Any update on this?

shahafss commented 6 years ago

Hi Andre, Unfortunately we didn't get a chance to address these issues on latest release as we were focused on MOAT development for the past month. We will include a fix for that on our next release which should occur during next week.

Sorry for the inconvenience.

Best, -Shahaf

shahafss commented 6 years ago

Hi Andre, Do you create a new Mobfox ad instance for every request? Can you share an example code in which you used to request Mobfox ads?

Thanks, -Shahaf

andre-arsenault commented 6 years ago

Hi Shahaf,

I only create a Mobfox ad instance if one does not already exist. My code is based on the provided Cordova plugin, with changes. You can see the iOS version here. The calling code enters in showBanner, which calls into createBanner. I save the Mobfox ad instance in the banner data member.

From my testing, the MobFoxPlugin object is the same between calls (at 0x1550f030 in the above logs) but the CustomEvent objects (MobFoxCustomEventConversant in the above logs) are newly-allocated every time.

Andre

shahafss commented 6 years ago

Hi Andre, Just wanted to let you know that we have found what's causing this issue and that it will have a fix on our next release.

Thank you for your patience. Best, -Shahaf

andre-arsenault commented 6 years ago

That's good news, thanks!

andre-arsenault commented 6 years ago

Is this fix included in the 3.3.1 release?

nabriski commented 6 years ago

Hi,

No, 3.3.1 is bug fix for a crash that was reported on some creatives (not pertaining to the classes you are using). The fix for this issue is part of our regular release schedule, planned for next week or, if testing goes slowly, the week after. The relevant code is already committed.

Thanks, Itamar

shahafss commented 6 years ago

fixed in 3.3.3

andre-arsenault commented 6 years ago

That's great, thanks. I'll take a look soon.

Is a fix for the same issue coming soon to the Android SDK as well?

nabriski commented 6 years ago

Yes, it is, but it's more complex for Android so it will take a few ore weeks