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

Issue 752382 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 2
Type: Bug-Regression


Participants' hotlists:
Hotlist-1


Sign in to add a comment

transform matrix3d crashes chrome

Reported by duscin...@gmail.com, Aug 4 2017

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.90 Safari/537.36

Steps to reproduce the problem:
1. open the HTML attached
2. chrome crashes

What is the expected behavior?
no crash

What went wrong?
Once the root div in the attached html is styled with the given transform matrix3d, the browser just crashes.

Crashed report ID: a55f9d2c-5204-404a-a15f-40ddc37faccf

How much crashed? Just one tab

Is it a problem with a plugin? No 

Did this work before? Yes 59.x 64-bit on windows 10

Chrome version: 60.0.3112.90  Channel: stable
OS Version: 10.0
Flash Version:
 
crash.html
2.2 KB View Download
Components: Internals>Compositing Blink>Compositing>Transform3D
Labels: ReleaseBlock-Stable M-60 OS-Linux OS-Mac
Owner: vmp...@chromium.org
Status: Assigned (was: Unconfirmed)
duscin.fz@, thank you for the report.

Here is the narrow bisect result:
https://chromium.googlesource.com/chromium/src/+log/27d4a2f28593194c2642514f9f28457d14fab406..3e4e1942c0b44ce8018f9936bfc1d0dbd3344171

vmpstr@, can you please look into this change (https://chromium.googlesource.com/chromium/src/+/dc5e1afbd1f6b01e44b58ee16f4c556d2ceaa04c) if possible?

Thank you!
Labels: hasbisect
 Issue 752959  has been merged into this issue.
Status: Started (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 8 2017

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

commit 4b44a058abace01b33f1b75798274dd07db13ddb
Author: Vladimir Levin <vmpstr@chromium.org>
Date: Tue Aug 08 19:29:07 2017

cc: Fix for large scale case when approximate perspective scales.

This patch caps the scale we would use when approximating perspective
transform scales to be such that the whole layer can be covered by at
most 5x5 tiles.

R=enne@chromium.org, chrishtr@chromium.org

Bug:  752382 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ie377d1c24f45b9448a39626699bd2775daa752c1
Reviewed-on: https://chromium-review.googlesource.com/604933
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Commit-Queue: Vladimir Levin <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492726}
[modify] https://crrev.com/4b44a058abace01b33f1b75798274dd07db13ddb/cc/layers/layer_impl.cc
[modify] https://crrev.com/4b44a058abace01b33f1b75798274dd07db13ddb/cc/layers/layer_impl_unittest.cc

Status: Fixed (was: Started)
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-60; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-60 label, otherwise remove Merge-TBD label. Thanks.
Cc: chrishtr@chromium.org enne@chromium.org
Labels: -Merge-TBD
m60 is in stable, I don't think a merge is warranted for that unless there is a significant amount of problems resulting from this.

FWIW, the patch is simple enough to be merged, but I think we prefer not to do so since it's not a severe enough regression (imo).

+enne, +chrishtr for opinions

Comment 9 by enne@chromium.org, Aug 8 2017

Does crash suggest that this is a frequent occurrence?
Labels: -M-60 M-61
Status: Assigned (was: Fixed)
I agree that we can skip M60. Marking as blocking for M61 though, we should try
to merge to that branch.
Labels: Merge-Request-61
I can't find the referenced crash id. Locally this isn't so much a crash as a hang in the sense that the compositor is doing work, there's just too much work to do.

Adding merge request 61 as per #10.
Before we approve merge to M61, please answer followings:
* Is the change well baked/verified in Canary, having enough automation tests coverage and safe to merge to M61?
* Any other important details to justify the merge.

Please note M61 is already in Beta, so merge bar is very high. Thank you.
* Is the change well baked/verified in Canary, having enough automation tests coverage and safe to merge to M61?

- The change is simple and it has unittests to confirm that it works (as well as manual verification with the test case provided here).
- It hasn't hit canary yet, so we can wait a couple of days if that's preferred.

* Any other important details to justify the merge.

- It's a regression in M60 causing hangs on pages with certain perspective transforms.
- The fix is isolated and simple, but it does affect all platforms

Thank you  vmpstr@.

I see, change landed 2 hrs back so not in canary yet. 
Lets wait until change is baked in Canary for few days. Please update the bug with Canary result. If canary result looks good, I will approve the merge.
Project Member

Comment 15 by sheriffbot@chromium.org, Aug 9 2017

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

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
[Bulk Edit]
URGENT - PTAL.
Your bug is labelled as M61 Stable ReleaseBlock, pls make sure to land the fix and get it merged into the release branch ASAP.

Know that this issue shouldn't block the release?  Remove the ReleaseBlock-Stable label.

Thank you.

How is the change looking in Canary?
I haven't seen any issues arising from this patch. So, I think it's safe to merge.
Labels: -Merge-Review-61 Merge-Approved-61
Approving merge to M61 branch 3163 based on comment #13 and #18. Please merge ASAP. Thank you.
Project Member

Comment 20 by bugdroid1@chromium.org, Aug 10 2017

Labels: -merge-approved-61 merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e3bd735055d0705d292ea075a73a7765f55923e7

commit e3bd735055d0705d292ea075a73a7765f55923e7
Author: Vladimir Levin <vmpstr@chromium.org>
Date: Thu Aug 10 21:11:25 2017

cc: Fix for large scale case when approximate perspective scales.

This patch caps the scale we would use when approximating perspective
transform scales to be such that the whole layer can be covered by at
most 5x5 tiles.

R=chrishtr@chromium.org, enne@chromium.org
TBR=vmpstr@chromium.org

(cherry picked from commit 4b44a058abace01b33f1b75798274dd07db13ddb)

Bug:  752382 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ie377d1c24f45b9448a39626699bd2775daa752c1
Reviewed-on: https://chromium-review.googlesource.com/604933
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Commit-Queue: Vladimir Levin <vmpstr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#492726}
Reviewed-on: https://chromium-review.googlesource.com/611135
Reviewed-by: Vladimir Levin <vmpstr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#461}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/e3bd735055d0705d292ea075a73a7765f55923e7/cc/layers/layer_impl.cc
[modify] https://crrev.com/e3bd735055d0705d292ea075a73a7765f55923e7/cc/layers/layer_impl_unittest.cc

Status: Fixed (was: Assigned)
Cc: vmp...@chromium.org hdodda@chromium.org
 Issue 754163  has been merged into this issue.

Sign in to add a comment