Stop ViewabilityTimer when ViewForTracking has become nil

[jsligh]

Closes #908

[YuriyVelichkoPI]

See the comment in the review

[shinwan2]

Thank you so much for handling this @YuriyVelichkoPI and @jsligh.

If I may be so bold, below are my use cases and expectations described by unit tests for faster feedback loop.
https://github.com/prebid/prebid-mobile-ios/compare/master...smartnews:prebid-mobile-ios:feature/optimize-view-tracking?expand=1

Let me know if I can be of more help.

[YuriyVelichkoPI]

@shinwan2, thank you for the clarification of the use case.

If I understood correctly, you want to display the same ad in the feed several times. Do you want to do it regardless of the impression event, Or if the ad is tracked the impression, you don't need to display it one more time in the feed?

To support your scenario, I believe you should use some additional container view as a point of viewability tracking and add this whole container to the different cells in the feed.

The pseudo-code solution:

 let nativeAd = NativeAd.create(cacheId: cacheId!)!
 let adContainer: UIView = createMyNativeAd(from: nativeAd)
 
 nativeAd.registerView(adContainer ...)
 
 cell.addSubview(adContainer)

// Later, after scrolling do the same, the native ad will still track the viewability of the adContainer but not the cell in the feed
cell.addSubview(adContainer)

[YuriyVelichkoPI]

In any casy after expecting the use case and the current codebase I think that we still need several additional enchancmets regarding the impression tracking of the native ads:

• Once the viewabilityTimer is fired, and the viewForTracking is nil (destroyed due to the app ligic) - invalidate the timer in order to prevent further useless cycles.
• The registerView function should do nothing if:
  • the viewForTracking is not nil
  • the impressionHasBeenTracked is true

It will prevent

• viewability tracking of non existing view
• mess in timers and tracking flows

Pubilshers in their turn should register the view right before it should be displayed. Even if publisher wants to change the ad view placement they still should do it as fast as possible. So the timer still shouldn't harm so much. Cahing the ad for a long time is impossible (due to the ad expiration policy) and not appreciated in any case. Still waiting for the detailed feedback about the use case.

[shinwan2]

@YuriyVelichkoPI

Do you want to do it regardless of the impression event

Yes, this one. The ad once assigned to a given slot (e.g. 4th slot on a feed) will always be shown on that same slot (using UITableViewCell).

I believe you should use some additional container view as a point of viewability tracking and add this whole container to the different cells in the feed.

I see, I believe it should be OK to clear the UIImageView.image (showing the ad image) to save memory when it's out of view?

[YuriyVelichkoPI]

LGTM