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

Issue metadata

Status: Fixed
Owner:
Not on Chrome anymore
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature

Blocking:
issue 628043



Sign in to add a comment

Be able to change the way ComputedStyle fields are grouped in CSSProperties.json5

Project Member Reported by nainar@chromium.org, Apr 12 2017

Issue description

Currently, there are groups of properties that are not directly stored in ComputedStyle but are stored instead as pointers to dynamically allocated instances. For example, the class StyleBackgroundData contains fields representing all background information. This is so that multiple elements can save memory by sharing the same instance of StyleBackgroundData if they have the same background properties. 

shend@ is working on generating the groups of properties together as a whole (e.g. here). In the future, we would like to be able to change the way that properties are grouped.

Design doc here: https://docs.google.com/document/d/1dSQCX54G3sGKYXNRazq_O4bRNhhgjR_MKENbO19qCIU/edit?usp=sharing
 

Comment 1 by nainar@chromium.org, Apr 12 2017

Components: Blink>CSS
Labels: Update-Monthly

Comment 2 by nainar@chromium.org, Apr 18 2017

Status: Assigned (was: Untriaged)
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 20 2017

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

commit 7185f26bb323f0650f8831f6e2d2c976fd373e55
Author: nainar <nainar@chromium.org>
Date: Thu Apr 20 06:43:30 2017

Generate getters/setters for some fields on groups in ComputedStyle

This patch generates getters/setters as needed for monotic_flags,
external and storage_only fields on groups in ComputedStyle. In a
following patch we will use this to access members of groups through
these getters/setters instead of directly as <group_name>.<field_name>.

Generated diff here: https://gist.github.com/nainar/bf6ca12c12ff7283d90ebed7640cf7be/revisions

Please note these functions are unused for now.

BUG= 710938 

