Skip to content

Commit

Permalink
[DuckPlayer] 12. Ensure DuckPlayer does not leak without the proper c…
Browse files Browse the repository at this point in the history 
…onfig (#3147)

Task/Issue URL: https://app.asana.com/0/414235014887631/1207900008665900/f

Description:
Add checks to ensure DuckPlayer does not show up in any scenario if the feature flag is off.
  • Loading branch information
@afterxleep
afterxleep authored Jul 26, 2024
1 parent 8209dc6 commit 4e66be5
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 11 deletions.
38 changes: 37 additions & 1 deletion DuckDuckGo/DuckPlayer/DuckPlayerNavigationHandler.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,14 @@ import ContentScopeScripts
import WebKit
import Core
import Common
import BrowserServicesKit

final class DuckPlayerNavigationHandler {

var duckPlayer: DuckPlayerProtocol
var referrer: DuckPlayerReferrer = .other
var lastHandledVideoID: String?
var featureFlagger: FeatureFlagger

private struct Constants {
static let SERPURL = "https://duckduckgo.com/"
Expand All @@ -45,8 +47,10 @@ final class DuckPlayerNavigationHandler {
static let urlInternalReferrer = "embeds_referring_euri"
}

init(duckPlayer: DuckPlayerProtocol = DuckPlayer()) {
init(duckPlayer: DuckPlayerProtocol = DuckPlayer(),
featureFlagger: FeatureFlagger = AppDependencyProvider.shared.featureFlagger) {
self.duckPlayer = duckPlayer
self.featureFlagger = featureFlagger
}

static var htmlTemplatePath: String {
Expand Down Expand Up @@ -99,6 +103,10 @@ final class DuckPlayerNavigationHandler {

guard let url else { return }

guard featureFlagger.isFeatureOn(.duckPlayer) else {
return
}

if let (videoID, _) = url.youtubeVideoParams,
videoID == lastHandledVideoID {
os_log("DP: URL (%s) already handled, skipping", log: .duckPlayerLog, type: .debug, url.absoluteString)
Expand Down Expand Up @@ -136,6 +144,10 @@ extension DuckPlayerNavigationHandler: DuckNavigationHandling {

guard let url = navigationAction.request.url else { return }

guard featureFlagger.isFeatureOn(.duckPlayer) else {
return
}

// Handle Youtube internal links like "Age restricted" and "Copyright restricted" videos
// These should not be handled by DuckPlayer
if url.isYoutubeVideo,
Expand Down Expand Up @@ -202,6 +214,11 @@ extension DuckPlayerNavigationHandler: DuckNavigationHandling {
return
}

guard featureFlagger.isFeatureOn(.duckPlayer) else {
completion(.allow)
return
}

if let (videoID, _) = url.youtubeVideoParams,
videoID == lastHandledVideoID,
!url.hasWatchInYoutubeQueryParameter {
Expand Down Expand Up @@ -240,6 +257,11 @@ extension DuckPlayerNavigationHandler: DuckNavigationHandling {

@MainActor
func handleJSNavigation(url: URL?, webView: WKWebView) {

guard featureFlagger.isFeatureOn(.duckPlayer) else {
return
}

handleURLChange(url: url, webView: webView)
}

Expand All @@ -248,6 +270,11 @@ extension DuckPlayerNavigationHandler: DuckNavigationHandling {

os_log("DP: Handling Back Navigation", log: .duckPlayerLog, type: .debug)

guard featureFlagger.isFeatureOn(.duckPlayer) else {
webView.goBack()
return
}

lastHandledVideoID = nil
webView.stopLoading()

Expand Down Expand Up @@ -280,6 +307,11 @@ extension DuckPlayerNavigationHandler: DuckNavigationHandling {
@MainActor
func handleReload(webView: WKWebView) {

guard featureFlagger.isFeatureOn(.duckPlayer) else {
webView.reload()
return
}

lastHandledVideoID = nil
webView.stopLoading()
if let url = webView.url, url.isDuckPlayer,
Expand All @@ -296,6 +328,10 @@ extension DuckPlayerNavigationHandler: DuckNavigationHandling {
@MainActor
func handleAttach(webView: WKWebView) {

guard featureFlagger.isFeatureOn(.duckPlayer) else {
return
}

if let url = webView.url, url.isDuckPlayer,
!url.isDuckURLScheme,
duckPlayer.settings.mode == .enabled || duckPlayer.settings.mode == .alwaysAsk {
Expand Down
7 changes: 7 additions & 0 deletions DuckDuckGoTests/DuckPlayerMocks.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,3 +152,10 @@ final class MockDuckPlayer: DuckPlayerProtocol {
nil
}
}

final class MockDuckPlayerFeatureFlagger: FeatureFlagger {
func isFeatureOn<F>(forProvider: F) -> Bool where F: BrowserServicesKit.FeatureFlagSourceProviding {
return true
}

}
23 changes: 13 additions & 10 deletions DuckDuckGoTests/YoutublePlayerNavigationHandlerTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class DuckPlayerNavigationHandlerTests: XCTestCase {
var mockPrivacyConfig: PrivacyConfigurationManagerMock!
var playerSettings: MockDuckPlayerSettings!
var player: MockDuckPlayer!
var featureFlagger: FeatureFlagger!

override func setUp() {
super.setUp()
Expand All @@ -42,7 +43,9 @@ class DuckPlayerNavigationHandlerTests: XCTestCase {
mockNavigationDelegate = MockWKNavigationDelegate()
mockAppSettings = AppSettingsMock()
mockPrivacyConfig = PrivacyConfigurationManagerMock()
featureFlagger = MockDuckPlayerFeatureFlagger()
webView.navigationDelegate = mockNavigationDelegate

}

override func tearDown() {
Expand Down Expand Up @@ -129,7 +132,7 @@ class DuckPlayerNavigationHandlerTests: XCTestCase {
let expectation = self.expectation(description: "Completion handler called")
let playerSettings = MockDuckPlayerSettings(appSettings: mockAppSettings, privacyConfigManager: mockPrivacyConfig)
let player = MockDuckPlayer(settings: playerSettings)
let handler = DuckPlayerNavigationHandler(duckPlayer: player)
let handler = DuckPlayerNavigationHandler(duckPlayer: player, featureFlagger: featureFlagger)
var navigationPolicy: WKNavigationActionPolicy?

handler.handleDecidePolicyFor(navigationAction, completion: { policy in
Expand All @@ -152,7 +155,7 @@ class DuckPlayerNavigationHandlerTests: XCTestCase {
let playerSettings = MockDuckPlayerSettings(appSettings: mockAppSettings, privacyConfigManager: mockPrivacyConfig)
playerSettings.mode = .enabled
let player = MockDuckPlayer(settings: playerSettings)
let handler = DuckPlayerNavigationHandler(duckPlayer: player)
let handler = DuckPlayerNavigationHandler(duckPlayer: player, featureFlagger: featureFlagger)
var navigationPolicy: WKNavigationActionPolicy?

handler.handleDecidePolicyFor(navigationAction, completion: { policy in
Expand All @@ -175,7 +178,7 @@ class DuckPlayerNavigationHandlerTests: XCTestCase {
let playerSettings = MockDuckPlayerSettings(appSettings: mockAppSettings, privacyConfigManager: mockPrivacyConfig)
playerSettings.mode = .enabled
let player = MockDuckPlayer(settings: playerSettings)
let handler = DuckPlayerNavigationHandler(duckPlayer: player)
let handler = DuckPlayerNavigationHandler(duckPlayer: player, featureFlagger: featureFlagger)
var navigationPolicy: WKNavigationActionPolicy?

handler.handleDecidePolicyFor(navigationAction, completion: { policy in
Expand All @@ -200,7 +203,7 @@ class DuckPlayerNavigationHandlerTests: XCTestCase {

let playerSettings = MockDuckPlayerSettings(appSettings: mockAppSettings, privacyConfigManager: mockPrivacyConfig)
let player = MockDuckPlayer(settings: playerSettings)
let handler = DuckPlayerNavigationHandler(duckPlayer: player)
let handler = DuckPlayerNavigationHandler(duckPlayer: player, featureFlagger: featureFlagger)

handler.lastHandledVideoID = "abc123"
handler.handleJSNavigation(url: youtubeURL, webView: webView)
Expand All @@ -217,7 +220,7 @@ class DuckPlayerNavigationHandlerTests: XCTestCase {

let playerSettings = MockDuckPlayerSettings(appSettings: mockAppSettings, privacyConfigManager: mockPrivacyConfig)
let player = MockDuckPlayer(settings: playerSettings)
let handler = DuckPlayerNavigationHandler(duckPlayer: player)
let handler = DuckPlayerNavigationHandler(duckPlayer: player, featureFlagger: featureFlagger)

handler.handleJSNavigation(url: youtubeURL, webView: webView)

Expand All @@ -233,7 +236,7 @@ class DuckPlayerNavigationHandlerTests: XCTestCase {
let playerSettings = MockDuckPlayerSettings(appSettings: mockAppSettings, privacyConfigManager: mockPrivacyConfig)
playerSettings.mode = .enabled
let player = MockDuckPlayer(settings: playerSettings)
let handler = DuckPlayerNavigationHandler(duckPlayer: player)
let handler = DuckPlayerNavigationHandler(duckPlayer: player, featureFlagger: featureFlagger)

handler.handleJSNavigation(url: youtubeURL, webView: webView)

Expand All @@ -250,7 +253,7 @@ class DuckPlayerNavigationHandlerTests: XCTestCase {
let playerSettings = MockDuckPlayerSettings(appSettings: mockAppSettings, privacyConfigManager: mockPrivacyConfig)
playerSettings.mode = .enabled
let player = MockDuckPlayer(settings: playerSettings)
let handler = DuckPlayerNavigationHandler(duckPlayer: player)
let handler = DuckPlayerNavigationHandler(duckPlayer: player, featureFlagger: featureFlagger)

handler.handleNavigation(navigationAction, webView: webView)
XCTAssertEqual(webView.url, nil)
Expand All @@ -265,7 +268,7 @@ class DuckPlayerNavigationHandlerTests: XCTestCase {
let playerSettings = MockDuckPlayerSettings(appSettings: mockAppSettings, privacyConfigManager: mockPrivacyConfig)
playerSettings.mode = .enabled
let player = MockDuckPlayer(settings: playerSettings)
let handler = DuckPlayerNavigationHandler(duckPlayer: player)
let handler = DuckPlayerNavigationHandler(duckPlayer: player, featureFlagger: featureFlagger)

handler.handleNavigation(navigationAction, webView: webView)

Expand All @@ -287,7 +290,7 @@ class DuckPlayerNavigationHandlerTests: XCTestCase {
let playerSettings = MockDuckPlayerSettings(appSettings: mockAppSettings, privacyConfigManager: mockPrivacyConfig)
playerSettings.mode = .enabled
let player = MockDuckPlayer(settings: playerSettings)
let handler = DuckPlayerNavigationHandler(duckPlayer: player)
let handler = DuckPlayerNavigationHandler(duckPlayer: player, featureFlagger: featureFlagger)
handler.handleReload(webView: mockWebView)

if let loadedRequest = mockWebView.lastLoadedRequest {
Expand All @@ -305,7 +308,7 @@ class DuckPlayerNavigationHandlerTests: XCTestCase {
let playerSettings = MockDuckPlayerSettings(appSettings: mockAppSettings, privacyConfigManager: mockPrivacyConfig)
playerSettings.mode = .enabled
let player = MockDuckPlayer(settings: playerSettings)
let handler = DuckPlayerNavigationHandler(duckPlayer: player)
let handler = DuckPlayerNavigationHandler(duckPlayer: player, featureFlagger: featureFlagger)
handler.handleAttach(webView: mockWebView)

if let loadedRequest = mockWebView.lastLoadedRequest {
Expand Down

0 comments on commit 4e66be5

Please sign in to comment.