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

Issue 2312293003: [CSSTypedOM] Computed StylePropertyMap use ComputedStyle for Lengths (Closed)

Created:
4 years, 3 months ago by rjwright
Modified:
3 years, 10 months ago
Reviewers:
meade_UTC10, dstockwell
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, blink-reviews-style_chromium.org, chromium-reviews, dglazkov+blink, haraken, meade_UTC10, rwlbuis
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[CSSTypedOM] Computed StylePropertyMap use ComputedStyle for Lengths Currently only working for top, left, bottom, right, width, height. I have removed the old code which returned resolved style values. This means that for now we support fewer properties, and I have marked some layout tests as expected failures. Will work on getting them passing again in follow-up patches. BUG=648495

Patch Set 1 #

Patch Set 2 : Fixed unit test #

Total comments: 3

Patch Set 3 : force style recalc in typed computed style value getter #

Patch Set 4 : Add CSSKeywordValue::create(const AtomicString& keyword) (no exception state). #

Patch Set 5 : presub #

Patch Set 6 : Move logic into ComputedStylePropertyMap and delete old code #

Patch Set 7 : Remove unnecessary switch. Change to using Length refs not pointers. #

Patch Set 8 : For unsupported properties return CSSUnsupportedStyleValue made from resolved style cssText #

Patch Set 9 : only make CSSUnsupportedStyleValue if the CSSValue exists. #

Patch Set 10 : Rebase #

Total comments: 6

Patch Set 11 : cleanup #

Total comments: 2

Patch Set 12 : rename styleValueForLengthProperty to styleValueForLength #

Patch Set 13 : rebase #

Total comments: 10

Patch Set 14 : Eddy's comments #

Patch Set 15 : remove unneeded braces #

Total comments: 2

Patch Set 16 : Change style recalc. Use CSSComputedStyleDec instead of CSSStyleDec. #

Total comments: 1

Patch Set 17 : small cleanup #

Patch Set 18 : Expose methods instead of friending. But now we want to try avoiding using a Style Declaration at a… #

Patch Set 19 : Remove reliance on CSSStyleDeclaration for non-custom case #

Patch Set 20 : rebase #

Patch Set 21 : oops #

