diff --git a/Signal/src/ViewControllers/ConversationView/CV/CVComponentState+GroupLink.swift b/Signal/src/ViewControllers/ConversationView/CV/CVComponentState+GroupLink.swift index a80517bd93..bf4b69f73e 100644 --- a/Signal/src/ViewControllers/ConversationView/CV/CVComponentState+GroupLink.swift +++ b/Signal/src/ViewControllers/ConversationView/CV/CVComponentState+GroupLink.swift @@ -157,11 +157,7 @@ extension CVComponentState { touchMessage() }.catch { error in // TODO: Add retry? - if IsNetworkConnectivityFailure(error) { - Logger.warn("Error: \(error)") - } else { - owsFailDebug("Error: \(error)") - } + owsFailDebugUnlessNetworkFailure(error) } return GroupInviteLinkViewModel(url: url, diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController+CVC.swift b/Signal/src/ViewControllers/ConversationView/ConversationViewController+CVC.swift index 8c7defeb5a..0b7f4a6934 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController+CVC.swift +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController+CVC.swift @@ -329,10 +329,10 @@ extension ConversationViewController: CVLoadCoordinatorDelegate { self.layout.invalidateLayout() } - updateViewToReflectLoad(loadedRenderState: self.renderState) - scrollToInitialPosition(animated: false) + updateViewToReflectLoad(loadedRenderState: self.renderState) + loadCoordinator.enqueueReload() setHasAppliedFirstLoadIfNecessary() @@ -342,11 +342,19 @@ extension ConversationViewController: CVLoadCoordinatorDelegate { private func updateForMinorUpdate(scrollAction: CVScrollAction) { Logger.verbose("") + // If the scroll action is not animated, perform it _before_ + // updateViewToReflectLoad(). + if !scrollAction.isAnimated { + self.perform(scrollAction: scrollAction) + } + updateViewToReflectLoad(loadedRenderState: self.renderState) loadDidLand() - perform(scrollAction: scrollAction) + if scrollAction.isAnimated { + self.perform(scrollAction: scrollAction) + } } private func updateWithFirstLoad() { @@ -368,16 +376,16 @@ extension ConversationViewController: CVLoadCoordinatorDelegate { benchSteps.step("2") - updateViewToReflectLoad(loadedRenderState: self.renderState) + scrollToInitialPosition(animated: false) + clearInitialScrollState() benchSteps.step("3") - loadDidLand() + updateViewToReflectLoad(loadedRenderState: self.renderState) benchSteps.step("4") - scrollToInitialPosition(animated: false) - clearInitialScrollState() + loadDidLand() benchSteps.step("5") @@ -396,22 +404,6 @@ extension ConversationViewController: CVLoadCoordinatorDelegate { } viewState.hasAppliedFirstLoad = true clearInitialScrollState() - - guard hasViewWillAppearEverBegun else { - Logger.verbose("Not fading in collection view.") - return - } - - // If we apply a load for the first time after the view - // appeared, we should fade in the collection view. - let useFadeAnimations = false - if useFadeAnimations { - // Fade in the collection view. - collectionView.alpha = 0.05 - UIView.animate(withDuration: 0.025) { - self.collectionView.alpha = 1.0 - } - } } private func updateReloadingAll(renderState: CVRenderState, @@ -426,9 +418,17 @@ extension ConversationViewController: CVLoadCoordinatorDelegate { DispatchQueue.main.async { [weak self] in guard let self = self else { return } + + // If the scroll action is not animated, perform it _before_ + // updateViewToReflectLoad(). + if !scrollAction.isAnimated { + self.perform(scrollAction: scrollAction) + } self.updateViewToReflectLoad(loadedRenderState: renderState) self.loadDidLand() - self.perform(scrollAction: scrollAction) + if scrollAction.isAnimated { + self.perform(scrollAction: scrollAction) + } } } @@ -560,11 +560,19 @@ extension ConversationViewController: CVLoadCoordinatorDelegate { return } + // If the scroll action is not animated, perform it _before_ + // updateViewToReflectLoad(). + if !scrollAction.isAnimated { + self.perform(scrollAction: scrollAction) + } + self.updateViewToReflectLoad(loadedRenderState: renderState) self.loadDidLand() - self.perform(scrollAction: scrollAction) + if scrollAction.isAnimated { + self.perform(scrollAction: scrollAction) + } viewState.scrollActionForUpdate = nil diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController+LastVisibleSortId.swift b/Signal/src/ViewControllers/ConversationView/ConversationViewController+LastVisibleSortId.swift index e598aaf9e8..e17ea36858 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController+LastVisibleSortId.swift +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController+LastVisibleSortId.swift @@ -16,7 +16,7 @@ extension ConversationViewController { /// The index path of the last item in the collection view's visible rect @objc - var firstVisibleIndexPath: IndexPath? { + public var firstVisibleIndexPath: IndexPath? { // For people looking at this in the future, UICollectionView has a very similar looking // property: -indexPathsForVisibleItems. Why aren't we using that? // @@ -39,7 +39,7 @@ extension ConversationViewController { /// The index path of the last item in the collection view's visible rect @objc - var lastVisibleIndexPath: IndexPath? { + public var lastVisibleIndexPath: IndexPath? { // For people looking at this in the future, UICollectionView has a very similar looking // property: -indexPathsForVisibleItems. Why aren't we using that? // diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController+Scroll.swift b/Signal/src/ViewControllers/ConversationView/ConversationViewController+Scroll.swift index d0542f3ca5..3a893ffde3 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController+Scroll.swift +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController+Scroll.swift @@ -468,7 +468,8 @@ extension ConversationViewController { return isScrolledToBottom } - private var safeDistanceFromBottom: CGFloat { + @objc + public var safeDistanceFromBottom: CGFloat { // This is a bit subtle. // // The _wrong_ way to determine if we're scrolled to the bottom is to diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index c3ad50bc16..e8d3f3a934 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -2007,8 +2007,6 @@ typedef enum : NSUInteger { { OWSAssertIsOnMainThread(); - // TODO: If you open CVC with one unread message, the scroll down button - // briefly flashes. if (!self.hasAppearedAndHasAppliedFirstLoad) { self.scrollDownButton.hidden = YES; self.scrollToNextMentionButton.hidden = YES; @@ -2034,7 +2032,6 @@ typedef enum : NSUInteger { if (self.isInPreviewPlatter) { scrollDownIsHidden = YES; scrollToNextMentionIsHidden = YES; - } else if (self.isPresentingMessageActions) { // Content offset calculations get messed up when we're presenting message actions // Don't change button visibility if we're presenting actions diff --git a/Signal/src/ViewControllers/GroupInviteLinksUI.swift b/Signal/src/ViewControllers/GroupInviteLinksUI.swift index 5b8478e78b..75d32c5e8b 100644 --- a/Signal/src/ViewControllers/GroupInviteLinksUI.swift +++ b/Signal/src/ViewControllers/GroupInviteLinksUI.swift @@ -201,11 +201,9 @@ class GroupInviteLinksActionSheet: ActionSheetController { }.catch { [weak self] error in if case GroupsV2Error.expiredGroupInviteLink = error { self?.applyExpiredGroupInviteLink() - } else if IsNetworkConnectivityFailure(error) { - // TODO: Retry errors? - Logger.warn("Error: \(error)") } else { - owsFailDebug("Error: \(error)") + // TODO: Retry errors? + owsFailDebugUnlessNetworkFailure(error) } } } @@ -218,11 +216,7 @@ class GroupInviteLinksActionSheet: ActionSheetController { self?.applyGroupAvatar(groupAvatar) }.catch { error in // TODO: Add retry? - if IsNetworkConnectivityFailure(error) { - Logger.warn("Error: \(error)") - } else { - owsFailDebug("Error: \(error)") - } + owsFailDebugUnlessNetworkFailure(error) } } diff --git a/SignalMessaging/groups/GroupsV2Impl.swift b/SignalMessaging/groups/GroupsV2Impl.swift index 8242b0aa23..266e9bc688 100644 --- a/SignalMessaging/groups/GroupsV2Impl.swift +++ b/SignalMessaging/groups/GroupsV2Impl.swift @@ -1058,11 +1058,7 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift { }.done { _ in Logger.verbose("Update succeeded.") }.catch { error in - if IsNetworkConnectivityFailure(error) { - Logger.warn("Error: \(error)") - } else { - owsFailDebug("Error: \(error)") - } + owsFailDebugUnlessNetworkFailure(error) } } diff --git a/SignalMessaging/groups/GroupsV2Migration.swift b/SignalMessaging/groups/GroupsV2Migration.swift index cec8f3191b..e2c18e906d 100644 --- a/SignalMessaging/groups/GroupsV2Migration.swift +++ b/SignalMessaging/groups/GroupsV2Migration.swift @@ -225,8 +225,7 @@ public extension GroupsV2Migration { } return ContactDiscoveryTask(phoneNumbers: phoneNumbersWithoutUuids).perform().asVoid() }.recover(on: .global()) { (error: Error) -> Promise in - // Log but otherwise ignore errors in CDS lookup. - owsFailDebug("Error: \(error)") + owsFailDebugUnlessNetworkFailure(error) return Promise.value(()) }.map(on: .global()) { _ in Logger.verbose("") @@ -257,7 +256,7 @@ public extension GroupsV2Migration { } } }.catch(on: .global()) { error in - owsFailDebug("Error: \(error)") + owsFailDebugUnlessNetworkFailure(error) } } } @@ -398,12 +397,7 @@ fileprivate extension GroupsV2Migration { return firstly { discoveryTask.perform().asVoid() }.recover(on: .global()) { error -> Promise in - // Log but ignore errors. - if IsNetworkConnectivityFailure(error) { - Logger.warn("Error: \(error)") - } else { - owsFailDebug("Error: \(error)") - } + owsFailDebugUnlessNetworkFailure(error) return Promise.value(()) } }.then(on: .global()) { () -> Promise in @@ -446,12 +440,7 @@ fileprivate extension GroupsV2Migration { // Do not ignore throttling errors. throw error } - // Log but ignore errors. - if IsNetworkConnectivityFailure(error) { - Logger.warn("Error: \(error)") - } else { - owsFailDebug("Error: \(error)") - } + owsFailDebugUnlessNetworkFailure(error) return Promise.value(()) } } diff --git a/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift b/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift index af4c202e9b..41e0644370 100644 --- a/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift +++ b/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift @@ -635,12 +635,7 @@ public class OWSLinkPreviewManager: NSObject { }.map { (avatarData: Data) -> Data? in return avatarData }.recover { (error: Error) -> Promise in - if IsNetworkConnectivityFailure(error) { - Logger.warn("Error: \(error)") - - } else { - owsFailDebug("Error: \(error)") - } + owsFailDebugUnlessNetworkFailure(error) return Promise.value(nil) } }.then(on: Self.workQueue) { (imageData: Data?) -> Promise in diff --git a/SignalServiceKit/src/Network/API/NetworkManager.swift b/SignalServiceKit/src/Network/API/NetworkManager.swift index 24587aceee..f07785cee6 100644 --- a/SignalServiceKit/src/Network/API/NetworkManager.swift +++ b/SignalServiceKit/src/Network/API/NetworkManager.swift @@ -157,3 +157,18 @@ public extension Error { HTTPRetryAfterDateForError(self) } } + +// MARK: - + +@inlinable +public func owsFailDebugUnlessNetworkFailure(_ error: Error, + file: String = #file, + function: String = #function, + line: Int = #line) { + if IsNetworkConnectivityFailure(error) { + // Log but otherwise ignore network failures. + Logger.warn("Error: \(error)", file: file, function: function, line: line) + } else { + owsFailDebug("Error: \(error)", file: file, function: function, line: line) + } +} diff --git a/SignalServiceKit/src/Network/API/OWSUpload.swift b/SignalServiceKit/src/Network/API/OWSUpload.swift index a4fed9804b..99e10e5dba 100644 --- a/SignalServiceKit/src/Network/API/OWSUpload.swift +++ b/SignalServiceKit/src/Network/API/OWSUpload.swift @@ -723,11 +723,7 @@ public extension OWSUpload { AppExpiry.shared.setHasAppExpiredAtCurrentVersion() } - if IsNetworkConnectivityFailure(error) { - Logger.warn("Error: \(error)") - } else { - owsFailDebug("Error: \(error)") - } + owsFailDebugUnlessNetworkFailure(error) resolver.reject(error) }) } diff --git a/SignalServiceKit/src/Util/StorageService.swift b/SignalServiceKit/src/Util/StorageService.swift index 2e6afde462..46bf781ef8 100644 --- a/SignalServiceKit/src/Util/StorageService.swift +++ b/SignalServiceKit/src/Util/StorageService.swift @@ -443,13 +443,8 @@ public struct StorageService { return StorageResponse(status: status, data: responseData) }.recover(on: .global()) { (error: Error) -> Promise in - if IsNetworkConnectivityFailure(error) { - throw StorageError.networkError(statusCode: 0, underlyingError: error) - } else { - // This should never happen. - owsFailDebug("Error: \(error)") - throw StorageError.networkError(statusCode: 0, underlyingError: error) - } + owsFailDebugUnlessNetworkFailure(error) + throw StorageError.networkError(statusCode: 0, underlyingError: error) } } }