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

Issue 2705143002: Split StyleSurroundData::offset into separate members. (Closed)

Created:
3 years, 10 months ago by shend
Modified:
3 years, 8 months ago
Reviewers:
meade_UTC10, haraken, nainar
CC:
blink-reviews, blink-reviews-style_chromium.org, chromium-reviews, kinuko+watch
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Split StyleSurroundData::offset into separate members. In generating ComputedStyle, every generated data member must correspond to a CSS property (whether long hand or short hand). StyleSurroundData is part of ComputedStyle and we would like to generate it as well. Unfortunately, the 'left', 'right', 'top' and 'bottom' CSS properties are grouped as one data member 'offset', which does not actually correspond to either a long or short hand CSS property, unlike 'margin' and 'padding'. Thus, we cannot generate this data member directly. This patch splits 'offset' into 'left', 'right', 'top' and 'bottom' so that every data member in StyleSurroundData corresponds to a CSS property. This allows us to generate it in a later patch. BUG=628043 Review-Url: https://codereview.chromium.org/2705143002 Cr-Commit-Position: refs/heads/master@{#461356} Committed: https://chromium.googlesource.com/chromium/src/+/542dc4c9f70f7c9e1a3ec096c6b62bceceb5e47f

Patch Set 1 #

Total comments: 1

Patch Set 2 : Stay with static #

Patch Set 3 : Rebase #

Patch Set 4 : Fix typo #

Unified diffs Side-by-side diffs Delta from patch set Stats (+86 lines, -34 lines) Patch
M third_party/WebKit/Source/core/style/ComputedStyle.h View 1 2 3 2 chunks +16 lines, -12 lines 0 comments Download
M third_party/WebKit/Source/core/style/ComputedStyle.cpp View 1 2 3 2 chunks +2 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/style/StyleSurroundData.h View 1 chunk +4 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/style/StyleSurroundData.cpp View 1 2 3 1 chunk +6 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/platform/LengthBox.h View 1 1 chunk +16 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/LengthBox.cpp View 1 chunk +42 lines, -16 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 28 (15 generated)
shend
Hi Naina, PTAL
3 years, 10 months ago (2017-02-21 00:55:44 UTC) #2
nainar
https://codereview.chromium.org/2705143002/diff/1/third_party/WebKit/Source/platform/LengthBox.h File third_party/WebKit/Source/platform/LengthBox.h (right): https://codereview.chromium.org/2705143002/diff/1/third_party/WebKit/Source/platform/LengthBox.h#newcode72 third_party/WebKit/Source/platform/LengthBox.h:72: // TODO(shend): these are independent of LengthBox, make them ...
3 years, 10 months ago (2017-02-21 02:28:37 UTC) #3
shend
On 2017/02/21 at 02:28:37, nainar wrote: > https://codereview.chromium.org/2705143002/diff/1/third_party/WebKit/Source/platform/LengthBox.h > File third_party/WebKit/Source/platform/LengthBox.h (right): > > https://codereview.chromium.org/2705143002/diff/1/third_party/WebKit/Source/platform/LengthBox.h#newcode72 ...
3 years, 10 months ago (2017-02-21 02:31:17 UTC) #4
nainar
On 2017/02/21 at 02:31:17, shend wrote: > On 2017/02/21 at 02:28:37, nainar wrote: > > ...
3 years, 10 months ago (2017-02-21 03:16:48 UTC) #5
shend
On 2017/02/21 at 03:16:48, nainar wrote: > On 2017/02/21 at 02:31:17, shend wrote: > > ...
3 years, 10 months ago (2017-02-21 03:46:18 UTC) #6
shend
On 2017/02/21 at 03:16:48, nainar wrote: > On 2017/02/21 at 02:31:17, shend wrote: > > ...
3 years, 10 months ago (2017-02-21 21:31:47 UTC) #7
nainar
On 2017/02/21 at 21:31:47, shend wrote: > On 2017/02/21 at 03:16:48, nainar wrote: > > ...
3 years, 10 months ago (2017-02-21 22:53:19 UTC) #8
shend
Hi Eddy, PTAL and WDYT of the static methods?
3 years, 10 months ago (2017-02-22 00:18:39 UTC) #10
meade_UTC10
lgtm Sorry for being so slow on this review. I think this seems fine. I'm ...
3 years, 9 months ago (2017-03-01 06:32:16 UTC) #11
shend
Hi haraken, can you PTAL for platform/LengthBox? Specifically, I'm duplicating some LengthBox methods as static ...
3 years, 8 months ago (2017-03-30 03:19:46 UTC) #13
haraken
On 2017/03/30 03:19:46, shend wrote: > Hi haraken, can you PTAL for platform/LengthBox? Specifically, I'm ...
3 years, 8 months ago (2017-03-30 14:08:26 UTC) #14
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/2705143002/60001
3 years, 8 months ago (2017-04-03 01:06:19 UTC) #25
commit-bot: I haz the power
3 years, 8 months ago (2017-04-03 01:12:18 UTC) #28
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/542dc4c9f70f7c9e1a3ec096c6b6...

Powered by Google App Engine
This is Rietveld 408576698