Skip to content

Commit

Permalink
Improve code
Browse files Browse the repository at this point in the history
  • Loading branch information
@mvasilak
mvasilak committed Mar 4, 2025
1 parent 8b825ea commit 61a67e8
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 58 deletions.
2 changes: 1 addition & 1 deletion ZShare/Controllers/TranslationWebViewHandler.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ final class TranslationWebViewHandler {
self.observable.on(.error(Error.noSuccessfulTranslators))

case .log:
DDLogInfo("JSLOG: \(body)")
DDLogInfo("TranslationWebViewHandler: JSLOG - \(body)")
}
}
}
39 changes: 17 additions & 22 deletions Zotero/Controllers/PDFWorkerController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,6 @@ final class PDFWorkerController {
let kind: Kind
}

enum PDFWorkState {
case enqueued
case inProgress
}

// MARK: Properties
private let dispatchSpecificKey: DispatchSpecificKey<String>
private let accessQueueLabel: String
Expand All @@ -52,11 +47,9 @@ final class PDFWorkerController {

// Accessed only via accessQueue
private static let maxConcurrentPDFWorkers: Int = 1
// Using an OrderedDictionary instead of an Array, so we can O(1) when cancelling a work that is still queued.
private var queue: OrderedDictionary<PDFWork, PublishSubject<Update>> = [:]
private var queue: [PDFWork] = []
private var subjectsByPDFWork: [PDFWork: PublishSubject<Update>] = [:]
private var pdfWorkerWebViewHandlersByPDFWork: [PDFWork: PDFWorkerWebViewHandler] = [:]
private var statesByPDFWork: [PDFWork: PDFWorkState] = [:]

// MARK: Object Lifecycle
init() {
Expand All @@ -76,9 +69,8 @@ final class PDFWorkerController {
existingSubject.bind(to: subject).disposed(by: disposeBag)
return
}
queue[work] = subject
queue.append(work)
subjectsByPDFWork[work] = subject
statesByPDFWork[work] = .enqueued

startWorkIfNeeded()
}
Expand All @@ -87,7 +79,11 @@ final class PDFWorkerController {

private func startWorkIfNeeded() {
guard pdfWorkerWebViewHandlersByPDFWork.count < Self.maxConcurrentPDFWorkers, !queue.isEmpty else { return }
let (work, subject) = queue.removeFirst()
let work = queue.removeFirst()
guard let subject = subjectsByPDFWork[work] else {
startWorkIfNeeded()
return
}
start(work: work, subject: subject)
startWorkIfNeeded()

Expand All @@ -110,7 +106,6 @@ final class PDFWorkerController {
return
}

statesByPDFWork[work] = .inProgress
setupObserver(for: pdfWorkerWebViewHandler)
subject.on(.next(Update(work: work, kind: .inProgress)))
switch work.kind {
Expand All @@ -125,11 +120,11 @@ final class PDFWorkerController {
pdfWorkerWebViewHandler.observable
.subscribe(onNext: { [weak self] in
guard let self else { return }
process(result: $0)
process(result: $0, self: self)
})
.disposed(by: disposeBag)

func process(result: Result<PDFWorkerWebViewHandler.PDFWorkerData, Error>) {
func process(result: Result<PDFWorkerWebViewHandler.PDFWorkerData, Error>, self: PDFWorkerController) {
switch result {
case .success(let data):
switch data {
Expand Down Expand Up @@ -162,7 +157,7 @@ final class PDFWorkerController {
pdfWorkerWebViewHandlersByPDFWork.values.forEach { $0.webViewHandler.removeFromSuperviewAsynchronously() }
pdfWorkerWebViewHandlersByPDFWork = [:]
// Then cancel actual works, and send cancelled event for each queued work.
let works = subjectsByPDFWork.keys + Array(queue.keys)
let works = subjectsByPDFWork.keys + queue
for work in works {
cancel(work: work)
}
Expand All @@ -176,20 +171,20 @@ final class PDFWorkerController {
return Disposables.create()
}
if DispatchQueue.getSpecific(key: dispatchSpecificKey) == accessQueueLabel {
cleanup(for: work, maybe: maybe)
cleanup(for: work, maybe: maybe, self: self)
} else {
accessQueue.async(flags: .barrier) {
cleanup(for: work, maybe: maybe)
accessQueue.async(flags: .barrier) { [weak self] in
guard let self else { return }
cleanup(for: work, maybe: maybe, self: self)
}
}
return Disposables.create()
}

func cleanup(for work: PDFWork, maybe: (MaybeEvent<PublishSubject<Update>>) -> Void) {
let subject = queue[work] ?? subjectsByPDFWork[work]
queue[work] = nil
func cleanup(for work: PDFWork, maybe: (MaybeEvent<PublishSubject<Update>>) -> Void, self: PDFWorkerController) {
let subject = subjectsByPDFWork[work]
queue.removeAll(where: { $0 == work })
subjectsByPDFWork[work] = nil
statesByPDFWork[work] = nil
DDLogInfo("PDFWorkerController: cleaned up for \(work)")
pdfWorkerWebViewHandlersByPDFWork.removeValue(forKey: work)?.webViewHandler.removeFromSuperviewAsynchronously()
if let subject {
Expand Down
16 changes: 8 additions & 8 deletions Zotero/Controllers/Web View Handling/LookupWebViewHandler.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ final class LookupWebViewHandler {
case item([String: Any])
}

private enum InitializationResult {
private enum InitializationState {
case initialized
case inProgress
case failed(Swift.Error)
Expand All @@ -51,14 +51,14 @@ final class LookupWebViewHandler {
private let disposeBag: DisposeBag
let observable: PublishSubject<Result<LookupData, Swift.Error>>

private var isLoading: BehaviorRelay<InitializationResult>
private var initializationState: BehaviorRelay<InitializationState>

init(webView: WKWebView, translatorsController: TranslatorsAndStylesController) {
self.translatorsController = translatorsController
webViewHandler = WebViewHandler(webView: webView, javascriptHandlers: JSHandlers.allCases.map({ $0.rawValue }))
observable = PublishSubject()
disposeBag = DisposeBag()
isLoading = BehaviorRelay(value: .inProgress)
initializationState = BehaviorRelay(value: .inProgress)

webViewHandler.receivedMessageHandler = { [weak self] name, body in
self?.receiveMessage(name: name, body: body)
Expand All @@ -69,10 +69,10 @@ final class LookupWebViewHandler {
.observe(on: MainScheduler.instance)
.subscribe(onSuccess: { [weak self] _ in
DDLogInfo("LookupWebViewHandler: initialization succeeded")
self?.isLoading.accept(.initialized)
self?.initializationState.accept(.initialized)
}, onFailure: { [weak self] error in
DDLogInfo("LookupWebViewHandler: initialization failed - \(error)")
self?.isLoading.accept(.failed(error))
self?.initializationState.accept(.failed(error))
})
.disposed(by: disposeBag)

Expand Down Expand Up @@ -134,15 +134,15 @@ final class LookupWebViewHandler {
}

func lookUp(identifier: String) {
switch isLoading.value {
switch initializationState.value {
case .failed(let error):
observable.on(.next(.failure(error)))

case .initialized:
performLookUp(for: identifier)

case .inProgress:
isLoading.filter { result in
initializationState.filter { result in
switch result {
case .inProgress:
return false
Expand Down Expand Up @@ -210,7 +210,7 @@ final class LookupWebViewHandler {
observable.on(.next(.success(.identifiers(rawData))))

case .log:
DDLogInfo("JSLOG: \(body)")
DDLogInfo("LookupWebViewHandler: JSLOG - \(body)")

case .request:
guard let body = body as? [String: Any],
Expand Down
35 changes: 17 additions & 18 deletions Zotero/Controllers/Web View Handling/PDFWorkerWebViewHandler.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ final class PDFWorkerWebViewHandler {
case fullText(data: [String: Any])
}

private enum InitializationResult {
private enum InitializationState {
case initialized
case inProgress
case failed(Swift.Error)
Expand All @@ -43,13 +43,13 @@ final class PDFWorkerWebViewHandler {
private let disposeBag: DisposeBag
let observable: PublishSubject<Result<PDFWorkerData, Swift.Error>>

private var isLoading: BehaviorRelay<InitializationResult>
private var initializationState: BehaviorRelay<InitializationState>

init(webView: WKWebView) {
webViewHandler = WebViewHandler(webView: webView, javascriptHandlers: JSHandlers.allCases.map({ $0.rawValue }))
observable = PublishSubject()
disposeBag = DisposeBag()
isLoading = BehaviorRelay(value: .inProgress)
initializationState = BehaviorRelay(value: .inProgress)

webViewHandler.receivedMessageHandler = { [weak self] name, body in
self?.receiveMessage(name: name, body: body)
Expand All @@ -60,19 +60,16 @@ final class PDFWorkerWebViewHandler {
.observe(on: MainScheduler.instance)
.subscribe(onSuccess: { [weak self] _ in
DDLogInfo("PDFWorkerWebViewHandler: initialization succeeded")
self?.isLoading.accept(.initialized)
self?.initializationState.accept(.initialized)
}, onFailure: { [weak self] error in
DDLogInfo("PDFWorkerWebViewHandler: initialization failed - \(error)")
self?.isLoading.accept(.failed(error))
self?.initializationState.accept(.failed(error))
})
.disposed(by: disposeBag)

func initialize() -> Single<Any> {
func initialize() -> Single<()> {
DDLogInfo("PDFWorkerWebViewHandler: initialize web view")
return loadIndex()
.flatMap { _ in
Single.just(Void())
}

func loadIndex() -> Single<()> {
guard let indexUrl = Bundle.main.url(forResource: "worker", withExtension: "html") else {
Expand All @@ -84,15 +81,15 @@ final class PDFWorkerWebViewHandler {
}

private func performCall(completion: @escaping () -> Void) {
switch isLoading.value {
switch initializationState.value {
case .failed(let error):
observable.on(.next(.failure(error)))

case .initialized:
completion()

case .inProgress:
isLoading.filter { result in
initializationState.filter { result in
switch result {
case .inProgress:
return false
Expand Down Expand Up @@ -121,11 +118,12 @@ final class PDFWorkerWebViewHandler {

func recognize(file: FileData) {
let filePath = file.createUrl().path
performCall {
performRecognize(for: filePath)
performCall { [weak self] in
guard let self else { return }
performRecognize(for: filePath, self: self)
}

func performRecognize(for path: String) {
func performRecognize(for path: String, self: PDFWorkerWebViewHandler) {
DDLogInfo("PDFWorkerWebViewHandler: call recognize js")
return webViewHandler.call(javascript: "recognize('\(path)');")
.subscribe(on: MainScheduler.instance)
Expand All @@ -140,11 +138,12 @@ final class PDFWorkerWebViewHandler {

func getFullText(file: FileData) {
let filePath = file.createUrl().path
performCall {
performGetFullText(for: filePath)
performCall { [weak self] in
guard let self else { return }
performGetFullText(for: filePath, self: self)
}

func performGetFullText(for path: String) {
func performGetFullText(for path: String, self: PDFWorkerWebViewHandler) {
DDLogInfo("PDFWorkerWebViewHandler: call getFullText js")
return webViewHandler.call(javascript: "getFullText('\(path)');")
.subscribe(on: MainScheduler.instance)
Expand Down Expand Up @@ -172,7 +171,7 @@ final class PDFWorkerWebViewHandler {
observable.on(.next(.success(.recognizerData(data: data))))

case .log:
DDLogInfo("JSLOG: \(body)")
DDLogInfo("PDFWorkerWebViewHandler: JSLOG - \(body)")
}
}
}
9 changes: 0 additions & 9 deletions Zotero/Controllers/Web View Handling/WebViewHandler.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,6 @@ final class WebViewHandler: NSObject {
return createWebLoadedSingle()
}

func loadHTMLString(_ string: String, baseURL: URL?) -> Single<()> {
guard let webView else {
DDLogError("WebViewHandler: web view is nil")
return .error(Error.webViewMissing)
}
webView.loadHTMLString(string, baseURL: baseURL)
return createWebLoadedSingle()
}

func load(webUrl: URL) -> Single<()> {
guard let webView else {
DDLogError("WebViewHandler: web view is nil")
Expand Down

0 comments on commit 61a67e8

Please sign in to comment.