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

Issue 742691 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

Window turns black when exiting Fullscreen Low Power on High Sierra w/ full size content view

Project Member Reported by sdy@chromium.org, Jul 14 2017

Issue description

Chrome Version: 61.0.3156.0
OS: 10.13 Beta (17a306f)

What steps will reproduce the problem?
(1) Play a YouTube video full screen.
(2) Make sure annotations are turned off, and wait for the controls to disappear.
(3) Move the mouse to bring back the controls.

What is the expected result?
The controls come back.

What happens instead?
The screen turns black until you exit fullscreen or wait for the FSLP window to become active again.

This only happens with High Sierra *and* ShouldUseFullSizeContentView() together.
 

Comment 1 by sdy@chromium.org, Jul 14 2017

Cc: -ccameron@chromium.org
Owner: ccameron@chromium.org
Status: Assigned (was: Untriaged)
ccameron@, do you think you have time to look at this?
Even letting the window return to FSLP doesn't seem to fix this for me; you have to exit fullscreen and re-enter it to get the video back. This seems pretty bad.
Not sure if I'll have too much time to grab this -- how far are we away from creating a new window for fullscreen? That will ultimately allow us to avoid the FSLP window hacks (hopefully).

Comment 4 by shrike@chromium.org, Jul 18 2017

Cc: ellyjo...@chromium.org ccameron@chromium.org
Owner: sdy@chromium.org
ccameron@ suggests we disable FSLP mode (there's a flag: disable-fullscreen-low-power-mode) for branch. He's going to look into it and maybe come up with a solution that lets us turn it back on for M61, or we spend a milestone exploring a fix.

sdy@ - can you land a CL to turn off FSLP mode?

Comment 5 by shrike@chromium.org, Jul 18 2017

BTW, I tried the flag and it seems to work around the problem.

Comment 6 by sdy@chromium.org, Jul 18 2017

shrike@: OK. I’ll just turn FSLP off for 10.13, right?
ccameron@: erickzul@ and jessye@ are making good progress, but it’s still early in the project! M62 is plausible.

Comment 7 by shrike@chromium.org, Jul 18 2017

Correct, this would be for just 10.13.
Project Member

Comment 8 by bugdroid1@chromium.org, Jul 19 2017

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

commit fbf0087bbdf9e947ae065bb80d8d7aa9ece45b1d
Author: Sidney San Martín <sdy@chromium.org>
Date: Wed Jul 19 20:11:45 2017

Disable FSLP on 10.13 while a bug is investigated.

Bug:  742691 
Change-Id: I4a80e92818261149d2a0b23e713245ae389ed5f3
Reviewed-on: https://chromium-review.googlesource.com/575846
Commit-Queue: Sidney San Martin <sdy@chromium.org>
Reviewed-by: ccameron chromium <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487941}
[modify] https://crrev.com/fbf0087bbdf9e947ae065bb80d8d7aa9ece45b1d/chrome/browser/ui/cocoa/browser_window_controller_private.mm

Comment 9 by sdy@chromium.org, Jul 19 2017

Cc: -ccameron@chromium.org
Labels: -ReleaseBlock-Stable
Owner: ccameron@chromium.org
OK, done.
Labels: TE-NeedsTriageFromMTV
As Mac 10.13 is not available from our end,could anyone from MTV team please look into this issue.
Adding 'TE-NeedsTriageFromMTV' label.
Thanks..!!
Cc: ccameron@chromium.org
Labels: -TE-NeedsTriageFromMTV
Owner: manoranj...@chromium.org
Cc: -ccameron@chromium.org
Owner: ccameron@chromium.org
Not seeing this issue anymore (with FSLP disabled) on Latest Canary#62.0.3167.0 for OS X 10.13 (17A315i).
What is the next step for turning this back on on 10.13? I want to make sure we're working towards that goal.
Summary: Window turns black when exiting Fullscreen Low Power on High Sierra w/ full size content view (was: Window turns black when exiting FSLP on High Sierra w/ full size content view)
The long-term stable state for this is to have fullscreen video pop out into its own window, and ensure that the implementation there is functional.
Cc: spqc...@chromium.org
Do we have plans for content-fullscreen (creating a new window for fullscreen videos)? That would be the most natural place to fix this, but I don't know if we've decided to do that.
Cc: sdy@chromium.org

Comment 18 by sdy@chromium.org, Sep 21 2017

ccameron@: That is the plan, but it may be a little while before someone works on it. I was planning to explore the fixes mentioned in the email thread I forwarded you (but you could, too).

Comment 19 by sdy@chromium.org, Oct 18 2017

Owner: sdy@chromium.org

Comment 20 by sdy@chromium.org, Oct 18 2017

Labels: -Pri-1 -M-61 M-64 Pri-2
Project Member

Comment 21 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 22 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 23 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 24 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 25 by sdy@chromium.org, Nov 16 2017

Status: Fixed (was: Assigned)
Still needs some cleanup, but this issue is resolved.
Project Member

