Skip to content

Suppress screen-orientation lock/unlock when entering/leaving PiP - #37848

Merged
simoarpe merged 5 commits into
masterfrom
simone/pip-orientation-lock
Jul 8, 2026
Merged

simoarpe merged 5 commits into
masterfrom
simone/pip-orientation-lock

Conversation

@simoarpe

@simoarpe simoarpe commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

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 current NavigationEntry. 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:

  1. 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 NavigationEntry and stopped writing the shared state the delegate reads. Nothing wrote what the delegate read, so the suppression never triggered.

  2. Key identity across components. The shared key was a header defined inline constexpr array. SupportsUserData matches 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, so GetUserData missed 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 NavigationEntry the 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 the SupportsUserData key a single file local definition. The two layers now call functions instead of sharing a key, so the address mismatch is impossible by construction.

  • New content/public/browser/navigation_entry_fullscreen.{h,cc} with SetNavigationEntryFullscreenRequested and IsNavigationEntryFullscreenRequested, backed by a file local presence key.
  • The delegate reads through IsNavigationEntryFullscreenRequested.
  • The YouTube PiP tab helper writes through SetNavigationEntryFullscreenRequested.
  • Removed the previous fullscreen_page_data.h (exported struct plus exported key).

This follows the pattern content/public uses for other cross layer predicates (for example IsIsolatedContext) and the key handling Chromium documents for WebContentsUserData.

Test plan

Phone only; not reproducible on tablet.

  1. Open m.youtube.com and play a video.
  2. Enter PiP. The system UI and video must not rotate to landscape.
  3. Expand back to the browser view. Again no landscape rotation.
  4. Repeat the cycle a few times.
  5. Observe no flash entering or leaving PiP in either direction.

Notes for reviewers

  • Boolean cross layer state is exposed as free functions rather than an exported UserData key on purpose. The key must be one address across the browser and content components; a single file local definition guarantees that, while a header inline key does not.
  • brave/content/public/browser/DEPS gains +content/public/browser so the new .cc can include navigation_entry.h and reach NavigationEntry's SupportsUserData methods. This matches the sibling content/browser/screen_orientation/DEPS.
  • No automated test is added: the path is Android only, cross layer, and driven by the renderer entering fullscreen, so it is not practical to unit test. This is consistent with the original Fix Picture-in-Picture orientation issues for YouTube videos #30578.

@simoarpe simoarpe self-assigned this Jul 8, 2026
@simoarpe simoarpe added CI/skip-ios Do not run CI builds for iOS CI/skip-windows-x64 Do not run CI builds for Windows x64 CI/skip-macos-arm64 Do not run CI builds for macOS arm64 CI/skip-origin Do not run CI builds for Origin labels Jul 8, 2026
Comment thread content/public/browser/sources.gni
Comment thread content/public/browser/sources.gni

@samartnik samartnik left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

++

@simoarpe
simoarpe enabled auto-merge (squash) July 8, 2026 14:35
@simoarpe
simoarpe merged commit d8f31f3 into master Jul 8, 2026
23 checks passed
@simoarpe
simoarpe deleted the simone/pip-orientation-lock branch July 8, 2026 14:46
@brave-builds brave-builds added this to the 1.94.x - Nightly milestone Jul 8, 2026
@brave-builds

Copy link
Copy Markdown
Collaborator

Released in v1.94.50

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI/skip-ios Do not run CI builds for iOS CI/skip-macos-arm64 Do not run CI builds for macOS arm64 CI/skip-origin Do not run CI builds for Origin CI/skip-windows-x64 Do not run CI builds for Windows x64

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Switching to or from PiP briefly rotates the system UI and video to landscape mode on the phone

3 participants