Regression: Bouncing of 'Cast' overlay is observed on changing the Cast modes.
Reported by
dchau...@etouch.net,
Feb 12 2018
|
|||||||||
Issue descriptionChrome Version: 66.0.3344.0 (Official Build) Revision 95f7d22cfff65db8271ee240486238f7d204586b-refs/heads/master@{#535592} 32/64-bit. OS: Win(7,8,8.1,10), Mac(10.12.6, 10.13.1,10.13.4), Linux(14.04 LTS). What steps will reproduce the problem? 1. Launch Chrome, go to chrome://downloads page, right click on it to open context menu and select 'Cast' option. 2. Click on 'OK, GOT IT' button and click on down arrow at 'Cast to' header to view cast modes list. 3. Now click on 'Cast tab' and 'Cast desktop' options and observe the Cast overlay. (Refer attached video) Bouncing of Cast overlay is seen when clicking on 'Cast tab' and 'Cast desktop' options. Cast overlay should not bounce when changing the cast modes. This is a regression issue, broken in M-65 series, below is manual regression range. Good build: 65.0.3317.0 (Revision: 528120) Bad build: 65.0.3318.0 (Revision: 528541) Using the per-revision bisect providing the bisect results: You are probably looking for a change made after 528476 (known good), but no later than 528477 (first known bad). CHANGELOG URL: The script might not always return single CL as suspect as some perf builds might get missing due to failure. https://chromium.googlesource.com/chromium/src/+log/2140d23a8add3f894fd9c94f5916e92974feb435..966d29e791e4811124037ae509754c4277391fd1 Suspecting: https://chromium.googlesource.com/chromium/src/+/966d29e791e4811124037ae509754c4277391fd1 @fsamuel: Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to concern owner. NOTE: This issue is also reproducible on M-65 Beta (build # 65.0.3325.51), M-66 Dev(build # 66.0.3343.3) Kindly review the attached screen-cast for reference. Thank you.
,
Feb 12 2018
I think what's going on here is we're not throttling auto-resizes. We issue them to the browser as fast as we can when surface sync is on. Maybe what I'll try doing is adding a pending flag.
,
Feb 12 2018
,
Feb 12 2018
,
Feb 12 2018
The right solution to this is being implemented by cblume@ but it won't be ready for M65. I have a short term workaround we can use in M65 and maybe M66.
,
Feb 13 2018
After conversation with fsamuel@ the plan is to merge a fix into M65. Marking RBS.
,
Feb 15 2018
M65 Stable promotion is coming VERY soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge into the release branch ASAP. Thank you.
,
Feb 15 2018
CL is in the CQ: https://chromium-review.googlesource.com/c/chromium/src/+/920341/5
,
Feb 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1ee6fb5c206f8f63176196bfc31aa40721b734c7 commit 1ee6fb5c206f8f63176196bfc31aa40721b734c7 Author: Fady Samuel <fsamuel@chromium.org> Date: Fri Feb 16 03:27:46 2018 Surface synchronization: Throttle auto-resize renderer side This is a short term fix until child-allocated LocalSurfaceIDs are implemented for auto-resize. Prior to surface synchronization, child renderers would allocate LocalSurfaceIds for auto-resize. When the renderer receives a DidReceiveCompositorFrameAck message then it knows the compositor has received the frame, and informs the browser process which then resizes the associated aura window to match the frame generated by the child. This ensured that the aura window would only be resized to a size that had a surface. With surface synchronization, every auto-resize resulted in a request for a LocalSurfaceId from the parent. The parent would immediately resize the aura window and embed the newly allocated LocalSurfaceId and then tell the child about it. In the meantime, the child may auto-resize again. Surface synchronization would block the parent on that LocalSurfaceId that never resolves until the child makes another request. After the deadline passes, the user sees flicker. This CL does not address the root cause of the problem (allocating IDs for surfaces that are never created), but it does significantly reduce the problem. Instead of immediately informing the browser when an auto-resize happens in the renderer. The ACK is posted as a task on the renderer's queue. The renderer may auto-resize multiple times until that task is handled, and so we effectively coalesce resize requests before making a request to the browser. This significantly reduces flicker. This is not the final state of auto-resize, though. In the future, the renderer will immediately allocate a LocalSurfaceId and submit a CompositorFrame, and then tell the browser about it to embed it. Occasionally, there will be a conflict between browser and renderer changes, but this should be very rare. Bug: 812127 , 811181 , 672962 Change-Id: I08839c4b498b68ba06bb5d818337738e191cf9be Reviewed-on: https://chromium-review.googlesource.com/920341 Commit-Queue: Fady Samuel <fsamuel@chromium.org> Reviewed-by: Antoine Labour <piman@chromium.org> Cr-Commit-Position: refs/heads/master@{#537173} [modify] https://crrev.com/1ee6fb5c206f8f63176196bfc31aa40721b734c7/content/renderer/render_view_impl.cc [modify] https://crrev.com/1ee6fb5c206f8f63176196bfc31aa40721b734c7/content/renderer/render_widget.cc [modify] https://crrev.com/1ee6fb5c206f8f63176196bfc31aa40721b734c7/content/renderer/render_widget.h
,
Feb 19 2018
Update:- Re-tested this issue on Windows (7,8,10) and Linux(14.04 LTS) machines using latest Chrome canary build# 66.0.3350.0 and fix is working as expected. Unable to check the Mac behavior due to issue 812173 Please find the attached screen-cast for reference. Thanks..!
,
Feb 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e8769aa4fe6812af29b772a6d7f456ab15fe3cd1 commit e8769aa4fe6812af29b772a6d7f456ab15fe3cd1 Author: Fady Samuel <fsamuel@chromium.org> Date: Mon Feb 19 20:18:12 2018 Surface synchronization: Throttle auto-resize renderer side This is a short term fix until child-allocated LocalSurfaceIDs are implemented for auto-resize. Prior to surface synchronization, child renderers would allocate LocalSurfaceIds for auto-resize. When the renderer receives a DidReceiveCompositorFrameAck message then it knows the compositor has received the frame, and informs the browser process which then resizes the associated aura window to match the frame generated by the child. This ensured that the aura window would only be resized to a size that had a surface. With surface synchronization, every auto-resize resulted in a request for a LocalSurfaceId from the parent. The parent would immediately resize the aura window and embed the newly allocated LocalSurfaceId and then tell the child about it. In the meantime, the child may auto-resize again. Surface synchronization would block the parent on that LocalSurfaceId that never resolves until the child makes another request. After the deadline passes, the user sees flicker. This CL does not address the root cause of the problem (allocating IDs for surfaces that are never created), but it does significantly reduce the problem. Instead of immediately informing the browser when an auto-resize happens in the renderer. The ACK is posted as a task on the renderer's queue. The renderer may auto-resize multiple times until that task is handled, and so we effectively coalesce resize requests before making a request to the browser. This significantly reduces flicker. This is not the final state of auto-resize, though. In the future, the renderer will immediately allocate a LocalSurfaceId and submit a CompositorFrame, and then tell the browser about it to embed it. Occasionally, there will be a conflict between browser and renderer changes, but this should be very rare. TBR=fsamuel@chromium.org (cherry picked from commit 1ee6fb5c206f8f63176196bfc31aa40721b734c7) Bug: 812127 , 811181 , 672962 Change-Id: I08839c4b498b68ba06bb5d818337738e191cf9be Reviewed-on: https://chromium-review.googlesource.com/920341 Commit-Queue: Fady Samuel <fsamuel@chromium.org> Reviewed-by: Antoine Labour <piman@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#537173} Reviewed-on: https://chromium-review.googlesource.com/925542 Reviewed-by: Fady Samuel <fsamuel@chromium.org> Cr-Commit-Position: refs/branch-heads/3325@{#500} Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369} [modify] https://crrev.com/e8769aa4fe6812af29b772a6d7f456ab15fe3cd1/content/renderer/render_view_impl.cc [modify] https://crrev.com/e8769aa4fe6812af29b772a6d7f456ab15fe3cd1/content/renderer/render_widget.cc [modify] https://crrev.com/e8769aa4fe6812af29b772a6d7f456ab15fe3cd1/content/renderer/render_widget.h
,
Feb 19 2018
This has been fixed and merged back into M65 as of this: https://chromium.googlesource.com/chromium/src/+/e8769aa4fe6812af29b772a6d7f456ab15fe3cd1 Marking as FIXED.
,
Feb 21 2018
Update:- Re-tested this issue on Windows (7,8,8.1,10), Mac(10.12.6, 10.13.1, 10.13.4) and Linux(14.04 LTS) machines using latest Chrome Beta build# 65.0.3325.88 and fix is working as expected.. Hence adding TE Verified labels. Please find the attached screen-cast for reference. Thanks..!
,
Feb 23 2018
Update:- Re-tested this issue on Windows (7,8,8.1,10), Mac(10.12.6, 10.13.1, 10.13.4) and Linux(14.04 LTS) machines using latest Chrome Canary build# 66.0.3353.0 and fix is working as expected.. Hence adding TE-Verified labels. Please find the attached screen-cast for reference. Thanks..!
,
Feb 28 2018
Issue 794463 has been merged into this issue. |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by dchau...@etouch.net
, Feb 12 2018