Split StyleSurroundData::margin into four individual Lengths.

Split StyleSurroundData::margin into four individual Lengths.

In StyleSurroundData, the margin longhand properties are stored in a
single LengthBox. This makes it difficult to generate because the
generator assumes that properties don't share storage.

This patch splits the LengthBox margin into four Lengths: margin_left,
margin_right, margin_top, margin_bottom. This enables us to generate
the margin properties properly.

This patch also adds a MarginEqual helper function to check if two
margins have the same value.

BUG=628043
Review-Url: https://codereview.chromium.org/2824973002
Cr-Commit-Position: refs/heads/master@{#466233}
Committed: https://chromium.googlesource.com/chromium/src/+/786615ba373b0e7a6e488364c49f2cf56942de3f

[shend, 2017-04-18 01:53:59]

shend@chromium.org changed reviewers:
+ nainar@chromium.org

[shend, 2017-04-18 01:53:59]

Hi Naina, PTAL :)

[shend, 2017-04-18 01:54:03]

The CQ bit was checked by shend@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-18 01:54:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-18 03:54:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-18 03:54:18]

Dry run: This issue passed the CQ dry run.

[nainar, 2017-04-18 06:58:49]

lgtm

https://codereview.chromium.org/2824973002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/style/ComputedStyle.h (right):

https://codereview.chromium.org/2824973002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/style/ComputedStyle.h:2866: bool
MarginEqual(const ComputedStyle& other) const {
Please mention in the description that you are adding this function

[shend, 2017-04-18 07:25:52]

Description was changed from

==========
Split StyleSurroundData::margin into four individual Lengths.

In StyleSurroundData, the margin longhand properties are stored in a
single LengthBox. This makes it difficult to generate because the
generator assumes that properties don't share storage.

This patch splits the LengthBox margin into four Lengths: margin_left,
margin_right, margin_top, margin_bottom. This enables us to generate
the margin properties properly.

BUG=628043
==========

to

==========
Split StyleSurroundData::margin into four individual Lengths.

In StyleSurroundData, the margin longhand properties are stored in a
single LengthBox. This makes it difficult to generate because the
generator assumes that properties don't share storage.

This patch splits the LengthBox margin into four Lengths: margin_left,
margin_right, margin_top, margin_bottom. This enables us to generate
the margin properties properly.

This patch also adds a MarginEqual helper function to check if two
margins have the same value.

BUG=628043
==========

[shend, 2017-04-18 07:54:25]

shend@chromium.org changed reviewers:
+ alancutter@chromium.org

[shend, 2017-04-18 07:54:25]

Hi Alan, PTAL

https://codereview.chromium.org/2824973002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/style/ComputedStyle.h (right):

https://codereview.chromium.org/2824973002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/style/ComputedStyle.h:2866: bool
MarginEqual(const ComputedStyle& other) const {
On 2017/04/18 at 06:58:49, nainar wrote:
> Please mention in the description that you are adding this function

Done.

[alancutter (OOO until 2018), 2017-04-19 07:01:21]

lgtm

https://codereview.chromium.org/2824973002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/style/StyleSurroundData.cpp (right):

https://codereview.chromium.org/2824973002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/style/StyleSurroundData.cpp:51: border_ ==
o.border_;
Nit: Let's group related properties with parentheses to make it a bit more
readable.

https://codereview.chromium.org/2824973002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/LengthBox.cpp (right):

https://codereview.chromium.org/2824973002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/LengthBox.cpp:111: }
Thought: I don't think layout logic belongs in this container class, it should
probably live in platform/LengthFunctions.h instead. No action required for this
patch.

[shend, 2017-04-19 07:19:24]

https://codereview.chromium.org/2824973002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/style/StyleSurroundData.cpp (right):

https://codereview.chromium.org/2824973002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/style/StyleSurroundData.cpp:51: border_ ==
o.border_;
On 2017/04/19 at 07:01:21, alancutter wrote:
> Nit: Let's group related properties with parentheses to make it a bit more
readable.

Done.

https://codereview.chromium.org/2824973002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/LengthBox.cpp (right):

https://codereview.chromium.org/2824973002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/LengthBox.cpp:111: }
On 2017/04/19 at 07:01:21, alancutter wrote:
> Thought: I don't think layout logic belongs in this container class, it should
probably live in platform/LengthFunctions.h instead. No action required for this
patch.

Aha, I was looking for a better place to put these.

[shend, 2017-04-19 22:50:42]

The CQ bit was checked by shend@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-19 22:51:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-19 22:55:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-19 22:55:05]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[shend, 2017-04-19 23:00:50]

The CQ bit was checked by shend@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-19 23:01:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-20 00:43:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-20 00:43:43]

Dry run: This issue passed the CQ dry run.

[shend, 2017-04-20 01:53:26]

The CQ bit was checked by shend@chromium.org

[shend, 2017-04-20 01:53:26]

The patchset sent to the CQ was uploaded after l-g-t-m from nainar@chromium.org,
alancutter@chromium.org
Link to the patchset: https://codereview.chromium.org/2824973002/#ps40001
(title: "Rebase")

[commit-bot: I haz the power, 2017-04-20 01:54:06]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-20 02:02:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-20 02:02:36]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[shend, 2017-04-20 02:47:42]

shend@chromium.org changed reviewers:
+ haraken@chromium.org

[shend, 2017-04-20 02:47:42]

Hi Haraken, PTAL for platform/

[haraken, 2017-04-20 07:43:39]

On 2017/04/20 02:47:42, shend wrote:
> Hi Haraken, PTAL for platform/

LGTM

[shend, 2017-04-20 23:58:42]

The CQ bit was checked by shend@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-20 23:59:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-21 01:23:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-21 01:23:27]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[shend, 2017-04-21 01:39:55]

The CQ bit was checked by shend@chromium.org

[commit-bot: I haz the power, 2017-04-21 01:40:29]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-21 02:26:17]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1492738795815920,
"parent_rev": "3e769db4f707bbc5962ee1aa0a1c727081ee754a", "commit_rev":
"786615ba373b0e7a6e488364c49f2cf56942de3f"}

[commit-bot: I haz the power, 2017-04-21 02:26:27]

Description was changed from

==========
Split StyleSurroundData::margin into four individual Lengths.

In StyleSurroundData, the margin longhand properties are stored in a
single LengthBox. This makes it difficult to generate because the
generator assumes that properties don't share storage.

This patch splits the LengthBox margin into four Lengths: margin_left,
margin_right, margin_top, margin_bottom. This enables us to generate
the margin properties properly.

This patch also adds a MarginEqual helper function to check if two
margins have the same value.

BUG=628043
==========

to

==========
Split StyleSurroundData::margin into four individual Lengths.

In StyleSurroundData, the margin longhand properties are stored in a
single LengthBox. This makes it difficult to generate because the
generator assumes that properties don't share storage.

This patch splits the LengthBox margin into four Lengths: margin_left,
margin_right, margin_top, margin_bottom. This enables us to generate
the margin properties properly.

This patch also adds a MarginEqual helper function to check if two
margins have the same value.

BUG=628043

Review-Url: https://codereview.chromium.org/2824973002
Cr-Commit-Position: refs/heads/master@{#466233}
Committed:
https://chromium.googlesource.com/chromium/src/+/786615ba373b0e7a6e488364c49f...
==========

[commit-bot: I haz the power, 2017-04-21 02:26:28]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/786615ba373b0e7a6e488364c49f...