Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure thread safety #32

Closed
wants to merge 7 commits into from
48 changes: 36 additions & 12 deletions Source/Bridge.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ public enum BridgeError: Error {
case missingWebView
}

@MainActor
protocol Bridgable: AnyObject {
var delegate: BridgeDelegate? { get set }
var webView: WKWebView? { get }
Expand All @@ -17,25 +18,30 @@ protocol Bridgable: AnyObject {

/// `Bridge` is the object for configuring a web view and
/// the channel for sending/receiving messages
@MainActor
svara marked this conversation as resolved.
Show resolved Hide resolved
public final class Bridge: Bridgable {
public typealias InitializationCompletionHandler = () -> Void
weak var delegate: BridgeDelegate?
weak var webView: WKWebView?

public static func initialize(_ webView: WKWebView) {
nonisolated public static func initialize(_ webView: WKWebView, completion: InitializationCompletionHandler?) {
svara marked this conversation as resolved.
Show resolved Hide resolved
Task { @MainActor in
await initialize(webView)
completion?()
}
}

public static func initialize(_ webView: WKWebView) async {
if getBridgeFor(webView) == nil {
initialize(Bridge(webView: webView))
}
}

init(webView: WKWebView) {
self.webView = webView
loadIntoWebView()
}

deinit {
webView?.configuration.userContentController.removeScriptMessageHandler(forName: scriptHandlerName)
}

svara marked this conversation as resolved.
Show resolved Hide resolved
// MARK: - Internal API

/// Register a single component
Expand Down Expand Up @@ -72,15 +78,14 @@ public final class Bridge: Bridgable {
// let replyMessage = message.replacing(data: data)
// callBridgeFunction("send", arguments: [replyMessage.toJSON()])
// }
@MainActor
@discardableResult
func evaluate(javaScript: String) async throws -> Any {
func evaluate(javaScript: String) async throws -> Any? {
guard let webView else {
throw BridgeError.missingWebView
}

do {
return try await webView.evaluateJavaScript(javaScript)
return try await webView.evaluateJavaScriptAsync(javaScript)
} catch {
Comment on lines 83 to 85
Copy link
Collaborator Author

@svara svara Feb 28, 2024

Choose a reason for hiding this comment

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

When using async/await version of evaluateJavaScript, WKWebView expects JavaScript to return with a value (something other than Void). If there's no value returning from the JavaScript that you evaluate it will crash because it tries to unwrap a nil value to Any.

logger.error("Error evaluating JavaScript: \(error)")
throw error
Expand All @@ -90,7 +95,7 @@ public final class Bridge: Bridgable {
/// Evaluates a JavaScript function with optional arguments by encoding the arguments
/// Function should not include the parens
/// Usage: evaluate(function: "console.log", arguments: ["test"])
func evaluate(function: String, arguments: [Any] = []) async throws -> Any {
func evaluate(function: String, arguments: [Any] = []) async throws -> Any? {
try await evaluate(javaScript: JavaScript(functionName: function, arguments: arguments).toString())
}

Expand Down Expand Up @@ -151,7 +156,7 @@ public final class Bridge: Bridgable {
// MARK: - JavaScript Evaluation

@discardableResult
private func evaluate(javaScript: JavaScript) async throws -> Any {
private func evaluate(javaScript: JavaScript) async throws -> Any? {
do {
return try await evaluate(javaScript: javaScript.toString())
} catch {
Expand All @@ -168,7 +173,6 @@ public final class Bridge: Bridgable {
}

extension Bridge: ScriptMessageHandlerDelegate {
@MainActor
func scriptMessageHandlerDidReceiveMessage(_ scriptMessage: WKScriptMessage) async throws {
if let event = scriptMessage.body as? String, event == "ready" {
try await delegate?.bridgeDidInitialize()
Expand All @@ -183,3 +187,23 @@ extension Bridge: ScriptMessageHandlerDelegate {
logger.warning("Unhandled message received: \(String(describing: scriptMessage.body))")
}
}

private extension WKWebView {
/// NOTE: The async version crashes the app with `Fatal error: Unexpectedly found nil while implicitly unwrapping an Optional value`
/// in case the function doesn't return anything.
/// This is a workaround. See https://forums.developer.apple.com/forums/thread/701553 for more details.
@discardableResult
func evaluateJavaScriptAsync(_ str: String) async throws -> Any? {
return try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<Any?, Error>) in
DispatchQueue.main.async {
svara marked this conversation as resolved.
Show resolved Hide resolved
self.evaluateJavaScript(str) { data, error in
if let error = error {
continuation.resume(throwing: error)
} else {
continuation.resume(returning: data)
}
}
}
}
}
}
7 changes: 5 additions & 2 deletions Source/BridgeComponent.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Foundation

@MainActor
protocol BridgingComponent: AnyObject {
static var name: String { get }
var delegate: BridgingDelegate { get }
Expand All @@ -22,6 +23,7 @@ protocol BridgingComponent: AnyObject {
func viewDidDisappear()
}

@MainActor
open class BridgeComponent: BridgingComponent {
public typealias ReplyCompletionHandler = (Result<Bool, Error>) -> Void

Expand All @@ -30,7 +32,7 @@ open class BridgeComponent: BridgingComponent {
/// Subclasses must provide their own implementation of this property.
///
/// - Note: This property is used for identifying the component.
open class var name: String {
nonisolated open class var name: String {
fatalError("BridgeComponent subclass must provide a unique 'name'")
}

Expand Down Expand Up @@ -149,7 +151,7 @@ open class BridgeComponent: BridgingComponent {

@discardableResult
/// Replies to the web with the last received message for a given `event`, replacing its `jsonData`
/// with the provided `Encodable` object.
/// with the provided `Encodable` object.
///
/// NOTE: If a message has not been received for the given `event`, the reply will be ignored.
///
Expand Down Expand Up @@ -275,3 +277,4 @@ open class BridgeComponent: BridgingComponent {

private var receivedMessages = [String: Message]()
}

3 changes: 2 additions & 1 deletion Source/BridgeDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import WebKit

public protocol BridgeDestination: AnyObject {}

@MainActor
public protocol BridgingDelegate: AnyObject {
var location: String { get }
var destination: BridgeDestination { get }
Expand All @@ -24,6 +25,7 @@ public protocol BridgingDelegate: AnyObject {
func bridgeDidReceiveMessage(_ message: Message) -> Bool
}

@MainActor
public final class BridgeDelegate: BridgingDelegate {
public let location: String
public unowned let destination: BridgeDestination
Expand Down Expand Up @@ -153,4 +155,3 @@ public final class BridgeDelegate: BridgingDelegate {
return component
}
}

3 changes: 2 additions & 1 deletion Tests/BridgeComponentTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import XCTest
import WebKit
import Strada

@MainActor
class BridgeComponentTest: XCTestCase {
private var delegate: BridgeDelegateSpy!
private var destination: AppBridgeDestination!
Expand Down Expand Up @@ -223,6 +224,6 @@ class BridgeComponentTest: XCTestCase {
}
}

private extension TimeInterval {
extension TimeInterval {
static let expectationTimeout: TimeInterval = 5
}
svara marked this conversation as resolved.
Show resolved Hide resolved
1 change: 1 addition & 0 deletions Tests/BridgeDelegateTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import XCTest
import WebKit
@testable import Strada

@MainActor
class BridgeDelegateTests: XCTestCase {
private var delegate: BridgeDelegate!
private var destination: AppBridgeDestination!
Expand Down
53 changes: 41 additions & 12 deletions Tests/BridgeTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,67 @@ import XCTest
import WebKit
@testable import Strada

@MainActor
class BridgeTests: XCTestCase {
func testInitWithANewWebViewAutomaticallyLoadsIntoWebView() {
func testInitWithANewWebViewAutomaticallyLoadsIntoWebView() async {
let webView = WKWebView()
let userContentController = webView.configuration.userContentController
XCTAssertTrue(userContentController.userScripts.isEmpty)

Bridge.initialize(webView)
await Bridge.initialize(webView)
XCTAssertEqual(userContentController.userScripts.count, 1)
}

func testInitWithTheSameWebViewDoesNotLoadTwice() {
func testInitWithTheSameWebViewDoesNotLoadTwice() async {
let webView = WKWebView()
let userContentController = webView.configuration.userContentController
XCTAssertTrue(userContentController.userScripts.isEmpty)

Bridge.initialize(webView)
await Bridge.initialize(webView)
XCTAssertEqual(userContentController.userScripts.count, 1)

Bridge.initialize(webView)
await Bridge.initialize(webView)
XCTAssertEqual(userContentController.userScripts.count, 1)
}

func testInitWithANewWebViewAutomaticallyLoadsIntoWebView() {
let webView = WKWebView()
let userContentController = webView.configuration.userContentController
XCTAssertTrue(userContentController.userScripts.isEmpty)

let expectation = expectation(description: "Wait for completion.")
Bridge.initialize(webView) {
XCTAssertEqual(userContentController.userScripts.count, 1)
expectation.fulfill()
}

wait(for: [expectation], timeout: .expectationTimeout)
}

func testInitWithTheSameWebViewDoesNotLoadTwice() {
let webView = WKWebView()
let userContentController = webView.configuration.userContentController
XCTAssertTrue(userContentController.userScripts.isEmpty)

let expectation1 = expectation(description: "Wait for completion.")
Bridge.initialize(webView) {
XCTAssertEqual(userContentController.userScripts.count, 1)
expectation1.fulfill()
}

let expectation2 = expectation(description: "Wait for completion.")

Bridge.initialize(webView) {
XCTAssertEqual(userContentController.userScripts.count, 1)
expectation2.fulfill()
}

wait(for: [expectation1, expectation2], timeout: .expectationTimeout)
}

/// NOTE: Each call to `webView.evaluateJavaScript(String)` will throw an error.
/// We intentionally disregard any thrown errors (`try? await bridge...`)
/// because we validate the evaluated JavaScript string ourselves.
@MainActor
func testRegisterComponentCallsJavaScriptFunction() async throws {
let webView = TestWebView()
let bridge = Bridge(webView: webView)
Expand All @@ -37,7 +72,6 @@ class BridgeTests: XCTestCase {
XCTAssertEqual(webView.lastEvaluatedJavaScript, "window.nativeBridge.register(\"test\")")
}

@MainActor
func testRegisterComponentsCallsJavaScriptFunction() async throws {
let webView = TestWebView()
let bridge = Bridge(webView: webView)
Expand All @@ -47,7 +81,6 @@ class BridgeTests: XCTestCase {
XCTAssertEqual(webView.lastEvaluatedJavaScript, "window.nativeBridge.register([\"one\",\"two\"])")
}

@MainActor
func testUnregisterComponentCallsJavaScriptFunction() async throws {
let webView = TestWebView()
let bridge = Bridge(webView: webView)
Expand All @@ -57,7 +90,6 @@ class BridgeTests: XCTestCase {
XCTAssertEqual(webView.lastEvaluatedJavaScript, "window.nativeBridge.unregister(\"test\")")
}

@MainActor
func testSendCallsJavaScriptFunction() async throws {
let webView = TestWebView()
let bridge = Bridge(webView: webView)
Expand All @@ -78,7 +110,6 @@ class BridgeTests: XCTestCase {
XCTAssertEqual(webView.lastEvaluatedJavaScript, "window.nativeBridge.replyWith({\"component\":\"page\",\"event\":\"connect\",\"data\":{\"title\":\"Page-title\"},\"id\":\"1\"})")
}

@MainActor
func testEvaluateJavaScript() async throws {
let webView = TestWebView()
let bridge = Bridge(webView: webView)
Expand All @@ -88,7 +119,6 @@ class BridgeTests: XCTestCase {
XCTAssertEqual(webView.lastEvaluatedJavaScript, "test(1,2,3)")
}

@MainActor
func testEvaluateJavaScriptReturnsErrorForNoWebView() async throws {
let bridge = Bridge(webView: WKWebView())
bridge.webView = nil
Expand All @@ -101,7 +131,6 @@ class BridgeTests: XCTestCase {
XCTAssertEqual(bridgeError, BridgeError.missingWebView)
}

@MainActor
func testEvaluateFunction() async throws {
let webView = TestWebView()
let bridge = Bridge(webView: webView)
Expand Down
1 change: 1 addition & 0 deletions Tests/ComponentTestExample/ComposerComponentTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import XCTest
import WebKit
import Strada

@MainActor
final class ComposerComponentTests: XCTestCase {
private var delegate: BridgeDelegateSpy!
private var destination: AppBridgeDestination!
Expand Down
Loading