Review-Url: https://codereview.chromium.org/2826653002
Cr-Commit-Position: refs/heads/master@{#465919}

[modify] https://crrev.com/7185f26bb323f0650f8831f6e2d2c976fd373e55/third_party/WebKit/Source/build/scripts/make_computed_style_base.py
[modify] https://crrev.com/7185f26bb323f0650f8831f6e2d2c976fd373e55/third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.h.tmpl
[modify] https://crrev.com/7185f26bb323f0650f8831f6e2d2c976fd373e55/third_party/WebKit/Source/build/scripts/templates/fields/base.tmpl
[modify] https://crrev.com/7185f26bb323f0650f8831f6e2d2c976fd373e55/third_party/WebKit/Source/build/scripts/templates/fields/external.tmpl
[modify] https://crrev.com/7185f26bb323f0650f8831f6e2d2c976fd373e55/third_party/WebKit/Source/build/scripts/templates/fields/field.tmpl
[modify] https://crrev.com/7185f26bb323f0650f8831f6e2d2c976fd373e55/third_party/WebKit/Source/build/scripts/templates/fields/keyword.tmpl
[modify] https://crrev.com/7185f26bb323f0650f8831f6e2d2c976fd373e55/third_party/WebKit/Source/build/scripts/templates/fields/monotonic_flag.tmpl
[modify] https://crrev.com/7185f26bb323f0650f8831f6e2d2c976fd373e55/third_party/WebKit/Source/build/scripts/templates/fields/primitive.tmpl
[modify] https://crrev.com/7185f26bb323f0650f8831f6e2d2c976fd373e55/third_party/WebKit/Source/build/scripts/templates/fields/storage_only.tmpl

Project Member

Comment 5 by bugdroid1@chromium.org, May 4 2017

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

commit a26e3d4f16764fda0ed56f200af4d7fd17109ffe
Author: nainar <nainar@chromium.org>
Date: Thu May 04 05:01:11 2017

Split up the if checks in ComputedStyle to make them easy to generate

This patch splits up the iff checks in the diffing function
ComputedStyle::UpdatePropertySpecificDifferences with the intention of
making it easy to generate with the macro fieldwise_diff created here:
https://codereview.chromium.org/2858863002

As can be seen in the try bot runs this has no perf impact

BUG= 710938 

Review-Url: https://codereview.chromium.org/2855873003
Cr-Commit-Position: refs/heads/master@{#469268}

[modify] https://crrev.com/a26e3d4f16764fda0ed56f200af4d7fd17109ffe/third_party/WebKit/Source/core/style/ComputedStyle.cpp

Project Member

Comment 6 by bugdroid1@chromium.org, May 4 2017

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

commit 997462ff044512808bc9f9ccc64b11148b625e91
Author: nainar <nainar@chromium.org>
Date: Thu May 04 06:25:44 2017

Add macro to diff the groups (and their members) in ComputedStyleBase

This patch adds the fieldwise_diff macro and then uses it to generate
the diff functions on the groups that have been generated so far
(StyleSurroundData) in ComputedStyleBase.

Please note that it can only be used for memebers of those groups too
that have already been generated. This is why the diffing for
BorderData has been left to a later CL.

Diff: https://gist.github.com/nainar/04f49165c4cb5ecb30371fbde1491ddf/revisions

BUG= 710938 

Review-Url: https://codereview.chromium.org/2858863002
Cr-Commit-Position: refs/heads/master@{#469286}

[modify] https://crrev.com/997462ff044512808bc9f9ccc64b11148b625e91/third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.cpp.tmpl
[modify] https://crrev.com/997462ff044512808bc9f9ccc64b11148b625e91/third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.h.tmpl
[modify] https://crrev.com/997462ff044512808bc9f9ccc64b11148b625e91/third_party/WebKit/Source/build/scripts/templates/fields/field.tmpl
[modify] https://crrev.com/997462ff044512808bc9f9ccc64b11148b625e91/third_party/WebKit/Source/core/style/ComputedStyle.cpp

Project Member

Comment 7 by bugdroid1@chromium.org, May 12 2017

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

commit 2fd9b8f36a1752754039ee9b817cbc08d721daf8
Author: shend <shend@chromium.org>
Date: Fri May 12 07:13:22 2017

Remove 'const' prefix from functions that return BorderValue by value.

There are several functions that create a BorderValue and return it by
value. However, the return type is 'const BorderValue' not 'BorderValue'
which makes it confusing because the 'const' is extraneous. This patch
removes the 'const' prefix.

BUG= 710938 

Review-Url: https://codereview.chromium.org/2878823002
Cr-Commit-Position: refs/heads/master@{#471242}

[modify] https://crrev.com/2fd9b8f36a1752754039ee9b817cbc08d721daf8/third_party/WebKit/Source/core/style/ComputedStyle.cpp
[modify] https://crrev.com/2fd9b8f36a1752754039ee9b817cbc08d721daf8/third_party/WebKit/Source/core/style/ComputedStyle.h

Project Member

Comment 8 by bugdroid1@chromium.org, May 15 2017

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

commit 6f28abba06b92fafe20371b82d81b13e6a8b6d6c
Author: nainar <nainar@chromium.org>
Date: Mon May 15 04:17:38 2017

Move code from from ComputedStyleBase.cpp.tmpl to .h.tmpl

This patch moves code from ComputedStyleBase.cpp.tmpl to the h.tmpl
file in preparation to make it a template so that we can make
ComputedStyle a templated argument and access both functions on
ComputedStyleBase and ComputedStyle in the base class.

Diff: https://gist.github.com/nainar/f7cb9953674378504ac6795be365bb1f/revisions

BUG= 710938 

Review-Url: https://codereview.chromium.org/2875233002
Cr-Commit-Position: refs/heads/master@{#471664}

[modify] https://crrev.com/6f28abba06b92fafe20371b82d81b13e6a8b6d6c/third_party/WebKit/Source/build/scripts/make_computed_style_base.py
[delete] https://crrev.com/234813ae8d52f622a1664e11365642751e8f26ba/third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.cpp.tmpl
[modify] https://crrev.com/6f28abba06b92fafe20371b82d81b13e6a8b6d6c/third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.h.tmpl
[modify] https://crrev.com/6f28abba06b92fafe20371b82d81b13e6a8b6d6c/third_party/WebKit/Source/core/BUILD.gn

Project Member

Comment 9 by bugdroid1@chromium.org, May 15 2017

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

commit 0066ff300549155904a9d1708488fc617f705404
Author: nainar <nainar@chromium.org>
Date: Mon May 15 04:28:15 2017

Refactor code generation to allow us to diff generic expressions and not just fields

This patch refactors the diffing generator code to allow us to diff
field getters and other expressions instead of just fields themselves in
the future.

It does so by creating a tree of groups to diff (DiffGroups)
that store the subgroups to diff as well as the expressions to diff.
For now the expressions are just the field accessors. In the future,
they will contain expressions like BorderLeftWidth() - a public getter
for border-left-width, and HasTransform() - which is not a field or
its getter.

Diff: https://gist.github.com/d14529581f729e810842254cbf7dc1f5/revisions

BUG= 710938 

Review-Url: https://codereview.chromium.org/2879563002
Cr-Commit-Position: refs/heads/master@{#471666}

[modify] https://crrev.com/0066ff300549155904a9d1708488fc617f705404/third_party/WebKit/Source/build/scripts/make_computed_style_base.py
[modify] https://crrev.com/0066ff300549155904a9d1708488fc617f705404/third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.h.tmpl
[modify] https://crrev.com/0066ff300549155904a9d1708488fc617f705404/third_party/WebKit/Source/build/scripts/templates/fields/field.tmpl
[modify] https://crrev.com/0066ff300549155904a9d1708488fc617f705404/third_party/WebKit/Source/core/BUILD.gn
[add] https://crrev.com/0066ff300549155904a9d1708488fc617f705404/third_party/WebKit/Source/core/css/ComputedStyleDiffFunctions.json5
[modify] https://crrev.com/0066ff300549155904a9d1708488fc617f705404/third_party/WebKit/Source/core/style/ComputedStyle.cpp

Project Member

Comment 10 by bugdroid1@chromium.org, May 15 2017

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

commit 9937b524e266fe058f637e442f96efba14b35c1f
Author: nainar <nainar@chromium.org>
Date: Mon May 15 06:31:19 2017

Generate diffing functions for generated subgroup BoxData in ComputedStyle

This patch generates the diffing functions for StyleBoxData which is now
a generated subgroup of ComputedStyle.

Please note that this patch doesn't generate the diffing for
VerticalAlign as it is accessed through both BoxData (when a length) and
a separate getter on ComputedStyle (when a keyword.) This will be
generated once we can generate diffs using functions.

Please note: No added callsite for ComputedStyleBase::ScrollAnchorDisablingPropertyChanged has been added
since it is already being called. We have just appended to that JSON
entry.

Diff: https://gist.github.com/5701f6d9a1be397677cbde5ad7033259/revisions

BUG= 710938 

Review-Url: https://codereview.chromium.org/2876353002
Cr-Commit-Position: refs/heads/master@{#471683}

[modify] https://crrev.com/9937b524e266fe058f637e442f96efba14b35c1f/third_party/WebKit/Source/core/css/ComputedStyleDiffFunctions.json5
[modify] https://crrev.com/9937b524e266fe058f637e442f96efba14b35c1f/third_party/WebKit/Source/core/style/ComputedStyle.cpp

Project Member

Comment 11 by bugdroid1@chromium.org, May 16 2017

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

commit 9b38f0b5a429e3046188ae68e65765d57dbe018c
Author: shend <shend@chromium.org>
Date: Tue May 16 23:06:29 2017

Remove references to box_data_ in ComputedStyle.

To allow changes to where fields are stored, ComputedStyle code should
not refer directly to a group, as that code will break when we change
groups. This patch removes references to box_data_ in ComputedStyle,
replacing with generated or handwritten getters. This patch does not
remove references within diffing functions as those will soon be
generated.

BUG= 710938 

Review-Url: https://codereview.chromium.org/2888513002
Cr-Commit-Position: refs/heads/master@{#472238}

[modify] https://crrev.com/9b38f0b5a429e3046188ae68e65765d57dbe018c/third_party/WebKit/Source/core/style/ComputedStyle.h

Project Member

Comment 12 by bugdroid1@chromium.org, May 17 2017

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

commit a400161f978b4d2a42c8805e9b3c7ad510ce9cb3
Author: shend <shend@chromium.org>
Date: Wed May 17 01:23:22 2017

Remove references to visual_data_ in ComputedStyle.

To allow changes to where fields are stored, ComputedStyle code should
not refer directly to a group, as that code will break when we change
groups. This patch removes references to visual_data_ in ComputedStyle,
replacing with generated or handwritten getters. This patch does not
remove references within diffing functions as those will soon be
generated.

BUG= 710938 

Review-Url: https://codereview.chromium.org/2884833003
Cr-Commit-Position: refs/heads/master@{#472282}

[modify] https://crrev.com/a400161f978b4d2a42c8805e9b3c7ad510ce9cb3/third_party/WebKit/Source/core/style/ComputedStyle.h

Project Member

Comment 13 by bugdroid1@chromium.org, May 17 2017

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

commit de633202221100c5bc812309bdff65f7faa6b0d3
Author: nainar <nainar@chromium.org>
Date: Wed May 17 04:19:51 2017

Remove references to surround_data_ in ComputedStyle.

To allow changes to where fields are stored, ComputedStyle code should
not refer directly to a group, as that code will break when we change
groups. This patch removes references to surround_data_ in ComputedStyle,
replacing with generated or handwritten getters.

References in the cpp file have already been removed.

BUG= 710938 

Review-Url: https://codereview.chromium.org/2887433005
Cr-Commit-Position: refs/heads/master@{#472322}

[modify] https://crrev.com/de633202221100c5bc812309bdff65f7faa6b0d3/third_party/WebKit/Source/core/style/ComputedStyle.h

Project Member

Comment 14 by bugdroid1@chromium.org, May 17 2017

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

commit 0e9b44fea13a90aff88c5a0e7e0164fa99e2418a
Author: nainar <nainar@chromium.org>
Date: Wed May 17 07:05:26 2017

Remove references to background_data_ in ComputedStyle.

To allow changes to where fields are stored, ComputedStyle code should
not refer directly to a group, as that code will break when we change
groups. This patch removes references to background_data_ in ComputedStyle,
replacing with generated or handwritten getters.

References in the cpp file didn't exist.

BUG= 710938 

Review-Url: https://codereview.chromium.org/2885753002
Cr-Commit-Position: refs/heads/master@{#472364}

[modify] https://crrev.com/0e9b44fea13a90aff88c5a0e7e0164fa99e2418a/third_party/WebKit/Source/core/style/ComputedStyle.h

Project Member

Comment 15 by bugdroid1@chromium.org, May 17 2017

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

commit 1f05e0767786c4b551c6cd3846dce6c183500747
Author: nainar <nainar@chromium.org>
Date: Wed May 17 23:55:27 2017

Generate diffs for fields in ComputedStyle that use their public getters

This patch allows us to diff fields using their public getters when
these getters do added work.

We specify the getters in a map of expressions in
ComputedStyleDiffFunctions.json5.

This can be used to diff not just public getter but any function that
uses the field. Hence the name map_of_expressions.

Diff: https://gist.github.com/ae54c525ef49c24b7707848546e78c73/revisions

BUG= 710938 

Review-Url: https://codereview.chromium.org/2876803003
Cr-Commit-Position: refs/heads/master@{#472605}

[modify] https://crrev.com/1f05e0767786c4b551c6cd3846dce6c183500747/third_party/WebKit/Source/build/scripts/make_computed_style_base.py
[modify] https://crrev.com/1f05e0767786c4b551c6cd3846dce6c183500747/third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.h.tmpl
[modify] https://crrev.com/1f05e0767786c4b551c6cd3846dce6c183500747/third_party/WebKit/Source/build/scripts/templates/fields/field.tmpl
[modify] https://crrev.com/1f05e0767786c4b551c6cd3846dce6c183500747/third_party/WebKit/Source/core/css/ComputedStyleDiffFunctions.json5
[modify] https://crrev.com/1f05e0767786c4b551c6cd3846dce6c183500747/third_party/WebKit/Source/core/style/ComputedStyle.cpp

Project Member

Comment 16 by bugdroid1@chromium.org, May 19 2017

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

commit 702510f88b10b7ab5df0a87ada3d09f6d1139873
Author: nainar <nainar@chromium.org>
Date: Fri May 19 01:26:03 2017

Generate diffing functions for generated subgroup InheritedData in ComputedStyle

This patch generates the diffing functions for StyleInheritedData which is now
a generated subgroup of ComputedStyle.

Please note: No added callsite for
ComputedStyleBase::DiffNeedsFullLayoutAndPaintInvalidation has been
added since it is already being called. We have just appended to that
JSON entry.

Diff: https://gist.github.com/nainar/5650d8c7dc6b9cc5d784c5be2409b338/revisions

BUG= 710938 

Review-Url: https://codereview.chromium.org/2886233004
Cr-Commit-Position: refs/heads/master@{#473015}

[modify] https://crrev.com/702510f88b10b7ab5df0a87ada3d09f6d1139873/third_party/WebKit/Source/core/css/ComputedStyleDiffFunctions.json5
[modify] https://crrev.com/702510f88b10b7ab5df0a87ada3d09f6d1139873/third_party/WebKit/Source/core/style/ComputedStyle.cpp

Project Member

Comment 17 by bugdroid1@chromium.org, May 20 2017

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

commit 1b298605df3e00d8bb378fb2c17a8014d0d1c578
Author: shend <shend@chromium.org>
Date: Sat May 20 12:44:46 2017

Remove references to inherited_data_ in ComputedStyle.

To allow changes to where fields are stored, ComputedStyle code should
not refer directly to a group, as that code will break when we change
groups. This patch removes references to inherited_data_ in ComputedStyle,
replacing with generated or handwritten getters. This patch does not
remove references within diffing functions as those will soon be
generated.

BUG= 710938 

Review-Url: https://codereview.chromium.org/2886293004
Cr-Commit-Position: refs/heads/master@{#473433}

[modify] https://crrev.com/1b298605df3e00d8bb378fb2c17a8014d0d1c578/third_party/WebKit/Source/core/style/ComputedStyle.cpp
[modify] https://crrev.com/1b298605df3e00d8bb378fb2c17a8014d0d1c578/third_party/WebKit/Source/core/style/ComputedStyle.h

Project Member

Comment 18 by bugdroid1@chromium.org, May 24 2017

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

commit ef53b0adb151b2eb77e4c5dcda0d9cea47e97a6d
Author: nainar <nainar@chromium.org>
Date: Wed May 24 03:23:16 2017

Assert that ComputedStyleBase is only templated with ComputedStyle

This patch adds a static_assert to assert at compile time that a
ComputedStyleBase can only be templated with a ComputedStyle.

Diff: https://gist.github.com/nainar/ea6c15963c9fe91a4d0c8c9cc5eddbad/revisions

BUG= 710938 

Review-Url: https://codereview.chromium.org/2896233002
Cr-Commit-Position: refs/heads/master@{#474146}

[modify] https://crrev.com/ef53b0adb151b2eb77e4c5dcda0d9cea47e97a6d/third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.h.tmpl

Project Member

Comment 19 by bugdroid1@chromium.org, May 24 2017

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

commit dd5069631f086ea22e7c43d9a1f85b0a83fc87df
Author: nainar <nainar@chromium.org>
Date: Wed May 24 09:46:47 2017

Generate diffs for properties that are generated in ComputedStyle

This patch generates the diffing functions for properties:
1. that are stored directly on ComputedStyle
2. that were left behind when previously generating diffs for
   properties that are on generated groups.

Diff: https://gist.github.com/nainar/7ee584319877b3da59a04b80a1690908/revisions

BUG= 710938 

Review-Url: https://codereview.chromium.org/2889353002
Cr-Commit-Position: refs/heads/master@{#474227}

[modify] https://crrev.com/dd5069631f086ea22e7c43d9a1f85b0a83fc87df/third_party/WebKit/Source/build/scripts/make_computed_style_base.py
[modify] https://crrev.com/dd5069631f086ea22e7c43d9a1f85b0a83fc87df/third_party/WebKit/Source/core/css/ComputedStyleDiffFunctions.json5
[modify] https://crrev.com/dd5069631f086ea22e7c43d9a1f85b0a83fc87df/third_party/WebKit/Source/core/style/ComputedStyle.cpp

Project Member

Comment 20 by bugdroid1@chromium.org, May 25 2017

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

commit 2ee241f419323d7dd924c8bf4b4506189b0319ab
Author: nainar <nainar@chromium.org>
Date: Thu May 25 05:37:48 2017

Generates predicates to test in diff functions in ComputedStyle

This patch adds the ability to generate the predicates that encapsulate
diffing logic when generating diff functions in ComputedStyle.

We specify the predicates to test as a list of predicates to test in
ComputedStyleDiffFunctions.json5.

A diff has been generated for two predicates for properties stored on
StyleInheritedData. Further generation will be done in a separate patch.

Diff: https://gist.github.com/nainar/e8956f13f3569d2195183513ffa11600/revisions

BUG= 710938 

Review-Url: https://codereview.chromium.org/2902433002
Cr-Commit-Position: refs/heads/master@{#474567}

[modify] https://crrev.com/2ee241f419323d7dd924c8bf4b4506189b0319ab/third_party/WebKit/Source/build/scripts/make_computed_style_base.py
[modify] https://crrev.com/2ee241f419323d7dd924c8bf4b4506189b0319ab/third_party/WebKit/Source/build/scripts/templates/fields/field.tmpl
[modify] https://crrev.com/2ee241f419323d7dd924c8bf4b4506189b0319ab/third_party/WebKit/Source/core/css/ComputedStyleDiffFunctions.json5
[modify] https://crrev.com/2ee241f419323d7dd924c8bf4b4506189b0319ab/third_party/WebKit/Source/core/style/ComputedStyle.cpp

Project Member

Comment 21 by bugdroid1@chromium.org, May 25 2017

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

commit f77fd909021aefd1cb49085c1f98dea3259dbba1
Author: nainar <nainar@chromium.org>
Date: Thu May 25 05:46:53 2017

Move code in ComputedStyle::UpdatePropertySpecificDifferences to generate diffing helpers

This patch moves code around in
ComputedStyle::UpdatePropertySpecificDifferences to allow us to easily
generate one diffing function helper per flag to be set on
StyleDifference.

BUG= 710938 

Review-Url: https://codereview.chromium.org/2900973004
Cr-Commit-Position: refs/heads/master@{#474571}

[modify] https://crrev.com/f77fd909021aefd1cb49085c1f98dea3259dbba1/third_party/WebKit/Source/core/style/ComputedStyle.cpp

Project Member

Comment 22 by bugdroid1@chromium.org, May 26 2017

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

commit 09743e506fb47d8ecc66a8bc72c83fb5635c2e21
Author: nainar <nainar@chromium.org>
Date: Fri May 26 03:18:32 2017

Sort all fields in alphabetical order to make the diffs deterministic

Currently the jinja template generates the properties getters/setters
in the order they appear in the group. This means changing groups will
change the location of the getter/setter making the diff generated hard
to read.
See diff here for example: https://codereview.chromium.org/2889323002

This CL sorts fields and subgroups deterministically in alphabetical
order.

Diff: https://gist.github.com/7b50d7f151021057eded58e5a405e724/revisions

BUG= 710938 

Review-Url: https://codereview.chromium.org/2905803002
Cr-Commit-Position: refs/heads/master@{#474896}

[modify] https://crrev.com/09743e506fb47d8ecc66a8bc72c83fb5635c2e21/third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.h.tmpl

Project Member

Comment 23 by bugdroid1@chromium.org, May 26 2017

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

commit 3b3e6c62644f6bf31d924c3f9256c105489a7f5d
Author: nainar <nainar@chromium.org>
Date: Fri May 26 06:04:42 2017

Ensure the diff functions are generated in order specified in the JSON file

This CL ensures that the diff functions are generated in the order that
they are specified in the JSOn input file as opposed to in the alignment
order as specified in the groups.

Diff: https://gist.github.com/ffa56bb0036497ad3a104a1e2f79d0bf/revisions

BUG= 710938 

Review-Url: https://codereview.chromium.org/2911593002
Cr-Commit-Position: refs/heads/master@{#474935}

[modify] https://crrev.com/3b3e6c62644f6bf31d924c3f9256c105489a7f5d/third_party/WebKit/Source/build/scripts/make_computed_style_base.py

Project Member

Comment 24 by bugdroid1@chromium.org, Jun 2 2017

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

commit e6cbdad3d4c498263f2ab30a2e193e3a634afb02
Author: nainar <nainar@chromium.org>
Date: Fri Jun 02 08:38:01 2017

Remove superfluous diff code in ComputedStyle::UpdatePropertySpecificDifferences

This code was generated in https://codereview.chromium.org/2897193005
and the generated function is being called too. I seem to have forgotten
to remove the handwritten code. The diff of the generated code is here:
https://gist.github.com/7db928ec119076f5526f3f6d2117d407/revisions

BUG= 710938 

Review-Url: https://codereview.chromium.org/2920943002
Cr-Commit-Position: refs/heads/master@{#476603}

[modify] https://crrev.com/e6cbdad3d4c498263f2ab30a2e193e3a634afb02/third_party/WebKit/Source/core/style/ComputedStyle.cpp

Project Member

Comment 25 by bugdroid1@chromium.org, Jun 2 2017

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

commit e6cbdad3d4c498263f2ab30a2e193e3a634afb02
Author: nainar <nainar@chromium.org>
Date: Fri Jun 02 08:38:01 2017

Remove superfluous diff code in ComputedStyle::UpdatePropertySpecificDifferences

This code was generated in https://codereview.chromium.org/2897193005
and the generated function is being called too. I seem to have forgotten
to remove the handwritten code. The diff of the generated code is here:
https://gist.github.com/7db928ec119076f5526f3f6d2117d407/revisions

BUG= 710938 

Review-Url: https://codereview.chromium.org/2920943002
Cr-Commit-Position: refs/heads/master@{#476603}

[modify] https://crrev.com/e6cbdad3d4c498263f2ab30a2e193e3a634afb02/third_party/WebKit/Source/core/style/ComputedStyle.cpp

Project Member

Comment 26 by bugdroid1@chromium.org, Jun 5 2017

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

commit 7f9edaaa091bddbff260234e49b6c2988720c5a6
Author: nainar <nainar@chromium.org>
Date: Mon Jun 05 05:05:15 2017

Remove references to rare_inherited_data_ in ComputedStyle.

To allow changes to where fields are stored, ComputedStyle code should
not refer directly to a group, as that code will break when we change
groups. This patch removes references to rare_inherited_data_ in
ComputedStyle, replacing with generated or handwritten getters.

This patch also edits a callsite of
ComputedStyle::SetTextOrientation to make it uniform with other
setters in ComputedStyle.

BUG= 710938 

Review-Url: https://codereview.chromium.org/2918983002
Cr-Commit-Position: refs/heads/master@{#476943}

[modify] https://crrev.com/7f9edaaa091bddbff260234e49b6c2988720c5a6/third_party/WebKit/Source/core/css/resolver/StyleResolverState.h
[modify] https://crrev.com/7f9edaaa091bddbff260234e49b6c2988720c5a6/third_party/WebKit/Source/core/style/ComputedStyle.cpp
[modify] https://crrev.com/7f9edaaa091bddbff260234e49b6c2988720c5a6/third_party/WebKit/Source/core/style/ComputedStyle.h

Project Member

Comment 27 by bugdroid1@chromium.org, Jun 14 2017

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

commit c4efddc762cbbfb2bce13cfcb49a1e9a589de079
Author: nainar <nainar@chromium.org>
Date: Wed Jun 14 07:13:18 2017

Generate diffs for all fields on StyleRareNonInheritedData

This patch generates diffs for all fields directly stored on
StyleRareNonInheritedData. A future patch will generate diffs for the
groups stored on StyleRareNonInheritedDatat and fields stored on those
groups.

Please note to acheive this some tests had to be turned into functions
so that they could be tested as predicates.

Diff: https://gist.github.com/nainar/024af3fe4f7e834f0342c4000aaaa57f/revisions

BUG= 710938 

Review-Url: https://codereview.chromium.org/2933303002
Cr-Commit-Position: refs/heads/master@{#479313}

[modify] https://crrev.com/c4efddc762cbbfb2bce13cfcb49a1e9a589de079/third_party/WebKit/Source/core/css/ComputedStyleDiffFunctions.json5
[modify] https://crrev.com/c4efddc762cbbfb2bce13cfcb49a1e9a589de079/third_party/WebKit/Source/core/style/ComputedStyle.cpp
[modify] https://crrev.com/c4efddc762cbbfb2bce13cfcb49a1e9a589de079/third_party/WebKit/Source/core/style/ComputedStyle.h

Project Member

Comment 28 by bugdroid1@chromium.org, Jun 14 2017

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

commit 4c9217a8f484ed4624f4ab103a2a8cdf8d8a515e
Author: nainar <nainar@chromium.org>
Date: Wed Jun 14 07:20:51 2017

Remove references to groups directly stored on ComputedStyle in .cpp file

This patch ensures that there are no references to the following groups
in ComputedStyle.cpp:
1. BoxData
2. BackgroundData
3. VisualData
4. SurroundData
5. InheritedData
6. RareInheritedData

RareNonInheritedData is left to a separate CL

Diff: https://gist.github.com/nainar/0052646dd5dbb1b50af360551122b442/revisions

BUG= 710938 

Review-Url: https://codereview.chromium.org/2938543006
Cr-Commit-Position: refs/heads/master@{#479314}

[modify] https://crrev.com/4c9217a8f484ed4624f4ab103a2a8cdf8d8a515e/third_party/WebKit/Source/core/css/ComputedStyleDiffFunctions.json5
[modify] https://crrev.com/4c9217a8f484ed4624f4ab103a2a8cdf8d8a515e/third_party/WebKit/Source/core/style/ComputedStyle.cpp

Project Member

Comment 29 by bugdroid1@chromium.org, Jun 14 2017

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

commit 4e8dd49ac7fa437b85db4086b66fe47841793191
Author: nainar <nainar@chromium.org>
Date: Wed Jun 14 07:49:12 2017

Remove duplicate entries in methods to diff and predicates to test

This patch ensures that in the case where a method/predicate depends on
multiple properties in a generated group the function only appears in
the generator once.

Diff: https://gist.github.com/nainar/c4d5c74597627e5dcde5f6caad377391/revisions

BUG= 710938 

Review-Url: https://codereview.chromium.org/2940613003
Cr-Commit-Position: refs/heads/master@{#479320}

[modify] https://crrev.com/4e8dd49ac7fa437b85db4086b66fe47841793191/third_party/WebKit/Source/build/scripts/make_computed_style_base.py

Project Member

Comment 30 by bugdroid1@chromium.org, Jun 22 2017

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

commit 85bd385ada17f3c847c075729a774a5025379209
Author: Naina Raisinghani <nainar@chromium.org>
Date: Wed Jun 21 23:59:19 2017

Create public initial functions for storage_only properties.

This patch creates public Initial functions for storage_only properties
since those don't need any special logic. These will be used in future
patches to reset properties to reduce dependencies on groups.

Diff: https://gist.github.com/nainar/8866acd15bd987a8041ec882463507ca/revisions

Bug:  710938 
Change-Id: Ia4a810dcca4a9dff52e641bc32d3fc054e471ab6
Reviewed-on: https://chromium-review.googlesource.com/542276
Commit-Queue: nainar <nainar@chromium.org>
Reviewed-by: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481361}
[modify] https://crrev.com/85bd385ada17f3c847c075729a774a5025379209/third_party/WebKit/Source/build/scripts/templates/fields/storage_only.tmpl
[modify] https://crrev.com/85bd385ada17f3c847c075729a774a5025379209/third_party/WebKit/Source/core/css/CSSProperties.json5
[modify] https://crrev.com/85bd385ada17f3c847c075729a774a5025379209/third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5

Project Member

Comment 31 by bugdroid1@chromium.org, Jun 22 2017

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

commit a048528fb5c16d84d0b0dc8eb3d3384a61671812
Author: Naina Raisinghani <nainar@chromium.org>
Date: Thu Jun 22 05:06:46 2017

Remove reference to RareNonInheritedData and subgroups from ComputedStyle

This patch generates the diff functions for subgroups on
RareNonInheritedData. It also removes references to groups in
ComputedStyle.cpp

Diff: https://gist.github.com/nainar/5181b6dbbd7181cffc869a359b57b96a/revisions

Bug:  710938 
Change-Id: If9030a551b375cbe35caf486a838e29d31950f4c
Reviewed-on: https://chromium-review.googlesource.com/542997
Commit-Queue: nainar <nainar@chromium.org>
Reviewed-by: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481445}
[modify] https://crrev.com/a048528fb5c16d84d0b0dc8eb3d3384a61671812/third_party/WebKit/Source/core/css/ComputedStyleDiffFunctions.json5
[modify] https://crrev.com/a048528fb5c16d84d0b0dc8eb3d3384a61671812/third_party/WebKit/Source/core/style/ComputedStyle.cpp
[modify] https://crrev.com/a048528fb5c16d84d0b0dc8eb3d3384a61671812/third_party/WebKit/Source/core/style/ComputedStyle.h
[modify] https://crrev.com/a048528fb5c16d84d0b0dc8eb3d3384a61671812/third_party/WebKit/Source/core/style/GridPosition.h

Project Member

Comment 32 by bugdroid1@chromium.org, Jun 22 2017

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

commit e9b00011edaaec3cba772c280806058d0d7084ff
Author: Darren Shen <shend@chromium.org>
Date: Thu Jun 22 23:48:31 2017

Remove references to some groups in ComputedStyle.h.

This patch removes direct references to:
 - StyleRareNonInheritedData
 - StyleWillChangeData
 - StyleGridItemData
 - StyleFlexibleBoxData

and replaces them with internal accessors.

Bug:  710938 
Change-Id: I757411ece02c28a3e3fb8bad0f29537594db4ebb
Reviewed-on: https://chromium-review.googlesource.com/544635
Reviewed-by: nainar <nainar@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481722}
[modify] https://crrev.com/e9b00011edaaec3cba772c280806058d0d7084ff/third_party/WebKit/Source/core/style/ComputedStyle.h

Project Member

Comment 33 by bugdroid1@chromium.org, Jun 23 2017

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

commit fd73d906631e01ab8fdd64f8c7ad9f0cc46d6767
Author: Naina Raisinghani <nainar@chromium.org>
Date: Fri Jun 23 03:11:16 2017

Remove references to some groups in ComputedStyle.h.

This patch removes references to the following groups and replaces
those accesses with direct getters/setters:
1. TransformData
2. MultiColData
3. GridData
4. ScrollSnapData
5. DeprecatedFlexibleBoxData

Bug:  710938 
Change-Id: I7ea899d943beae130f3289f7fba37d6f4e3eb6f2
Reviewed-on: https://chromium-review.googlesource.com/544715
Commit-Queue: nainar <nainar@chromium.org>
Reviewed-by: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481797}
[modify] https://crrev.com/fd73d906631e01ab8fdd64f8c7ad9f0cc46d6767/third_party/WebKit/Source/core/css/ComputedStyleDiffFunctions.json5
[modify] https://crrev.com/fd73d906631e01ab8fdd64f8c7ad9f0cc46d6767/third_party/WebKit/Source/core/style/ComputedStyle.h

Project Member

Comment 34 by bugdroid1@chromium.org, Jun 23 2017

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

commit 82e425e14a72daaa367c9761e486f6c2589f41c5
Author: Naina Raisinghani <nainar@chromium.org>
Date: Fri Jun 23 03:22:21 2017

Make all groups on ComputedStyle private

This patch makes all subgroups on ComputedStyle private to ensure that
no function depends on the way grouping happens in ComputedStyle.

Diff: https://gist.github.com/nainar/13876826757ad4d2c99520f99a8ac875/revisions

Bug:  710938 
Change-Id: I2d2d7d5cff3fc64361b9bf42c4ea2496f3130f7f
Reviewed-on: https://chromium-review.googlesource.com/544450
Reviewed-by: Darren Shen <shend@chromium.org>
Commit-Queue: nainar <nainar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481803}
[modify] https://crrev.com/82e425e14a72daaa367c9761e486f6c2589f41c5/third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.h.tmpl
[modify] https://crrev.com/82e425e14a72daaa367c9761e486f6c2589f41c5/third_party/WebKit/Source/core/style/ComputedStyle.h

Status: Fixed (was: Assigned)
This is done. 

Accessors for all fields have been generated. 
No diff function depends on properties being on a certain group. 
diff functions are now generated from a JSON file. 

For any further questions please raise a bug or email shend@chromium.org and nainar@chromium.org.

Sign in to add a comment