New issue
Advanced search Search tips

Issue 810214 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

-webkit-box-reflect regression: Missing masked reflection during animation

Project Member Reported by wangxianzhu@chromium.org, Feb 8 2018

Issue description

Chrome Version: M65 and M66

What steps will reproduce the problem?
(1) Open the attached test case

What is the expected result?
During the animation, the lower half of the square with red border should show a masked reflection of the upper half.

What happens instead?
No reflection during animation.

Please ignore the wrong rendering at the end of the animation which is  bug 809402 .

Bisected locally and found that the test starts to crash since r529820, and starts to behaves as described above since r531130. I also reproduced the bug at r529820 with r531130 cherry-picked on it.

I guess the serialization might miss the mask image of the reflection (as a reference filter).
 
reflect.html
546 bytes View Download
Project Member

Comment 2 by sheriffbot@chromium.org, Feb 12 2018

This issue is marked as a release blocker with no milestone associated. Please add an appropriate milestone.

All release blocking issues should have milestones associated to it, so that the issue can tracked and the fixes can be pushed promptly.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: M-65
Labels: M-66
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 14 2018

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

commit 252af33418128e61b71b9afe20eba98ad5ed6a5c
Author: Khushal <khushalsagar@chromium.org>
Date: Wed Feb 14 05:03:09 2018

cc/ipc: Don't fail filter serialization on insufficent buffer.

When serializing a filter on a CompositorFrame, we would fail if the
total size exceeds 8K. Instead compute the size required before
serializing the filter.

Note that there are cases where the PaintOpWriter can not trivially
compute the exact size required, with complex types like PaintRecords.
But those should never be serialized with security constraints imposed
on PaintFilters serialized with CompositorFrames.

R=enne@chromium.org

Bug:  810214 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I2733e3fc36a0b5d8076cb064e5f8de1fc14e33b3
Reviewed-on: https://chromium-review.googlesource.com/909960
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536650}
[modify] https://crrev.com/252af33418128e61b71b9afe20eba98ad5ed6a5c/cc/ipc/cc_param_traits.cc
[modify] https://crrev.com/252af33418128e61b71b9afe20eba98ad5ed6a5c/cc/paint/paint_filter.cc
[modify] https://crrev.com/252af33418128e61b71b9afe20eba98ad5ed6a5c/cc/paint/paint_filter.h
[modify] https://crrev.com/252af33418128e61b71b9afe20eba98ad5ed6a5c/cc/paint/paint_flags.cc
[modify] https://crrev.com/252af33418128e61b71b9afe20eba98ad5ed6a5c/cc/paint/paint_flags.h
[modify] https://crrev.com/252af33418128e61b71b9afe20eba98ad5ed6a5c/cc/paint/paint_op_buffer_unittest.cc
[modify] https://crrev.com/252af33418128e61b71b9afe20eba98ad5ed6a5c/cc/paint/paint_op_reader.cc
[modify] https://crrev.com/252af33418128e61b71b9afe20eba98ad5ed6a5c/cc/paint/paint_op_writer.cc
[modify] https://crrev.com/252af33418128e61b71b9afe20eba98ad5ed6a5c/cc/paint/paint_op_writer.h
[modify] https://crrev.com/252af33418128e61b71b9afe20eba98ad5ed6a5c/cc/paint/paint_shader.cc
[modify] https://crrev.com/252af33418128e61b71b9afe20eba98ad5ed6a5c/cc/paint/paint_shader.h
[modify] https://crrev.com/252af33418128e61b71b9afe20eba98ad5ed6a5c/services/viz/public/cpp/compositing/paint_filter_struct_traits.h

Labels: Merge-Request-65
Project Member

Comment 7 by sheriffbot@chromium.org, Feb 14 2018

Labels: -Merge-Request-65 Merge-Review-65 Hotlist-Merge-Review
This bug requires manual review: M65 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 8 by cmasso@google.com, Feb 14 2018

Please verify in canary

Comment 9 by gov...@chromium.org, Feb 15 2018

How is this change looking in canary?
Works as expected on 66.0.3348.0.
The patch is safe to merge, its constrained to the fixing size allocation when serializing a filter. And without it, cases like the one on this bug will have broken rendering.
Labels: -Merge-Review-65 Merge-Approved-65
Approving merge to M65 branch 3325 based on comments #10 & #11. Please merge ASAP. Thank you.
Project Member

Comment 13 by bugdroid1@chromium.org, Feb 15 2018

Labels: -merge-approved-65 merge-merged-3325
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6bc9982ac0180e0727edcc696eef023580007b92

commit 6bc9982ac0180e0727edcc696eef023580007b92
Author: Khushal <khushalsagar@chromium.org>
Date: Thu Feb 15 22:01:54 2018

cc/ipc: Don't fail filter serialization on insufficent buffer.

When serializing a filter on a CompositorFrame, we would fail if the
total size exceeds 8K. Instead compute the size required before
serializing the filter.

Note that there are cases where the PaintOpWriter can not trivially
compute the exact size required, with complex types like PaintRecords.
But those should never be serialized with security constraints imposed
on PaintFilters serialized with CompositorFrames.

R=enne@chromium.org
TBR=khushalsagar@chromium.org

(cherry picked from commit 252af33418128e61b71b9afe20eba98ad5ed6a5c)

Bug:  810214 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I2733e3fc36a0b5d8076cb064e5f8de1fc14e33b3
Reviewed-on: https://chromium-review.googlesource.com/909960
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#536650}
Reviewed-on: https://chromium-review.googlesource.com/922928
Reviewed-by: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#483}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/6bc9982ac0180e0727edcc696eef023580007b92/cc/ipc/cc_param_traits.cc
[modify] https://crrev.com/6bc9982ac0180e0727edcc696eef023580007b92/cc/paint/paint_filter.cc
[modify] https://crrev.com/6bc9982ac0180e0727edcc696eef023580007b92/cc/paint/paint_filter.h
[modify] https://crrev.com/6bc9982ac0180e0727edcc696eef023580007b92/cc/paint/paint_flags.cc
[modify] https://crrev.com/6bc9982ac0180e0727edcc696eef023580007b92/cc/paint/paint_flags.h
[modify] https://crrev.com/6bc9982ac0180e0727edcc696eef023580007b92/cc/paint/paint_op_buffer_unittest.cc
[modify] https://crrev.com/6bc9982ac0180e0727edcc696eef023580007b92/cc/paint/paint_op_reader.cc
[modify] https://crrev.com/6bc9982ac0180e0727edcc696eef023580007b92/cc/paint/paint_op_writer.cc
[modify] https://crrev.com/6bc9982ac0180e0727edcc696eef023580007b92/cc/paint/paint_op_writer.h
[modify] https://crrev.com/6bc9982ac0180e0727edcc696eef023580007b92/cc/paint/paint_shader.cc
[modify] https://crrev.com/6bc9982ac0180e0727edcc696eef023580007b92/cc/paint/paint_shader.h
[modify] https://crrev.com/6bc9982ac0180e0727edcc696eef023580007b92/services/viz/public/cpp/compositing/paint_filter_struct_traits.h

Status: Fixed (was: Assigned)

Sign in to add a comment