Merge pull request #25285 from brave/issues/40605

[ads] Fix NTT video background may be blank after changing NTP media type setting
brave · Aug 23, 2024 · de2674c · de2674c
2 parents 3cf6dbf + e20246f
commit de2674c
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 14 deletions.
diff --git a/...ces/Brave/Frontend/Browser/New Tab Page/Backgrounds/NewTabPageBackgroundButtonsView.swift b/...ces/Brave/Frontend/Browser/New Tab Page/Backgrounds/NewTabPageBackgroundButtonsView.swift
@@ -73,7 +73,7 @@ class NewTabPageBackgroundButtonsView: UIView, PreferencesObserver {
   private let playButton = PlayButton().then {
     $0.isHidden = true
   }
-  private var shouldShowPlayButton = false
+  private var isVideoBackground = false
   private var playButtonGestureRecognizer: UITapGestureRecognizer?
 
   /// The parent safe area insets (since UICollectionView doesn't feed down
@@ -175,6 +175,11 @@ class NewTabPageBackgroundButtonsView: UIView, PreferencesObserver {
     tappedBackgroundDuringAutoplay?()
   }
 
+  func resetVideoBackgroundButtons() {
+    isVideoBackground = false
+    updatePlayButtonVisibility()
+  }
+
   func videoAutoplayStarted() {
     let tapGesture = UITapGestureRecognizer(
       target: self,
@@ -185,7 +190,7 @@ class NewTabPageBackgroundButtonsView: UIView, PreferencesObserver {
   }
 
   func videoAutoplayFinished() {
-    shouldShowPlayButton = true
+    isVideoBackground = true
     updatePlayButtonVisibility()
 
     if let playButtonGestureRecognizer = playButtonGestureRecognizer {
@@ -201,7 +206,7 @@ class NewTabPageBackgroundButtonsView: UIView, PreferencesObserver {
     if isLandscape && UIDevice.isPhone {
       playButton.isHidden = true
     } else {
-      playButton.isHidden = !shouldShowPlayButton
+      playButton.isHidden = !isVideoBackground
     }
   }
 

diff --git a/...os/Sources/Brave/Frontend/Browser/New Tab Page/Backgrounds/NewTabPageBackgroundView.swift b/...os/Sources/Brave/Frontend/Browser/New Tab Page/Backgrounds/NewTabPageBackgroundView.swift
@@ -37,6 +37,11 @@ class NewTabPageBackgroundView: UIView {
     }
   }
 
+  func resetPlayerLayer() {
+    playerLayer.player = nil
+    playerLayer.removeFromSuperlayer()
+  }
+
   override init(frame: CGRect) {
     super.init(frame: frame)
 

diff --git a/...e-ios/Sources/Brave/Frontend/Browser/New Tab Page/Backgrounds/NewTabPageVideoPlayer.swift b/...e-ios/Sources/Brave/Frontend/Browser/New Tab Page/Backgrounds/NewTabPageVideoPlayer.swift
@@ -45,7 +45,6 @@ class NewTabPageVideoPlayer {
 
   init(_ backgroundVideoPath: URL) {
     self.backgroundVideoPath = backgroundVideoPath
-    createPlayer()
   }
 
   deinit {

diff --git a/ios/brave-ios/Sources/Brave/Frontend/Browser/New Tab Page/NewTabPageViewController.swift b/ios/brave-ios/Sources/Brave/Frontend/Browser/New Tab Page/NewTabPageViewController.swift
@@ -257,7 +257,18 @@ class NewTabPageViewController: UIViewController {
     }
 
     background.changed = { [weak self] in
-      self?.setupBackgroundImage()
+      guard let self else { return }
+      setupBackgroundImage()
+
+      let isTabVisible = viewIfLoaded?.window != nil
+      setupBackgroundVideoIfNeeded(shouldCreatePlayer: isTabVisible)
+      // Load the video asset here, as viewDidAppear is not called when the view
+      // is already visible
+      if isTabVisible {
+        videoPlayer?.loadAndAutoplayVideoAssetIfNeeded(
+          shouldAutoplay: false
+        )
+      }
     }
 
     Preferences.BraveNews.isEnabled.observe(from: self)
@@ -330,7 +341,7 @@ class NewTabPageViewController: UIViewController {
     }
 
     setupBackgroundImage()
-    setupBackgroundVideoIfNeeded()
+    setupBackgroundVideoIfNeeded(shouldCreatePlayer: true)
     backgroundView.snp.makeConstraints {
       $0.edges.equalToSuperview()
     }
@@ -394,7 +405,6 @@ class NewTabPageViewController: UIViewController {
       self?.reportSponsoredBackgroundEvent(.viewedImpression)
     }
 
-    setupBackgroundVideoIfNeeded()
     videoPlayer?.loadAndAutoplayVideoAssetIfNeeded(
       shouldAutoplay: shouldShowBackgroundVideo()
     )
@@ -487,20 +497,22 @@ class NewTabPageViewController: UIViewController {
     )
   }
 
-  func setupBackgroundVideoIfNeeded() {
+  func setupBackgroundVideoIfNeeded(shouldCreatePlayer: Bool) {
     videoButtonsView.isHidden = true
 
-    // Setup the background video only if:
-    // - it hasn't been setup before
-    // - the current NTP background is a video
-    guard videoPlayer == nil,
-      let backgroundVideoPath = background.backgroundVideoPath
-    else {
+    guard let backgroundVideoPath = background.backgroundVideoPath else {
+      videoPlayer = nil
+      backgroundView.resetPlayerLayer()
+      backgroundButtonsView.resetVideoBackgroundButtons()
       return
     }
 
     gradientView.isHidden = false
     videoPlayer = NewTabPageVideoPlayer(backgroundVideoPath)
+    if shouldCreatePlayer {
+      videoPlayer?.createPlayer()
+    }
+
     backgroundView.setupPlayerLayer(backgroundVideoPath, player: videoPlayer?.player)
 
     videoButtonsView.tappedBackgroundVideo = { [weak videoPlayer] in