Patch Set 22 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+172 lines, -51 lines) Patch
M third_party/WebKit/LayoutTests/TestExpectations View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +7 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/typedcssom/computedstyle/width.html View 1 2 3 4 5 6 1 chunk +32 lines, -19 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSComputedStyleDeclaration.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 3 chunks +7 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/core/css/cssom/CSSCalcLength.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/cssom/CSSCalcLength.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +9 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/cssom/CSSKeywordValue.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/cssom/CSSKeywordValue.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +5 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 3 chunks +15 lines, -7 lines 0 comments Download
M third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +76 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/core/css/cssom/FilteredComputedStylePropertyMap.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 2 chunks +5 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/css/cssom/FilteredComputedStylePropertyMap.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +3 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/css/cssom/FilteredComputedStylePropertyMapTest.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 4 chunks +7 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/core/css/cssom/WindowGetComputedStyle.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +1 line, -3 lines 0 comments Download
M third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 52 (37 generated)
rjwright
Hmm. I guess I should store the ComputedStyle instead of the Node when we make ...
4 years, 3 months ago (2016-09-08 02:09:44 UTC) #2
meade_UTC10
https://codereview.chromium.org/2312293003/diff/20001/third_party/WebKit/Source/core/css/cssom/CSSKeywordValue.h File third_party/WebKit/Source/core/css/cssom/CSSKeywordValue.h (right): https://codereview.chromium.org/2312293003/diff/20001/third_party/WebKit/Source/core/css/cssom/CSSKeywordValue.h#newcode31 third_party/WebKit/Source/core/css/cssom/CSSKeywordValue.h:31: explicit CSSKeywordValue(const AtomicString& keyword) It's fine to create another ...
4 years, 3 months ago (2016-09-09 01:56:27 UTC) #4
rjwright1
4 years, 3 months ago (2016-09-22 00:51:34 UTC) #13
meade_UTC10
https://codereview.chromium.org/2312293003/diff/180001/third_party/WebKit/Source/core/css/cssom/CSSCalcLength.cpp File third_party/WebKit/Source/core/css/cssom/CSSCalcLength.cpp (right): https://codereview.chromium.org/2312293003/diff/180001/third_party/WebKit/Source/core/css/cssom/CSSCalcLength.cpp#newcode77 third_party/WebKit/Source/core/css/cssom/CSSCalcLength.cpp:77: result->set(values.pixels, CSSPrimitiveValue::UnitType::Pixels); Shouldn't fixed lengths become CSSSimpleLengths? (you may ...
4 years, 3 months ago (2016-09-22 05:34:36 UTC) #14
rjwright
https://codereview.chromium.org/2312293003/diff/180001/third_party/WebKit/Source/core/css/cssom/CSSCalcLength.cpp File third_party/WebKit/Source/core/css/cssom/CSSCalcLength.cpp (right): https://codereview.chromium.org/2312293003/diff/180001/third_party/WebKit/Source/core/css/cssom/CSSCalcLength.cpp#newcode77 third_party/WebKit/Source/core/css/cssom/CSSCalcLength.cpp:77: result->set(values.pixels, CSSPrimitiveValue::UnitType::Pixels); On 2016/09/22 05:34:35, Eddy wrote: > Shouldn't ...
4 years, 3 months ago (2016-09-23 04:50:04 UTC) #15
meade_UTC10
lgtm https://codereview.chromium.org/2312293003/diff/200001/third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp File third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp (right): https://codereview.chromium.org/2312293003/diff/200001/third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp#newcode15 third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp:15: CSSStyleValue* styleValueForLengthProperty(const Length& length) nit: just styleValueForLength for ...
4 years, 3 months ago (2016-09-23 05:21:25 UTC) #16
rjwright
https://codereview.chromium.org/2312293003/diff/200001/third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp File third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp (right): https://codereview.chromium.org/2312293003/diff/200001/third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp#newcode15 third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp:15: CSSStyleValue* styleValueForLengthProperty(const Length& length) On 2016/09/23 05:21:25, Eddy wrote: ...
4 years, 3 months ago (2016-09-23 05:39:29 UTC) #17
rjwright
4 years, 2 months ago (2016-09-26 05:29:32 UTC) #29
meade_UTC10
https://codereview.chromium.org/2312293003/diff/240001/third_party/WebKit/Source/core/css/cssom/CSSCalcLength.cpp File third_party/WebKit/Source/core/css/cssom/CSSCalcLength.cpp (right): https://codereview.chromium.org/2312293003/diff/240001/third_party/WebKit/Source/core/css/cssom/CSSCalcLength.cpp#newcode71 third_party/WebKit/Source/core/css/cssom/CSSCalcLength.cpp:71: CSSCalcLength* CSSCalcLength::fromLength(const Length& length) [no action required] btw I'm ...
4 years, 2 months ago (2016-09-27 04:41:39 UTC) #32
rjwright
https://codereview.chromium.org/2312293003/diff/240001/third_party/WebKit/Source/core/css/cssom/CSSCalcLength.h File third_party/WebKit/Source/core/css/cssom/CSSCalcLength.h (right): https://codereview.chromium.org/2312293003/diff/240001/third_party/WebKit/Source/core/css/cssom/CSSCalcLength.h#newcode60 third_party/WebKit/Source/core/css/cssom/CSSCalcLength.h:60: CSSLengthValue* addInternal(const CSSLengthValue* other) override; On 2016/09/27 04:41:39, Eddy ...
4 years, 2 months ago (2016-09-28 01:38:24 UTC) #33
meade_UTC10
https://codereview.chromium.org/2312293003/diff/240001/third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp File third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp (right): https://codereview.chromium.org/2312293003/diff/240001/third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp#newcode38 third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp:38: // Need to look back to it and see ...
4 years, 2 months ago (2016-09-28 01:51:32 UTC) #34
nainar
As per real life discussion with meade@ https://codereview.chromium.org/2312293003/diff/280001/third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp File third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp (right): https://codereview.chromium.org/2312293003/diff/280001/third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp#newcode43 third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp:43: document.updateStyleAndLayoutTreeForNode(m_node); this ...
4 years, 2 months ago (2016-09-28 02:53:26 UTC) #36
rjwright
In the latest patch set (#16) I have added ComputedStylePropertyMap as a friend of CSSComputedStyleDeclaration, ...
4 years, 2 months ago (2016-09-29 04:00:28 UTC) #37
rjwright
https://codereview.chromium.org/2312293003/diff/300001/third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.h File third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.h (right): https://codereview.chromium.org/2312293003/diff/300001/third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.h#newcode19 third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.h:19: static ComputedStylePropertyMap* create(Node* node, const String& pseudoElement) I think ...
4 years, 2 months ago (2016-09-29 04:08:17 UTC) #38
rjwright
4 years, 2 months ago (2016-09-29 06:15:01 UTC) #39
I still need to double-check my style recalc stuff, as per code comments. I also
really should test some pseudoelement stuff.

Powered by Google App Engine
This is Rietveld 408576698