Fix: dimensions of CS Ad are wrong after the first pre roll. #3652
Conversation
|
@theodab can you review it? |
| this.imaAdsManager_.resize(this.video_.offsetWidth, | ||
| this.video_.offsetHeight, viewMode); | ||
| }); | ||
| this.resizeObserver_.observe(this.video_); |
There was a problem hiding this comment.
Hm. We should also call this.resizeObserver_.disconnect() when the ad manager is disconnected from the video, so as to not leave hanging observers.
Unfortunately, now that I am looking at it, we don't have any appropriate lifecycle events for that. AdManager.onAssetUnload is called when the player unloads, but that's not the right moment; if you unload one asset and load another, the ad manager should still be there. We'd want to call it when the player detaches instead, I think, and that isn't passed on to the ad manager.
For now, I guess I'll approve and merge this current CL. I'll make a follow-up CL to make Player.detach also dispose of the ad manager, since it's kind of outside the scope of the issue you are fixing here.
|
All tests passed! |
github-actions
bot
added
the
status: archived
Close: #3638