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

Issue 1181023004: Move rem handling out of CSSParserValues.cpp (Closed)

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

Description

Move rem handling out of CSSParserValues Move rem handling out of CSSParserValues and instead keep track of whether rem units were applied per ComputedStyle. Then once styleForElement is finished if rem units were used we inform StyleEngine. Add rem-calc-dynamic-scaling.html for using rem with calc(). BUG=499780, 515611 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=199770

Patch Set 1 #

Total comments: 1

Patch Set 2 : Small test #

Patch Set 3 : V3 #

Patch Set 4 : Add test #

Total comments: 2

Patch Set 5 : Patch for landing #

Patch Set 6 : Patch for relanding #

Patch Set 7 : Another attempt #

Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -45 lines) Patch
A + LayoutTests/fast/css/rem-calc-dynamic-scaling.html View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
A + LayoutTests/platform/linux/fast/css/rem-calc-dynamic-scaling-expected.png View 1 2 3 4 Binary file 0 comments Download
A + LayoutTests/platform/linux/fast/css/rem-calc-dynamic-scaling-expected.txt View 1 2 3 4 0 chunks +-1 lines, --1 lines 0 comments Download
A + LayoutTests/platform/mac/fast/css/rem-calc-dynamic-scaling-expected.png View 1 2 3 4 5 Binary file 0 comments Download
A + LayoutTests/platform/mac/fast/css/rem-calc-dynamic-scaling-expected.txt View 1 2 3 4 5 0 chunks +-1 lines, --1 lines 0 comments Download
A + LayoutTests/platform/win-xp/fast/css/rem-calc-dynamic-scaling-expected.png View 1 2 3 4 5 Binary file 0 comments Download
A + LayoutTests/platform/win-xp/fast/css/rem-calc-dynamic-scaling-expected.txt View 1 2 3 4 5 0 chunks +-1 lines, --1 lines 0 comments Download
A + LayoutTests/platform/win/fast/css/rem-calc-dynamic-scaling-expected.png View 1 2 3 4 5 Binary file 0 comments Download
A + LayoutTests/platform/win/fast/css/rem-calc-dynamic-scaling-expected.txt View 1 2 3 4 5 0 chunks +-1 lines, --1 lines 0 comments Download
M Source/core/css/CSSToLengthConversionData.h View 1 2 1 chunk +1 line, -1 line 0 comments Download
M Source/core/css/CSSToLengthConversionData.cpp View 1 2 1 chunk +6 lines, -0 lines 0 comments Download
M Source/core/css/StyleSheetContents.h View 1 2 3 chunks +0 lines, -4 lines 0 comments Download
M Source/core/css/StyleSheetContents.cpp View 1 2 2 chunks +0 lines, -2 lines 0 comments Download
M Source/core/css/parser/CSSParserImpl.cpp View 1 2 1 chunk +1 line, -4 lines 0 comments Download
M Source/core/css/parser/CSSParserValues.h View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M Source/core/css/parser/CSSParserValues.cpp View 1 2 3 4 5 2 chunks +1 line, -4 lines 0 comments Download
M Source/core/css/parser/CSSParserValuesTest.cpp View 1 2 1 chunk +1 line, -2 lines 0 comments Download
M Source/core/css/resolver/StyleResolver.cpp View 1 2 1 chunk +3 lines, -0 lines 0 comments Download
M Source/core/dom/Document.cpp View 1 2 2 chunks +0 lines, -6 lines 0 comments Download
M Source/core/dom/DocumentStyleSheetCollection.cpp View 1 2 3 1 chunk +0 lines, -2 lines 0 comments Download
M Source/core/dom/ShadowTreeStyleSheetCollection.cpp View 1 2 3 1 chunk +0 lines, -1 line 0 comments Download
M Source/core/dom/StyleEngine.cpp View 1 2 3 1 chunk +0 lines, -1 line 0 comments Download
M Source/core/dom/TreeScopeStyleSheetCollection.h View 1 2 3 3 chunks +0 lines, -4 lines 0 comments Download
M Source/core/dom/TreeScopeStyleSheetCollection.cpp View 1 2 3 2 chunks +0 lines, -15 lines 0 comments Download
M Source/core/style/ComputedStyle.h View 1 2 3 4 5 3 chunks +7 lines, -1 line 0 comments Download
M Source/core/style/ComputedStyle.cpp View 1 2 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 30 (12 generated)
rwlbuis
PTAL
5 years, 6 months ago (2015-06-17 00:51:22 UTC) #2
Timothy Loh
https://codereview.chromium.org/1181023004/diff/1/Source/core/css/parser/CSSPropertyParser.cpp File Source/core/css/parser/CSSPropertyParser.cpp (right): https://codereview.chromium.org/1181023004/diff/1/Source/core/css/parser/CSSPropertyParser.cpp#newcode388 Source/core/css/parser/CSSPropertyParser.cpp:388: if (value->unit == CSSPrimitiveValue::CSS_REMS) { Probably better in createPrimitiveNumericValue, ...
5 years, 6 months ago (2015-06-17 01:35:18 UTC) #3
Timothy Loh
Err I don't think this works for calc :/
5 years, 6 months ago (2015-06-17 03:27:53 UTC) #4
rwlbuis
On 2015/06/17 03:27:53, Timothy Loh wrote: > Err I don't think this works for calc ...
5 years, 4 months ago (2015-07-29 21:46:27 UTC) #6
Timothy Loh
lgtm! https://codereview.chromium.org/1181023004/diff/80001/LayoutTests/fast/css/rem-dynamic-scaling.html File LayoutTests/fast/css/rem-dynamic-scaling.html (right): https://codereview.chromium.org/1181023004/diff/80001/LayoutTests/fast/css/rem-dynamic-scaling.html#newcode9 LayoutTests/fast/css/rem-dynamic-scaling.html:9: <div style="font-size:calc(0% + 1rem)"> Uhhh.. put this in ...
5 years, 4 months ago (2015-07-30 01:06:55 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1181023004/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1181023004/120001
5 years, 4 months ago (2015-07-30 13:15:33 UTC) #11
commit-bot: I haz the power
Committed patchset #5 (id:120001) as https://src.chromium.org/viewvc/blink?view=rev&revision=199737
5 years, 4 months ago (2015-07-30 14:38:24 UTC) #12
dmurph
I think this killed MSAN bot. Unfortunately I need to revert manually. https://code.google.com/p/chromium/issues/detail?id=515535
5 years, 4 months ago (2015-07-30 16:27:22 UTC) #14
rwlbuis
On 2015/07/30 16:27:22, dmurph wrote: > I think this killed MSAN bot. Unfortunately I need ...
5 years, 4 months ago (2015-07-30 17:35:03 UTC) #15
blink-reviews
Is there a memory sanatizer trybot? You might need to build with memsan enabled, and ...
5 years, 4 months ago (2015-07-30 17:37:16 UTC) #16
dmurph
Is there a memory sanatizer trybot? You might need to build with memsan enabled, and ...
5 years, 4 months ago (2015-07-30 17:37:44 UTC) #17
rwlbuis
On 2015/07/30 17:37:44, dmurph wrote: > Is there a memory sanatizer trybot? You might need ...
5 years, 4 months ago (2015-07-30 17:47:42 UTC) #18
Lei Zhang
While verifying http://crbug.com/514925, I got new uninit memory errors here: UninitCondition Conditional jump or move ...
5 years, 4 months ago (2015-07-30 18:37:49 UTC) #20
rwlbuis
On 2015/07/30 18:37:49, Lei Zhang wrote: > While verifying http://crbug.com/514925, I got new uninit memory ...
5 years, 4 months ago (2015-07-30 19:12:09 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1181023004/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1181023004/140001
5 years, 4 months ago (2015-07-30 21:36:16 UTC) #24
commit-bot: I haz the power
Try jobs failed on following builders: linux_blink_rel on tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/73253)
5 years, 4 months ago (2015-07-30 22:22:29 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1181023004/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1181023004/160001
5 years, 4 months ago (2015-07-30 23:54:58 UTC) #29
commit-bot: I haz the power
5 years, 4 months ago (2015-07-31 00:59:12 UTC) #30
Message was sent while issue was closed.
Committed patchset #7 (id:160001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=199770

Powered by Google App Engine
This is Rietveld 408576698