Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2174703002:
Organize public ComputedStyle methods by property (Closed)
DescriptionOrganize public ComputedStyle methods by property
Organized ComputedStyle public methods by property. This is part of a
larger effort to clean up ComputedStyle and ultimately generate it. Also
added a comment to the top of ComputedStyle explaining the new style, as
well as TODOs to classes used by ComputedStyle to merge their logic into
ComputedStyle.
BUG=628043
Committed: https://crrev.com/a64e4958441c43f0bafc5c71d54fd259589598b1
Cr-Commit-Position: refs/heads/master@{#408603}
Patch Set 1 #Patch Set 2 : Done #
Total comments: 339
Patch Set 3 : Review feedback & rebase #Patch Set 4 : Rebase #Patch Set 5 : Rebase #Dependent Patchsets: Messages
Total messages: 21 (12 generated)
Description was changed from ========== Organizing ComputedStyle methods by Property (WIP) BUG=628043 ========== to ========== Organize public ComputedStyle methods by property Organized ComputedStyle public methods by property. This is part of a larger effort to clean up ComputedStyle and ultimately generate it. Also added a comment to the top of ComputedStyle explaining the new style, as well as TODOs to classes used by ComputedStyle to merge their logic into ComputedStyle. BUG=628043 ==========
sashab@chromium.org changed reviewers: + nainar@chromium.org
I am so sorry for bikeshedding! https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/style/ComputedStyle.h (right): https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:143: protected: remove inserted new line here https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:430: // align-content (aka -webkit-align-content) insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:439: // justify-content (aka -webkit-justify-content) insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:506: // border-image-repeat why mention it if there are no methods here also new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:507: // border-image-slice insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:510: // border-image-source insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:514: // border-image-width insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:517: // border-image-outset insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:523: // border-top-width insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:526: // border-bottom-width insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:529: // border-left-width insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:532: // border-right-width insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:541: // border-right-style insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:544: // border-left-style insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:547: // border-bottom-style insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:554: // border-right-color insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:556: // border-top-color insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:558: // border-bottom-color insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:563: // border-top-left-radius (aka -webkit-border-top-left-radius) insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:566: // border-top-right-radius (aka -webkit-border-top-right-radius) insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:569: // border-bottom-left-radius (aka -webkit-border-bottom-left-radius) insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:572: // border-bottom-right-radius (aka -webkit-border-bottom-right-radius) insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:578: // left insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:581: // right insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:584: // top insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:587: // bottom insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:603: void setClear(EClear v) { m_nonInheritedData.m_clear = v; } reverse order of clear() and setClear() https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:611: // break-before (shorthand for page-break-before and -webkit-column-break-before) insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:615: // break-inside (shorthand for page-break-inside and -webkit-column-break-inside) insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:634: // column-fill insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:638: // column-gap (aka -webkit-column-gap) insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:643: // column-rule (aka -webkit-column-rule) longhands=column-rule-width;column-rule-style;column-rule-color insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:646: // column-rule-style (aka -webkit-column-rule-style) type_name=EBorderStyle, initial=initialBorderStyle insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:649: // column-rule-width (aka -webkit-column-rule-width) insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:653: // column-span (aka -webkit-column-span) insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:655: ColumnSpan getColumnSpan() const { return static_cast<ColumnSpan>(m_rareNonInheritedData->m_multiCol->m_columnSpan); } submit an earlier patch that changes this to columnSpan()? https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:657: // column-width (aka -webkit-column-width) insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:658: void setColumnWidth(float f) { SET_NESTED_VAR(m_rareNonInheritedData, m_multiCol, m_autoWidth, false); SET_NESTED_VAR(m_rareNonInheritedData, m_multiCol, m_width, f); } change order of setColumnWidth() and columnWidth() https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:665: void setContain(Containment contain) { SET_VAR(m_rareNonInheritedData, m_contain, contain); } reverse order of contain() and setContain() https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:669: bool hasContent() const { return contentData(); } bring to end - add all misc functions to end https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:685: // flex-direction (aka -webkit-flex-direction) insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:689: // flex-grow (aka -webkit-flex-grow) insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:693: // flex-shrink (aka -webkit-flex-shrink) insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:697: // flex-wrap (aka -webkit-flex-wrap) insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:701: // -webkit-box-flex type_name=float insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:705: // -webkit-box-flex-group type_name=unsigned int insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:709: // -webkit-box-align insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:714: // -webkit-box-decoration-break insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:718: // -webkit-box-lines insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:722: // -webkit-box-ordinal-group insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:726: // -webkit-box-orient insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:730: // -webkit-box-pack insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:734: // -webkit-box-reflect insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:758: // grid-auto-columns insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:762: // grid-auto-flow insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:765: // grid-auto-rows insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:769: // grid-column-gap insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:774: // grid-column-start insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:778: // grid-column-end insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:782: // grid-row-gap insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:786: // grid-row-start insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:790: // grid-row-end insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:790: // grid-row-end insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:794: // grid-template-areas why have this here when there are no methods? https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:799: // grid-template-rows insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:807: // justify-items insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:814: // width insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:817: // height insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:822: // max-width insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:825: // max-height insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:830: // min-width insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:833: // min-height insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:854: // margin-top insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:857: // margin-bottom insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:860: // margin-left insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:863: // margin-right insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:871: // -webkit-margin-after-collapse (aka -webkit-margin-bottom-collapse) insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:885: // motion-offset insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:889: // motion-rotation insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:896: ObjectFit getObjectFit() const { return static_cast<ObjectFit>(m_rareNonInheritedData->m_objectFit); } getObjectFit() -> objectFit() in an earlier patch https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:898: // object-position insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:911: // We restrict the smallest value to int min + 2 because we use int min and int min + 1 as special values in a hash set. delete line https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:918: static OutlineIsAuto initialOutlineStyleIsAuto() { return OutlineIsAutoOff; } move to after setOutlineStyle() https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:923: // outline-width insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:926: int outlineWidth() const move to before setOutlineWidth() https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:947: // overflow-x insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:951: // overflow-y insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:958: // padding-bottom insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:961: // padding-left insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:964: // padding-right insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:967: // padding-top insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:975: // perspective-origin (aka -webkit-perspective-origin) insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:979: // -webkit-perspective-origin-x insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:983: // -webkit-perspective-origin-y insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1003: // transform-origin (aka -webkit-transform-origin) insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1007: // transform-style (aka -webkit-transform-style) insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1011: // -webkit-transform-origin-x insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1015: // -webkit-transform-origin-y insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1019: // -webkit-transform-origin-z insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1029: // rotate insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1033: // scale insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1041: ScrollBehavior getScrollBehavior() const { return static_cast<ScrollBehavior>(m_rareNonInheritedData->m_scrollBehavior); } rename getScrollBehavior() -> scrollBehavior() in an earlier patch https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1043: // scroll-snap-coordinate insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1047: // scroll-snap-destination insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1051: // scroll-snap-points-x insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1055: // scroll-snap-points-y insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1059: // scroll-snap-type insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1061: ScrollSnapType getScrollSnapType() const { return static_cast<ScrollSnapType>(m_rareNonInheritedData->m_scrollSnapType); } rename getScrollSnapType() -> scrollSnapType() in an earlier patch https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1072: // shape-margin (aka -webkit-shape-margin) insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1076: // shape-outside (aka -webkit-shape-outside) insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1084: ShapeValue* shapeOutside() const { return m_rareNonInheritedData->m_shapeOutside.get(); } put before setShapeOutside() https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1088: PageSizeType getPageSizeType() const { return static_cast<PageSizeType>(m_rareNonInheritedData->m_pageSizeType); } rename getPageSizeType() -> pageSizeType() in an earlier patch https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1099: TextDecoration getTextDecoration() const { return static_cast<TextDecoration>(m_visual->textDecoration); } rename getTextDecoration() -> textDecoration() in an earlier patch https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1101: // text-decoration-color insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1103: // text-decoration-style insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1105: TextDecorationStyle getTextDecorationStyle() const { return static_cast<TextDecorationStyle>(m_rareNonInheritedData->m_textDecorationStyle); } rename getTextDecorationStyle() -> textDecorationStyle() in an earlier patch https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1107: // text-underline-position runtime_flag=CSS3TextDecorations, inherited, type_name=TextUnderlinePosition insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1110: TextUnderlinePosition getTextUnderlinePosition() const { return static_cast<TextUnderlinePosition>(m_rareInheritedData->m_textUnderlinePosition); } a) switch order with setTextUnderlinePosition b) getTextUnderlinePosition() -> textUnderlinePosition() in an earlier patch https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1114: TextOverflow getTextOverflow() const { return static_cast<TextOverflow>(m_rareNonInheritedData->textOverflow); } rename getTextOverflow() -> textOverflow() in an earlier patch https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1119: TouchAction getTouchAction() const { return static_cast<TouchAction>(m_rareNonInheritedData->m_touchAction); } rename getTouchAction() -> touchAction() in an earlier patch https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1130: const Length& getVerticalAlignLength() const { return m_box->verticalAlign(); } rename getVerticalAlignLength() -> verticalAlignLength() in an earlier patch https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1137: // wrap-through insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1164: DraggableRegionMode getDraggableRegionMode() const { return m_rareNonInheritedData->m_draggableRegionMode; } rename getDraggableRegionMode() -> draggableRegionMode() in an earlier patch https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1189: // -webkit-mask-box-image-repeat why have this here if there are no methods? https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1196: // -webkit-mask-box-image-source insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1200: // -webkit-mask-box-image-width insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1250: Hyphens getHyphens() const { return static_cast<Hyphens>(m_rareInheritedData->hyphens); } rename getHyphens() -> hyphens() in an earlier patch https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1252: // -webkit-hyphenate-character insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1267: // list-style-position insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1271: // list-style-image insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1280: // widows insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1314: TabSize getTabSize() const { return m_rareInheritedData->m_tabSize; } rename getTabSize() ->tabSize() in an earlier patch https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1321: // text-align-last insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1323: TextAlignLast getTextAlignLast() const { return static_cast<TextAlignLast>(m_rareInheritedData->m_textAlignLast); } rename getTextAlignLast() ->textAlignLast() in an earlier patch https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1328: TextCombine getTextCombine() const { return static_cast<TextCombine>(m_rareInheritedData->m_textCombine); } rename getTextCombine() -> textCombine() in an earlier patch https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1336: TextIndentLine getTextIndentLine() const { return static_cast<TextIndentLine>(m_rareInheritedData->m_textIndentLine); } rename getTextIndentLine() -> textIndentLine() in an earlier patch https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1337: TextIndentType getTextIndentType() const { return static_cast<TextIndentType>(m_rareInheritedData->m_textIndentType); } rename getTextIndentType() -> textIndentType() in an earlier patch https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1344: TextJustify getTextJustify() const { return static_cast<TextJustify>(m_rareInheritedData->m_textJustify); } rename getTextJustify() -> textJustify() in an earlier patch https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1349: TextOrientation getTextOrientation() const { return static_cast<TextOrientation>(m_rareInheritedData->m_textOrientation); } rename getTextOrientation() -> textOrientation() in an earlier patch https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1369: void setVisibility(EVisibility v) { m_inheritedData.m_visibility = v; } put after visibility() https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1384: LineBreak getLineBreak() const { return static_cast<LineBreak>(m_rareInheritedData->lineBreak); } rename getLineBreak() -> lineBreak() in an earlier patch https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1389: WritingMode getWritingMode() const { return static_cast<WritingMode>(m_inheritedData.m_writingMode); } rename getWritingMode() -> writingMode() in an earlier patch https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1403: TextEmphasisFill getTextEmphasisFill() const { return static_cast<TextEmphasisFill>(m_rareInheritedData->textEmphasisFill); } rename all getTextEmphasis*() -> textEmphasis*() in an earlier patch https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1410: // -webkit-text-emphasis-color (aka -epub-text-emphasis-color) insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1412: // -webkit-text-emphasis-position insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1414: TextEmphasisPosition getTextEmphasisPosition() const { return static_cast<TextEmphasisPosition>(m_rareInheritedData->textEmphasisPosition); } rename getTextEmphasisPosition() -> textEmphasisPosition() in an earlier patch https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1434: PrintColorAdjust getPrintColorAdjust() const { return static_cast<PrintColorAdjust>(m_inheritedData.m_printColorAdjust); } rename getPrintColorAdjust() -> printColorAdjust() in an earlier patch https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1445: RubyPosition getRubyPosition() const { return static_cast<RubyPosition>(m_rareInheritedData->m_rubyPosition); } a) put this before the setter b) rename getRubyPosition() -> rubyPosition() in an earlier patch https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1450: Color tapHighlightColor() const { return m_rareInheritedData->tapHighlightColor; } put this before the setter https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1462: // -webkit-text-stroke-width insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1471: // -webkit-user-modify insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1475: // -webkit-user-select insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1490: // font-size-adjust insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1493: // font-weight insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1497: bool setFontDescription(const FontDescription&); insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1504: static float initialLetterWordSpacing() { return 0.0f; } should this be initialLetterSpacing() ? https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1507: // word-spacing insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1552: // flood-color insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1554: // lighting-color insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1568: // stroke-dasharray insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1571: // stroke-dashoffset insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1574: // stroke-miterlimit insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1577: // stroke-opacity insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1580: // stroke-width insert new line before https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1624: void setHasViewportUnits(bool hasViewportUnits = true) const { m_nonInheritedData.m_hasViewportUnits = hasViewportUnits; } reverse order of getter and setter https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1627: void setHasRemUnits() const { m_nonInheritedData.m_hasRemUnits = true; } reverse order of getter and setter https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1634: remove new line https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1656: bool hasExplicitlyInheritedProperties() const { return m_nonInheritedData.m_explicitInheritance; } reverse order with setter
The CQ bit was checked by sashab@chromium.org to run a CQ dry run
Thanks Naina :) In future no need to comment on individual lines, just say the overall feedback (e.g. 'put a newline before each property') and I can do it :) In this case I intentionally hadn't done the newlines thing, but now that you put it that way I think it is better with the newlines :) Also the reorder comments were really helpful, thanks!! As for the renames, I added TODOs but then saw this patch: https://chromium.googlesource.com/chromium/src/+/02dda935229d062978dc75a80a22... The functions were renamed to getFoo() if there is already a type named Foo, and changing to chromium codestyle (calling it Foo()) would cause a name conflict. I think the way to fix this is to have better scoped enums (so the type isn't called Foo, it's called FooType or something, but we can do more of that once ComputedStyle is generated). Thanks again!! :) Sasha https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/style/ComputedStyle.h (right): https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:143: protected: On 2016/07/25 at 07:43:27, nainar wrote: > remove inserted new line here Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:430: // align-content (aka -webkit-align-content) On 2016/07/25 at 07:43:27, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:439: // justify-content (aka -webkit-justify-content) On 2016/07/25 at 07:43:22, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:506: // border-image-repeat On 2016/07/25 at 07:43:20, nainar wrote: > why mention it if there are no methods here > > also new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:507: // border-image-slice On 2016/07/25 at 07:43:26, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:510: // border-image-source On 2016/07/25 at 07:43:23, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:514: // border-image-width On 2016/07/25 at 07:43:19, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:517: // border-image-outset On 2016/07/25 at 07:43:26, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:523: // border-top-width On 2016/07/25 at 07:43:19, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:526: // border-bottom-width On 2016/07/25 at 07:43:19, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:529: // border-left-width On 2016/07/25 at 07:43:22, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:532: // border-right-width On 2016/07/25 at 07:43:26, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:541: // border-right-style On 2016/07/25 at 07:43:21, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:544: // border-left-style On 2016/07/25 at 07:43:24, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:547: // border-bottom-style On 2016/07/25 at 07:43:23, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:554: // border-right-color On 2016/07/25 at 07:43:21, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:556: // border-top-color On 2016/07/25 at 07:43:20, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:558: // border-bottom-color On 2016/07/25 at 07:43:25, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:563: // border-top-left-radius (aka -webkit-border-top-left-radius) On 2016/07/25 at 07:43:27, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:566: // border-top-right-radius (aka -webkit-border-top-right-radius) On 2016/07/25 at 07:43:23, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:566: // border-top-right-radius (aka -webkit-border-top-right-radius) On 2016/07/25 at 07:43:23, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:569: // border-bottom-left-radius (aka -webkit-border-bottom-left-radius) On 2016/07/25 at 07:43:24, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:572: // border-bottom-right-radius (aka -webkit-border-bottom-right-radius) On 2016/07/25 at 07:43:20, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:578: // left On 2016/07/25 at 07:43:19, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:581: // right On 2016/07/25 at 07:43:20, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:584: // top On 2016/07/25 at 07:43:24, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:587: // bottom On 2016/07/25 at 07:43:20, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:611: // break-before (shorthand for page-break-before and -webkit-column-break-before) On 2016/07/25 at 07:43:21, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:615: // break-inside (shorthand for page-break-inside and -webkit-column-break-inside) On 2016/07/25 at 07:43:18, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:634: // column-fill On 2016/07/25 at 07:43:27, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:638: // column-gap (aka -webkit-column-gap) On 2016/07/25 at 07:43:18, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:643: // column-rule (aka -webkit-column-rule) longhands=column-rule-width;column-rule-style;column-rule-color On 2016/07/25 at 07:43:24, nainar wrote: > insert new line before Done, also removed this line https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:646: // column-rule-style (aka -webkit-column-rule-style) type_name=EBorderStyle, initial=initialBorderStyle On 2016/07/25 at 07:43:24, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:649: // column-rule-width (aka -webkit-column-rule-width) On 2016/07/25 at 07:43:23, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:653: // column-span (aka -webkit-column-span) On 2016/07/25 at 07:43:27, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:655: ColumnSpan getColumnSpan() const { return static_cast<ColumnSpan>(m_rareNonInheritedData->m_multiCol->m_columnSpan); } On 2016/07/25 at 07:43:24, nainar wrote: > submit an earlier patch that changes this to columnSpan()? There are lots of these, will do these in a later patch. Added a todo. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:657: // column-width (aka -webkit-column-width) On 2016/07/25 at 07:43:19, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:658: void setColumnWidth(float f) { SET_NESTED_VAR(m_rareNonInheritedData, m_multiCol, m_autoWidth, false); SET_NESTED_VAR(m_rareNonInheritedData, m_multiCol, m_width, f); } On 2016/07/25 at 07:43:24, nainar wrote: > change order of setColumnWidth() and columnWidth() Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:665: void setContain(Containment contain) { SET_VAR(m_rareNonInheritedData, m_contain, contain); } On 2016/07/25 at 07:43:26, nainar wrote: > reverse order of contain() and setContain() Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:669: bool hasContent() const { return contentData(); } On 2016/07/25 at 07:43:18, nainar wrote: > bring to end - add all misc functions to end Done!! Ty so much https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:685: // flex-direction (aka -webkit-flex-direction) On 2016/07/25 at 07:43:27, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:689: // flex-grow (aka -webkit-flex-grow) On 2016/07/25 at 07:43:19, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:693: // flex-shrink (aka -webkit-flex-shrink) On 2016/07/25 at 07:43:19, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:697: // flex-wrap (aka -webkit-flex-wrap) On 2016/07/25 at 07:43:26, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:701: // -webkit-box-flex type_name=float On 2016/07/25 at 07:43:23, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:705: // -webkit-box-flex-group type_name=unsigned int On 2016/07/25 at 07:43:25, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:709: // -webkit-box-align On 2016/07/25 at 07:43:27, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:714: // -webkit-box-decoration-break On 2016/07/25 at 07:43:25, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:718: // -webkit-box-lines On 2016/07/25 at 07:43:27, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:722: // -webkit-box-ordinal-group On 2016/07/25 at 07:43:21, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:726: // -webkit-box-orient On 2016/07/25 at 07:43:19, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:730: // -webkit-box-pack On 2016/07/25 at 07:43:22, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:734: // -webkit-box-reflect On 2016/07/25 at 07:43:20, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:758: // grid-auto-columns On 2016/07/25 at 07:43:25, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:762: // grid-auto-flow On 2016/07/25 at 07:43:20, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:765: // grid-auto-rows On 2016/07/25 at 07:43:26, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:769: // grid-column-gap On 2016/07/25 at 07:43:22, nainar wrote: > insert new line before Done, and removed newline below https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:774: // grid-column-start On 2016/07/25 at 07:43:27, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:778: // grid-column-end On 2016/07/25 at 07:43:18, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:782: // grid-row-gap On 2016/07/25 at 07:43:25, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:786: // grid-row-start On 2016/07/25 at 07:43:20, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:790: // grid-row-end On 2016/07/25 at 07:43:18, nainar wrote: > insert new line before Done Done ;) https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:794: // grid-template-areas On 2016/07/25 at 07:43:23, nainar wrote: > why have this here when there are no methods? Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:799: // grid-template-rows On 2016/07/25 at 07:43:26, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:807: // justify-items On 2016/07/25 at 07:43:18, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:814: // width On 2016/07/25 at 07:43:21, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:817: // height On 2016/07/25 at 07:43:20, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:822: // max-width On 2016/07/25 at 07:43:19, nainar wrote: > insert new line before Done, and moved initializer up to top https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:825: // max-height On 2016/07/25 at 07:43:24, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:830: // min-width On 2016/07/25 at 07:43:26, nainar wrote: > insert new line before Done, and moved initializer up to top https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:833: // min-height On 2016/07/25 at 07:43:27, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:854: // margin-top On 2016/07/25 at 07:43:19, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:857: // margin-bottom On 2016/07/25 at 07:43:25, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:860: // margin-left On 2016/07/25 at 07:43:25, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:863: // margin-right On 2016/07/25 at 07:43:23, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:871: // -webkit-margin-after-collapse (aka -webkit-margin-bottom-collapse) On 2016/07/25 at 07:43:20, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:885: // motion-offset On 2016/07/25 at 07:43:23, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:889: // motion-rotation On 2016/07/25 at 07:43:18, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:896: ObjectFit getObjectFit() const { return static_cast<ObjectFit>(m_rareNonInheritedData->m_objectFit); } On 2016/07/25 at 07:43:23, nainar wrote: > getObjectFit() -> objectFit() in an earlier patch added TODO, will do in later patch :) https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:898: // object-position On 2016/07/25 at 07:43:26, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:911: // We restrict the smallest value to int min + 2 because we use int min and int min + 1 as special values in a hash set. On 2016/07/25 at 07:43:19, nainar wrote: > delete line ?? No newline here, and this comment is informative :) https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:918: static OutlineIsAuto initialOutlineStyleIsAuto() { return OutlineIsAutoOff; } On 2016/07/25 at 07:43:19, nainar wrote: > move to after setOutlineStyle() Nice!! Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:923: // outline-width On 2016/07/25 at 07:43:21, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:926: int outlineWidth() const On 2016/07/25 at 07:43:18, nainar wrote: > move to before setOutlineWidth() Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:947: // overflow-x On 2016/07/25 at 07:43:22, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:951: // overflow-y On 2016/07/25 at 07:43:21, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:958: // padding-bottom On 2016/07/25 at 07:43:19, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:961: // padding-left On 2016/07/25 at 07:43:21, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:964: // padding-right On 2016/07/25 at 07:43:22, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:967: // padding-top On 2016/07/25 at 07:43:23, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:975: // perspective-origin (aka -webkit-perspective-origin) On 2016/07/25 at 07:43:25, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:979: // -webkit-perspective-origin-x On 2016/07/25 at 07:43:20, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:983: // -webkit-perspective-origin-y On 2016/07/25 at 07:43:26, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1003: // transform-origin (aka -webkit-transform-origin) On 2016/07/25 at 07:43:22, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1007: // transform-style (aka -webkit-transform-style) On 2016/07/25 at 07:43:18, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1011: // -webkit-transform-origin-x On 2016/07/25 at 07:43:21, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1015: // -webkit-transform-origin-y On 2016/07/25 at 07:43:21, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1019: // -webkit-transform-origin-z On 2016/07/25 at 07:43:23, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1029: // rotate On 2016/07/25 at 07:43:26, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1033: // scale On 2016/07/25 at 07:43:25, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1041: ScrollBehavior getScrollBehavior() const { return static_cast<ScrollBehavior>(m_rareNonInheritedData->m_scrollBehavior); } On 2016/07/25 at 07:43:20, nainar wrote: > rename getScrollBehavior() -> scrollBehavior() in an earlier patch Added TODO https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1043: // scroll-snap-coordinate On 2016/07/25 at 07:43:25, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1047: // scroll-snap-destination On 2016/07/25 at 07:43:21, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1051: // scroll-snap-points-x On 2016/07/25 at 07:43:23, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1055: // scroll-snap-points-y On 2016/07/25 at 07:43:18, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1059: // scroll-snap-type On 2016/07/25 at 07:43:24, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1061: ScrollSnapType getScrollSnapType() const { return static_cast<ScrollSnapType>(m_rareNonInheritedData->m_scrollSnapType); } On 2016/07/25 at 07:43:26, nainar wrote: > rename getScrollSnapType() -> scrollSnapType() in an earlier patch Added TODO https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1072: // shape-margin (aka -webkit-shape-margin) On 2016/07/25 at 07:43:23, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1076: // shape-outside (aka -webkit-shape-outside) On 2016/07/25 at 07:43:21, nainar wrote: > insert new line before Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1084: ShapeValue* shapeOutside() const { return m_rareNonInheritedData->m_shapeOutside.get(); } On 2016/07/25 at 07:43:19, nainar wrote: > put before setShapeOutside() Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1088: PageSizeType getPageSizeType() const { return static_cast<PageSizeType>(m_rareNonInheritedData->m_pageSizeType); } On 2016/07/25 at 07:43:22, nainar wrote: > rename getPageSizeType() -> pageSizeType() in an earlier patch Done https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1099: TextDecoration getTextDecoration() const { return static_cast<TextDecoration>(m_visual->textDecoration); } On 2016/07/25 at 07:43:21, nainar wrote: > rename getTextDecoration() -> textDecoration() in an earlier patch Added todo https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1101: // text-decoration-color On 2016/07/25 at 07:43:25, nainar wrote: > insert new line before Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1103: // text-decoration-style On 2016/07/25 at 07:43:24, nainar wrote: > insert new line before Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1105: TextDecorationStyle getTextDecorationStyle() const { return static_cast<TextDecorationStyle>(m_rareNonInheritedData->m_textDecorationStyle); } On 2016/07/25 at 07:43:24, nainar wrote: > rename getTextDecorationStyle() -> textDecorationStyle() in an earlier patch Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1105: TextDecorationStyle getTextDecorationStyle() const { return static_cast<TextDecorationStyle>(m_rareNonInheritedData->m_textDecorationStyle); } On 2016/07/25 at 07:43:24, nainar wrote: > rename getTextDecorationStyle() -> textDecorationStyle() in an earlier patch Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1107: // text-underline-position runtime_flag=CSS3TextDecorations, inherited, type_name=TextUnderlinePosition On 2016/07/25 at 07:43:22, nainar wrote: > insert new line before Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1110: TextUnderlinePosition getTextUnderlinePosition() const { return static_cast<TextUnderlinePosition>(m_rareInheritedData->m_textUnderlinePosition); } On 2016/07/25 at 07:43:22, nainar wrote: > a) switch order with setTextUnderlinePosition > b) getTextUnderlinePosition() -> textUnderlinePosition() in an earlier patch Done Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1114: TextOverflow getTextOverflow() const { return static_cast<TextOverflow>(m_rareNonInheritedData->textOverflow); } On 2016/07/25 at 07:43:24, nainar wrote: > rename getTextOverflow() -> textOverflow() in an earlier patch Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1119: TouchAction getTouchAction() const { return static_cast<TouchAction>(m_rareNonInheritedData->m_touchAction); } On 2016/07/25 at 07:43:26, nainar wrote: > rename getTouchAction() -> touchAction() in an earlier patch Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1130: const Length& getVerticalAlignLength() const { return m_box->verticalAlign(); } On 2016/07/25 at 07:43:20, nainar wrote: > rename getVerticalAlignLength() -> verticalAlignLength() in an earlier patch Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1137: // wrap-through On 2016/07/25 at 07:43:26, nainar wrote: > insert new line before Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1164: DraggableRegionMode getDraggableRegionMode() const { return m_rareNonInheritedData->m_draggableRegionMode; } On 2016/07/25 at 07:43:24, nainar wrote: > rename getDraggableRegionMode() -> draggableRegionMode() in an earlier patch Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1189: // -webkit-mask-box-image-repeat On 2016/07/25 at 07:43:25, nainar wrote: > why have this here if there are no methods? Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1196: // -webkit-mask-box-image-source On 2016/07/25 at 07:43:26, nainar wrote: > insert new line before Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1196: // -webkit-mask-box-image-source On 2016/07/25 at 07:43:26, nainar wrote: > insert new line before Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1200: // -webkit-mask-box-image-width On 2016/07/25 at 07:43:21, nainar wrote: > insert new line before Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1250: Hyphens getHyphens() const { return static_cast<Hyphens>(m_rareInheritedData->hyphens); } On 2016/07/25 at 07:43:25, nainar wrote: > rename getHyphens() -> hyphens() in an earlier patch Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1252: // -webkit-hyphenate-character On 2016/07/25 at 07:43:20, nainar wrote: > insert new line before Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1267: // list-style-position On 2016/07/25 at 07:43:22, nainar wrote: > insert new line before Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1271: // list-style-image On 2016/07/25 at 07:43:23, nainar wrote: > insert new line before Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1280: // widows On 2016/07/25 at 07:43:20, nainar wrote: > insert new line before Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1314: TabSize getTabSize() const { return m_rareInheritedData->m_tabSize; } On 2016/07/25 at 07:43:23, nainar wrote: > rename getTabSize() ->tabSize() in an earlier patch Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1321: // text-align-last On 2016/07/25 at 07:43:22, nainar wrote: > insert new line before Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1323: TextAlignLast getTextAlignLast() const { return static_cast<TextAlignLast>(m_rareInheritedData->m_textAlignLast); } On 2016/07/25 at 07:43:19, nainar wrote: > rename getTextAlignLast() ->textAlignLast() in an earlier patch Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1328: TextCombine getTextCombine() const { return static_cast<TextCombine>(m_rareInheritedData->m_textCombine); } On 2016/07/25 at 07:43:23, nainar wrote: > rename getTextCombine() -> textCombine() in an earlier patch Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1336: TextIndentLine getTextIndentLine() const { return static_cast<TextIndentLine>(m_rareInheritedData->m_textIndentLine); } On 2016/07/25 at 07:43:27, nainar wrote: > rename getTextIndentLine() -> textIndentLine() in an earlier patch Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1337: TextIndentType getTextIndentType() const { return static_cast<TextIndentType>(m_rareInheritedData->m_textIndentType); } On 2016/07/25 at 07:43:20, nainar wrote: > rename getTextIndentType() -> textIndentType() in an earlier patch Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1344: TextJustify getTextJustify() const { return static_cast<TextJustify>(m_rareInheritedData->m_textJustify); } On 2016/07/25 at 07:43:25, nainar wrote: > rename getTextJustify() -> textJustify() in an earlier patch Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1349: TextOrientation getTextOrientation() const { return static_cast<TextOrientation>(m_rareInheritedData->m_textOrientation); } On 2016/07/25 at 07:43:20, nainar wrote: > rename getTextOrientation() -> textOrientation() in an earlier patch Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1369: void setVisibility(EVisibility v) { m_inheritedData.m_visibility = v; } On 2016/07/25 at 07:43:21, nainar wrote: > put after visibility() Done!! Ty https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1384: LineBreak getLineBreak() const { return static_cast<LineBreak>(m_rareInheritedData->lineBreak); } On 2016/07/25 at 07:43:23, nainar wrote: > rename getLineBreak() -> lineBreak() in an earlier patch Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1389: WritingMode getWritingMode() const { return static_cast<WritingMode>(m_inheritedData.m_writingMode); } On 2016/07/25 at 07:43:21, nainar wrote: > rename getWritingMode() -> writingMode() in an earlier patch Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1403: TextEmphasisFill getTextEmphasisFill() const { return static_cast<TextEmphasisFill>(m_rareInheritedData->textEmphasisFill); } On 2016/07/25 at 07:43:25, nainar wrote: > rename all getTextEmphasis*() -> textEmphasis*() in an earlier patch Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1410: // -webkit-text-emphasis-color (aka -epub-text-emphasis-color) On 2016/07/25 at 07:43:22, nainar wrote: > insert new line before Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1412: // -webkit-text-emphasis-position On 2016/07/25 at 07:43:23, nainar wrote: > insert new line before Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1414: TextEmphasisPosition getTextEmphasisPosition() const { return static_cast<TextEmphasisPosition>(m_rareInheritedData->textEmphasisPosition); } On 2016/07/25 at 07:43:25, nainar wrote: > rename getTextEmphasisPosition() -> textEmphasisPosition() in an earlier patch Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1434: PrintColorAdjust getPrintColorAdjust() const { return static_cast<PrintColorAdjust>(m_inheritedData.m_printColorAdjust); } On 2016/07/25 at 07:43:24, nainar wrote: > rename getPrintColorAdjust() -> printColorAdjust() in an earlier patch Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1445: RubyPosition getRubyPosition() const { return static_cast<RubyPosition>(m_rareInheritedData->m_rubyPosition); } On 2016/07/25 at 07:43:25, nainar wrote: > a) put this before the setter > b) rename getRubyPosition() -> rubyPosition() in an earlier patch Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1450: Color tapHighlightColor() const { return m_rareInheritedData->tapHighlightColor; } On 2016/07/25 at 07:43:22, nainar wrote: > put this before the setter Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1462: // -webkit-text-stroke-width On 2016/07/25 at 07:43:18, nainar wrote: > insert new line before Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1462: // -webkit-text-stroke-width On 2016/07/25 at 07:43:18, nainar wrote: > insert new line before Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1471: // -webkit-user-modify On 2016/07/25 at 07:43:23, nainar wrote: > insert new line before Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1475: // -webkit-user-select On 2016/07/25 at 07:43:21, nainar wrote: > insert new line before Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1490: // font-size-adjust On 2016/07/25 at 07:43:22, nainar wrote: > insert new line before Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1493: // font-weight On 2016/07/25 at 07:43:24, nainar wrote: > insert new line before Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1497: bool setFontDescription(const FontDescription&); On 2016/07/25 at 07:43:24, nainar wrote: > insert new line before Done, and moved setfontdescription up to the top https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1504: static float initialLetterWordSpacing() { return 0.0f; } On 2016/07/25 at 07:43:24, nainar wrote: > should this be initialLetterSpacing() ? It's used for both. :) https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1507: // word-spacing On 2016/07/25 at 07:43:26, nainar wrote: > insert new line before Done and fixed up FIXME to be clearer. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1540: void setRy(const Length& ry) { accessSVGStyle().setRy(ry); } Added newlines before each of these. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1552: // flood-color On 2016/07/25 at 07:43:25, nainar wrote: > insert new line before Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1554: // lighting-color On 2016/07/25 at 07:43:21, nainar wrote: > insert new line before Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1568: // stroke-dasharray On 2016/07/25 at 07:43:25, nainar wrote: > insert new line before Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1571: // stroke-dashoffset On 2016/07/25 at 07:43:27, nainar wrote: > insert new line before Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1574: // stroke-miterlimit On 2016/07/25 at 07:43:19, nainar wrote: > insert new line before Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1577: // stroke-opacity On 2016/07/25 at 07:43:26, nainar wrote: > insert new line before Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1580: // stroke-width On 2016/07/25 at 07:43:19, nainar wrote: > insert new line before Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1624: void setHasViewportUnits(bool hasViewportUnits = true) const { m_nonInheritedData.m_hasViewportUnits = hasViewportUnits; } On 2016/07/25 at 07:43:22, nainar wrote: > reverse order of getter and setter Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1627: void setHasRemUnits() const { m_nonInheritedData.m_hasRemUnits = true; } On 2016/07/25 at 07:43:26, nainar wrote: > reverse order of getter and setter Done. https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1634: On 2016/07/25 at 07:43:22, nainar wrote: > remove new line Done, also broke these up. :) https://codereview.chromium.org/2174703002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:1656: bool hasExplicitlyInheritedProperties() const { return m_nonInheritedData.m_explicitInheritance; } On 2016/07/25 at 07:43:24, nainar wrote: > reverse order with setter Done.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm - took a look at the diff between patch 2 and patch 3 - and all the comments were addressed. Thanks so much for taking a look at all the comments! To the OWNERS!
sashab@chromium.org changed reviewers: + esprehn@chromium.org
Ty nainar :) esprehn!
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Alf thinks this patch is great! http://i.imgur.com/kykI3by.gif rs lgtm
The CQ bit was checked by sashab@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by sashab@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from nainar@chromium.org, esprehn@chromium.org Link to the patchset: https://codereview.chromium.org/2174703002/#ps80001 (title: "Rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== Organize public ComputedStyle methods by property Organized ComputedStyle public methods by property. This is part of a larger effort to clean up ComputedStyle and ultimately generate it. Also added a comment to the top of ComputedStyle explaining the new style, as well as TODOs to classes used by ComputedStyle to merge their logic into ComputedStyle. BUG=628043 ========== to ========== Organize public ComputedStyle methods by property Organized ComputedStyle public methods by property. This is part of a larger effort to clean up ComputedStyle and ultimately generate it. Also added a comment to the top of ComputedStyle explaining the new style, as well as TODOs to classes used by ComputedStyle to merge their logic into ComputedStyle. BUG=628043 Committed: https://crrev.com/a64e4958441c43f0bafc5c71d54fd259589598b1 Cr-Commit-Position: refs/heads/master@{#408603} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/a64e4958441c43f0bafc5c71d54fd259589598b1 Cr-Commit-Position: refs/heads/master@{#408603} |
