Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(445)

Issue 1005523002: WebKitCSSMatrix: Avoid crash resolving relative lengths. (Closed)

Created:
5 years, 9 months ago by rune
Modified:
5 years, 9 months ago
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, dglazkov+blink, ed+blinkwatch_opera.com, rwlbuis
Base URL:
https://chromium.googlesource.com/chromium/blink.git@default-initial-20150312
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

WebKitCSSMatrix: Avoid crash resolving relative lengths. The initial styles created to resolve lengths did not properly construct a font which caused a crash trying to resolve 'ex' and 'ch' units. Call Font::update() to construct a dummy font. There is no document here to the deduce the initial font from. What to do with relative lengths and percentages has been discussed for the standardization of DOMMatrix, and the current draft says only absolute lengths should be allowed [1]. I chose not to try to align with that as WebKitCSSMatrix is a proprietary api. FWIW, viewport relative units were already resolved against an emtpy viewport. [1] http://dev.w3.org/fxtf/geometry/#dommatrix-constructors R=leviw@chromium.org BUG=414145 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=191865

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+18 lines, -2 lines) Patch
A LayoutTests/transforms/cssmatrix-crash.html View 1 chunk +9 lines, -0 lines 0 comments Download
A LayoutTests/transforms/cssmatrix-crash-expected.txt View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/css/CSSMatrix.cpp View 2 chunks +8 lines, -2 lines 0 comments Download

Messages

Total messages: 7 (2 generated)
rune
This is based on top of https://codereview.chromium.org/1001833002/
5 years, 9 months ago (2015-03-12 15:22:10 UTC) #1
rune
I just realized timloh has looked at this before and actually reported the bug.
5 years, 9 months ago (2015-03-13 10:02:32 UTC) #3
leviw_travelin_and_unemployed
LGTM.
5 years, 9 months ago (2015-03-13 18:23:34 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1005523002/1
5 years, 9 months ago (2015-03-13 18:30:15 UTC) #6
commit-bot: I haz the power
5 years, 9 months ago (2015-03-13 20:03:08 UTC) #7
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=191865

Powered by Google App Engine
This is Rietveld 408576698