Skip to content

refactor: move webkit integration into browser engine core#237

Merged
yonaries merged 7 commits intomainfrom
refactor/browser-engine-module
Mar 14, 2026
Merged

refactor: move webkit integration into browser engine core#237
yonaries merged 7 commits intomainfrom
refactor/browser-engine-module

Conversation

@yonaries
Copy link
Copy Markdown
Contributor

Summary

  • move WebKit page, profile, script bridge, and download handling into Core/BrowserEngine
  • update tabs and browser UI to consume BrowserPage / BrowserPageView instead of owning raw WKWebView integration
  • remove legacy Features/Tabs/Web wiring and keep the tabs-specific adapter in Features/Tabs/Browser

Testing

  • xcodebuild -project Ora.xcodeproj -scheme ora -configuration Debug -destination "platform=macOS" -derivedDataPath /tmp/ora-derived CODE_SIGNING_ALLOWED=NO build
  • manual smoke test: browsing, navigation, tabs, downloads, privacy, passwords, and media flows

@yonaries yonaries requested a review from kenenisa as a code owner March 14, 2026 13:29
@tembo tembo bot added the enhancement improvements label Mar 14, 2026
@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 14, 2026

Greptile Summary

This PR introduces a clean BrowserEngine abstraction layer in Core/BrowserEngine that encapsulates all WebKit integration (WKWebView, WKWebsiteDataStore, WKDownload, user scripts, and navigation delegation) behind framework-agnostic types. Feature modules (Tabs, Downloads, Privacy, FindInPage, Passwords, Player, Browser) are updated to consume the new BrowserPage/BrowserPageView API instead of accessing WKWebView directly, and three legacy files under Features/Tabs/Web are removed.

  • The new BrowserPage class wraps WKWebView with a delegate-based API (BrowserPageDelegate) that features like tab management, downloads, and passwords can implement without importing WebKit
  • Tab no longer creates a web view in its init; instead, restoreTransientState lazily initializes a BrowserPage when the tab becomes active — this is a nice improvement for startup performance
  • Download handling is refactored from direct WKDownloadDelegate conformance in the navigation delegate to a callback-based BrowserDownloadTask integrated via DownloadManager.handleDownload()
  • There is a potential race condition in Tab.stopMedia() where browserPage is nilled synchronously while async teardown is still in progress
  • BrowserWebsiteDataType.cache maps to allWebsiteDataTypes() (including cookies), which preserves prior behavior but is a semantic mismatch in the new abstraction layer

Confidence Score: 4/5

  • This PR is a well-structured refactor with one potential race condition in stopMedia; overall safe to merge with that fix.
  • The refactor is thorough and consistent — all 27 files are updated coherently, the build passes, and the abstraction layer is cleanly designed. The one logic concern (stopMedia race) is unlikely to trigger in normal usage but could cause issues during rapid tab close/reopen cycles. The style suggestions are non-blocking improvements.
  • ora/Features/Tabs/Models/Tab.swift — potential race condition in stopMedia() between synchronous nil assignment and async teardown completion

Important Files Changed

Filename Overview
ora/Core/BrowserEngine/BrowserPage.swift New core abstraction wrapping WKWebView with navigation/download/script delegation. Clean separation of WebKit concerns.
ora/Core/BrowserEngine/BrowserPageView.swift NSViewRepresentable wrapper for BrowserPage. Faithfully ports mouse event handling from old WebView. Missing downloadManager parameter compared to old Coordinator.
ora/Core/BrowserEngine/BrowserEngine.swift Singleton factory for profiles and pages. makeProfile creates new BrowserEngineProfile each call instead of caching, but WKWebsiteDataStore handles deduplication internally.
ora/Core/BrowserEngine/BrowserEngineProfile.swift WebKit data store wrapper with data clearing. The .cache type maps to allWebsiteDataTypes() which is broader than just cache but matches prior behavior.
ora/Core/BrowserEngine/Scripts/OraBrowserScripts.swift Consolidates user scripts previously in TabScriptHandler. Bridge, navigation/media, and password manager scripts cleanly organized.
ora/Features/Tabs/Models/Tab.swift Major refactor: replaces WKWebView with BrowserPage. Init no longer creates the web view; defers to restoreTransientState. Race condition in stopMedia where browserPage is nilled before async teardown completes.
ora/Features/Tabs/Browser/TabBrowserPageDelegate.swift New delegate bridging BrowserPage events to Tab state updates. Cleanly handles navigation phases, script messages, JS dialogs, and downloads.
ora/Features/Downloads/Services/DownloadManager.swift New handleDownload method with callback-based BrowserDownloadTask integration. Progress timer and cleanup logic are well-structured.
ora/Features/Browser/Views/BrowserView.swift Sidebar shield JS changed from safe callAsyncJavaScript with parameterized arguments to direct string interpolation. Values are controlled internally so no injection risk in practice.
ora/Features/Privacy/Services/PrivacyService.swift Simplified to use BrowserEngineProfile for data clearing. Behavior preserved.

Class Diagram

%%{init: {'theme': 'neutral'}}%%
classDiagram
    class BrowserEngine {
        +shared: BrowserEngine
        +makeProfile(identifier, isPrivate) BrowserEngineProfile
        +makePage(profile, configuration, delegate) BrowserPage
    }

    class BrowserEngineProfile {
        +identifier: UUID
        +isPrivate: Bool
        +dataStore: WKWebsiteDataStore
        +clearData(ofTypes, forHost, completion)
    }

    class BrowserPage {
        -webView: WKWebView
        +delegate: BrowserPageDelegate
        +contentView: NSView
        +currentURL: URL?
        +load(URLRequest)
        +reload()
        +goBack()
        +goForward()
        +evaluateJavaScript(String)
        +takeSnapshot(configuration)
        +teardown()
    }

    class BrowserPageDelegate {
        <<protocol>>
        +decidePolicyFor(navigationAction)
        +didUpdateNavigation(event)
        +didFailNavigationWith(error)
        +didReceiveScriptMessage(message)
        +didStartDownload(download)
        +requestPermission(permission)
    }

    class TabBrowserPageDelegate {
        +tab: Tab
        +mediaController: MediaController
        +passwordCoordinator: PasswordAutofillCoordinator
    }

    class Tab {
        +browserPage: BrowserPage?
        +pageDelegate: TabBrowserPageDelegate?
        +restoreTransientState()
        +setupBrowserPageDelegate(page)
    }

    class BrowserPageView {
        +page: BrowserPage
    }

    BrowserEngine --> BrowserEngineProfile : creates
    BrowserEngine --> BrowserPage : creates
    BrowserPage --> BrowserPageDelegate : delegates to
    TabBrowserPageDelegate ..|> BrowserPageDelegate : implements
    Tab --> BrowserPage : owns
    Tab --> TabBrowserPageDelegate : owns
    BrowserPageView --> BrowserPage : wraps
Loading

Last reviewed commit: e68ca61

@yonaries yonaries merged commit c55e8e9 into main Mar 14, 2026
2 checks passed
@yonaries yonaries deleted the refactor/browser-engine-module branch March 14, 2026 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant