mirror of
https://github.com/Novaloop-AG/element-x-ios.git
synced 2026-11-03 06:55:59 +00:00
Revert weak media provider (#4483)
* Revert "Address the real lifetime issue of the SDK's `Client` by making `Context.mediaProvider` weak. (#4466)"
This reverts commit c794144ef1.
* Better docs.
This commit is contained in:
@@ -17,7 +17,7 @@ class ClientProxy: ClientProxyProtocol {
|
||||
private let networkMonitor: NetworkMonitorProtocol
|
||||
private let appSettings: AppSettings
|
||||
|
||||
private let mediaLoader: MediaLoaderProtocol
|
||||
let mediaLoader: MediaLoaderProtocol
|
||||
private let clientQueue: DispatchQueue
|
||||
|
||||
private var roomListService: RoomListService
|
||||
@@ -1117,20 +1117,6 @@ class ClientProxy: ClientProxyProtocol {
|
||||
}
|
||||
}
|
||||
|
||||
extension ClientProxy: MediaLoaderProtocol {
|
||||
func loadMediaContentForSource(_ source: MediaSourceProxy) async throws -> Data {
|
||||
try await mediaLoader.loadMediaContentForSource(source)
|
||||
}
|
||||
|
||||
func loadMediaThumbnailForSource(_ source: MediaSourceProxy, width: UInt, height: UInt) async throws -> Data {
|
||||
try await mediaLoader.loadMediaThumbnailForSource(source, width: width, height: height)
|
||||
}
|
||||
|
||||
func loadMediaFileForSource(_ source: MediaSourceProxy, filename: String?) async throws -> MediaFileHandleProxy {
|
||||
try await mediaLoader.loadMediaFileForSource(source, filename: filename)
|
||||
}
|
||||
}
|
||||
|
||||
private class ClientDelegateWrapper: ClientDelegate {
|
||||
private let authErrorCallback: (Bool) -> Void
|
||||
|
||||
|
||||
@@ -73,7 +73,7 @@ enum TimelineMediaVisibility: Decodable {
|
||||
}
|
||||
|
||||
// sourcery: AutoMockable
|
||||
protocol ClientProxyProtocol: AnyObject, MediaLoaderProtocol {
|
||||
protocol ClientProxyProtocol: AnyObject {
|
||||
var actionsPublisher: AnyPublisher<ClientProxyAction, Never> { get }
|
||||
|
||||
var loadingStatePublisher: CurrentValuePublisher<ClientProxyLoadingState, Never> { get }
|
||||
@@ -105,6 +105,8 @@ protocol ClientProxyProtocol: AnyObject, MediaLoaderProtocol {
|
||||
|
||||
var pusherNotificationClientIdentifier: String? { get }
|
||||
|
||||
var mediaLoader: MediaLoaderProtocol { get }
|
||||
|
||||
var roomSummaryProvider: RoomSummaryProviderProtocol { get }
|
||||
|
||||
/// Used for listing rooms that shouldn't be affected by the main `roomSummaryProvider` filtering
|
||||
|
||||
@@ -16,7 +16,17 @@ private final class MediaRequest {
|
||||
}
|
||||
|
||||
actor MediaLoader: MediaLoaderProtocol {
|
||||
private let client: ClientProtocol
|
||||
// We noticed that the keyboard appears to hold onto a reference to the `Context` of the last
|
||||
// screen that had text input focus, resulting in its MediaProvider staying alive which in
|
||||
// turn keeps this loader alive: https://github.com/element-hq/element-x-ios/issues/4465
|
||||
// Therefore the client is `weak` so that the underlying `MatrixRustSDK.Client` is released
|
||||
// when e.g. clearing the cache, otherwise we have the potential for 2 `Client`s to be alive
|
||||
// at the same time causing havoc.
|
||||
//
|
||||
// Whilst a more correct fix would be to make `Context.mediaProvider` weak, this requires a
|
||||
// bunch of workarounds in our preview tests to keep the mock provider alive as some ViewModels
|
||||
// don't have an accompanying ClientMock to own it.
|
||||
private weak var client: ClientProtocol?
|
||||
private var ongoingRequests = [MediaSourceProxy: MediaRequest]()
|
||||
|
||||
init(client: ClientProtocol) {
|
||||
@@ -24,18 +34,21 @@ actor MediaLoader: MediaLoaderProtocol {
|
||||
}
|
||||
|
||||
func loadMediaContentForSource(_ source: MediaSourceProxy) async throws -> Data {
|
||||
try await enqueueLoadMediaRequest(forSource: source) {
|
||||
try await self.client.getMediaContent(mediaSource: source.underlyingSource)
|
||||
try await enqueueLoadMediaRequest(forSource: source) { [weak client] in
|
||||
guard let client else { throw MediaLoaderError.missingClient }
|
||||
return try await client.getMediaContent(mediaSource: source.underlyingSource)
|
||||
}
|
||||
}
|
||||
|
||||
func loadMediaThumbnailForSource(_ source: MediaSourceProxy, width: UInt, height: UInt) async throws -> Data {
|
||||
try await enqueueLoadMediaRequest(forSource: source) {
|
||||
try await self.client.getMediaThumbnail(mediaSource: source.underlyingSource, width: UInt64(width), height: UInt64(height))
|
||||
try await enqueueLoadMediaRequest(forSource: source) { [weak client] in
|
||||
guard let client else { throw MediaLoaderError.missingClient }
|
||||
return try await client.getMediaThumbnail(mediaSource: source.underlyingSource, width: UInt64(width), height: UInt64(height))
|
||||
}
|
||||
}
|
||||
|
||||
func loadMediaFileForSource(_ source: MediaSourceProxy, filename: String?) async throws -> MediaFileHandleProxy {
|
||||
guard let client else { throw MediaLoaderError.missingClient }
|
||||
let result = try await client.getMediaFile(mediaSource: source.underlyingSource,
|
||||
filename: filename,
|
||||
mimeType: source.mimeType ?? "application/octet-stream",
|
||||
|
||||
@@ -7,6 +7,10 @@
|
||||
|
||||
import Foundation
|
||||
|
||||
enum MediaLoaderError: Error {
|
||||
case missingClient
|
||||
}
|
||||
|
||||
// sourcery: AutoMockable
|
||||
protocol MediaLoaderProtocol {
|
||||
func loadMediaContentForSource(_ source: MediaSourceProxy) async throws -> Data
|
||||
|
||||
@@ -9,7 +9,7 @@ import Combine
|
||||
import Kingfisher
|
||||
import UIKit
|
||||
|
||||
class MediaProvider: MediaProviderProtocol {
|
||||
struct MediaProvider: MediaProviderProtocol {
|
||||
private let mediaLoader: MediaLoaderProtocol
|
||||
private let imageCache: Kingfisher.ImageCache
|
||||
private let homeserverReachabilityPublisher: CurrentValuePublisher<NetworkMonitorReachability, Never>?
|
||||
|
||||
@@ -17,7 +17,7 @@ enum MediaProviderError: Error {
|
||||
}
|
||||
|
||||
// sourcery: AutoMockable
|
||||
protocol MediaProviderProtocol: AnyObject {
|
||||
protocol MediaProviderProtocol {
|
||||
func imageFromSource(_ source: MediaSourceProxy?, size: CGSize?) -> UIImage?
|
||||
func loadImageFromSource(_ source: MediaSourceProxy, size: CGSize?) async -> Result<UIImage, MediaProviderError>
|
||||
func loadImageDataFromSource(_ source: MediaSourceProxy) async -> Result<Data, MediaProviderError>
|
||||
|
||||
@@ -94,7 +94,7 @@ class UserSessionStore: UserSessionStoreProtocol {
|
||||
// MARK: - Private
|
||||
|
||||
private func buildUserSessionWithClient(_ clientProxy: ClientProxyProtocol) -> UserSessionProtocol {
|
||||
let mediaProvider = MediaProvider(mediaLoader: clientProxy,
|
||||
let mediaProvider = MediaProvider(mediaLoader: clientProxy.mediaLoader,
|
||||
imageCache: .onlyInMemory,
|
||||
homeserverReachabilityPublisher: clientProxy.homeserverReachabilityPublisher)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user