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

Issue 2921543002: Make ComputedStyleBase use MemberCopy when copying pointer fields. (Closed)

Created:
3 years, 6 months ago by shend
Modified:
3 years, 6 months ago
CC:
blink-reviews, blink-reviews-style_chromium.org, chromium-reviews
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Make ComputedStyleBase use DataCopy when copying pointer fields. ComputedStyleBase has a constructor that copies every field using the default copy behaviour. However, when we generate RareNonInheritedData, some of the pointer fields require special copying behaviour (e.g. those stored in a std::unique_ptr). To inject custom copying behaviour, we copy pointer fields with a new function called MemberCopy. MemberCopy is a function that takes a variable of some type and returns a copy of it. It can be overloaded to have custom copying behaviour for some type. Since ComputedStyleBase currently has no special copying behaviour, we simply overload MemberCopy with RefPtr and Persistent to use the default copying behaviour. When we generate RareNonInheritedData, we will need to introduce overloads for std::unique_ptr and DataPersistent. This patch does not introduce changes in behaviour. Diff of generated files: https://gist.github.com/7a2b410da029e4ed71af5509c511cba2/revisions BUG=628043 Review-Url: https://codereview.chromium.org/2921543002 Cr-Commit-Position: refs/heads/master@{#477206} Committed: https://chromium.googlesource.com/chromium/src/+/ba2889de7991c1f5162cd12dd6cc6e4de3bc3ac0

Patch Set 1 #

Patch Set 2 : Address comments #

Patch Set 3 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+31 lines, -0 lines) Patch
M third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.h.tmpl View 1 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/build/scripts/templates/fields/group.tmpl View 1 1 chunk +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/style/BUILD.gn View 1 1 chunk +1 line, -0 lines 0 comments Download
A third_party/WebKit/Source/core/style/MemberCopy.h View 1 1 chunk +25 lines, -0 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 28 (20 generated)
shend
Hi Alan, PTAL
3 years, 6 months ago (2017-06-01 03:43:54 UTC) #7
alancutter (OOO until 2018)
Please mention in the description if a change has no change in behaviour. The name ...
3 years, 6 months ago (2017-06-02 01:16:38 UTC) #8
shend
On 2017/06/02 at 01:16:38, alancutter wrote: > Please mention in the description if a change ...
3 years, 6 months ago (2017-06-02 01:33:37 UTC) #10
alancutter (OOO until 2018)
On 2017/06/02 at 01:33:37, shend wrote: > On 2017/06/02 at 01:16:38, alancutter wrote: > > ...
3 years, 6 months ago (2017-06-02 03:07:39 UTC) #11
shend
On 2017/06/02 at 03:07:39, alancutter wrote: > On 2017/06/02 at 01:33:37, shend wrote: > > ...
3 years, 6 months ago (2017-06-06 03:54:43 UTC) #22
alancutter (OOO until 2018)
lgtm
3 years, 6 months ago (2017-06-06 05:33:41 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2921543002/40001
3 years, 6 months ago (2017-06-06 05:45:00 UTC) #25
commit-bot: I haz the power
3 years, 6 months ago (2017-06-06 05:49:07 UTC) #28
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/ba2889de7991c1f5162cd12dd6cc...

Powered by Google App Engine
This is Rietveld 408576698