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

Issue 594460 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug

Blocked on:
issue 594449
issue 594452

Blocking:
issue 555388



Sign in to add a comment

Mac: Update content fullscreen mode to allow for entering detached mode

Project Member Reported by ccameron@chromium.org, Mar 14 2016

Issue description

Detached mode is a special fullscreen mode for video that drops GPU power roughly in half.

Actually entering the mode seems to require a very special dance.
- The setting appears to be attached to an NSWindow
- A number of things can knock you out of detached mode, and if an NSWindow is knocked out, it never gets back into detached mode
- I've only ever gotten windows with the style mask 
NSTitledWindowMask|NSResizableWindowMask|NSFullSizeContentViewWindowMask to enter detached mode
- I've only gotten windows to enter detached mode when they already have a 420v-backed AVSampleBufferDisplayLayer present when they go fullscreen (you can't add it later)
- Only solid black parent layers may be present in the CALayer tree
- The AVSampleBufferDisplayLayer may have no child layers

A scheme that seems like it may be reasonable for allowing Chrome to enter this mode is as follows
- When going content-fullscreen, put 2 windows in the customWindowsToEnterFullScreenForWindow group
- One of these windows is the "content" window, which is what we have now. That window is in front.
- The other window is the "low power" window, which is created with all of the above requirements met/
- By default, we put the "content" window on top of the "low power" window, and the "low power" window doesn't do anything (it just has a black 420v frame up)
- If we detect that there is nothing but the video present, we start feeding the video frames to the "low power" window
- At some point, we move the "content" window behind the "low power" window (using setLevel:), so that only the "low power" window shows
 
Blockedon: 594452 594449
Blocking: 555388
Cc: charliea@chromium.org

Comment 4 by a...@chromium.org, Mar 23 2016

Cc: a...@chromium.org
It feels to me like we're missing something of the bigger picture; this feels too intricate and tricky.

Have we talked to Apple about this, how to achieve this more easily or reliably?
Agree -- I've filed a few radars on it, and I'm filing more. They seem to have made it much more restrictive in 10.11, breaking tons of simple apps that would hit it in 10.10. I haven't heard back from them about why that happened.

Of note is that there is momentary juddering/flickering when entering detached mode on my laptop (even in QuickTime), so my guess is that they have been ratcheting down support every time they hit some hardware bug.
spqchan and I hacked together the following patch which gets us into detached mode:
https://codereview.chromium.org/1844053002

One of the things I suggested about detached mode above is false. It is not actually the case that if you ever get kicked out of detached mode then you never get back in. What is the case is that:
* You need to have the AVSampleBufferDisplayLayer already present when entering fullscreen
* If you ever remove the AVSampleBufferDisplayLayer, then your NSWindow is out for good
* You are free to add and remove child layers. Child layers will take you out of detached mode, but you'll re-enter when you remove them
* You do need the special style mask
* You do need only solid-black parent CALayers for the AVSampleBufferDisplayLayer.
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 27 2016

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

commit 5796159f793cc27a4b2f6ccdef91b8bf87b00721
Author: ccameron <ccameron@chromium.org>
Date: Wed Apr 27 21:13:57 2016

Mac: Add detection for low power video compatible frames

Create a persistent AVSampleBufferDisplayLayer in CALayerTreeHost
for fullscreen low power video.

Add a function to CARendererLayerTree to check to see if the
frame can be represented by a single video frame on a black
background, and, if so, update the AVSampleBufferDisplayLayer
to show this video frame.

The AVSampleBufferDisplayLayer then doesn't go anywhere.
The next patch will plumb it through to the
AcceleratedWidgetMac.

