duckduckgo · afterxleep · Dec 9, 2024 · Dec 9, 2024 · Dec 9, 2024
diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj
@@ -1045,7 +1045,6 @@
 		D6ACEA322BBD55BF008FADDF /* TabURLInterceptor.swift in Sources */ = {isa = PBXBuildFile; fileRef = D6ACEA312BBD55BF008FADDF /* TabURLInterceptor.swift */; };
 		D6B67A122C332B6E002122EB /* DuckPlayerMocks.swift in Sources */ = {isa = PBXBuildFile; fileRef = D6B67A112C332B6E002122EB /* DuckPlayerMocks.swift */; };
 		D6B9E8D22CDA4420002B640C /* DuckPlayerOverlayUsagePixels.swift in Sources */ = {isa = PBXBuildFile; fileRef = D6B9E8D12CDA4418002B640C /* DuckPlayerOverlayUsagePixels.swift */; };
-		D6B9E8D42CDA8375002B640C /* DuckPlayerOverlayUsagePixelsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D6B9E8D32CDA8369002B640C /* DuckPlayerOverlayUsagePixelsTests.swift */; };
 		D6BC8ACB2C5AA3860025375B /* DuckPlayer in Frameworks */ = {isa = PBXBuildFile; productRef = D6BC8ACA2C5AA3860025375B /* DuckPlayer */; };
 		D6BFCB5F2B7524AA0051FF81 /* SubscriptionPIRView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D6BFCB5E2B7524AA0051FF81 /* SubscriptionPIRView.swift */; };
 		D6BFCB612B7525160051FF81 /* SubscriptionPIRViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = D6BFCB602B7525160051FF81 /* SubscriptionPIRViewModel.swift */; };
