Conversation
- WebContents: onShow()/onHide() were removed in favor of updateWebContentsVisibility(@visibility int). - AccountManagerFacadeProvider.setInstance() was removed (only setInstanceForTests() remains); drop the AccountManagerFacade initialization (Wolvic does not require it). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- DeviceUtils: addDeviceSpecificUserAgentSwitch() was removed; use updateDeviceSpecificUserAgentSwitch(Context). - WebContentsObserver: the (WebContents) constructor and destroy() were replaced by a no-arg constructor + observe(WebContents) and webContentsDestroyed(). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- BrowserStartupController.startBrowserProcessesAsync() gained singleProcess and scheduleFlushStartupTasks parameters; pass false/false (Wolvic runs multiprocess; flush scheduling is WebView-only). - WebContentsDelegate.enterFullscreenModeForTab() gained a leading long requestingFrame parameter. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- TabWebContentsDelegate.enterFullscreenModeForTab now takes (RenderFrameHost, prefersNavigationBar, prefersStatusBar, displayId) to match M144's WebContentsDelegateAndroid signature. - M144 renamed WebContentsDelegateAndroid.onUpdateUrl -> onUpdateTargetUrl and changed its semantics: it now fires on every hovered-link target-URL change (passing the hovered URL), not once per navigation with the page URL. The old override drove the mobile-YouTube -> desktop rewrite, which under the new semantics caused an endless navigation loop. Remove the override (its onLocationChange was already delivered by TabWebContentsObserver) and move the rewrite to TabWebContentsObserver.didStartNavigationInPrimaryMainFrame -- the URL is rewritten before the request is initiated, with the redirect deferred via PostTask to avoid re-entering the navigation machinery. YoutubeUrlHelper is now idempotent (returns an already-desktop URL verbatim) so the redirect cannot loop. - Fix YouTube fullscreen video not filling the window: YouTube sizes its fullscreen <video> element against window.outerWidth/ outerHeight. On M144 the field-trial testing config enables the AndroidUseCorrectWindowBounds feature, which makes GetBoundsInRootWindow() (and thus window.outerWidth/Height) report the physical Android panel instead of the WebContents view bounds. Wolvic renders web content to an offscreen VR texture that is larger than the panel, so reporting the panel shrinks YouTube's fullscreen video to a small box in the top-left corner. The feature is meant for windowed/multi- window Android apps and is irrelevant to Wolvic, which never runs as a positioned Android window; its pre-M14x behaviour (view/surface bounds) is the correct "window" size for our offscreen VR compositor. Disable AndroidUseCorrectWindowBounds so window.outerWidth/Height track the content surface again. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
MediaSession.fromWebContents() now returns null until the native MediaSession is created lazily on first media use (it uses GetIfExists() instead of creating one). Constructing TabMediaSessionObserver eagerly at tab creation therefore passed a null session into MediaSessionObserver's constructor, which dereferences it -> startup NPE. Defer observing: attach the observer only if a session already exists, otherwise create it from TabWebContentsObserver.mediaSessionCreated() when the session appears. The session is created at most once per WebContents, so no dedup/cleanup is needed. Guard onMediaFullscreen() against a not-yet-created observer. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
svillar
added
chromium
The M150 Chromium AAR ships Java 21 bytecode (class file version 65),
which the JDK 17 toolchain refuses to read on the compile classpath
("class file has wrong version 65.0, should be 61.0"). Raise the
toolchain to 21; source/target compatibility stays at 1.8, so emitted
bytecode is unchanged. Also flip the CI default JDK to 21.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Not ready to approve
There is a confirmed Java logic bug in RuntimeImpl where BuildConfig.FLAVOR_abi is compared with ==, which can break the intended x64 feature-disabling behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Updates Wolvic’s Chromium backend to be compatible with Chromium M150 by adapting to upstream API changes across navigation, media session, visibility, and startup plumbing, and aligning the build toolchain with the new baseline.
Changes:
- Move YouTube “app=desktop” URL rewriting earlier in the navigation lifecycle and make it idempotent.
- Update WebContents / MediaSession observer and delegate integrations to match new Chromium M150 APIs.
- Bump Java toolchain/CI to JDK 21 and adjust runtime startup flags / UA switch initialization.
File summaries
| File | Description |
|---|---|
| app/src/common/chromium/com/igalia/wolvic/browser/api/impl/YoutubeUrlHelper.java | Makes YouTube URL rewrite idempotent by returning the original spec when already in desktop mode. |
| app/src/common/chromium/com/igalia/wolvic/browser/api/impl/TabWebContentsObserver.java | Rewrites YouTube watch navigations pre-request; updates payment handler observer lifecycle; hooks MediaSession creation callback. |
| app/src/common/chromium/com/igalia/wolvic/browser/api/impl/TabWebContentsDelegate.java | Adapts fullscreen entry API signature change and removes URL rewrite logic formerly in onUpdateUrl. |
| app/src/common/chromium/com/igalia/wolvic/browser/api/impl/TabMediaSessionObserver.java | Updates observer construction to accept a lazily-created MediaSession instance directly. |
| app/src/common/chromium/com/igalia/wolvic/browser/api/impl/TabImpl.java | Defers MediaSession observer creation until the native MediaSession exists; null-guards fullscreen media callbacks. |
| app/src/common/chromium/com/igalia/wolvic/browser/api/impl/SessionImpl.java | Replaces onShow/onHide with WebContents visibility updates (VISIBLE/HIDDEN). |
| app/src/common/chromium/com/igalia/wolvic/browser/api/impl/RuntimeImpl.java | Adjusts startup switches (disable features), updates UA switch API, and adapts BrowserStartupController callback signature. |
| app/build.gradle | Bumps Android Java toolchain language level to 21. |
| .github/workflows/build.yml | Updates CI default JDK to 21 to match the new toolchain baseline. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 2
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Required API changes to bump the underlying Chromium version to M150
There is one commit per version bump, the migration was done in 4-release steps except the last one.
There is an extra commit on top of M150 which was left alone on purpose because it also affects gecko