From 7182df02c8ada69db2aff5de5da0d2f159e3ded0 Mon Sep 17 00:00:00 2001 From: Daniel Bernal Date: Fri, 6 Dec 2024 19:19:16 +0100 Subject: [PATCH 1/3] Refactor Overlay Pixels Refactors Overlay Pixels to use WebKit KVO and backForward lists for improved reliablity --- .../DuckPlayerTabExtension.swift | 39 ++++-- .../DuckPlayerOverlayPixels.swift | 123 ++++++++++-------- 2 files changed, 91 insertions(+), 71 deletions(-) diff --git a/DuckDuckGo/Tab/TabExtensions/DuckPlayerTabExtension.swift b/DuckDuckGo/Tab/TabExtensions/DuckPlayerTabExtension.swift index 0fc03ac3f5..f41adb6ea9 100644 --- a/DuckDuckGo/Tab/TabExtensions/DuckPlayerTabExtension.swift +++ b/DuckDuckGo/Tab/TabExtensions/DuckPlayerTabExtension.swift @@ -49,6 +49,9 @@ final class DuckPlayerTabExtension { didSet { youtubeOverlayScript?.webView = webView youtubePlayerScript?.webView = webView + if duckPlayerOverlayUsagePixels.webView == nil { + duckPlayerOverlayUsagePixels.webView = webView + } } } private weak var youtubeOverlayScript: YoutubeOverlayUserScript? @@ -56,6 +59,7 @@ final class DuckPlayerTabExtension { private let onboardingDecider: DuckPlayerOnboardingDecider private var shouldSelectNextNewTab: Bool? private var duckPlayerOverlayUsagePixels: DuckPlayerOverlayPixelFiring + private var duckPlayerModeCancellable: AnyCancellable? init(duckPlayer: DuckPlayer, isBurner: Bool, @@ -69,6 +73,7 @@ final class DuckPlayerTabExtension { self.preferences = preferences self.onboardingDecider = onboardingDecider self.duckPlayerOverlayUsagePixels = duckPlayerOverlayPixels + webViewPublisher.sink { [weak self] webView in self?.webView = webView }.store(in: &cancellables) @@ -84,6 +89,15 @@ final class DuckPlayerTabExtension { self?.setUpYoutubeScriptsIfNeeded(for: self?.webView?.url) } }.store(in: &cancellables) + + // Add a DuckPlayerMode observer + setupPlayerModeObserver() + + } + + deinit { + duckPlayerModeCancellable?.cancel() + duckPlayerModeCancellable = nil } @MainActor @@ -152,6 +166,14 @@ final class DuckPlayerTabExtension { Debounce.lastFireTime = now PixelKit.fire(GeneralPixel.duckPlayerOverlayYoutubeImpressions) } + + private func setupPlayerModeObserver() { + duckPlayerModeCancellable = preferences.$duckPlayerMode + .sink { [weak self] mode in + self?.duckPlayerOverlayUsagePixels.duckPlayerMode = mode + } + } + } extension DuckPlayerTabExtension: YoutubeOverlayUserScriptDelegate { @@ -238,16 +260,9 @@ extension DuckPlayerTabExtension: NavigationResponder { } } - // Fire DuckPlayer Temporary Pixels on Reload + // Duck Player Overlay Reload Pixel if case .reload = navigationAction.navigationType { - if let url = navigationAction.request.url { - duckPlayerOverlayUsagePixels.handleNavigationAndFirePixels(url: url, duckPlayerMode: duckPlayer.mode) - } - } - - // Fire DuckPlayer temporary pixels on navigating outside Youtube - if let url = navigationAction.request.url, !url.isYoutube, navigationAction.isForMainFrame { - duckPlayerOverlayUsagePixels.handleNavigationAndFirePixels(url: url, duckPlayerMode: duckPlayer.mode) + duckPlayerOverlayUsagePixels.fireReloadPixelIfNeeded(url: navigationAction.url) } // when in Private Player, don't directly reload current URL when it‘s a Private Player target URL @@ -319,11 +334,7 @@ extension DuckPlayerTabExtension: NavigationResponder { // Fire Overlay Shown Pixels fireOverlayShownPixelIfNeeded(url: navigation.url) - - // Fire DuckPlayer Overlay Temporary Pixels - if let url = navigation.request.url { - duckPlayerOverlayUsagePixels.handleNavigationAndFirePixels(url: url, duckPlayerMode: duckPlayer.mode) - } + } @MainActor diff --git a/DuckDuckGo/YoutubePlayer/DuckPlayerOverlayPixels.swift b/DuckDuckGo/YoutubePlayer/DuckPlayerOverlayPixels.swift index 8988465794..95982c3c4d 100644 --- a/DuckDuckGo/YoutubePlayer/DuckPlayerOverlayPixels.swift +++ b/DuckDuckGo/YoutubePlayer/DuckPlayerOverlayPixels.swift @@ -16,84 +16,93 @@ // limitations under the License. // import PixelKit +import WebKit protocol DuckPlayerOverlayPixelFiring { var pixelFiring: PixelFiring? { get set } - var navigationHistory: [URL] { 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? - var navigationHistory: [URL] = [] + var duckPlayerMode: DuckPlayerMode = .disabled + + weak var webView: WKWebView? { + didSet { + if let webView = webView { + addObservers(to: webView) + } + } + } - private var idleTimer: Timer? - private var idleTimeInterval: TimeInterval + private var lastVisitedURL: URL? // Tracks the last known URL - init(pixelFiring: PixelFiring? = PixelKit.shared, - navigationHistory: [URL] = [], - timeoutInterval: TimeInterval = 30.0) { + init(pixelFiring: PixelFiring? = PixelKit.shared) { 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 { + removeObservers() + } - // Fire the reload pixel if this is a reload navigation - if isReload { - pixelFiring?.fire(GeneralPixel.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 { - pixelFiring?.fire(GeneralPixel.duckPlayerYouTubeOverlayNavigationBack) - } else if previousURL.isYoutubeWatch && currentURL.isYoutube { - // Forward navigation within YouTube (including non-video URLs) - pixelFiring?.fire(GeneralPixel.duckPlayerYouTubeNavigationWithinYouTube) - } else if previousURL.isYoutubeWatch && !currentURL.isYoutube && !currentURL.isDuckPlayer { - // Navigation outside YouTube - pixelFiring?.fire(GeneralPixel.duckPlayerYouTubeOverlayNavigationOutsideYoutube) - navigationHistory.removeAll() - } + func fireNavigationPixelsIfNeeded(webView: WKWebView) { + + 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.. Date: Mon, 9 Dec 2024 15:41:30 +0100 Subject: [PATCH 2/3] Fix lint issues --- DuckDuckGo.xcodeproj/project.pbxproj | 6 - .../DuckPlayerTabExtension.swift | 4 +- .../DuckPlayerOverlayPixels.swift | 2 +- .../DuckPlayerOverlayPixelsTests.swift | 133 ------------------ 4 files changed, 3 insertions(+), 142 deletions(-) delete mode 100644 UnitTests/YoutubePlayer/DuckPlayerOverlayPixelsTests.swift diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index e5c180269a..a0e1df525d 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -3090,8 +3090,6 @@ D6BC8AC82C5A95B10025375B /* DuckPlayer in Frameworks */ = {isa = PBXBuildFile; productRef = D6BC8AC72C5A95B10025375B /* DuckPlayer */; }; D6E0ACB12CE36DCA005D3486 /* DuckPlayerOverlayPixels.swift in Sources */ = {isa = PBXBuildFile; fileRef = D6E0ACB02CE36DC4005D3486 /* DuckPlayerOverlayPixels.swift */; }; D6E0ACB22CE36DCA005D3486 /* DuckPlayerOverlayPixels.swift in Sources */ = {isa = PBXBuildFile; fileRef = D6E0ACB02CE36DC4005D3486 /* DuckPlayerOverlayPixels.swift */; }; - D6E0ACB42CE36FB0005D3486 /* DuckPlayerOverlayPixelsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D6E0ACB32CE36FA8005D3486 /* DuckPlayerOverlayPixelsTests.swift */; }; - D6E0ACB52CE36FB0005D3486 /* DuckPlayerOverlayPixelsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D6E0ACB32CE36FA8005D3486 /* DuckPlayerOverlayPixelsTests.swift */; }; EA0BA3A9272217E6002A0B6C /* ClickToLoadUserScript.swift in Sources */ = {isa = PBXBuildFile; fileRef = EA0BA3A8272217E6002A0B6C /* ClickToLoadUserScript.swift */; }; EA18D1CA272F0DC8006DC101 /* social_images in Resources */ = {isa = PBXBuildFile; fileRef = EA18D1C9272F0DC8006DC101 /* social_images */; }; EA8AE76A279FBDB20078943E /* ClickToLoadTDSTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = EA8AE769279FBDB20078943E /* ClickToLoadTDSTests.swift */; }; @@ -4973,7 +4971,6 @@ CDE248A72C821FFE00F9399D /* MaliciousSiteProtectionState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MaliciousSiteProtectionState.swift; sourceTree = ""; }; D64A5FF72AEA5C2B00B6D6E7 /* HomeButtonMenuFactory.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HomeButtonMenuFactory.swift; sourceTree = ""; }; D6E0ACB02CE36DC4005D3486 /* DuckPlayerOverlayPixels.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DuckPlayerOverlayPixels.swift; sourceTree = ""; }; - D6E0ACB32CE36FA8005D3486 /* DuckPlayerOverlayPixelsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DuckPlayerOverlayPixelsTests.swift; sourceTree = ""; }; EA0BA3A8272217E6002A0B6C /* ClickToLoadUserScript.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ClickToLoadUserScript.swift; sourceTree = ""; }; EA18D1C9272F0DC8006DC101 /* social_images */ = {isa = PBXFileReference; lastKnownFileType = folder; path = social_images; sourceTree = ""; }; EA8AE769279FBDB20078943E /* ClickToLoadTDSTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ClickToLoadTDSTests.swift; sourceTree = ""; }; @@ -5846,7 +5843,6 @@ 376718FE28E58504003A2A15 /* YoutubePlayer */ = { isa = PBXGroup; children = ( - D6E0ACB32CE36FA8005D3486 /* DuckPlayerOverlayPixelsTests.swift */, 3199AF812C80736B003AEBDC /* DuckPlayerOnboardingLocationValidatorTests.swift */, 3714B1E828EDBAAB0056C57A /* DuckPlayerTests.swift */, 567DA94429E95C3F008AC5EE /* YoutubeOverlayUserScriptTests.swift */, @@ -12421,7 +12417,6 @@ 3706FE27293F661700E42796 /* AppPrivacyConfigurationTests.swift in Sources */, B626A7652992506A00053070 /* SerpHeadersNavigationResponderTests.swift in Sources */, 9F6434712BECBA2800D2D8A0 /* SubscriptionRedirectManagerTests.swift in Sources */, - D6E0ACB52CE36FB0005D3486 /* DuckPlayerOverlayPixelsTests.swift in Sources */, 9F26060C2B85C20B00819292 /* AddEditBookmarkDialogViewModelTests.swift in Sources */, 567A23DF2C89980A0010F66C /* OnboardingNavigationDelegateTests.swift in Sources */, 562532A12BC069190034D316 /* ZoomPopoverViewModelTests.swift in Sources */, @@ -14009,7 +14004,6 @@ B630E7FE29C887ED00363609 /* NSErrorAdditionalInfo.swift in Sources */, 370270C02C78EB13002E44E4 /* HomePageSettingsModelTests.swift in Sources */, 4B9292BB2667103100AD2C21 /* BookmarkNodeTests.swift in Sources */, - D6E0ACB42CE36FB0005D3486 /* DuckPlayerOverlayPixelsTests.swift in Sources */, 4B0219A825E0646500ED7DEA /* WebsiteDataStoreTests.swift in Sources */, AAC9C01E24CB6BEB00AD1325 /* TabCollectionViewModelTests.swift in Sources */, 56CE77612C7DFCF800AC1ED2 /* OnboardingSuggestedSearchesProviderTests.swift in Sources */, diff --git a/DuckDuckGo/Tab/TabExtensions/DuckPlayerTabExtension.swift b/DuckDuckGo/Tab/TabExtensions/DuckPlayerTabExtension.swift index f41adb6ea9..1df6438c24 100644 --- a/DuckDuckGo/Tab/TabExtensions/DuckPlayerTabExtension.swift +++ b/DuckDuckGo/Tab/TabExtensions/DuckPlayerTabExtension.swift @@ -73,7 +73,7 @@ final class DuckPlayerTabExtension { self.preferences = preferences self.onboardingDecider = onboardingDecider self.duckPlayerOverlayUsagePixels = duckPlayerOverlayPixels - + webViewPublisher.sink { [weak self] webView in self?.webView = webView }.store(in: &cancellables) @@ -334,7 +334,7 @@ extension DuckPlayerTabExtension: NavigationResponder { // Fire Overlay Shown Pixels fireOverlayShownPixelIfNeeded(url: navigation.url) - + } @MainActor diff --git a/DuckDuckGo/YoutubePlayer/DuckPlayerOverlayPixels.swift b/DuckDuckGo/YoutubePlayer/DuckPlayerOverlayPixels.swift index 95982c3c4d..c3c5362c2f 100644 --- a/DuckDuckGo/YoutubePlayer/DuckPlayerOverlayPixels.swift +++ b/DuckDuckGo/YoutubePlayer/DuckPlayerOverlayPixels.swift @@ -55,7 +55,7 @@ final class DuckPlayerOverlayUsagePixels: NSObject, DuckPlayerOverlayPixelFiring guard let currentURL = webView.url else { return } - + let backItemURL = webView.backForwardList.backItem?.url if lastVisitedURL != nil { diff --git a/UnitTests/YoutubePlayer/DuckPlayerOverlayPixelsTests.swift b/UnitTests/YoutubePlayer/DuckPlayerOverlayPixelsTests.swift deleted file mode 100644 index d7e47b0190..0000000000 --- a/UnitTests/YoutubePlayer/DuckPlayerOverlayPixelsTests.swift +++ /dev/null @@ -1,133 +0,0 @@ -// -// DuckPlayerOverlayPixelsTests.swift -// -// Copyright © 2024 DuckDuckGo. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -import XCTest -import PixelKit -@testable import DuckDuckGo_Privacy_Browser - -final class PixelFiringMock: PixelFiring { - - static var lastPixelsFired = [PixelKitEventV2]() - - static func tearDown() { - lastPixelsFired.removeAll() - } - - func fire(_ event: PixelKitEventV2) { - Self.lastPixelsFired.append(event) - } - - func fire(_ event: PixelKitEventV2, frequency: PixelKit.Frequency) { - Self.lastPixelsFired.append(event) - } -} - -class DuckPlayerOverlayUsagePixelsTests: XCTestCase { - - var duckPlayerOverlayPixels: DuckPlayerOverlayUsagePixels! - - override func setUp() { - super.setUp() - PixelFiringMock.tearDown() - duckPlayerOverlayPixels = DuckPlayerOverlayUsagePixels(pixelFiring: PixelFiringMock(), timeoutInterval: 3.0) - } - - override func tearDown() { - PixelFiringMock.tearDown() - duckPlayerOverlayPixels = nil - super.tearDown() - } - - func testRegisterNavigationAppendsURLToHistory() { - let testURL1 = URL(string: "https://www.youtube.com/watch?v=example1")! - let testURL2 = URL(string: "https://www.youtube.com/playlist?list=PL-example")! - let testURL3 = URL(string: "https://www.example.com")! - - duckPlayerOverlayPixels.handleNavigationAndFirePixels(url: testURL1, duckPlayerMode: .alwaysAsk) - duckPlayerOverlayPixels.handleNavigationAndFirePixels(url: testURL2, duckPlayerMode: .alwaysAsk) - duckPlayerOverlayPixels.handleNavigationAndFirePixels(url: testURL3, duckPlayerMode: .alwaysAsk) - - XCTAssertEqual(duckPlayerOverlayPixels.navigationHistory.count, 3) - XCTAssertEqual(duckPlayerOverlayPixels.navigationHistory[0], testURL1) - XCTAssertEqual(duckPlayerOverlayPixels.navigationHistory[1], testURL2) - XCTAssertEqual(duckPlayerOverlayPixels.navigationHistory[2], testURL3) - } - - func testBackNavigationTriggersBackPixel() { - let firstURL = URL(string: "https://www.youtube.com/watch?v=example1")! - let secondURL = URL(string: "https://www.youtube.com/watch?v=example2")! - - duckPlayerOverlayPixels.handleNavigationAndFirePixels(url: firstURL, duckPlayerMode: .alwaysAsk) - duckPlayerOverlayPixels.handleNavigationAndFirePixels(url: secondURL, duckPlayerMode: .alwaysAsk) - duckPlayerOverlayPixels.handleNavigationAndFirePixels(url: firstURL, duckPlayerMode: .alwaysAsk) - - XCTAssertEqual(PixelFiringMock.lastPixelsFired.last?.name, GeneralPixel.duckPlayerYouTubeOverlayNavigationBack.name) - } - - func testReloadNavigationTriggersRefreshPixel() { - let testURL = URL(string: "https://www.youtube.com/watch?v=XTWWSS")! - - duckPlayerOverlayPixels.handleNavigationAndFirePixels(url: testURL, duckPlayerMode: .alwaysAsk) - duckPlayerOverlayPixels.handleNavigationAndFirePixels(url: testURL, duckPlayerMode: .alwaysAsk) - - XCTAssertEqual(PixelFiringMock.lastPixelsFired.last?.name, GeneralPixel.duckPlayerYouTubeOverlayNavigationRefresh.name) - } - - func testNavigateWithinYoutubeTriggersWithinYouTubePixel() { - let videoURL = URL(string: "https://www.youtube.com/watch?v=example1")! - let playlistURL = URL(string: "https://www.youtube.com/playlist?list=PL-example")! - - duckPlayerOverlayPixels.handleNavigationAndFirePixels(url: videoURL, duckPlayerMode: .alwaysAsk) - duckPlayerOverlayPixels.handleNavigationAndFirePixels(url: playlistURL, duckPlayerMode: .alwaysAsk) - - XCTAssertEqual(PixelFiringMock.lastPixelsFired.last?.name, GeneralPixel.duckPlayerYouTubeNavigationWithinYouTube.name) - } - - func testNavigateOutsideYoutubeTriggersOutsideYouTubePixel() { - let youtubeURL = URL(string: "https://www.youtube.com/watch?v=example1")! - let outsideURL = URL(string: "https://www.example.com")! - - duckPlayerOverlayPixels.handleNavigationAndFirePixels(url: youtubeURL, duckPlayerMode: .alwaysAsk) - duckPlayerOverlayPixels.handleNavigationAndFirePixels(url: outsideURL, duckPlayerMode: .alwaysAsk) - - XCTAssertEqual(PixelFiringMock.lastPixelsFired.last?.name, GeneralPixel.duckPlayerYouTubeOverlayNavigationOutsideYoutube.name) - } - - func testBackNavigationDoesNotTriggerWithinOrOutsideYouTubePixel() { - let firstURL = URL(string: "https://www.youtube.com/watch?v=example1")! - let secondURL = URL(string: "https://www.youtube.com/watch?v=example2")! - let backURL = URL(string: "https://www.youtube.com/watch?v=example1")! - - duckPlayerOverlayPixels.handleNavigationAndFirePixels(url: firstURL, duckPlayerMode: .alwaysAsk) - duckPlayerOverlayPixels.handleNavigationAndFirePixels(url: secondURL, duckPlayerMode: .alwaysAsk) - duckPlayerOverlayPixels.handleNavigationAndFirePixels(url: backURL, duckPlayerMode: .alwaysAsk) - - XCTAssertNotEqual(PixelFiringMock.lastPixelsFired.last?.name, GeneralPixel.duckPlayerYouTubeNavigationWithinYouTube.name) - XCTAssertNotEqual(PixelFiringMock.lastPixelsFired.last?.name, GeneralPixel.duckPlayerYouTubeOverlayNavigationOutsideYoutube.name) - } - - func testReloadNavigationDoesNotTriggerWithinOrOutsideYouTubePixel() { - let testURL = URL(string: "https://www.youtube.com/watch?v=example")! - - duckPlayerOverlayPixels.handleNavigationAndFirePixels(url: testURL, duckPlayerMode: .alwaysAsk) - duckPlayerOverlayPixels.handleNavigationAndFirePixels(url: testURL, duckPlayerMode: .alwaysAsk) - - XCTAssertNotEqual(PixelFiringMock.lastPixelsFired.last?.name, GeneralPixel.duckPlayerYouTubeNavigationWithinYouTube.name) - XCTAssertNotEqual(PixelFiringMock.lastPixelsFired.last?.name, GeneralPixel.duckPlayerYouTubeOverlayNavigationOutsideYoutube.name) - } -} From a49ad5667018277e2c9e7461a710c5b2eb7b5965 Mon Sep 17 00:00:00 2001 From: Daniel Bernal Date: Mon, 9 Dec 2024 16:05:28 +0100 Subject: [PATCH 3/3] Add isObserving prop and removing observers --- .../YoutubePlayer/DuckPlayerOverlayPixels.swift | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/DuckDuckGo/YoutubePlayer/DuckPlayerOverlayPixels.swift b/DuckDuckGo/YoutubePlayer/DuckPlayerOverlayPixels.swift index c3c5362c2f..9b23441abf 100644 --- a/DuckDuckGo/YoutubePlayer/DuckPlayerOverlayPixels.swift +++ b/DuckDuckGo/YoutubePlayer/DuckPlayerOverlayPixels.swift @@ -31,10 +31,11 @@ final class DuckPlayerOverlayUsagePixels: NSObject, DuckPlayerOverlayPixelFiring var pixelFiring: PixelFiring? var duckPlayerMode: DuckPlayerMode = .disabled + private var isObserving = false weak var webView: WKWebView? { didSet { - if let webView = webView { + if let webView { addObservers(to: webView) } } @@ -47,7 +48,9 @@ final class DuckPlayerOverlayUsagePixels: NSObject, DuckPlayerOverlayPixelFiring } deinit { - removeObservers() + if let webView { + removeObservers(from: webView) + } } func fireNavigationPixelsIfNeeded(webView: WKWebView) { @@ -84,12 +87,16 @@ final class DuckPlayerOverlayUsagePixels: NSObject, DuckPlayerOverlayPixelFiring // 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() { - guard let webView = webView else { return } - webView.removeObserver(self, forKeyPath: #keyPath(WKWebView.url)) + private func removeObservers(from webView: WKWebView) { + if isObserving { + webView.removeObserver(self, forKeyPath: #keyPath(WKWebView.url)) + isObserving = false + } } override func observeValue(forKeyPath keyPath: String?, of object: Any?, change: [NSKeyValueChangeKey: Any]?, context: UnsafeMutableRawPointer?) {