New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 644133 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Bug



Sign in to add a comment

Video is visible behind the browser window while exiting full screen from detached mode

Project Member Reported by sdy@chromium.org, Sep 5 2016

Issue description

Version: 55.0.2850.0
OS: macOS 10.11.6
Hardware: MacBook Pro (Retina, 15-inch, Late 2013), 2.6 GHz, Intel Iris Pro + NVIDIA GeForce GT 750M

What steps will reproduce the problem?
(1) Play a video full screen and wait for UI to disappear.
(2) Exit full screen with a keyboard shortcut (e.g. F or Esc on YouTube).
(3) Repeat the above, but exit full screen when player UI is still visible.

What is the expected output?
Clean transition.

What do you see instead?
As the window transitions out of full screen, the full screen video remains visible behind the window. This does not happen when UI is visible (i.e. we're not in detached mode, if I'm using that term correctly).
 
detached_mode_video_in_bg.mov
13.3 MB Download
Cc: spqc...@chromium.org
Owner: ccameron@chromium.org
Status: Assigned (was: Available)
Hmm, we need to ensure that the FSLP window be hidden before the transition out of fullscreen.
I looked at this a while ago and found that calling -[NSWindow close] on a NSWindow that is in detached mode causes system hangs and janky transitions.

So if you press escape while in detached mode, we can't really make a clean transition if we're doing this 2-window business.

That said, we can do better if, say, you move the mouse to click on the get-out-of-fullscreen button. In that case, we can detect that we haven't been in detached mode for a while, and can just -[NSWindow close] the fullscreen window.
Is it just as janky if we hide (not close) the window right before we make the transition out of fullscreen?
I tested locally on 10.12 just doing a -[NSWindow close], and it seems to work fine, and we don't have the janky transition.

I'm a bit afraid to enable it, because I remember there being issues ... but so much has changed that this may be fine (or maybe the bugs are just fixed in 10.12).
Owner: ----
Status: Available (was: Assigned)
Marking as available.

We should circle back on the content fullscreen plan at some point.
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3a9e676f5eab10c442af9e492a2ca285f5496524

commit 3a9e676f5eab10c442af9e492a2ca285f5496524
Author: Sidney San Martín <sdy@chromium.org>
Date: Thu Nov 02 02:36:40 2017

Close the status bubble window when it's not visible.

This is a prerequisite for achieving detachment (a.k.a. FSLP, low power
fullscreen video playback) with the browser window, which requires that
no other windows are in front of it.

Prior to this change, status bubble windows were left on screen for the
lifetime of their parent windows. It looks like that was to work around
a bug with Spaces: if a browser window was in another Space, the user
would be switched to that space if the status bubble showed itself due
to network activity ( crbug.com/31821 ). I can't get this to happen on
macOS 10.9+, so I think it's safe to orderOut: the status bubble when
hidden.

Bug:  644133 ,  742691 
Change-Id: Ic1b6f2f45fee14d1e7033411de9aff4c64c1ec42
Reviewed-on: https://chromium-review.googlesource.com/742468
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Sidney San Martín <sdy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513382}
[modify] https://crrev.com/3a9e676f5eab10c442af9e492a2ca285f5496524/chrome/browser/ui/cocoa/status_bubble_mac.mm
[modify] https://crrev.com/3a9e676f5eab10c442af9e492a2ca285f5496524/chrome/browser/ui/cocoa/web_contents_modal_dialog_manager_views_mac_browsertest.mm

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/523954ceafee2535cb9026fff5b683af7e7f24c1

commit 523954ceafee2535cb9026fff5b683af7e7f24c1
Author: Sidney San Martín <sdy@chromium.org>
Date: Thu Nov 02 04:58:54 2017

Use the browser window for fullscreen layer detachment (FSLP).

Chrome has used a secondary NSWindow to show fullscreen video in a way
that lets macOS "detach" the video layer and use much less power for
graphics than it would otherwise. There are currently a couple of bugs
in this code and it's disabled for High Sierra. From discussions with
Apple, it looks like the browser window itself was only ineligible for
detachment because of its invisible status bubble child window.

- crrev/c/742468 changes the status bubble to orderOut: when not in use.

- This change has CARendererLayerTree::RootLayer just build a
  detachment-friendly layer tree when possible, instead of returning a
  second layer tree to the browser. The browser's whole FSLP code path
  can be removed in a followup CL.

With both changes, the browser window itself can become detached \o/.

Future work: Figure out why the normal layer tree isn't
detachment-friendly. Two possibilities I've found so far: First, a color
correction hack (crbug/633805) renders background colors as surfaces
instead of specifying a color on the layer. Second, backgrounds are
rendered with tiles instead of as one big layer. Neither of those should
be necessary, and fixing them might make detachment "just work".

