Skip to content

Commit

Permalink
Refactor Overlay Pixels (#3644)
Browse files Browse the repository at this point in the history 
Task/Issue URL:
https://app.asana.com/0/1204099484721401/1208674332470432/f
Tech Design URL:
CC:

**Description**:
Refactors Overlay Pixels to use WebKit KVO and backForward lists for
improved reliability
  • Loading branch information
@afterxleep
afterxleep authored Dec 9, 2024
1 parent b927078 commit 8a4f89e
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 208 deletions.
6 changes: 0 additions & 6 deletions DuckDuckGo.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3052,8 +3052,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 */; };
Expand Down Expand Up @@ -4914,7 +4912,6 @@
CDE248A72C821FFE00F9399D /* MaliciousSiteProtectionState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MaliciousSiteProtectionState.swift; sourceTree = "<group>"; };
D64A5FF72AEA5C2B00B6D6E7 /* HomeButtonMenuFactory.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HomeButtonMenuFactory.swift; sourceTree = "<group>"; };
D6E0ACB02CE36DC4005D3486 /* DuckPlayerOverlayPixels.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DuckPlayerOverlayPixels.swift; sourceTree = "<group>"; };
D6E0ACB32CE36FA8005D3486 /* DuckPlayerOverlayPixelsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DuckPlayerOverlayPixelsTests.swift; sourceTree = "<group>"; };
EA0BA3A8272217E6002A0B6C /* ClickToLoadUserScript.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ClickToLoadUserScript.swift; sourceTree = "<group>"; };
EA18D1C9272F0DC8006DC101 /* social_images */ = {isa = PBXFileReference; lastKnownFileType = folder; path = social_images; sourceTree = "<group>"; };
EA8AE769279FBDB20078943E /* ClickToLoadTDSTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ClickToLoadTDSTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -5797,7 +5794,6 @@
376718FE28E58504003A2A15 /* YoutubePlayer */ = {
isa = PBXGroup;
children = (
D6E0ACB32CE36FA8005D3486 /* DuckPlayerOverlayPixelsTests.swift */,
3199AF812C80736B003AEBDC /* DuckPlayerOnboardingLocationValidatorTests.swift */,
3714B1E828EDBAAB0056C57A /* DuckPlayerTests.swift */,
567DA94429E95C3F008AC5EE /* YoutubeOverlayUserScriptTests.swift */,
Expand Down Expand Up @@ -12313,7 +12309,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 */,
Expand Down Expand Up @@ -13878,7 +13873,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 */,
Expand Down
37 changes: 24 additions & 13 deletions DuckDuckGo/Tab/TabExtensions/DuckPlayerTabExtension.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,17 @@ final class DuckPlayerTabExtension {
didSet {
youtubeOverlayScript?.webView = webView
youtubePlayerScript?.webView = webView
if duckPlayerOverlayUsagePixels.webView == nil {
duckPlayerOverlayUsagePixels.webView = webView
}
}
}
private weak var youtubeOverlayScript: YoutubeOverlayUserScript?
private weak var youtubePlayerScript: YoutubePlayerUserScript?
private let onboardingDecider: DuckPlayerOnboardingDecider
private var shouldSelectNextNewTab: Bool?
private var duckPlayerOverlayUsagePixels: DuckPlayerOverlayPixelFiring
private var duckPlayerModeCancellable: AnyCancellable?

init(duckPlayer: DuckPlayer,
isBurner: Bool,
Expand All @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -320,10 +335,6 @@ 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
Expand Down
128 changes: 72 additions & 56 deletions DuckDuckGo/YoutubePlayer/DuckPlayerOverlayPixels.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,84 +16,100 @@
// 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
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? = 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 {
if let webView {
removeObservers(from: webView)
}
}

// 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
}

// 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))
let backItemURL = webView.backForwardList.backItem?.url

if lastVisitedURL != nil {
// Back navigation
if currentURL == backItemURL {
firePixelIfNeeded(GeneralPixel.duckPlayerYouTubeOverlayNavigationBack, url: lastVisitedURL)
}
// Regular navigation
else {
if currentURL.isYoutube {
firePixelIfNeeded(GeneralPixel.duckPlayerYouTubeNavigationWithinYouTube, url: lastVisitedURL)
} else {
firePixelIfNeeded(GeneralPixel.duckPlayerYouTubeOverlayNavigationOutsideYoutube, url: lastVisitedURL)
}
}
}

// Update the last visited URL
lastVisitedURL = currentURL
}

func fireReloadPixelIfNeeded(url: URL) {
firePixelIfNeeded(GeneralPixel.duckPlayerYouTubeOverlayNavigationRefresh, url: url)
}

// 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 firePixel(_ pixel: PixelKitEventV2) {
pixelFiring?.fire(pixel)
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?) {
guard let webView = object as? WKWebView else { return }

if keyPath == #keyPath(WKWebView.url) {
fireNavigationPixelsIfNeeded(webView: webView)
}
}

private func firePixelIfNeeded(_ pixel: PixelKitEventV2, url: URL?) {
if let url, url.isYoutubeWatch, duckPlayerMode == .alwaysAsk {
pixelFiring?.fire(pixel)
}
}
}
Loading

0 comments on commit 8a4f89e

Please sign in to comment.