Media loading flow changes (#483)

* Use an imageProvider directly from the view in the home screen

* Add support for media request coalescing

* Rename MediaProxy to MediaLoader

* Add new image loading mechanism to the room details screen avatar.

* Use the `SettingsScreen` prefix for all settings screen related components

* Add new image loading mechanism to the room header

* Add new image loading mechanism to the room member details screen

* Introduce a LoadableImage SwiftUI view that will automatically handle image loading

* Adopt the new LoadableImage where possible

* Fix LoadableImage not using/storing loaded images properly

* Simplify media loader enqueueing

* Made LodableImage load content after mediaSource updates. Adopt it on the home and settings screens

* Introduce a LoadableAvatarImage component and reuse it throughout the app

* Small logging tweaks, made some LoadableImage properties private

* Fix redacted skeletons avatar background color

* Fix placeholder avatars changing when backgrounding the app

* PR comments.

- Trim the @ sign off of mxid placeholders.
- Only expose AvatarSize on the avatar image, use CGSize elsewhere.

Co-authored-by: Doug <douglase@element.io>

ElementX/Sources/Services/Media/ImageProviderProtocol.swift

@@ -0,0 +1,45 @@
+//
+// Copyright 2023 New Vector Ltd
+//
+// 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 UIKit
+
+protocol ImageProviderProtocol {
+    func imageFromSource(_ source: MediaSourceProxy?, size: CGSize?) -> UIImage?
+    
+    @discardableResult func loadImageFromSource(_ source: MediaSourceProxy, size: CGSize?) async -> Result<UIImage, MediaProviderError>
+    
+    func imageFromURL(_ url: URL?, size: CGSize?) -> UIImage?
+    
+    @discardableResult func loadImageFromURL(_ url: URL, size: CGSize?) async -> Result<UIImage, MediaProviderError>
+}
+
+extension ImageProviderProtocol {
+    func imageFromSource(_ source: MediaSourceProxy?) -> UIImage? {
+        imageFromSource(source, size: nil)
+    }
+    
+    @discardableResult func loadImageFromSource(_ source: MediaSourceProxy) async -> Result<UIImage, MediaProviderError> {
+        await loadImageFromSource(source, size: nil)
+    }
+    
+    func imageFromURL(_ url: URL?) -> UIImage? {
+        imageFromURL(url, size: nil)
+    }
+    
+    @discardableResult func loadImageFromURL(_ url: URL) async -> Result<UIImage, MediaProviderError> {
+        await loadImageFromURL(url, size: nil)
+    }
+}

ElementX/Sources/Services/Media/MediaLoader.swift

@@ -0,0 +1,84 @@
+//
+// Copyright 2022 New Vector Ltd
+//
+// 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 Combine
+import Foundation
+import MatrixRustSDK
+import UIKit
+
+private final class MediaRequest {
+    var continuations: [CheckedContinuation<Data, Error>] = []
+}
+
+actor MediaLoader: MediaLoaderProtocol {
+    private let client: ClientProtocol
+    private let clientQueue: DispatchQueue
+    private var ongoingRequests = [MediaSourceProxy: MediaRequest]()
+
+    init(client: ClientProtocol,
+         clientQueue: DispatchQueue = .global()) {
+        self.client = client
+        self.clientQueue = clientQueue
+    }
+
+    func mediaSourceForURL(_ url: URL) -> MediaSourceProxy {
+        .init(url: url)
+    }
+    
+    func loadMediaContentForSource(_ source: MediaSourceProxy) async throws -> Data {
+        try await enqueueLoadMediaRequest(forSource: source) {
+            try self.client.getMediaContent(source: source.underlyingSource)
+        }
+    }
+    
+    func loadMediaThumbnailForSource(_ source: MediaSourceProxy, width: UInt, height: UInt) async throws -> Data {
+        try await enqueueLoadMediaRequest(forSource: source) {
+            try self.client.getMediaThumbnail(source: source.underlyingSource, width: UInt64(width), height: UInt64(height))
+        }
+    }
+    
+    // MARK: - Private
+    
+    private func enqueueLoadMediaRequest(forSource source: MediaSourceProxy, operation: @escaping () throws -> [UInt8]) async throws -> Data {
+        if let ongoingRequest = ongoingRequests[source] {
+            return try await withCheckedThrowingContinuation { continuation in
+                ongoingRequest.continuations.append(continuation)
+            }
+        }
+        
+        let ongoingRequest = MediaRequest()
+        ongoingRequests[source] = ongoingRequest
+        
+        defer {
+            ongoingRequests[source] = nil
+        }
+        
+        do {
+            let result = try await Task.dispatch(on: clientQueue) {
+                let bytes = try operation()
+                return Data(bytes: bytes, count: bytes.count)
+            }
+            
+            ongoingRequest.continuations.forEach { $0.resume(returning: result) }
+            
+            return result
+            
+        } catch {
+            ongoingRequest.continuations.forEach { $0.resume(throwing: error) }
+            throw error
+        }
+    }
+}

ElementX/Sources/Services/Media/MediaLoaderProtocol.swift

@@ -16,8 +16,8 @@
 
 import Foundation
 
-protocol MediaProxyProtocol {
-    func mediaSourceForURL(_ url: URL) -> MediaSourceProxy
+protocol MediaLoaderProtocol {
+    func mediaSourceForURL(_ url: URL) async -> MediaSourceProxy
 
     func loadMediaContentForSource(_ source: MediaSourceProxy) async throws -> Data
 

ElementX/Sources/Services/Media/MediaProvider.swift

@@ -18,53 +18,54 @@ import Kingfisher
 import UIKit
 
 struct MediaProvider: MediaProviderProtocol {
-    private let mediaProxy: MediaProxyProtocol
+    private let mediaLoader: MediaLoaderProtocol
     private let imageCache: Kingfisher.ImageCache
     private let fileCache: FileCacheProtocol
     private let backgroundTaskService: BackgroundTaskServiceProtocol?
     
-    init(mediaProxy: MediaProxyProtocol,
+    init(mediaLoader: MediaLoaderProtocol,
          imageCache: Kingfisher.ImageCache,
          fileCache: FileCacheProtocol,
          backgroundTaskService: BackgroundTaskServiceProtocol?) {
-        self.mediaProxy = mediaProxy
+        self.mediaLoader = mediaLoader
         self.imageCache = imageCache
         self.fileCache = fileCache
         self.backgroundTaskService = backgroundTaskService
     }
     
-    func imageFromSource(_ source: MediaSourceProxy?, avatarSize: AvatarSize?) -> UIImage? {
+    // MARK: Images
+    
+    func imageFromSource(_ source: MediaSourceProxy?, size: CGSize?) -> UIImage? {
         guard let url = source?.url else {
             return nil
         }
-        let cacheKey = cacheKeyForURL(url, avatarSize: avatarSize)
+        let cacheKey = cacheKeyForURL(url, size: size)
         return imageCache.retrieveImageInMemoryCache(forKey: cacheKey, options: nil)
     }
     
-    func imageFromURL(_ url: URL?, avatarSize: AvatarSize?) -> UIImage? {
+    func imageFromURL(_ url: URL?, size: CGSize?) -> UIImage? {
         guard let url else {
             return nil
         }
         
-        return imageFromSource(.init(url: url), avatarSize: avatarSize)
+        return imageFromSource(.init(url: url), size: size)
     }
     
-    func loadImageFromURL(_ url: URL, avatarSize: AvatarSize?) async -> Result<UIImage, MediaProviderError> {
-        await loadImageFromSource(.init(url: url), avatarSize: avatarSize)
+    func loadImageFromURL(_ url: URL, size: CGSize?) async -> Result<UIImage, MediaProviderError> {
+        await loadImageFromSource(.init(url: url), size: size)
     }
     
-    func loadImageFromSource(_ source: MediaSourceProxy, avatarSize: AvatarSize?) async -> Result<UIImage, MediaProviderError> {
-        if let image = imageFromSource(source, avatarSize: avatarSize) {
+    func loadImageFromSource(_ source: MediaSourceProxy, size: CGSize?) async -> Result<UIImage, MediaProviderError> {
+        if let image = imageFromSource(source, size: size) {
             return .success(image)
         }
         
-        #warning("Media loading should check for existing in flight operations and de-dupe requests.")
         let loadImageBgTask = await backgroundTaskService?.startBackgroundTask(withName: "LoadImage: \(source.url.hashValue)")
         defer {
             loadImageBgTask?.stop()
         }
         
-        let cacheKey = cacheKeyForURL(source.url, avatarSize: avatarSize)
+        let cacheKey = cacheKeyForURL(source.url, size: size)
 
         if case let .success(cacheResult) = await imageCache.retrieveImage(forKey: cacheKey),
            let image = cacheResult.image {
@@ -73,10 +74,10 @@ struct MediaProvider: MediaProviderProtocol {
 
         do {
             let imageData: Data
-            if let avatarSize {
-                imageData = try await mediaProxy.loadMediaThumbnailForSource(source, width: UInt(avatarSize.scaledValue), height: UInt(avatarSize.scaledValue))
+            if let size {
+                imageData = try await mediaLoader.loadMediaThumbnailForSource(source, width: UInt(size.width), height: UInt(size.height))
             } else {
-                imageData = try await mediaProxy.loadMediaContentForSource(source)
+                imageData = try await mediaLoader.loadMediaContentForSource(source)
             }
 
             guard let image = UIImage(data: imageData) else {
@@ -92,6 +93,8 @@ struct MediaProvider: MediaProviderProtocol {
             return .failure(.failedRetrievingImage)
         }
     }
+    
+    // MARK: Files
 
     func fileFromSource(_ source: MediaSourceProxy?, fileExtension: String) -> URL? {
         guard let source else {
@@ -100,6 +103,18 @@ struct MediaProvider: MediaProviderProtocol {
         let cacheKey = fileCacheKeyForURL(source.url)
         return fileCache.file(forKey: cacheKey, fileExtension: fileExtension)
     }
+    
+    func fileFromURL(_ url: URL?, fileExtension: String) -> URL? {
+        guard let url else {
+            return nil
+        }
+        
+        return fileFromSource(MediaSourceProxy(url: url), fileExtension: fileExtension)
+    }
+    
+    func loadFileFromURL(_ url: URL, fileExtension: String) async -> Result<URL, MediaProviderError> {
+        await loadFileFromSource(MediaSourceProxy(url: url), fileExtension: fileExtension)
+    }
 
     @discardableResult func loadFileFromSource(_ source: MediaSourceProxy, fileExtension: String) async -> Result<URL, MediaProviderError> {
         if let url = fileFromSource(source, fileExtension: fileExtension) {
@@ -114,7 +129,7 @@ struct MediaProvider: MediaProviderProtocol {
         let cacheKey = fileCacheKeyForURL(source.url)
         
         do {
-            let data = try await mediaProxy.loadMediaContentForSource(source)
+            let data = try await mediaLoader.loadMediaContentForSource(source)
             
             let url = try fileCache.store(data, with: fileExtension, forKey: cacheKey)
             return .success(url)
@@ -124,23 +139,11 @@ struct MediaProvider: MediaProviderProtocol {
         }
     }
     
-    func fileFromURL(_ url: URL?, fileExtension: String) -> URL? {
-        guard let url else {
-            return nil
-        }
-        
-        return fileFromSource(MediaSourceProxy(url: url), fileExtension: fileExtension)
-    }
-    
-    func loadFileFromURL(_ url: URL, fileExtension: String) async -> Result<URL, MediaProviderError> {
-        await loadFileFromSource(MediaSourceProxy(url: url), fileExtension: fileExtension)
-    }
-    
     // MARK: - Private
     
-    private func cacheKeyForURL(_ url: URL, avatarSize: AvatarSize?) -> String {
-        if let avatarSize {
-            return "\(url.absoluteString){\(avatarSize.scaledValue),\(avatarSize.scaledValue)}"
+    private func cacheKeyForURL(_ url: URL, size: CGSize?) -> String {
+        if let size {
+            return "\(url.absoluteString){\(size.width),\(size.height)}"
         } else {
             return url.absoluteString
         }

ElementX/Sources/Services/Media/MediaProviderProtocol.swift

@@ -23,15 +23,7 @@ enum MediaProviderError: Error {
     case invalidImageData
 }
 
-protocol MediaProviderProtocol {
-    func imageFromSource(_ source: MediaSourceProxy?, avatarSize: AvatarSize?) -> UIImage?
-    
-    @discardableResult func loadImageFromSource(_ source: MediaSourceProxy, avatarSize: AvatarSize?) async -> Result<UIImage, MediaProviderError>
-    
-    func imageFromURL(_ url: URL?, avatarSize: AvatarSize?) -> UIImage?
-    
-    @discardableResult func loadImageFromURL(_ url: URL, avatarSize: AvatarSize?) async -> Result<UIImage, MediaProviderError>
-
+protocol MediaProviderProtocol: ImageProviderProtocol {
     func fileFromSource(_ source: MediaSourceProxy?, fileExtension: String) -> URL?
 
     @discardableResult func loadFileFromSource(_ source: MediaSourceProxy, fileExtension: String) async -> Result<URL, MediaProviderError>
@@ -40,21 +32,3 @@ protocol MediaProviderProtocol {
 
     @discardableResult func loadFileFromURL(_ url: URL, fileExtension: String) async -> Result<URL, MediaProviderError>
 }
-
-extension MediaProviderProtocol {
-    func imageFromSource(_ source: MediaSourceProxy?) -> UIImage? {
-        imageFromSource(source, avatarSize: nil)
-    }
-    
-    @discardableResult func loadImageFromSource(_ source: MediaSourceProxy) async -> Result<UIImage, MediaProviderError> {
-        await loadImageFromSource(source, avatarSize: nil)
-    }
-    
-    func imageFromURL(_ url: URL?) -> UIImage? {
-        imageFromURL(url, avatarSize: nil)
-    }
-    
-    @discardableResult func loadImageFromURL(_ url: URL) async -> Result<UIImage, MediaProviderError> {
-        await loadImageFromURL(url, avatarSize: nil)
-    }
-}

ElementX/Sources/Services/Media/MediaProxy.swift

@@ -1,49 +0,0 @@
-//
-// Copyright 2022 New Vector Ltd
-//
-// 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 Combine
-import Foundation
-import MatrixRustSDK
-import UIKit
-
-class MediaProxy: MediaProxyProtocol {
-    private let client: ClientProtocol
-    private let clientQueue: DispatchQueue
-
-    init(client: ClientProtocol,
-         clientQueue: DispatchQueue = .global()) {
-        self.client = client
-        self.clientQueue = clientQueue
-    }
-
-    func mediaSourceForURL(_ url: URL) -> MediaSourceProxy {
-        .init(url: url)
-    }
-
-    func loadMediaContentForSource(_ source: MediaSourceProxy) async throws -> Data {
-        try await Task.dispatch(on: clientQueue) {
-            let bytes = try self.client.getMediaContent(source: source.underlyingSource)
-            return Data(bytes: bytes, count: bytes.count)
-        }
-    }
-
-    func loadMediaThumbnailForSource(_ source: MediaSourceProxy, width: UInt, height: UInt) async throws -> Data {
-        try await Task.dispatch(on: clientQueue) {
-            let bytes = try self.client.getMediaThumbnail(source: source.underlyingSource, width: UInt64(width), height: UInt64(height))
-            return Data(bytes: bytes, count: bytes.count)
-        }
-    }
-}

ElementX/Sources/Services/Media/MockMediaProvider.swift

@@ -18,31 +18,31 @@ import Foundation
 import UIKit
 
 struct MockMediaProvider: MediaProviderProtocol {
-    func imageFromSource(_ source: MediaSourceProxy?, avatarSize: AvatarSize?) -> UIImage? {
+    func imageFromSource(_ source: MediaSourceProxy?, size: CGSize?) -> UIImage? {
         nil
     }
     
-    func loadImageFromSource(_ source: MediaSourceProxy, avatarSize: AvatarSize?) async -> Result<UIImage, MediaProviderError> {
+    func loadImageFromSource(_ source: MediaSourceProxy, size: CGSize?) async -> Result<UIImage, MediaProviderError> {
         .failure(.failedRetrievingImage)
     }
     
-    func imageFromURL(_ url: URL?, avatarSize: AvatarSize?) -> UIImage? {
+    func imageFromURL(_ url: URL?, size: CGSize?) -> UIImage? {
         guard url != nil else {
             return nil
         }
         
-        if let avatarSize {
-            switch avatarSize {
-            case .room:
+        #warning("Fix me. this is stupid!")
+        if let size {
+            if size == AvatarSize.room(on: .details).scaledSize
+                || size == AvatarSize.room(on: .home).scaledSize
+                || size == AvatarSize.room(on: .timeline).scaledSize {
                 return Asset.Images.appLogo.image
-            default:
-                return UIImage(systemName: "photo")
             }
         }
         return UIImage(systemName: "photo")
     }
     
-    func loadImageFromURL(_ url: URL, avatarSize: AvatarSize?) async -> Result<UIImage, MediaProviderError> {
+    func loadImageFromURL(_ url: URL, size: CGSize?) async -> Result<UIImage, MediaProviderError> {
         guard let image = UIImage(systemName: "photo") else {
             fatalError()
         }