Comment 26 by bugdroid1@chromium.org, Dec 1 2017

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

commit 777537ee1b54b2701e3d7f7be06f574d561ece01
Author: Sidney San Martín <sdy@chromium.org>
Date: Fri Dec 01 08:14:43 2017

Delete the old fullscreen low power path on Mac.

Bug:  742691 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I14d358e6cc46abdd4a2574a75745b1cacf897399
Reviewed-on: https://chromium-review.googlesource.com/802020
Reviewed-by: ccameron <ccameron@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Sidney San Martín <sdy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520889}
[modify] https://crrev.com/777537ee1b54b2701e3d7f7be06f574d561ece01/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/777537ee1b54b2701e3d7f7be06f574d561ece01/chrome/browser/ui/cocoa/browser_window_controller.h
[modify] https://crrev.com/777537ee1b54b2701e3d7f7be06f574d561ece01/chrome/browser/ui/cocoa/browser_window_controller.mm
[modify] https://crrev.com/777537ee1b54b2701e3d7f7be06f574d561ece01/chrome/browser/ui/cocoa/browser_window_controller_private.mm
[delete] https://crrev.com/4ac683f23cfa0f50ac7240b6d675bd901a570607/chrome/browser/ui/cocoa/fullscreen_low_power_coordinator.h
[delete] https://crrev.com/4ac683f23cfa0f50ac7240b6d675bd901a570607/chrome/browser/ui/cocoa/fullscreen_low_power_coordinator.mm
[delete] https://crrev.com/4ac683f23cfa0f50ac7240b6d675bd901a570607/chrome/browser/ui/cocoa/fullscreen_low_power_coordinator_unittest.mm
[modify] https://crrev.com/777537ee1b54b2701e3d7f7be06f574d561ece01/chrome/common/chrome_switches.cc
[modify] https://crrev.com/777537ee1b54b2701e3d7f7be06f574d561ece01/chrome/common/chrome_switches.h
[modify] https://crrev.com/777537ee1b54b2701e3d7f7be06f574d561ece01/chrome/test/BUILD.gn
[modify] https://crrev.com/777537ee1b54b2701e3d7f7be06f574d561ece01/content/browser/compositor/gpu_output_surface_mac.mm
[modify] https://crrev.com/777537ee1b54b2701e3d7f7be06f574d561ece01/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/777537ee1b54b2701e3d7f7be06f574d561ece01/gpu/ipc/client/command_buffer_proxy_impl.cc
[modify] https://crrev.com/777537ee1b54b2701e3d7f7be06f574d561ece01/gpu/ipc/client/gpu_process_hosted_ca_layer_tree_params.h
[modify] https://crrev.com/777537ee1b54b2701e3d7f7be06f574d561ece01/gpu/ipc/common/gpu_messages.h
[modify] https://crrev.com/777537ee1b54b2701e3d7f7be06f574d561ece01/gpu/ipc/in_process_command_buffer.cc
[modify] https://crrev.com/777537ee1b54b2701e3d7f7be06f574d561ece01/gpu/ipc/service/gpu_command_buffer_stub.cc
[modify] https://crrev.com/777537ee1b54b2701e3d7f7be06f574d561ece01/gpu/ipc/service/image_transport_surface_delegate.h
[modify] https://crrev.com/777537ee1b54b2701e3d7f7be06f574d561ece01/gpu/ipc/service/image_transport_surface_overlay_mac.h
[modify] https://crrev.com/777537ee1b54b2701e3d7f7be06f574d561ece01/gpu/ipc/service/image_transport_surface_overlay_mac.mm
[modify] https://crrev.com/777537ee1b54b2701e3d7f7be06f574d561ece01/ui/accelerated_widget_mac/BUILD.gn
[modify] https://crrev.com/777537ee1b54b2701e3d7f7be06f574d561ece01/ui/accelerated_widget_mac/accelerated_widget_mac.h
[modify] https://crrev.com/777537ee1b54b2701e3d7f7be06f574d561ece01/ui/accelerated_widget_mac/accelerated_widget_mac.mm
[modify] https://crrev.com/777537ee1b54b2701e3d7f7be06f574d561ece01/ui/accelerated_widget_mac/ca_layer_tree_coordinator.h
[modify] https://crrev.com/777537ee1b54b2701e3d7f7be06f574d561ece01/ui/accelerated_widget_mac/ca_layer_tree_coordinator.mm
[modify] https://crrev.com/777537ee1b54b2701e3d7f7be06f574d561ece01/ui/accelerated_widget_mac/ca_layer_tree_unittest_mac.mm
[modify] https://crrev.com/777537ee1b54b2701e3d7f7be06f574d561ece01/ui/accelerated_widget_mac/ca_renderer_layer_tree.h
[delete] https://crrev.com/4ac683f23cfa0f50ac7240b6d675bd901a570607/ui/accelerated_widget_mac/fullscreen_low_power_coordinator.h

Sign in to add a comment