Skip to content

Commit

Permalink
Revert "Resolving automatic update edge cases (#3142)"
Browse files Browse the repository at this point in the history 
This reverts commit c165e74.

# Conflicts:
#	DuckDuckGo/Updates/BinaryOwnershipChecker.swift
#	DuckDuckGo/Updates/UpdateController.swift
  • Loading branch information
@quanganhdo
quanganhdo committed Sep 3, 2024
1 parent 10359ba commit 5c83056
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 242 deletions.
8 changes: 0 additions & 8 deletions DuckDuckGo.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@
1D220BF92B86192200F8BBC6 /* PreferencesEmailProtectionView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1D220BF72B86192200F8BBC6 /* PreferencesEmailProtectionView.swift */; };
1D220BFC2B87AACF00F8BBC6 /* PrivacyProtectionStatus.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1D220BFB2B87AACF00F8BBC6 /* PrivacyProtectionStatus.swift */; };
1D220BFD2B87AACF00F8BBC6 /* PrivacyProtectionStatus.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1D220BFB2B87AACF00F8BBC6 /* PrivacyProtectionStatus.swift */; };
1D232E942C7860DA0043840D /* BinaryOwnershipChecker.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1D232E932C7860DA0043840D /* BinaryOwnershipChecker.swift */; };
1D232E992C7870D90043840D /* BinaryOwnershipCheckerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1D232E962C786E7D0043840D /* BinaryOwnershipCheckerTests.swift */; };
1D26EBAC2B74BECB0002A93F /* NSImageSendable.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1D26EBAB2B74BECB0002A93F /* NSImageSendable.swift */; };
1D26EBAD2B74BECB0002A93F /* NSImageSendable.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1D26EBAB2B74BECB0002A93F /* NSImageSendable.swift */; };
1D26EBB02B74DB600002A93F /* TabSnapshotCleanupService.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1D26EBAF2B74DB600002A93F /* TabSnapshotCleanupService.swift */; };
Expand Down Expand Up @@ -3054,8 +3052,6 @@
1D1C36E529FB019C001FA40C /* HistoryTabExtensionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HistoryTabExtensionTests.swift; sourceTree = "<group>"; };
1D220BF72B86192200F8BBC6 /* PreferencesEmailProtectionView.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PreferencesEmailProtectionView.swift; sourceTree = "<group>"; };
1D220BFB2B87AACF00F8BBC6 /* PrivacyProtectionStatus.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PrivacyProtectionStatus.swift; sourceTree = "<group>"; };
1D232E932C7860DA0043840D /* BinaryOwnershipChecker.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BinaryOwnershipChecker.swift; sourceTree = "<group>"; };
1D232E962C786E7D0043840D /* BinaryOwnershipCheckerTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BinaryOwnershipCheckerTests.swift; sourceTree = "<group>"; };
1D26EBAB2B74BECB0002A93F /* NSImageSendable.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NSImageSendable.swift; sourceTree = "<group>"; };
1D26EBAF2B74DB600002A93F /* TabSnapshotCleanupService.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TabSnapshotCleanupService.swift; sourceTree = "<group>"; };
1D36E657298AA3BA00AA485D /* InternalUserDeciderStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InternalUserDeciderStore.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -4855,7 +4851,6 @@
1D72D59B2BFF61B200AEDE36 /* UpdateNotificationPresenter.swift */,
1D9297BE2C1B062900A38521 /* ApplicationUpdateDetector.swift */,
1D0DE93D2C3BA9840037ABC2 /* AppRestarter.swift */,
1D232E932C7860DA0043840D /* BinaryOwnershipChecker.swift */,
1D39E5762C2BFD5700757339 /* ReleaseNotesTabExtension.swift */,
1D39E5792C2C0F3700757339 /* ReleaseNotesUserScript.swift */,
1D0DE9402C3BB9CC0037ABC2 /* ReleaseNotesParser.swift */,
Expand Down Expand Up @@ -4896,7 +4891,6 @@
children = (
1D838A312C44F0180078373F /* ReleaseNotesParserTests.swift */,
1D638D602C44F2BA00530DD5 /* ApplicationUpdateDetectorTests.swift */,
1D232E962C786E7D0043840D /* BinaryOwnershipCheckerTests.swift */,
);
path = Updates;
sourceTree = "<group>";
Expand Down Expand Up @@ -12282,7 +12276,6 @@
37BF3F21286F0A7A00BD9014 /* PinnedTabsViewModel.swift in Sources */,
EEC4A6692B2C87D300F7C0AA /* VPNLocationView.swift in Sources */,
AAC5E4D225D6A709007F5990 /* BookmarkList.swift in Sources */,
1D232E942C7860DA0043840D /* BinaryOwnershipChecker.swift in Sources */,
B602E81D2A1E25B1006D261F /* NEOnDemandRuleExtension.swift in Sources */,
56A0543E2C215FB3007D8FAB /* OnboardingUserScript.swift in Sources */,
C1372EF42BBC5BAD003F8793 /* SecureTextField.swift in Sources */,
Expand Down Expand Up @@ -12601,7 +12594,6 @@
BDCB66D82C7CE1A600E8ABC9 /* VPNFeedbackFormViewModelTests.swift in Sources */,
986189E62A7CFB3E001B4519 /* LocalBookmarkStoreSavingTests.swift in Sources */,
AA652CD325DDA6E9009059CC /* LocalBookmarkManagerTests.swift in Sources */,
1D232E992C7870D90043840D /* BinaryOwnershipCheckerTests.swift in Sources */,
CBDD5DE329A67F2700832877 /* MockConfigurationStore.swift in Sources */,
9F3910692B68D87B00CB5112 /* ProgressExtensionTests.swift in Sources */,
B63ED0DC26AE7B1E00A9DAD1 /* WebViewMock.swift in Sources */,
Expand Down
66 changes: 0 additions & 66 deletions DuckDuckGo/Updates/BinaryOwnershipChecker.swift
View file

