Suppress screen-orientation lock/unlock when entering/leaving PiP - #37848
Merged
Merged
Conversation
Collaborator
|
Released in v1.94.50 |
6 tasks
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.
Resolves brave/brave-browser#56784
2026-07-08.at.15.06.34.mp4
Entering or leaving the Brave managed YouTube Picture-in-Picture window briefly rotated the system UI and the video to landscape on phones. This PR restores the orientation suppression that #30578 introduced and that later regressed, and hardens the mechanism so it cannot silently break the same way again.
Phone only. Not reproducible on tablet.
Root cause
The flash is suppressed by
BraveScreenOrientationDelegateAndroid, which skips the screen orientation lock/unlock while a fullscreen request is armed on the currentNavigationEntry. That state is written in the browser layer (the YouTube PiP tab helper) and read in the content layer (the delegate). Two independent problems broke the link:Disconnected state. PR Fix YouTube PiP script injection after navigation #37366 reworked the tab helper to track the request under a private key on the
NavigationEntryand stopped writing the shared state the delegate reads. Nothing wrote what the delegate read, so the suppression never triggered.Key identity across components. The shared key was a header defined
inline constexprarray.SupportsUserDatamatches keys by pointer address, not by string value, and a header defined key can resolve to a different address in each component. The browser layer writer and the content layer reader ended up with different addresses, soGetUserDatamissed even once the state was reconnected. This is the failure mode Chromium documents at crbug.com/589840.Both were confirmed on device: the delegate read the same
NavigationEntrythe tab helper wrote, yet found no data, because the two sides used different key addresses.Fix
Expose the per entry fullscreen flag through exported accessor functions in
content/public, and keep theSupportsUserDatakey a single file local definition. The two layers now call functions instead of sharing a key, so the address mismatch is impossible by construction.content/public/browser/navigation_entry_fullscreen.{h,cc}withSetNavigationEntryFullscreenRequestedandIsNavigationEntryFullscreenRequested, backed by a file local presence key.IsNavigationEntryFullscreenRequested.SetNavigationEntryFullscreenRequested.fullscreen_page_data.h(exported struct plus exported key).This follows the pattern
content/publicuses for other cross layer predicates (for exampleIsIsolatedContext) and the key handling Chromium documents forWebContentsUserData.Test plan
Phone only; not reproducible on tablet.
m.youtube.comand play a video.Notes for reviewers
brave/content/public/browser/DEPSgains+content/public/browserso the new.cccan includenavigation_entry.hand reachNavigationEntry'sSupportsUserDatamethods. This matches the siblingcontent/browser/screen_orientation/DEPS.