Bug:  742691 ,  644133 
Change-Id: Ia59ca2d2541e1484efaf8e7982d9f015a448e080
Reviewed-on: https://chromium-review.googlesource.com/739185
Commit-Queue: Sidney San Martín <sdy@chromium.org>
Reviewed-by: ccameron <ccameron@chromium.org>
Reviewed-by: Sarah Chan <spqchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513410}
[modify] https://crrev.com/523954ceafee2535cb9026fff5b683af7e7f24c1/chrome/browser/ui/cocoa/browser_window_controller_private.mm
[modify] https://crrev.com/523954ceafee2535cb9026fff5b683af7e7f24c1/ui/accelerated_widget_mac/ca_renderer_layer_tree.h
[modify] https://crrev.com/523954ceafee2535cb9026fff5b683af7e7f24c1/ui/accelerated_widget_mac/ca_renderer_layer_tree.mm

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/62e2e5bdc6a3991e3117113d1e292ddbc9f4c0ae

commit 62e2e5bdc6a3991e3117113d1e292ddbc9f4c0ae
Author: Finnur Thorarinsson <finnur@chromium.org>
Date: Thu Nov 02 10:23:31 2017

Revert "Close the status bubble window when it's not visible."

This reverts commit 3a9e676f5eab10c442af9e492a2ca285f5496524.

Reason for revert: 

Starting from this checkin, a unit test has been failing consistently:

StatusBubbleMacTest.Delete
https://build.chromium.org/p/chromium.mac/builders/Mac10.12%20Tests/builds/6903


Original change's description:
> Close the status bubble window when it's not visible.
> 
> This is a prerequisite for achieving detachment (a.k.a. FSLP, low power
> fullscreen video playback) with the browser window, which requires that
> no other windows are in front of it.
> 
> Prior to this change, status bubble windows were left on screen for the
> lifetime of their parent windows. It looks like that was to work around
> a bug with Spaces: if a browser window was in another Space, the user
> would be switched to that space if the status bubble showed itself due
> to network activity ( crbug.com/31821 ). I can't get this to happen on
> macOS 10.9+, so I think it's safe to orderOut: the status bubble when
> hidden.
> 
> Bug:  644133 ,  742691 
> Change-Id: Ic1b6f2f45fee14d1e7033411de9aff4c64c1ec42
> Reviewed-on: https://chromium-review.googlesource.com/742468
> Reviewed-by: Robert Sesek <rsesek@chromium.org>
> Commit-Queue: Sidney San Martín <sdy@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#513382}

TBR=rsesek@chromium.org,sdy@chromium.org

Change-Id: Id284030640edc01fa92cbd8066dcfe6daf463660
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  644133 ,  742691 
Reviewed-on: https://chromium-review.googlesource.com/749942
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Commit-Queue: Finnur Thorarinsson <finnur@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513451}
[modify] https://crrev.com/62e2e5bdc6a3991e3117113d1e292ddbc9f4c0ae/chrome/browser/ui/cocoa/status_bubble_mac.mm
[modify] https://crrev.com/62e2e5bdc6a3991e3117113d1e292ddbc9f4c0ae/chrome/browser/ui/cocoa/web_contents_modal_dialog_manager_views_mac_browsertest.mm

Project Member

Comment 9 by bugdroid1@chromium.org, Nov 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8f1989aa6b1becf26cafbcd3dc972a43009d3883

commit 8f1989aa6b1becf26cafbcd3dc972a43009d3883
Author: Sidney San Martín <sdy@chromium.org>
Date: Fri Nov 03 16:03:15 2017

Close the status bubble window when it's not visible.

This is a prerequisite for achieving detachment (a.k.a. FSLP: fullscreen
low power video playback) with the browser window because macOS won't
detach a window when another window is in front of it.

Prior to this change, status bubble windows were left on screen for the
lifetime of their parent windows to work around an issue when a browser
window in another space had network activity: if Chrome was active in
the current space, showing the status bubble would switch to the other
window's space ( crbug.com/31821 ). This change prevents that by setting
the bubble window's collectionBehavior, which wasn't possible on macOS
10.5, which Chrome supported when the bug was found.

This is a reland of 3a9e676f5eab10c442af9e492a2ca285f5496524 with a fix
for a failed test (and potential crash).

Bug:  644133 ,  742691 
Change-Id: Iec140be94b8d0dbdb72ae816469660a259a2f47f
Reviewed-on: https://chromium-review.googlesource.com/753803
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Sidney San Martín <sdy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513793}
[modify] https://crrev.com/8f1989aa6b1becf26cafbcd3dc972a43009d3883/chrome/browser/ui/cocoa/bubble_view.h
[modify] https://crrev.com/8f1989aa6b1becf26cafbcd3dc972a43009d3883/chrome/browser/ui/cocoa/bubble_view.mm
[modify] https://crrev.com/8f1989aa6b1becf26cafbcd3dc972a43009d3883/chrome/browser/ui/cocoa/bubble_view_unittest.mm
[modify] https://crrev.com/8f1989aa6b1becf26cafbcd3dc972a43009d3883/chrome/browser/ui/cocoa/status_bubble_mac.mm
[modify] https://crrev.com/8f1989aa6b1becf26cafbcd3dc972a43009d3883/chrome/browser/ui/cocoa/web_contents_modal_dialog_manager_views_mac_browsertest.mm

Comment 10 by sdy@chromium.org, Mar 5 2018

Owner: sdy@chromium.org
Status: Fixed (was: Available)

Sign in to add a comment