This file was deleted.

129 changes: 50 additions & 79 deletions DuckDuckGo/Updates/UpdateController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,26 +59,27 @@ final class UpdateController: NSObject, UpdateControllerProtocol {
lazy var notificationPresenter = UpdateNotificationPresenter()
let willRelaunchAppPublisher: AnyPublisher<Void, Never>

@Published private(set) var isUpdateBeingLoaded = false
var isUpdateBeingLoadedPublisher: Published<Bool>.Publisher { $isUpdateBeingLoaded }
init(internalUserDecider: InternalUserDecider,
appRestarter: AppRestarting = AppRestarter()) {
willRelaunchAppPublisher = willRelaunchAppSubject.eraseToAnyPublisher()
self.internalUserDecider = internalUserDecider
self.appRestarter = appRestarter
super.init()

// Struct used to cache data until the updater finishes checking for updates
struct UpdateCheckResult {
let item: SUAppcastItem
let isInstalled: Bool
configureUpdater()
}
private var updateCheckResult: UpdateCheckResult?

@Published private(set) var isUpdateBeingLoaded = false
var isUpdateBeingLoadedPublisher: Published<Bool>.Publisher { $isUpdateBeingLoaded }

@Published private(set) var latestUpdate: Update? {
didSet {
if let latestUpdate, !latestUpdate.isInstalled {
if !shouldShowManualUpdateDialog {
switch latestUpdate.type {
case .critical:
notificationPresenter.showUpdateNotification(icon: NSImage.criticalUpdateNotificationInfo, text: UserText.criticalUpdateNotification, presentMultiline: true)
case .regular:
notificationPresenter.showUpdateNotification(icon: NSImage.updateNotificationInfo, text: UserText.updateAvailableNotification, presentMultiline: true)
}
switch latestUpdate.type {
case .critical:
notificationPresenter.showUpdateNotification(icon: NSImage.criticalUpdateNotificationInfo, text: UserText.criticalUpdateNotification, presentMultiline: true)
case .regular:
notificationPresenter.showUpdateNotification(icon: NSImage.updateNotificationInfo, text: UserText.updateAvailableNotification, presentMultiline: true)
}
isUpdateAvailableToInstall = !latestUpdate.isInstalled
} else {
Expand Down Expand Up @@ -112,34 +113,15 @@ final class UpdateController: NSObject, UpdateControllerProtocol {
}
}

var automaticUpdateFlow: Bool {
// In case the current user is not the owner of the binary, we have to switch
// to manual update flow because the authentication is required.
return areAutomaticUpdatesEnabled && binaryOwnershipChecker.isCurrentUserOwner()
}

var shouldShowManualUpdateDialog = false

private(set) var updater: SPUStandardUpdaterController!
private var appRestarter: AppRestarting
private let willRelaunchAppSubject = PassthroughSubject<Void, Never>()
private var internalUserDecider: InternalUserDecider
private let binaryOwnershipChecker: BinaryOwnershipChecking

// MARK: - Public

init(internalUserDecider: InternalUserDecider,
appRestarter: AppRestarting = AppRestarter(),
binaryOwnershipChecker: BinaryOwnershipChecking = BinaryOwnershipChecker()) {
willRelaunchAppPublisher = willRelaunchAppSubject.eraseToAnyPublisher()
self.internalUserDecider = internalUserDecider
self.appRestarter = appRestarter
self.binaryOwnershipChecker = binaryOwnershipChecker
super.init()

configureUpdater()
}

func checkNewApplicationVersion() {
let updateStatus = ApplicationUpdateDetector.isApplicationUpdated()
switch updateStatus {
Expand All @@ -163,40 +145,42 @@ final class UpdateController: NSObject, UpdateControllerProtocol {
updater.updater.checkForUpdatesInBackground()
}

@objc func runUpdate() {
PixelKit.fire(DebugEvent(GeneralPixel.updaterDidRunUpdate))

if automaticUpdateFlow {
appRestarter.restart()
} else {
updater.userDriver.activeUpdateAlert?.hideUnnecessaryUpdateButtons()
shouldShowManualUpdateDialog = true
checkForUpdate()
}
}

// MARK: - Private

private func configureUpdater() {
// The default configuration of Sparkle updates is in Info.plist
updater = SPUStandardUpdaterController(updaterDelegate: self, userDriverDelegate: self)
shouldShowManualUpdateDialog = false

if updater.updater.automaticallyDownloadsUpdates != automaticUpdateFlow {
updater.updater.automaticallyDownloadsUpdates = automaticUpdateFlow
if updater.updater.automaticallyDownloadsUpdates != areAutomaticUpdatesEnabled {
updater.updater.automaticallyDownloadsUpdates = areAutomaticUpdatesEnabled
}

#if DEBUG
updater.updater.automaticallyChecksForUpdates = false
updater.updater.automaticallyDownloadsUpdates = false
updater.updater.updateCheckInterval = 0
#endif

checkForUpdateInBackground()
}

@objc private func openUpdatesPage() {
@objc func openUpdatesPage() {
notificationPresenter.openUpdatesPage()
}

@objc func runUpdate() {
PixelKit.fire(DebugEvent(GeneralPixel.updaterDidRunUpdate))

if areAutomaticUpdatesEnabled {
appRestarter.restart()
} else {
updater.userDriver.activeUpdateAlert?.hideUnnecessaryUpdateButtons()
shouldShowManualUpdateDialog = true
checkForUpdate()
}
}

}

extension UpdateController: SPUStandardUserDriverDelegate {
Expand All @@ -218,7 +202,6 @@ extension UpdateController: SPUUpdaterDelegate {
}

private func onUpdateCheckStart() {
updateCheckResult = nil
isUpdateBeingLoaded = true
}

Expand Down Expand Up @@ -252,59 +235,47 @@ extension UpdateController: SPUUpdaterDelegate {

PixelKit.fire(DebugEvent(GeneralPixel.updaterDidFindUpdate))

if !automaticUpdateFlow {
// For manual updates, we can present the available update without waiting for the update cycle to finish. The Sparkle flow downloads the update later
updateCheckResult = UpdateCheckResult(item: item, isInstalled: false)
onUpdateCheckEnd()
guard !areAutomaticUpdatesEnabled else {
// If automatic updates are enabled, we are waiting until the update is downloaded
return
}
// For manual updates, show the available update without downloading
onUpdateCheckEnd(item: item, isInstalled: false)
}

func updaterDidNotFindUpdate(_ updater: SPUUpdater, error: any Error) {
let item = (error as NSError).userInfo["SULatestAppcastItemFound"] as? SUAppcastItem
Logger.updates.debug("Updater did not find update: \(String(describing: item?.displayVersionString))(\(String(describing: item?.versionString)))")
if let item {
// User is running the latest version
updateCheckResult = UpdateCheckResult(item: item, isInstalled: true)
}

onUpdateCheckEnd(item: item, isInstalled: true)

PixelKit.fire(DebugEvent(GeneralPixel.updaterDidNotFindUpdate, error: error))
}

func updater(_ updater: SPUUpdater, didDownloadUpdate item: SUAppcastItem) {
Logger.updates.debug("Updater did download update: \(item.displayVersionString)(\(item.versionString))")

if automaticUpdateFlow {
// For automatic updates, the available item has to be downloaded
updateCheckResult = UpdateCheckResult(item: item, isInstalled: false)
guard areAutomaticUpdatesEnabled else {
// If manual are enabled, we don't download
return
}
// Automatic updates present the available update after it's downloaded
onUpdateCheckEnd(item: item, isInstalled: false)

PixelKit.fire(DebugEvent(GeneralPixel.updaterDidDownloadUpdate))
}

func updater(_ updater: SPUUpdater, didFinishUpdateCycleFor updateCheck: SPUUpdateCheck, error: (any Error)?) {
Logger.updates.debug("Updater did finish update cycle")

onUpdateCheckEnd()
}

private func onUpdateCheckEnd() {
guard isUpdateBeingLoaded else {
// The update check end is already handled
return
}

// If the update is available, present it
if let updateCheckResult = updateCheckResult {
latestUpdate = Update(appcastItem: updateCheckResult.item,
isInstalled: updateCheckResult.isInstalled)
private func onUpdateCheckEnd(item: SUAppcastItem?, isInstalled: Bool) {
if let item {
latestUpdate = Update(appcastItem: item, isInstalled: isInstalled)
} else {
latestUpdate = nil
}

// Clear cache
isUpdateBeingLoaded = false
updateCheckResult = nil
}

func updater(_ updater: SPUUpdater, didFinishUpdateCycleFor updateCheck: SPUUpdateCheck, error: (any Error)?) {
Logger.updates.debug("Updater did finish update cycle")
}

}
Expand Down
2 changes: 0 additions & 2 deletions LocalPackages/BuildToolPlugins/Plugins/InputFilesChecker/InputFilesChecker.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import PackagePlugin
import XcodeProjectPlugin

let nonSandboxedExtraInputFiles: Set<InputFile> = [
.init("BinaryOwnershipChecker.swift", .source),
.init("BWEncryption.m", .source),
.init("BWEncryptionOutput.m", .source),
.init("BWManager.swift", .source),
Expand Down Expand Up @@ -50,7 +49,6 @@ let extraInputFiles: [TargetName: Set<InputFile>] = [
"DuckDuckGo Privacy Pro": nonSandboxedExtraInputFiles,

"Unit Tests": [
.init("BinaryOwnershipCheckerTests.swift", .source),
.init("BWEncryptionTests.swift", .source),
.init("WKWebViewPrivateMethodsAvailabilityTests.swift", .source)
],
Expand Down
Loading

0 comments on commit 5c83056

Please sign in to comment.