Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions ios/Sources/GutenbergKit/Sources/EditorViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public final class EditorViewController: UIViewController, GutenbergEditorContro
private let editorService: EditorService
private let mediaPicker: MediaPickerController?
private let controller: GutenbergEditorController
private let bundleProvider = EditorAssetBundleProvider()
private let bundleProvider: EditorAssetBundleProvider

// MARK: - Private Properties (UI)

Expand Down Expand Up @@ -153,11 +153,21 @@ public final class EditorViewController: UIViewController, GutenbergEditorContro
configuration: EditorConfiguration,
dependencies: EditorDependencies? = nil,
mediaPicker: MediaPickerController? = nil,
httpClient: EditorHTTPClient? = nil,
isWarmupMode: Bool = false
) {
let httpClient = httpClient ?? EditorHTTPClient(
urlSession: URLSession.shared,
authHeader: configuration.authHeader
)

self.configuration = configuration
self.dependencies = dependencies
self.editorService = EditorService(configuration: configuration)
self.editorService = EditorService(
configuration: configuration,
httpClient: httpClient
)
self.bundleProvider = EditorAssetBundleProvider(httpClient: httpClient)
self.mediaPicker = mediaPicker
self.controller = GutenbergEditorController(configuration: configuration)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,15 @@ import WebKit
///
/// The provider must be bound to the WebView configuration before loading the editor,
/// and must have a bundle set before the editor requests assets.
public final class EditorAssetBundleProvider: NSObject, @unchecked Sendable {
@MainActor
public final class EditorAssetBundleProvider: NSObject {

private let lock = NSLock()
private var bundle: EditorAssetBundle?
private let urlSession: URLSession
private let httpClient: EditorHTTPClient
private var runningTasks: [Task<Void, Never>] = []

override public init() {
self.urlSession = URLSession(configuration: .default)
public init(httpClient: EditorHTTPClient) {
self.httpClient = httpClient
super.init()
}

Expand All @@ -40,9 +41,7 @@ public final class EditorAssetBundleProvider: NSObject, @unchecked Sendable {
///
/// - Parameter bundle: The downloaded asset bundle containing cached scripts and styles.
public func set(bundle: EditorAssetBundle) {
lock.withLock {
self.bundle = bundle
}
self.bundle = bundle
}

/// Registers this provider with a WebView configuration.
Expand Down Expand Up @@ -100,19 +99,15 @@ extension EditorAssetBundleProvider: WKScriptMessageHandlerWithReply {
extension EditorAssetBundleProvider: WKURLSchemeHandler {
public func webView(_ webView: WKWebView, start urlSchemeTask: any WKURLSchemeTask) {
logExecutionTime("Retrieved cached asset") {
var loggerMessages = ["📚 Editor requested a cached asset"]

defer {
Logger.assetLibrary.info("\(loggerMessages.joined(separator: "\n"))")
}
Logger.assetLibrary.info("📚 Editor requested a cached asset")

guard let url = urlSchemeTask.request.url else {
loggerMessages.append(" URL: <missing>")
Logger.assetLibrary.info(" URL: <missing>")
urlSchemeTask.didFailWithError(URLError(.badURL))
return
}

loggerMessages.append(" URL: \(url)")
Logger.assetLibrary.info(" URL: \(url)")

guard let bundle else {
preconditionFailure("Cannot read asset with no bundle present. This is a programmer error.")
Expand All @@ -124,24 +119,23 @@ extension EditorAssetBundleProvider: WKURLSchemeHandler {
let shouldFetchFromRemote = !bundle.isValidAssetPath(for: url) || !bundle.hasAssetData(for: url)

guard !shouldFetchFromRemote else {
loggerMessages.append(" Asset not in bundle – fetching from remote")
self.fetchFromRemote(url: url, urlSchemeTask: urlSchemeTask, loggerMessages: &loggerMessages)
Logger.assetLibrary.info(" Asset not in bundle – fetching from remote")
self.fetchFromRemote(for: urlSchemeTask)
return
}

do {
loggerMessages.append(" Path: \(bundle.assetDataPath(for: url))")
Logger.assetLibrary.info(" Path: \(bundle.assetDataPath(for: url))")

let data = try bundle.assetData(for: url)
let response = HTTPURLResponse(url: url, statusCode: 200, httpVersion: "HTTP/1.1", headerFields: nil)!
urlSchemeTask.didReceive(response)
urlSchemeTask.didReceive(data)
urlSchemeTask.didFinish()
} catch {
loggerMessages.append(" Error: \(error.localizedDescription)")
Logger.assetLibrary.warning(" Error: \(error.localizedDescription)")
urlSchemeTask.didFailWithError(error)
}

}
}

Expand All @@ -153,59 +147,56 @@ extension EditorAssetBundleProvider: WKURLSchemeHandler {
///
/// This is used when an asset isn't in the local bundle (e.g., images referenced
/// in CSS files that weren't downloaded because only JS/CSS files are cached).
private func fetchFromRemote(
url: URL,
urlSchemeTask: any WKURLSchemeTask,
loggerMessages: inout [String]
) {
guard let originalURL = self.originalURL(for: url) else {
loggerMessages.append(" Failed to construct original URL")
let response = HTTPURLResponse(url: url, statusCode: 400, httpVersion: nil, headerFields: nil)!
private func fetchFromRemote(for urlSchemeTask: any WKURLSchemeTask) {
guard let originalRequest = self.originalRequest(for: urlSchemeTask.request) else {
Logger.assetLibrary.info(" Failed to construct original URL")
let response = HTTPURLResponse(
url: urlSchemeTask.request.url!,
statusCode: 400,
httpVersion: nil,
headerFields: nil
)!
urlSchemeTask.didReceive(response)
urlSchemeTask.didFinish()
return
}

loggerMessages.append(" Fetching from: \(originalURL)")
Logger.assetLibrary.info(" Fetching: \(originalRequest)")

let task = urlSession.dataTask(with: originalURL) { data, response, error in
if let error {
let taskHandle = Task {
do {
let (data, response) = try await self.httpClient.perform(originalRequest)

urlSchemeTask.didReceive(response)
urlSchemeTask.didReceive(data)
urlSchemeTask.didFinish()
}
catch {
Logger.assetLibrary.error("📚 Failed to fetch remote asset: \(error.localizedDescription)")
urlSchemeTask.didFailWithError(error)
return
}

guard let httpResponse = response as? HTTPURLResponse, let data else {
urlSchemeTask.didFailWithError(URLError(.badServerResponse))
return
}

// Create a new response with the original URL scheme task's URL
let schemeResponse = HTTPURLResponse(
url: url,
statusCode: httpResponse.statusCode,
httpVersion: "HTTP/1.1",
headerFields: httpResponse.allHeaderFields as? [String: String]
)!

urlSchemeTask.didReceive(schemeResponse)
urlSchemeTask.didReceive(data)
urlSchemeTask.didFinish()
}
task.resume()

self.runningTasks.append(taskHandle)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I perceive these are never removed/emptied. Is there a memory leak concern? Should we concern ourselves with cleaning these up at some point or is the risk negligible?

}

/// Converts a `gbk-cache-https` URL back to its original `https` URL.
///
/// For example: `gbk-cache-https://example.com/path` → `https://example.com/path`
private func originalURL(for url: URL) -> URL? {
var components = URLComponents(url: url, resolvingAgainstBaseURL: false)
private func originalRequest(for request: URLRequest) -> URLRequest? {
guard let url = request.url, var components = URLComponents(url: url, resolvingAgainstBaseURL: false) else {
return nil
}

// The scheme "gbk-cache-https" encodes the original scheme after the prefix
let schemePrefix = "gbk-cache-"
guard let scheme = components?.scheme, scheme.hasPrefix(schemePrefix) else {
guard let scheme = components.scheme, scheme.hasPrefix(schemePrefix) else {
return nil
}
components?.scheme = String(scheme.dropFirst(schemePrefix.count))
return components?.url
components.scheme = String(scheme.dropFirst(schemePrefix.count))

var mutableCopy = request
mutableCopy.url = components.url
return mutableCopy
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ struct EditorAssetLibraryTests {
func existingBundleReturnsNilForMissingChecksum() async throws {
let library = makeLibrary()

let result = try await library.existingBundle(forManifestChecksum: "nonexistent-checksum-12345")
let result = await library.existingBundle(forManifestChecksum: "nonexistent-checksum-12345")
#expect(result == nil)
}

Expand Down