Add SYNC-3478 [v109] Poll for missing send tabs on interval

Account/FxAPushMessageHandler.swift

@@ -29,7 +29,7 @@ extension FxAPushMessageHandler {
     /// Accepts the raw Push message from Autopush.
     /// This method then decrypts it according to the content-encoding (aes128gcm or aesgcm)
     /// and then effects changes on the logged in account.
-    @discardableResult func handle(userInfo: [AnyHashable: Any]) -> PushMessageResult {
+    @discardableResult func handle(userInfo: [AnyHashable: Any]) -> PushMessageResults {
         let keychain = MZKeychainWrapper.sharedClientAppContainerKeychain
         guard let pushReg = keychain.object(forKey: KeychainKey.fxaPushRegistration) as? PushRegistration else {
             return deferMaybe(PushMessageError.accountError)
@@ -60,76 +60,83 @@ extension FxAPushMessageHandler {
         }
 
         // return handle(plaintext: string)
-        let deferred = PushMessageResult()
+        let deferred = PushMessageResults()
         RustFirefoxAccounts.reconfig(prefs: profile.prefs).uponQueue(.main) { accountManager in
             accountManager.deviceConstellation()?.processRawIncomingAccountEvent(pushPayload: string) {
                 result in
-                guard case .success(let events) = result, let firstEvent = events.first else {
+                guard case .success(let events) = result, !events.isEmpty else {
                     let err: PushMessageError
                     if case .failure(let error) = result {
+                        SentryIntegration.shared.send(message: "Failed to get any events from FxA", tag: .fxaClient, severity: .error, description: error.localizedDescription)
                         err = PushMessageError.messageIncomplete(error.localizedDescription)
                     } else {
+                        SentryIntegration.shared.send(message: "Got zero events from FxA", tag: .fxaClient, severity: .error, description: "no events retrieved from fxa")
                         err = PushMessageError.messageIncomplete("empty message")
                     }
                     deferred.fill(Maybe(failure: err))
                     return
                 }
-                if events.count > 1 {
-                    // Log to the console for debugging release builds
-                    os_log(
-                        "%{public}@",
-                        log: OSLog(subsystem: "org.mozilla.firefox",
-                                   category: "firefoxnotificationservice"),
-                        type: OSLogType.debug,
-                        "Multiple events arrived, only handling the first event.")
-                }
-                switch firstEvent {
-                case .commandReceived(let deviceCommand):
-                    switch deviceCommand {
-                    case .tabReceived(_, let tabData):
-                        let title = tabData.entries.last?.title ?? ""
-                        let url = tabData.entries.last?.url ?? ""
-                        let message = PushMessage.commandReceived(tab: ["title": title, "url": url])
-                        if let json = try? accountManager.gatherTelemetry() {
-                            let events = FxATelemetry.parseTelemetry(fromJSONString: json)
-                            events.forEach { $0.record(intoPrefs: self.profile.prefs) }
+                var messages: [PushMessage] = []
+
+                // It's possible one of the messages is a device disconnection
+                // in that case, we have an async call to get the name of the device
+                // we should make sure not to resolve our own value before that name retrieval
+                // is done
+                var waitForClient: Deferred<Maybe<String>>?
+                for event in events {
+                    switch event {
+                    case .commandReceived(let deviceCommand):
+                        switch deviceCommand {
+                        case .tabReceived(_, let tabData):
+                            let title = tabData.entries.last?.title ?? ""
+                            let url = tabData.entries.last?.url ?? ""
+                            messages.append(PushMessage.commandReceived(tab: ["title": title, "url": url]))
+                            if let json = try? accountManager.gatherTelemetry() {
+                                let events = FxATelemetry.parseTelemetry(fromJSONString: json)
+                                events.forEach { $0.record(intoPrefs: self.profile.prefs) }
+                            }
+                        }
+                    case .deviceConnected(let deviceName):
+                        messages.append(PushMessage.deviceConnected(deviceName))
+                    case let .deviceDisconnected(deviceId, isLocalDevice):
+                        if isLocalDevice {
+                            // We can't disconnect the device from the account until we have access to the application, so we'll handle this properly in the AppDelegate (as this code in an extension),
+                            // by calling the FxALoginHelper.applicationDidDisonnect(application).
+                            self.profile.prefs.setBool(true, forKey: PendingAccountDisconnectedKey)
+                            messages.append(PushMessage.thisDeviceDisconnected)
                         }
-                        deferred.fill(Maybe(success: message))
-                    }
-                case .deviceConnected(let deviceName):
-                    let message = PushMessage.deviceConnected(deviceName)
-                    deferred.fill(Maybe(success: message))
-                case let .deviceDisconnected(deviceId, isLocalDevice):
-                    if isLocalDevice {
-                        // We can't disconnect the device from the account until we have access to the application, so we'll handle this properly in the AppDelegate (as this code in an extension),
-                        // by calling the FxALoginHelper.applicationDidDisonnect(application).
-                        self.profile.prefs.setBool(true, forKey: PendingAccountDisconnectedKey)
-                        let message = PushMessage.thisDeviceDisconnected
-                        deferred.fill(Maybe(success: message))
-                        return
-                    }
 
-                    guard let profile = self.profile as? BrowserProfile else {
-                        // We can't look up a name in testing, so this is the same as not knowing about it.
-                        let message = PushMessage.deviceDisconnected(nil)
-                        deferred.fill(Maybe(success: message))
-                        return
-                    }
+                        guard let profile = self.profile as? BrowserProfile else {
+                            // We can't look up a name in testing, so this is the same as not knowing about it.
+                            messages.append(PushMessage.deviceDisconnected(nil))
+                            break
+                        }
 
-                    profile.remoteClientsAndTabs.getClient(fxaDeviceId: deviceId).uponQueue(.main) { result in
-                        guard let device = result.successValue else { return }
-                        let message = PushMessage.deviceDisconnected(device?.name)
-                        if let id = device?.guid {
-                            profile.remoteClientsAndTabs.deleteClient(guid: id).uponQueue(.main) { _ in
-                                print("deleted client")
+                        waitForClient = Deferred<Maybe<String>>()
+                        profile.remoteClientsAndTabs.getClient(fxaDeviceId: deviceId).uponQueue(.main) { result in
+                            guard let device = result.successValue else {
+                                waitForClient?.fill(Maybe(failure: result.failureValue ?? "Unknown Error"))
+                                return
+                            }
+                            messages.append(PushMessage.deviceDisconnected(device?.name))
+                            waitForClient?.fill(Maybe(success: device?.name ?? "Unknown Device"))
+                            if let id = device?.guid {
+                                profile.remoteClientsAndTabs.deleteClient(guid: id).uponQueue(.main) { _ in
+                                    print("deleted client")
+                                }
                             }
                         }
-
-                        deferred.fill(Maybe(success: message))
+                    default:
+                        // There are other events, but we ignore them at this level.
+                        do {}
+                    }
+                }
+                if let waitForClient = waitForClient {
+                    waitForClient.upon { _ in
+                        deferred.fill(Maybe(success: messages))
                     }
-                default:
-                    // There are other events, but we ignore them at this level.
-                    do {}
+                } else {
+                    deferred.fill(Maybe(success: messages))
                 }
             }
         }
@@ -192,7 +199,7 @@ enum PushMessage: Equatable {
     }
 }
 
-typealias PushMessageResult = Deferred<Maybe<PushMessage>>
+typealias PushMessageResults = Deferred<Maybe<[PushMessage]>>
 
 enum PushMessageError: MaybeErrorType {
     case notDecrypted

Client/Application/AppDelegate+PushNotifications.swift

@@ -59,6 +59,8 @@ extension AppDelegate {
                 if newState.localDevice?.pushEndpointExpired ?? false {
                     MZKeychainWrapper.sharedClientAppContainerKeychain.removeObject(forKey: KeychainKey.apnsToken, withAccessibility: MZKeychainItemAccessibility.afterFirstUnlock)
                     NotificationCenter.default.post(name: .RegisterForPushNotifications, object: nil)
+                    // Our endpoint expired, we should check for missed messages
+                    self.profile.pollCommands(forcePoll: true)
                 }
             }
         }

Client/Application/AppDelegate+SyncSentTabs.swift

@@ -31,8 +31,8 @@ class AppSyncDelegate: SyncDelegate {
     }
 
     func displaySentTab(for url: URL, title: String, from deviceName: String?) {
-        DispatchQueue.main.sync {
-            if app.applicationState == .active {
+        DispatchQueue.main.async {
+            if self.app.applicationState == .active {
                 BrowserViewController.foregroundBVC().switchToTabForURLOrOpen(url)
                 return
             }

Client/Frontend/Settings/AppSettingsOptions.swift

@@ -292,6 +292,7 @@ class SyncNowSetting: WithAccountSetting {
 
         NotificationCenter.default.post(name: .UserInitiatedSyncManually, object: nil)
         profile.syncManager.syncEverything(why: .syncNow)
+        profile.pollCommands(forcePoll: true)
     }
 }
 

Extensions/NotificationService/NotificationService.swift

@@ -45,7 +45,23 @@ class NotificationService: UNNotificationServiceExtension {
         let handler = FxAPushMessageHandler(with: profile)
 
         handler.handle(userInfo: userInfo).upon { res in
-            self.didFinish(res.successValue, with: res.failureValue as? PushMessageError)
+            guard res.isSuccess, let events = res.successValue, let firstEvent = events.first else {
+                self.didFinish(nil, with: res.failureValue as? PushMessageError)
+                return
+            }
+            // We pass the first event to the notification handler, and add the rest directly
+            // to our own handling of send tab if they are send tabs so users don't miss them
+            for (idx, event) in events.enumerated() {
+                if  idx != 0,
+                    case let .commandReceived(tab) = event,
+                    let urlString = tab["url"],
+                    let url = URL(string: urlString),
+                    url.isWebPage(),
+                    let title = tab["title"] {
+                    self.profile?.syncDelegate?.displaySentTab(for: url, title: title, from: tab["deviceName"])
+                }
+            }
+            self.didFinish(firstEvent)
         }
     }
 

Providers/Profile.swift

@@ -146,6 +146,7 @@ protocol Profile: AnyObject {
     @discardableResult func storeTabs(_ tabs: [RemoteTab]) -> Deferred<Maybe<Int>>
 
     func sendItem(_ item: ShareItem, toDevices devices: [RemoteDevice]) -> Success
+    func pollCommands(forcePoll: Bool)
 
     var syncManager: SyncManager! { get }
     func hasSyncedLogins() -> Deferred<Maybe<Bool>>
@@ -643,6 +644,42 @@ open class BrowserProfile: Profile {
         return deferred
     }
 
+    /// Polls for missed send tabs and handles them
+    /// The method will not poll FxA if the interval hasn't passed
+    /// See AppConstants.FXA_COMMANDS_INTERVAL for the interval value
+    public func pollCommands(forcePoll: Bool = false) {
+        // We should only poll if the interval has passed to not
+        // overwhelm FxA
+        let lastPoll = self.prefs.timestampForKey(PrefsKeys.PollCommandsTimestamp)
+        let now = Date.now()
+        if let lastPoll = lastPoll, !forcePoll, now - lastPoll < AppConstants.FXA_COMMANDS_INTERVAL {
+            return
+        }
+        self.prefs.setTimestamp(now, forKey: PrefsKeys.PollCommandsTimestamp)
+        let accountManager = self.rustFxA.accountManager.peek()
+        accountManager?.deviceConstellation()?.pollForCommands { commands in
+            if let commands = try? commands.get() {
+                for command in commands {
+                    switch command {
+                    case .tabReceived(let sender, let tabData):
+                        // The tabData.entries is the tabs history
+                        // we only want the last item, which is the tab
+                        // to display
+                        let title = tabData.entries.last?.title ?? ""
+                        let url = tabData.entries.last?.url ?? ""
+                        if let json = try? accountManager?.gatherTelemetry() {
+                            let events = FxATelemetry.parseTelemetry(fromJSONString: json)
+                            events.forEach { $0.record(intoPrefs: self.prefs) }
+                        }
+                        if let url = URL(string: url) {
+                            self.syncDelegate?.displaySentTab(for: url, title: title, from: sender?.displayName)
+                        }
+                    }
+                }
+            }
+        }
+    }
+
     lazy var logins: RustLogins = {
         let sqlCipherDatabasePath = URL(fileURLWithPath: (try! files.getAndEnsureDirectory()), isDirectory: true).appendingPathComponent("logins.db").path
         let databasePath = URL(fileURLWithPath: (try! files.getAndEnsureDirectory()), isDirectory: true).appendingPathComponent("loginsPerField.db").path
@@ -1039,12 +1076,15 @@ open class BrowserProfile: Profile {
             }
 
             let clientSynchronizer = ready.synchronizer(ClientsSynchronizer.self, delegate: delegate, prefs: prefs, why: why)
-            return clientSynchronizer.synchronizeLocalClients(self.profile.remoteClientsAndTabs, withServer: ready.client, info: ready.info) >>== { result in
+            return clientSynchronizer.synchronizeLocalClients(
+                self.profile.remoteClientsAndTabs,
+                withServer: ready.client,
+                info: ready.info
+            ) >>== { result in
                 guard case .completed = result, let accountManager = self.profile.rustFxA.accountManager.peek() else {
                     return deferMaybe(result)
                 }
                 log.debug("Updating FxA devices list.")
-
                 accountManager.deviceConstellation()?.refreshState()
                 return deferMaybe(result)
             }
@@ -1450,6 +1490,7 @@ open class BrowserProfile: Profile {
 
         @objc func syncOnTimer() {
             self.syncEverything(why: .scheduled)
+            self.profile.pollCommands()
         }
 
         public func hasSyncedHistory() -> Deferred<Maybe<Bool>> {

Push/PushClient.swift

@@ -46,7 +46,6 @@ public extension PushClientImplementation {
                   completion: @escaping (PushRegistration?) -> Void) {
 
         let registerURL = endpointURL.appendingPathComponent("registration")!
-
         var mutableURLRequest = URLRequest(url: registerURL)
         mutableURLRequest.httpMethod = HTTPMethod.post.rawValue
 

RustFxA/PushNotificationSetup.swift

@@ -23,13 +23,15 @@ open class PushNotificationSetup {
             self.pushClient?.register(apnsToken) { [weak self] pushRegistration in
                 guard let pushRegistration = pushRegistration else { return }
                 self?.pushRegistration = pushRegistration
-                keychain.set(apnsToken, forKey: KeychainKey.apnsToken, withAccessibility: .afterFirstUnlock)
 
                 let subscription = pushRegistration.defaultSubscription
                 let devicePush = DevicePushSubscription(endpoint: subscription.endpoint.absoluteString,
                                                         publicKey: subscription.p256dhPublicKey,
                                                         authKey: subscription.authKey)
                 accountManager.deviceConstellation()?.setDevicePushSubscription(sub: devicePush)
+                // We set our apnsToken **after** the call to set the push subscription completes
+                // This helps ensure that if that call fails, we will try again with a new token next time
+                keychain.set(apnsToken, forKey: KeychainKey.apnsToken, withAccessibility: .afterFirstUnlock)
                 keychain.set(pushRegistration as NSCoding,
                              forKey: KeychainKey.fxaPushRegistration,
                              withAccessibility: .afterFirstUnlock)

Shared/AppConstants.swift

@@ -108,4 +108,7 @@ public struct AppConstants {
 
     /// Fixed short version for nightly builds
     public static let NIGHTLY_APP_VERSION = "9000"
+
+    /// Time that needs to pass before polling FxA for send tabs again, 86_400_000 milliseconds is 1 day
+    public static let FXA_COMMANDS_INTERVAL = 86_400_000
 }

Shared/Prefs.swift

@@ -130,6 +130,9 @@ public struct PrefsKeys {
     // Application Services Migration to Places DB
     public static let NewPlacesAPIDefaultKey = "NewPlacesAPI"
 
+    // The last timestamp we polled FxA for missing send tabs
+    public static let PollCommandsTimestamp = "PollCommandsTimestamp"
+
 }
 
 public struct PrefsDefaults {

Shared/SentryIntegration.swift

@@ -12,6 +12,7 @@ public enum SentryTag: String {
     case rustLogins = "RustLogins"
     case rustRemoteTabs = "RustRemoteTabs"
     case rustLog = "RustLog"
+    case fxaClient = "FxAClient"
     case notificationService = "NotificationService"
     case unifiedTelemetry = "UnifiedTelemetry"
     case general = "General"

Tests/ClientTests/Mocks/MockProfile.swift

@@ -237,6 +237,10 @@ open class MockProfile: Client.Profile {
         return succeed()
     }
 
+    public func pollCommands(forcePoll: Bool) {
+        return
+    }
+
     public func hasSyncedLogins() -> Deferred<Maybe<Bool>> {
         return deferMaybe(true)
     }