@@ -2894,7 +2893,6 @@
 		D6ACEA312BBD55BF008FADDF /* TabURLInterceptor.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TabURLInterceptor.swift; sourceTree = "<group>"; };
 		D6B67A112C332B6E002122EB /* DuckPlayerMocks.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DuckPlayerMocks.swift; sourceTree = "<group>"; };
 		D6B9E8D12CDA4418002B640C /* DuckPlayerOverlayUsagePixels.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DuckPlayerOverlayUsagePixels.swift; sourceTree = "<group>"; };
-		D6B9E8D32CDA8369002B640C /* DuckPlayerOverlayUsagePixelsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DuckPlayerOverlayUsagePixelsTests.swift; sourceTree = "<group>"; };
 		D6BFCB5E2B7524AA0051FF81 /* SubscriptionPIRView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SubscriptionPIRView.swift; sourceTree = "<group>"; };
 		D6BFCB602B7525160051FF81 /* SubscriptionPIRViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SubscriptionPIRViewModel.swift; sourceTree = "<group>"; };
 		D6D95CE22B6D9F8800960317 /* AsyncHeadlessWebViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AsyncHeadlessWebViewModel.swift; sourceTree = "<group>"; };
@@ -5535,7 +5533,6 @@
 		D62EC3B72C24695800FC9D04 /* DuckPlayer */ = {
 			isa = PBXGroup;
 			children = (
-				D6B9E8D32CDA8369002B640C /* DuckPlayerOverlayUsagePixelsTests.swift */,
 				D6B67A112C332B6E002122EB /* DuckPlayerMocks.swift */,
 				D62EC3BB2C2470E000FC9D04 /* DuckPlayerTests.swift */,
 				D62EC3B82C246A5600FC9D04 /* YoutublePlayerNavigationHandlerTests.swift */,
@@ -8272,7 +8269,6 @@
 				5694372B2BE3F2D900C0881B /* SyncErrorHandlerTests.swift in Sources */,
 				4B27FBB52C927435007E21A7 /* PersistentPixelTests.swift in Sources */,
 				987130C7294AAB9F00AB05E0 /* MenuBookmarksViewModelTests.swift in Sources */,
-				D6B9E8D42CDA8375002B640C /* DuckPlayerOverlayUsagePixelsTests.swift in Sources */,
 				858650D32469BFAD00C36F8A /* DaxDialogTests.swift in Sources */,
 				31C138B227A4097800FFD4B2 /* DownloadTestsHelper.swift in Sources */,
 				1E1D8B5D2994FFE100C96994 /* AutoconsentMessageProtocolTests.swift in Sources */,

diff --git a/DuckDuckGo/DuckPlayer/DuckPlayerNavigationHandler.swift b/DuckDuckGo/DuckPlayer/DuckPlayerNavigationHandler.swift
@@ -25,6 +25,7 @@ import Common
 import BrowserServicesKit
 import DuckPlayer
 import os.log
+import Combine
 
 /// Handles navigation and interactions related to Duck Player within the app.
 final class DuckPlayerNavigationHandler: NSObject {
@@ -75,6 +76,9 @@ final class DuckPlayerNavigationHandler: NSObject {
     /// Delegate for handling tab navigation events.
     weak var tabNavigationHandler: DuckPlayerTabNavigationHandling?
 
+    /// Cancellable for observing DuckPlayer Mode changes
+    private var duckPlayerModeCancellable: AnyCancellable?
+
     private struct Constants {
         static let SERPURL =  "duckduckgo.com/"
         static let refererHeader = "Referer"
@@ -125,6 +129,8 @@ final class DuckPlayerNavigationHandler: NSObject {
         self.dailyPixelFiring = dailyPixelFiring
         self.tabNavigationHandler = tabNavigationHandler
         self.duckPlayerOverlayUsagePixels = duckPlayerOverlayUsagePixels
+
+        super.init()
     }
 
     /// Returns the file path for the Duck Player HTML template.
@@ -552,6 +558,14 @@ final class DuckPlayerNavigationHandler: NSObject {
         return false
     }
 
+    /// Register a DuckPlayer mode Observe to handle events when the mode changes
+    private func setupPlayerModeObserver() {
+        duckPlayerModeCancellable =  duckPlayer.settings.duckPlayerSettingsPublisher
+            .sink { [weak self] in
+                self?.duckPlayerOverlayUsagePixels?.duckPlayerMode = self?.duckPlayer.settings.mode ?? .disabled
+            }
+    }
+
     /// // Handle "open in YouTube" links (duck://player/openInYoutube)
     ///
     /// - Parameter url: The `URL` used to determine the tab type.
@@ -577,6 +591,11 @@ final class DuckPlayerNavigationHandler: NSObject {
         }
     }
 
+    deinit {
+        duckPlayerModeCancellable?.cancel()
+        duckPlayerModeCancellable = nil
+    }
+
 }
 
 extension DuckPlayerNavigationHandler: DuckPlayerNavigationHandling {
@@ -636,7 +655,6 @@ extension DuckPlayerNavigationHandler: DuckPlayerNavigationHandling {
                 // Before performing the simulated request
                 DispatchQueue.main.asyncAfter(deadline: .now() + 0.15) {
                     self.performRequest(request: newRequest, webView: webView)
-                    self.duckPlayerOverlayUsagePixels?.handleNavigationAndFirePixels(url: url, duckPlayerMode: self.duckPlayerMode)
                     self.fireDuckPlayerPixels(webView: webView)
 
                 }
@@ -680,12 +698,6 @@ extension DuckPlayerNavigationHandler: DuckPlayerNavigationHandling {
             return .notHandled(.duplicateNavigation)
         }
 
-        // Overlay Usage Pixel handling
-        if let url = webView.url {
-            duckPlayerOverlayUsagePixels?.handleNavigationAndFirePixels(url: url, duckPlayerMode: duckPlayerMode)
-            lastURLChangeHandling = Date()
-        }
-
         // Check if DuckPlayer feature is enabled
         guard isDuckPlayerFeatureEnabled else {
             return .notHandled(.featureOff)
@@ -786,6 +798,9 @@ extension DuckPlayerNavigationHandler: DuckPlayerNavigationHandling {
         guard let url = webView.url else {
             return
         }
+
+        // Fire Reload Pixel
+        duckPlayerOverlayUsagePixels?.fireReloadPixelIfNeeded(url: url)
 
         if url.isDuckPlayer, duckPlayerMode != .disabled {
             redirectToDuckPlayerVideo(url: url, webView: webView, disableNewTab: true)
@@ -809,6 +824,11 @@ extension DuckPlayerNavigationHandler: DuckPlayerNavigationHandling {
 
         // Reset referrer and initial settings
         referrer = .other
+
+        // Attach WebView to OverlayPixels
+        duckPlayerOverlayUsagePixels?.webView = webView
+        duckPlayerOverlayUsagePixels?.duckPlayerMode = duckPlayer.settings.mode
+        setupPlayerModeObserver()
 
         // Ensure feature and mode are enabled
         guard isDuckPlayerFeatureEnabled,
@@ -825,6 +845,7 @@ extension DuckPlayerNavigationHandler: DuckPlayerNavigationHandling {
             referrer = parameters.referrer
             redirectToDuckPlayerVideo(url: url, webView: webView, disableNewTab: true)
         }
+
     }
 
     /// Updates the referrer after the web view finishes loading a page.
@@ -836,16 +857,6 @@ extension DuckPlayerNavigationHandler: DuckPlayerNavigationHandling {
         // Reset allowFirstVideo
         duckPlayer.settings.allowFirstVideo = false
 
-        // Overlay Usage Pixel handling for Direct Navigation
-        if let url = webView.url, !url.isYoutube {
-            duckPlayerOverlayUsagePixels?.handleNavigationAndFirePixels(url: url, duckPlayerMode: duckPlayerMode)
-        }
-        // Reset Overlay Last Fired pixel after the page is loaded
-        // A delay is required as Youtube sometimes performs an extra redirect on load
-        DispatchQueue.main.asyncAfter(deadline: .now() + 3) {
-            self.duckPlayerOverlayUsagePixels?.lastFiredPixel = nil
-        }
-
 
     }
 

diff --git a/DuckDuckGo/DuckPlayer/DuckPlayerOverlayUsagePixels.swift b/DuckDuckGo/DuckPlayer/DuckPlayerOverlayUsagePixels.swift
@@ -17,91 +17,110 @@
 //  limitations under the License.
 //
 
+import WebKit
 import Core
 
 protocol DuckPlayerOverlayPixelFiring {
-    
+
     var pixelFiring: PixelFiring.Type { get set }
-    var navigationHistory: [URL] { get set }
-    var lastFiredPixel: Pixel.Event? { get set }
-
-    func handleNavigationAndFirePixels(url: URL?, duckPlayerMode: DuckPlayerMode)
+    var webView: WKWebView? { get set }
+    var duckPlayerMode: DuckPlayerMode { get set }
+    func fireNavigationPixelsIfNeeded(webView: WKWebView)
+    func fireReloadPixelIfNeeded(url: URL)
 }
 
-final class DuckPlayerOverlayUsagePixels: DuckPlayerOverlayPixelFiring {
+final class DuckPlayerOverlayUsagePixels: NSObject, DuckPlayerOverlayPixelFiring {
 
     var pixelFiring: PixelFiring.Type
-    var navigationHistory: [URL] = []
-    var lastFiredPixel: Pixel.Event?
+    var duckPlayerMode: DuckPlayerMode = .disabled
+    private var isObserving = false
+
+    weak var webView: WKWebView? {
+        didSet {
+            if let webView {
+                addObservers(to: webView)
+            }
+        }
+    }
 
-    private var idleTimer: Timer?
-    private var idleTimeInterval: TimeInterval
+    private var lastVisitedURL: URL? // Tracks the last known URL
 
-    init(pixelFiring: PixelFiring.Type = Pixel.self,
-         navigationHistory: [URL] = [],
-         timeoutInterval: TimeInterval = 30.0) {
+    init(pixelFiring: PixelFiring.Type = Pixel.self) {
         self.pixelFiring = pixelFiring
-        self.idleTimeInterval = timeoutInterval
     }
 
-    func handleNavigationAndFirePixels(url: URL?, duckPlayerMode: DuckPlayerMode) {
-        guard let url = url else { return }
-        let comparisonURL = url.forComparison()
-
-        // Only append the URL if it's different from the last entry in normalized form
-        navigationHistory.append(comparisonURL)
-
-        // DuckPlayer is in Ask Mode, there's navigation history, and last URL is a YouTube Watch Video
-        guard duckPlayerMode == .alwaysAsk,
-              navigationHistory.count > 1,
-              let currentURL = navigationHistory.last,
-              let previousURL = navigationHistory.dropLast().last,
-              previousURL.isYoutubeWatch else { return }
-
-        var isReload = false
-        // Check for a reload condition: when current videoID is the same as Previous
-        if let currentVideoID = currentURL.youtubeVideoParams?.videoID,
-           let previousVideoID = previousURL.youtubeVideoParams?.videoID,
-           !previousURL.isDuckPlayer, !currentURL.isDuckPlayer {
-            isReload = currentVideoID == previousVideoID
+    deinit {
+        if let webView {
+            removeObservers(from: webView)
         }
+    }
+
+    func fireNavigationPixelsIfNeeded(webView: WKWebView) {
 
-        // Fire the reload pixel if this is a reload navigation
-        if isReload {
-            firePixel(.duckPlayerYouTubeOverlayNavigationRefresh)
-        } else {
-            // Determine if it’s a back navigation by looking further back in history
-            let isBackNavigation = navigationHistory.count > 2 &&
-                                   navigationHistory[navigationHistory.count - 3].forComparison() == currentURL.forComparison()
-
-            // Fire the appropriate pixel based on navigation type
-            if isBackNavigation {
-                firePixel(.duckPlayerYouTubeOverlayNavigationBack)
-            } else if previousURL.isYoutubeWatch && currentURL.isYoutube {
-                // Forward navigation within YouTube (including non-video URLs)
-                firePixel(.duckPlayerYouTubeNavigationWithinYouTube)
-            } else if previousURL.isYoutubeWatch && !currentURL.isYoutube && !currentURL.isDuckPlayer {
-                // Navigation outside YouTube
-                firePixel(.duckPlayerYouTubeOverlayNavigationOutsideYoutube)
-                navigationHistory.removeAll()
-            }
+        guard let currentURL = webView.url else {
+            return
         }
+
+        let backItemURL = webView.backForwardList.backItem?.url
 
-        // Truncation logic: Remove all URLs up to the last occurrence of the current URL in normalized form
-        if navigationHistory.count > 0 {
-            if let lastOccurrenceIndex = (0..<navigationHistory.count - 1).last(where: { navigationHistory[$0].forComparison() == comparisonURL }) {
-                navigationHistory = Array(navigationHistory.prefix(upTo: lastOccurrenceIndex + 1))
+
+        if let (currentVideoID, _) = currentURL.youtubeVideoParams,
+           let (oldVideoID, _) = lastVisitedURL?.youtubeVideoParams,
+            oldVideoID == currentVideoID {
+            return
+        }
+
+        if lastVisitedURL != nil {
+            // Back navigation
+            if currentURL == backItemURL {
+                firePixelIfNeeded(Pixel.Event.duckPlayerYouTubeOverlayNavigationBack, url: lastVisitedURL)
+            }
+            // Regular navigation
+            else {
+                if currentURL.isYoutube {
+                    firePixelIfNeeded(Pixel.Event.duckPlayerYouTubeNavigationWithinYouTube, url: lastVisitedURL)
+                } else {
+                    firePixelIfNeeded(Pixel.Event.duckPlayerYouTubeOverlayNavigationOutsideYoutube, url: lastVisitedURL)
+                }
             }
         }
+
+        // Update the last visited URL
+        lastVisitedURL = currentURL
     }
-
-    private func firePixel(_ pixel: Pixel.Event) {
-        if lastFiredPixel == .duckPlayerYouTubeOverlayNavigationRefresh && pixel == .duckPlayerYouTubeOverlayNavigationRefresh {
-            return
+
+    func fireReloadPixelIfNeeded(url: URL) {
+        firePixelIfNeeded(Pixel.Event.duckPlayerYouTubeOverlayNavigationRefresh, url: lastVisitedURL)
+    }
+
+    // MARK: - Observer Management
+
+    private func addObservers(to webView: WKWebView) {
+        removeObservers(from: webView)
+        webView.addObserver(self, forKeyPath: #keyPath(WKWebView.url), options: [.new, .old], context: nil)
+        isObserving = true
+    }
+
+    private func removeObservers(from webView: WKWebView) {
+        if isObserving {
+            webView.removeObserver(self, forKeyPath: #keyPath(WKWebView.url))
+            isObserving = false
         }
-        lastFiredPixel = pixel
-        pixelFiring.fire(pixel, withAdditionalParameters: [:])
     }
 
+    // swiftlint:disable block_based_kvo
+    override func observeValue(forKeyPath keyPath: String?, of object: Any?, change: [NSKeyValueChangeKey: Any]?, context: UnsafeMutableRawPointer?) {
+        guard let webView = object as? WKWebView else { return }
+
+        if keyPath == #keyPath(WKWebView.url) {
+            fireNavigationPixelsIfNeeded(webView: webView)
+        }
+    }
+    // swiftlint:enable block_based_kvo
 
+    private func firePixelIfNeeded(_ pixel: Pixel.Event, url: URL?) {
+        if let url, url.isYoutubeWatch, duckPlayerMode == .alwaysAsk {
+            pixelFiring.fire(pixel, withAdditionalParameters: [:])
+        }
+    }
 }
diff --git a/DuckDuckGo/TabViewController.swift b/DuckDuckGo/TabViewController.swift
@@ -601,6 +601,12 @@ class TabViewController: UIViewController {
         }
 
         instrumentation.didPrepareWebView()
+
+        // Initialize DuckPlayerNavigationHandler
+        if let handler = duckPlayerNavigationHandler,
+            let webView = webView {
+            handler.handleAttach(webView: webView)
+        }
 
         if consumeCookies {
             consumeCookiesThenLoadRequest(request)
@@ -623,12 +629,7 @@ class TabViewController: UIViewController {
                 // break a js-initiated popup request such as printing from a popup
                 guard self?.url != cleanURLRequest.url || loadingStopped || !loadingInitiatedByParentTab else { return }
                 self?.load(urlRequest: cleanURLRequest)
-
-
-                if let handler = self?.duckPlayerNavigationHandler,
-                    let webView = self?.webView {
-                    handler.handleAttach(webView: webView)
-                }
+
             })
         }