BUG= 594460 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/1920723002
Cr-Commit-Position: refs/heads/master@{#390187}

[modify] https://crrev.com/5796159f793cc27a4b2f6ccdef91b8bf87b00721/gpu/ipc/service/image_transport_surface_overlay_mac.mm
[modify] https://crrev.com/5796159f793cc27a4b2f6ccdef91b8bf87b00721/ui/accelerated_widget_mac/ca_layer_tree_coordinator.h
[modify] https://crrev.com/5796159f793cc27a4b2f6ccdef91b8bf87b00721/ui/accelerated_widget_mac/ca_layer_tree_coordinator.mm
[modify] https://crrev.com/5796159f793cc27a4b2f6ccdef91b8bf87b00721/ui/accelerated_widget_mac/ca_layer_tree_unittest_mac.mm
[modify] https://crrev.com/5796159f793cc27a4b2f6ccdef91b8bf87b00721/ui/accelerated_widget_mac/ca_renderer_layer_tree.h
[modify] https://crrev.com/5796159f793cc27a4b2f6ccdef91b8bf87b00721/ui/accelerated_widget_mac/ca_renderer_layer_tree.mm

Project Member

Comment 8 by bugdroid1@chromium.org, Apr 29 2016

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

commit 3ae0dfa02e589d5a0ca56685d0cb2a28ae3818b3
Author: ccameron <ccameron@chromium.org>
Date: Fri Apr 29 22:11:22 2016

Mac fullscreen low power: Plumb through to AcceleratedWidgetMac

Add a CAContext in the GPU process for the fullscreen low power
AVSampleBufferDisplayLayer recently added.

Plumb this CAContext's id through to AcceleratedWidgetMac's
GotCAContextFrame function.

Follow-on patches will enable showing a separate NSWindow
with this AVSampleBufferDisplayLayer as its contents.

TBR=sievers
BUG= 594460 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/1918723002
Cr-Commit-Position: refs/heads/master@{#390781}

[modify] https://crrev.com/3ae0dfa02e589d5a0ca56685d0cb2a28ae3818b3/content/browser/compositor/software_output_device_mac.mm
[modify] https://crrev.com/3ae0dfa02e589d5a0ca56685d0cb2a28ae3818b3/content/browser/gpu/gpu_process_host_ui_shim.cc
[modify] https://crrev.com/3ae0dfa02e589d5a0ca56685d0cb2a28ae3818b3/content/common/accelerated_surface_buffers_swapped_params_mac.h
[modify] https://crrev.com/3ae0dfa02e589d5a0ca56685d0cb2a28ae3818b3/content/common/gpu_host_messages.h
[modify] https://crrev.com/3ae0dfa02e589d5a0ca56685d0cb2a28ae3818b3/content/gpu/gpu_child_thread.cc
[modify] https://crrev.com/3ae0dfa02e589d5a0ca56685d0cb2a28ae3818b3/content/gpu/gpu_child_thread.h
[modify] https://crrev.com/3ae0dfa02e589d5a0ca56685d0cb2a28ae3818b3/gpu/ipc/service/gpu_channel_manager_delegate.h
[modify] https://crrev.com/3ae0dfa02e589d5a0ca56685d0cb2a28ae3818b3/gpu/ipc/service/gpu_channel_test_common.cc
[modify] https://crrev.com/3ae0dfa02e589d5a0ca56685d0cb2a28ae3818b3/gpu/ipc/service/gpu_channel_test_common.h
[modify] https://crrev.com/3ae0dfa02e589d5a0ca56685d0cb2a28ae3818b3/gpu/ipc/service/image_transport_surface_overlay_mac.h
[modify] https://crrev.com/3ae0dfa02e589d5a0ca56685d0cb2a28ae3818b3/gpu/ipc/service/image_transport_surface_overlay_mac.mm
[modify] https://crrev.com/3ae0dfa02e589d5a0ca56685d0cb2a28ae3818b3/ui/accelerated_widget_mac/accelerated_widget_mac.h
[modify] https://crrev.com/3ae0dfa02e589d5a0ca56685d0cb2a28ae3818b3/ui/accelerated_widget_mac/accelerated_widget_mac.mm
[modify] https://crrev.com/3ae0dfa02e589d5a0ca56685d0cb2a28ae3818b3/ui/views/widget/native_widget_mac_unittest.mm

Project Member

Comment 9 by bugdroid1@chromium.org, May 5 2016

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

commit 4202438c3b04ab7ed89767349b77706e1d118c07
Author: ccameron <ccameron@chromium.org>
Date: Thu May 05 20:29:40 2016

Mac: Plumb AcceleratedWidgetMac through RWHVMac

The AcceleratedWidgetMac has the CALayers for content and for low power
fullscreen mode. Expose this through RenderWidgetHostView, so that it
can be accessed by the fullscreen NSWindow.

BUG= 594460 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/1952003002
Cr-Commit-Position: refs/heads/master@{#391895}

[modify] https://crrev.com/4202438c3b04ab7ed89767349b77706e1d118c07/content/browser/frame_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/4202438c3b04ab7ed89767349b77706e1d118c07/content/browser/frame_host/render_widget_host_view_child_frame.h
[modify] https://crrev.com/4202438c3b04ab7ed89767349b77706e1d118c07/content/browser/renderer_host/render_widget_host_view_mac.h
[modify] https://crrev.com/4202438c3b04ab7ed89767349b77706e1d118c07/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/4202438c3b04ab7ed89767349b77706e1d118c07/content/browser/renderer_host/render_widget_host_view_mus.cc
[modify] https://crrev.com/4202438c3b04ab7ed89767349b77706e1d118c07/content/browser/renderer_host/render_widget_host_view_mus.h
[modify] https://crrev.com/4202438c3b04ab7ed89767349b77706e1d118c07/content/public/browser/render_widget_host_view.h
[modify] https://crrev.com/4202438c3b04ab7ed89767349b77706e1d118c07/content/test/test_render_view_host.cc
[modify] https://crrev.com/4202438c3b04ab7ed89767349b77706e1d118c07/content/test/test_render_view_host.h

Project Member

Comment 10 by bugdroid1@chromium.org, May 12 2016

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

commit 335667904fb55b54dc75c1be172347986b8f3763
Author: ccameron <ccameron@chromium.org>
Date: Thu May 12 18:28:45 2016

Mac fullscreen low power video: Add FullscreenLowPowerControllerCocoa

This is where the logic for transitioning in and out of fullscreen
low power mode lives. Do not hook up to the BrowserWindowController
yet.

BUG= 594460 

Review-Url: https://codereview.chromium.org/1952163002
Cr-Commit-Position: refs/heads/master@{#393311}

[modify] https://crrev.com/335667904fb55b54dc75c1be172347986b8f3763/chrome/browser/ui/BUILD.gn
[add] https://crrev.com/335667904fb55b54dc75c1be172347986b8f3763/chrome/browser/ui/cocoa/fullscreen_low_power_coordinator.h
[add] https://crrev.com/335667904fb55b54dc75c1be172347986b8f3763/chrome/browser/ui/cocoa/fullscreen_low_power_coordinator.mm
[modify] https://crrev.com/335667904fb55b54dc75c1be172347986b8f3763/chrome/chrome_browser_ui.gypi
[modify] https://crrev.com/335667904fb55b54dc75c1be172347986b8f3763/ui/accelerated_widget_mac/BUILD.gn
[modify] https://crrev.com/335667904fb55b54dc75c1be172347986b8f3763/ui/accelerated_widget_mac/accelerated_widget_mac.gyp
[modify] https://crrev.com/335667904fb55b54dc75c1be172347986b8f3763/ui/accelerated_widget_mac/accelerated_widget_mac.h
[modify] https://crrev.com/335667904fb55b54dc75c1be172347986b8f3763/ui/accelerated_widget_mac/accelerated_widget_mac.mm
[add] https://crrev.com/335667904fb55b54dc75c1be172347986b8f3763/ui/accelerated_widget_mac/fullscreen_low_power_coordinator.h

Project Member

Comment 11 by bugdroid1@chromium.org, May 23 2016

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

commit befa9c961bf06d34403d4f40cc469e1c774b829a
Author: ccameron <ccameron@chromium.org>
Date: Mon May 23 23:20:27 2016

Mac fullscreen low power: Add layout interactions and flag

This instantiates the NSWindow used for fullscreen low power mode, by
creating a FullscreenLowPowerCoordinator when entering fullscreen mode,
and adding it to the fullscreen transition group.

This also adds logic for determining if the layout of the browser chrome
is on-screen, and prevents moving the fullscreen low power window
in front if it is present.

Add a flag to enable this feature during development.

BUG= 594460 

Review-Url: https://codereview.chromium.org/1990163003
Cr-Commit-Position: refs/heads/master@{#395457}

[modify] https://crrev.com/befa9c961bf06d34403d4f40cc469e1c774b829a/chrome/browser/ui/cocoa/browser_window_controller.h
[modify] https://crrev.com/befa9c961bf06d34403d4f40cc469e1c774b829a/chrome/browser/ui/cocoa/browser_window_controller.mm
[modify] https://crrev.com/befa9c961bf06d34403d4f40cc469e1c774b829a/chrome/browser/ui/cocoa/browser_window_controller_private.mm
[modify] https://crrev.com/befa9c961bf06d34403d4f40cc469e1c774b829a/chrome/browser/ui/cocoa/fullscreen_low_power_coordinator.h
[modify] https://crrev.com/befa9c961bf06d34403d4f40cc469e1c774b829a/chrome/browser/ui/cocoa/fullscreen_low_power_coordinator.mm
[modify] https://crrev.com/befa9c961bf06d34403d4f40cc469e1c774b829a/chrome/common/chrome_switches.cc
[modify] https://crrev.com/befa9c961bf06d34403d4f40cc469e1c774b829a/chrome/common/chrome_switches.h

Project Member

Comment 12 by bugdroid1@chromium.org, May 24 2016

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

commit 80c04977423898bd64b1cfa91246ced5cc35d80b
Author: ccameron <ccameron@chromium.org>
Date: Tue May 24 16:41:53 2016

Mac fullscreen low power: Ignore WasOccluded by low power window

The fullscreen low power window will occlude the main NSWindow. Change
the RWHVMac to ignore WasOccluded messages when the fullscreen low
power window exists.

BUG= 594460 

Review-Url: https://codereview.chromium.org/2003243002
Cr-Commit-Position: refs/heads/master@{#395613}

[modify] https://crrev.com/80c04977423898bd64b1cfa91246ced5cc35d80b/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/80c04977423898bd64b1cfa91246ced5cc35d80b/ui/accelerated_widget_mac/accelerated_widget_mac.h
[modify] https://crrev.com/80c04977423898bd64b1cfa91246ced5cc35d80b/ui/accelerated_widget_mac/accelerated_widget_mac.mm

Project Member

Comment 13 by bugdroid1@chromium.org, Jun 8 2016

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

commit b1811073196ffed5ca106db948c5016dc2c2665b
Author: ccameron <ccameron@chromium.org>
Date: Wed Jun 08 00:06:23 2016

Mac low power video: Support child windows, sheets, and transitions

We do not want to use the fullscreen low power window when we have
a child window, an active sheet, or we are in a fullscreen transition.

Add listeners for these changes, and make them call into the
FullscreenLowPowerCoordinatorCocoa.

Add two exceptions for child windows that were determined empirically
(toolbar and a 1x1 bubble).

Also update the fullscreen low power window to not be in the window
cycle or the window list.

BUG= 594460 

Review-Url: https://codereview.chromium.org/2040253003
Cr-Commit-Position: refs/heads/master@{#398425}

[modify] https://crrev.com/b1811073196ffed5ca106db948c5016dc2c2665b/chrome/browser/ui/cocoa/browser_window_controller_private.mm
[modify] https://crrev.com/b1811073196ffed5ca106db948c5016dc2c2665b/chrome/browser/ui/cocoa/framed_browser_window.mm
[modify] https://crrev.com/b1811073196ffed5ca106db948c5016dc2c2665b/chrome/browser/ui/cocoa/fullscreen_low_power_coordinator.h
[modify] https://crrev.com/b1811073196ffed5ca106db948c5016dc2c2665b/chrome/browser/ui/cocoa/fullscreen_low_power_coordinator.mm

Project Member

Comment 14 by bugdroid1@chromium.org, Jun 11 2016

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

commit 515c79f4dcddc2bdcd0fa55af705ce7850e5258d
Author: ccameron <ccameron@chromium.org>
Date: Sat Jun 11 00:49:52 2016

Mac: Flip fullscreen low power layer geometry

This is done for the regular content layer, and needs to be done for
the fullscreen low power layer.

BUG= 594460 

Review-Url: https://codereview.chromium.org/2062463002
Cr-Commit-Position: refs/heads/master@{#399337}

[modify] https://crrev.com/515c79f4dcddc2bdcd0fa55af705ce7850e5258d/ui/accelerated_widget_mac/accelerated_widget_mac.h
[modify] https://crrev.com/515c79f4dcddc2bdcd0fa55af705ce7850e5258d/ui/accelerated_widget_mac/accelerated_widget_mac.mm

Awesome! ccameron, can you please add final power usages to show how it improved power usage for Mac?
ccameorn@, is this something that we might want a BattOr Telemetry test for? It seems especially important given how finnicky the constraints on entering the low power mode are.
Yes, we definitely want tests to ensure that this doesn't regress. Let's circle back on this towards the end of the week.
Project Member

Comment 19 by bugdroid1@chromium.org, Jun 15 2016

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

commit 515c79f4dcddc2bdcd0fa55af705ce7850e5258d
Author: ccameron <ccameron@chromium.org>
Date: Sat Jun 11 00:49:52 2016

Mac: Flip fullscreen low power layer geometry

This is done for the regular content layer, and needs to be done for
the fullscreen low power layer.

BUG= 594460 

Review-Url: https://codereview.chromium.org/2062463002
Cr-Commit-Position: refs/heads/master@{#399337}

[modify] https://crrev.com/515c79f4dcddc2bdcd0fa55af705ce7850e5258d/ui/accelerated_widget_mac/accelerated_widget_mac.h
[modify] https://crrev.com/515c79f4dcddc2bdcd0fa55af705ce7850e5258d/ui/accelerated_widget_mac/accelerated_widget_mac.mm

Status: Fixed (was: Assigned)

Sign in to add a comment