Skip to content

Commit

Permalink
Add support for local overrides for feature flags (#3545)
Browse files Browse the repository at this point in the history
Task/Issue URL: https://app.asana.com/0/72649045549333/1208716221426945/f
Tech Design URL: https://app.asana.com/0/481882893211075/1208716218352496/f

Description:
This change adds support for overriding feature flags locally. FeatureFlagLocalOverrides
is instantiated and passed to DefaultFeatureFlagger to allow local overrides. A dedicated
submenu within Debug menu was added to handle feature flags state. The menu is dynamically
populated and only available to internal users. Local overrides have no effect when
default user flag is disabled.
Currently only 1 flag uses the local overrides: the new HTML New Tab Page flag. It's a WIP feature
and the feature flag currently only presents an empty webview in place of the NTP.
  • Loading branch information
ayoy authored Nov 14, 2024
1 parent 8b531a5 commit 7bf460d
Show file tree
Hide file tree
Showing 18 changed files with 237 additions and 28 deletions.
8 changes: 7 additions & 1 deletion DuckDuckGo.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -1269,6 +1269,8 @@
37F19A6728E1B43200740DC6 /* DuckPlayerPreferences.swift in Sources */ = {isa = PBXBuildFile; fileRef = 37F19A6628E1B43200740DC6 /* DuckPlayerPreferences.swift */; };
37F19A6A28E2F2D000740DC6 /* DuckPlayer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 37F19A6928E2F2D000740DC6 /* DuckPlayer.swift */; };
37F44A5F298C17830025E7FE /* Navigation in Frameworks */ = {isa = PBXBuildFile; productRef = 37F44A5E298C17830025E7FE /* Navigation */; };
37F8ABD32CE3EE5B00CB0294 /* FeatureFlagOverridesMenu.swift in Sources */ = {isa = PBXBuildFile; fileRef = 37F8ABD22CE3EE5B00CB0294 /* FeatureFlagOverridesMenu.swift */; };
37F8ABD42CE3EE5B00CB0294 /* FeatureFlagOverridesMenu.swift in Sources */ = {isa = PBXBuildFile; fileRef = 37F8ABD22CE3EE5B00CB0294 /* FeatureFlagOverridesMenu.swift */; };
37FD78112A29EBD100B36DB1 /* SyncErrorHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 37FD78102A29EBD100B36DB1 /* SyncErrorHandler.swift */; };
37FD78122A29EBD100B36DB1 /* SyncErrorHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 37FD78102A29EBD100B36DB1 /* SyncErrorHandler.swift */; };
4B0135CE2729F1AA00D54834 /* NSPasteboardExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4B0135CD2729F1AA00D54834 /* NSPasteboardExtension.swift */; };
Expand Down Expand Up @@ -3712,6 +3714,7 @@
37F19A6428E1B3FB00740DC6 /* PreferencesDuckPlayerView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PreferencesDuckPlayerView.swift; sourceTree = "<group>"; };
37F19A6628E1B43200740DC6 /* DuckPlayerPreferences.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DuckPlayerPreferences.swift; sourceTree = "<group>"; };
37F19A6928E2F2D000740DC6 /* DuckPlayer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DuckPlayer.swift; sourceTree = "<group>"; };
37F8ABD22CE3EE5B00CB0294 /* FeatureFlagOverridesMenu.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeatureFlagOverridesMenu.swift; sourceTree = "<group>"; };
37FD78102A29EBD100B36DB1 /* SyncErrorHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SyncErrorHandler.swift; sourceTree = "<group>"; };
4B0135CD2729F1AA00D54834 /* NSPasteboardExtension.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NSPasteboardExtension.swift; sourceTree = "<group>"; };
4B02197F25E05FAC00ED7DEA /* FireproofingURLExtensions.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FireproofingURLExtensions.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -5302,6 +5305,7 @@
1D36E651298A84F600AA485D /* InternalUserDecider */ = {
isa = PBXGroup;
children = (
37F8ABD22CE3EE5B00CB0294 /* FeatureFlagOverridesMenu.swift */,
1D36E657298AA3BA00AA485D /* InternalUserDeciderStore.swift */,
);
path = InternalUserDecider;
Expand Down Expand Up @@ -11304,6 +11308,7 @@
3706FB13293F65D500E42796 /* Bookmark.swift in Sources */,
3706FB14293F65D500E42796 /* ConnectBitwardenViewModel.swift in Sources */,
3706FB15293F65D500E42796 /* NSNotificationName+DataImport.swift in Sources */,
37F8ABD42CE3EE5B00CB0294 /* FeatureFlagOverridesMenu.swift in Sources */,
EE6666702B56EDE4001D898D /* VPNLocationsHostingViewController.swift in Sources */,
3706FB16293F65D500E42796 /* StoredPermission.swift in Sources */,
3706FB17293F65D500E42796 /* FirePopoverCollectionViewHeader.swift in Sources */,
Expand Down Expand Up @@ -13055,6 +13060,7 @@
37F19A6A28E2F2D000740DC6 /* DuckPlayer.swift in Sources */,
AA5FA69A275F91C700DCE9C9 /* Favicon.swift in Sources */,
AABEE69A24A902A90043105B /* SuggestionContainerViewModel.swift in Sources */,
37F8ABD32CE3EE5B00CB0294 /* FeatureFlagOverridesMenu.swift in Sources */,
AA840A9827319D1600E63CDD /* FirePopoverWrapperViewController.swift in Sources */,
4B85A48028821CC500FC4C39 /* NSPasteboardItemExtension.swift in Sources */,
37CD54CA27F2FDD100F1F7B9 /* AutofillPreferencesModel.swift in Sources */,
Expand Down Expand Up @@ -15066,7 +15072,7 @@
repositoryURL = "https://github.com/duckduckgo/BrowserServicesKit";
requirement = {
kind = exactVersion;
version = 209.1.2;
version = 210.0.0;
};
};
9FF521422BAA8FF300B9819B /* XCRemoteSwiftPackageReference "lottie-spm" */ = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
"kind" : "remoteSourceControl",
"location" : "https://github.com/duckduckgo/BrowserServicesKit",
"state" : {
"revision" : "16a1f44f24fd492a87fc28571cac87022c1b740b",
"version" : "209.1.2"
"revision" : "cfb178099738bc6cd0c3a3d73df717420ef4a59f",
"version" : "210.0.0"
}
},
{
Expand Down Expand Up @@ -75,7 +75,7 @@
{
"identity" : "lottie-spm",
"kind" : "remoteSourceControl",
"location" : "https://github.com/airbnb/lottie-spm",
"location" : "https://github.com/airbnb/lottie-spm.git",
"state" : {
"revision" : "1d29eccc24cc8b75bff9f6804155112c0ffc9605",
"version" : "4.4.3"
Expand Down
8 changes: 7 additions & 1 deletion DuckDuckGo/Application/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import Configuration
import CoreData
import Crashes
import DDGSync
import FeatureFlags
import History
import MetricKit
import Networking
Expand Down Expand Up @@ -264,7 +265,12 @@ final class AppDelegate: NSObject, NSApplicationDelegate {

featureFlagger = DefaultFeatureFlagger(
internalUserDecider: internalUserDecider,
privacyConfigManager: AppPrivacyFeatures.shared.contentBlocking.privacyConfigurationManager
privacyConfigManager: AppPrivacyFeatures.shared.contentBlocking.privacyConfigurationManager,
localOverrides: FeatureFlagLocalOverrides(
keyValueStore: UserDefaults.appConfiguration,
actionHandler: FeatureFlagOverridesPublishingHandler<FeatureFlag>()
),
for: FeatureFlag.self
)

onboardingStateMachine = ContextualOnboardingStateMachine()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,10 @@ extension WKWebViewConfiguration {

if SupportedOSChecker.isCurrentOSReceivingUpdates {
if urlSchemeHandler(forURLScheme: URL.NavigationalScheme.duck.rawValue) == nil {
setURLSchemeHandler(DuckURLSchemeHandler(), forURLScheme: URL.NavigationalScheme.duck.rawValue)
setURLSchemeHandler(
DuckURLSchemeHandler(featureFlagger: NSApp.delegateTyped.featureFlagger),
forURLScheme: URL.NavigationalScheme.duck.rawValue
)
}
}

Expand Down
1 change: 0 additions & 1 deletion DuckDuckGo/Common/Utilities/UserDefaultsWrapper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,6 @@ public struct UserDefaultsWrapper<T> {

case lastBrokenSiteToastShownDate = "brokenSitePrompt.last-broken-site-toast-shown-date"
case toastDismissStreakCounter = "brokenSitePrompt.toast-dismiss-streak-counter"

}

enum RemovedKeys: String, CaseIterable {
Expand Down
112 changes: 112 additions & 0 deletions DuckDuckGo/InternalUserDecider/FeatureFlagOverridesMenu.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
//
// FeatureFlagOverridesMenu.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 AppKit
import BrowserServicesKit
import FeatureFlags

final class FeatureFlagOverridesMenu: NSMenu {

let featureFlagger: FeatureFlagger

let setInternalUserStateItem: NSMenuItem = {
let item = NSMenuItem(title: "Set Internal User State First")
item.isEnabled = false
return item
}()

init(featureFlagOverrides: FeatureFlagger) {
self.featureFlagger = featureFlagOverrides
super.init(title: "")

buildItems {
setInternalUserStateItem
NSMenuItem.separator()

FeatureFlag.allCases.filter(\.supportsLocalOverriding).map { flag in
NSMenuItem(
title: "\(flag.rawValue) (default: \(featureFlagger.isFeatureOn(for: flag, allowOverride: false) ? "on" : "off"))",
action: #selector(toggleFeatureFlag(_:)),
target: self,
representedObject: flag
)
}

NSMenuItem.separator()
NSMenuItem(title: "Remove All Overrides", action: #selector(resetAllOverrides(_:))).targetting(self)
}
}

required init(coder: NSCoder) {
fatalError("init(coder:) has not been implemented")
}

override func update() {
super.update()

items.forEach { item in
guard let flag = item.representedObject as? FeatureFlag else {
return
}
item.isHidden = !featureFlagger.internalUserDecider.isInternalUser
item.title = "\(flag.rawValue) (default: \(defaultValue(for: flag)), override: \(overrideValue(for: flag)))"
let override = featureFlagger.localOverrides?.override(for: flag)
item.state = override == true ? .on : .off

if override != nil {
item.submenu = NSMenu(items: [
NSMenuItem(
title: "Remove Override",
action: #selector(resetOverride(_:)),
target: self,
representedObject: flag
)
])
} else {
item.submenu = nil
}
}

setInternalUserStateItem.isHidden = featureFlagger.internalUserDecider.isInternalUser
}

private func defaultValue(for flag: FeatureFlag) -> String {
featureFlagger.isFeatureOn(for: flag, allowOverride: false) ? "on" : "off"
}

private func overrideValue(for flag: FeatureFlag) -> String {
guard let override = featureFlagger.localOverrides?.override(for: flag) else {
return "none"
}
return override ? "on" : "off"
}

@objc func toggleFeatureFlag(_ sender: NSMenuItem) {
guard let featureFlag = sender.representedObject as? FeatureFlag else { return }
featureFlagger.localOverrides?.toggleOverride(for: featureFlag)
}

@objc func resetOverride(_ sender: NSMenuItem) {
guard let featureFlag = sender.representedObject as? FeatureFlag else { return }
featureFlagger.localOverrides?.clearOverride(for: featureFlag)
}

@objc func resetAllOverrides(_ sender: NSMenuItem) {
featureFlagger.localOverrides?.clearAllOverrides(for: FeatureFlag.self)
}
}
4 changes: 4 additions & 0 deletions DuckDuckGo/Menus/MainMenu.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import BrowserServicesKit
import Cocoa
import Common
import Combine
import FeatureFlags
import OSLog
import SwiftUI
import WebKit
Expand Down Expand Up @@ -618,6 +619,9 @@ final class MainMenu: NSMenu {
@MainActor
private func setupDebugMenu() -> NSMenu {
let debugMenu = NSMenu(title: "Debug") {
NSMenuItem(title: "Feature Flag Overrides")
.submenu(FeatureFlagOverridesMenu(featureFlagOverrides: NSApp.delegateTyped.featureFlagger))
NSMenuItem.separator()
NSMenuItem(title: "Open Vanilla Browser", action: #selector(MainViewController.openVanillaBrowser)).withAccessibilityIdentifier("MainMenu.openVanillaBrowser")
NSMenuItem.separator()
NSMenuItem(title: "Tab") {
Expand Down
33 changes: 30 additions & 3 deletions DuckDuckGo/Tab/View/BrowserTabViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import BrowserServicesKit
import Cocoa
import Combine
import Common
import FeatureFlags
import SwiftUI
import WebKit
import Subscription
Expand Down Expand Up @@ -132,6 +133,7 @@ final class BrowserTabViewController: NSViewController {
hoverLabelContainer.alphaValue = 0
subscribeToTabs()
subscribeToSelectedTabViewModel()
subscribeToHTMLNewTabPageFeatureFlagChanges()

if let webViewContainer {
removeChild(in: self.containerStackView, webViewContainer: webViewContainer)
Expand Down Expand Up @@ -273,6 +275,25 @@ final class BrowserTabViewController: NSViewController {
}
}

private func subscribeToHTMLNewTabPageFeatureFlagChanges() {
guard let overridesHandler = featureFlagger.localOverrides?.actionHandler as? FeatureFlagOverridesPublishingHandler<FeatureFlag> else {
return
}

overridesHandler.flagDidChangePublisher
.filter { $0.0 == .htmlNewTabPage }
.asVoid()
.sink { [weak self] in
guard let self, let tabViewModel else {
return
}
if tabViewModel.tab.content == .newtab {
showTabContent(of: tabViewModel)
}
}
.store(in: &cancellables)
}

private func subscribeToSelectedTabViewModel() {
tabCollectionViewModel.$selectedTabViewModel
.sink { [weak self] selectedTabViewModel in
Expand Down Expand Up @@ -777,8 +798,12 @@ final class BrowserTabViewController: NSViewController {
updateTabIfNeeded(tabViewModel: tabViewModel)

case .newtab:
removeAllTabContent()
addAndLayoutChild(homePageViewControllerCreatingIfNeeded())
if NSApp.delegateTyped.featureFlagger.isFeatureOn(.htmlNewTabPage) {
updateTabIfNeeded(tabViewModel: tabViewModel)
} else {
removeAllTabContent()
addAndLayoutChild(homePageViewControllerCreatingIfNeeded())
}

case .dataBrokerProtection:
removeAllTabContent()
Expand Down Expand Up @@ -835,7 +860,9 @@ final class BrowserTabViewController: NSViewController {
switch tabViewModel.tab.content {
case .onboarding:
return
case .newtab, .settings:
case .newtab:
containsHostingView = !NSApp.delegateTyped.featureFlagger.isFeatureOn(.htmlNewTabPage)
case .settings:
containsHostingView = true
default:
containsHostingView = false
Expand Down
23 changes: 23 additions & 0 deletions DuckDuckGo/YoutubePlayer/DuckURLSchemeHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,21 @@
// limitations under the License.
//

import BrowserServicesKit
import FeatureFlags
import Foundation
import WebKit
import ContentScopeScripts
import PhishingDetection

final class DuckURLSchemeHandler: NSObject, WKURLSchemeHandler {

let featureFlagger: FeatureFlagger

init(featureFlagger: FeatureFlagger) {
self.featureFlagger = featureFlagger
}

func webView(_ webView: WKWebView, start urlSchemeTask: WKURLSchemeTask) {
guard let requestURL = webView.url ?? urlSchemeTask.request.url else {
assertionFailure("No URL for Duck scheme handler")
Expand All @@ -36,6 +44,12 @@ final class DuckURLSchemeHandler: NSObject, WKURLSchemeHandler {
handleDuckPlayer(requestURL: requestURL, urlSchemeTask: urlSchemeTask, webView: webView)
case .phishingErrorPage:
handleErrorPage(urlSchemeTask: urlSchemeTask)
case .newTab:
if featureFlagger.isFeatureOn(.htmlNewTabPage) {
handleSpecialPages(urlSchemeTask: urlSchemeTask)
} else {
handleNativeUIPages(requestURL: requestURL, urlSchemeTask: urlSchemeTask)
}
default:
handleNativeUIPages(requestURL: requestURL, urlSchemeTask: urlSchemeTask)
}
Expand Down Expand Up @@ -127,6 +141,8 @@ private extension DuckURLSchemeHandler {
directoryURL = URL(fileURLWithPath: "/pages/onboarding")
} else if url.isReleaseNotesScheme {
directoryURL = URL(fileURLWithPath: "/pages/release-notes")
} else if url.isNewTab {
directoryURL = URL(fileURLWithPath: "/pages/new-tab")
} else {
assertionFailure("Unknown scheme")
return nil
Expand Down Expand Up @@ -205,6 +221,7 @@ private extension DuckURLSchemeHandler {

extension URL {
enum URLType {
case newTab
case onboarding
case duckPlayer
case releaseNotes
Expand All @@ -220,6 +237,8 @@ extension URL {
return .phishingErrorPage
} else if self.isReleaseNotesScheme {
return .releaseNotes
} else if self.isNewTab {
return .newTab
} else {
return nil
}
Expand All @@ -229,6 +248,10 @@ extension URL {
return isDuckURLScheme && host == "onboarding"
}

var isNewTab: Bool {
return isDuckURLScheme && host == "newtab"
}

var isDuckURLScheme: Bool {
navigationalScheme == .duck
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,10 @@ class PhishingDetectionIntegrationTests: XCTestCase {
}

class MockFeatureFlagger: FeatureFlagger {
func isFeatureOn<F>(forProvider: F) -> Bool where F: BrowserServicesKit.FeatureFlagSourceProviding {
var internalUserDecider: InternalUserDecider = DefaultInternalUserDecider(store: MockInternalUserStoring())
var localOverrides: FeatureFlagLocalOverriding?

func isFeatureOn<Flag: FeatureFlagDescribing>(for featureFlag: Flag, allowOverride: Bool) -> Bool {
return true
}
}
Loading

0 comments on commit 7bf460d

Please sign in to comment.