New issue
Advanced search Search tips

Issue 781637 link

Starred by 4 users

Issue metadata

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



Sign in to add a comment

FSLP positioning messed up for some sites

Project Member Reported by sdy@chromium.org, Nov 5 2017

Issue description

Chrome Version: 64.0.3259.0
OS: macOS 10.12.6, 10.13.1

What steps will reproduce the problem?
(1) Load the attached HTML file and follow the instructions.

What is the expected result?
The layout doesn't change when the video controls show and hide

What happens instead?
The video is contained in an element with overflow: hidden. The new FSLP path introduced in r513410 handles this poorly and positions the video — and its black superlayer — almost entirely offscreen.
 
fslp_alignment.mp4
630 KB View Download
fslp_alignment.html
27.0 KB View Download
Please post a fully functional reduced testcase with a video.

Also, please clarify what you mean by "it seriously does change the layout". What exactly
is going wrong? Perhaps attaching a screencast of its behavior will explain.

Comment 2 by ajha@chromium.org, Nov 14 2017

Labels: Needs-Feedback

Comment 3 by sdy@chromium.org, Nov 14 2017

Description: Show this description

Comment 4 by sdy@chromium.org, Nov 14 2017

Labels: -Type-Bug Type-Bug-Regression

Comment 5 by sdy@chromium.org, Nov 14 2017

Sorry chrishtr@, I wrote the original bug description as a quick note-to-self, it wasn't meant for human consumption :). Added a proper testcase and video.

Comment 6 by ajha@chromium.org, Nov 20 2017

This is marked as Beta blocker and M-64 will be branched in ~1 week time. If possible, please plan the fix before branch point.

Thank you!
Project Member

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

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

commit a8745244573b42282c1edbb208b0442498a149c3
Author: Sidney San Martín <sdy@chromium.org>
Date: Mon Nov 20 19:03:55 2017

Use the regular layer tree for fullscreen low power detachment on Mac.

Fixes an issue where fullscreen video wasn't positioned correctly in the
low power tree for some sites, and a longstanding issue where clipped
videos become un-clipped in the fullscreen low power tree.

Made possible by two changes:

- Black and transparent backgrounds are no longer rendered as IOSurfaces
  for color correction. For macOS to consider a layer solid black, it
  needs to actually have a black background color (and no contents).
  Note: I think we can avoid IOSurface replacement by setting a flag or
  two on the CAContext. See https://crrev.com/c/742663.

- Instead of constructing a new layer tree for fullscreen low power,
  just make the root layer black and match its size to the background to
  meet the "opaque black superlayer" requirement for detachment. A nicer
  solution could be to send the browser a color, since it already has a
  solid color superlayer, in which case no special handling for
  fullscreen layer detachment would be needed at all, it would "just
  work". It still needs to be determined whether that would comply with
  the HTML fullscreen spec. See  https://crbug.com/785001 .

Bug:  781637 
Change-Id: I94abb1ade9c58c218296a27f9a7bc3630d4a0c64
Reviewed-on: https://chromium-review.googlesource.com/775139
Commit-Queue: Sidney San Martín <sdy@chromium.org>
Reviewed-by: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517879}
[modify] https://crrev.com/a8745244573b42282c1edbb208b0442498a149c3/ui/accelerated_widget_mac/ca_renderer_layer_tree.h
[modify] https://crrev.com/a8745244573b42282c1edbb208b0442498a149c3/ui/accelerated_widget_mac/ca_renderer_layer_tree.mm

Comment 8 by ajha@chromium.org, Nov 21 2017

Labels: -Needs-Feedback TE-Verified-M64 TE-Verified-64.0.3274.0
Verified the fix on the latest canary(64.0.3274.0) on Mac OS 10.12.6 using fslp_alignment.html and this seems to be working as intended. Hence adding the verified label.  
sdy@, please mark it as 'Fixed' if there is no other pending CL exists.

Thanks..!

Comment 10 by sdy@chromium.org, Nov 22 2017

Status: Fixed (was: Assigned)

Sign in to add a comment