Use getters for StyleSurroundData members in ComputedStyle.cpp

Use getters for StyleSurroundData members in ComputedStyle.cpp

In ComputedStyle, we directly access StyleSurroundData members via the
m_surround pointer (e.g. m_surround->m_border), but this assumes that
these members will always be stored in StyleSurroundData. As part of
clockwork, we may change where these members are stored, so this patch
uses getter methods instead, which are independent of where these
members are stored.

Places not affected by this patch:
- Getters/setters, since these will be eventually generated.
- When the surround pointer is used directly (e.g. for fast
  comparisons), since these are unavoidable.
- Modifying variables via m_surround.access(), since they're
  setters not getters. Setters will be refactored in a separate
  patch.

BUG=628043
Review-Url: https://codereview.chromium.org/2785343002
Cr-Commit-Position: refs/heads/master@{#465119}
Committed: https://chromium.googlesource.com/chromium/src/+/20be06ee11375df67a4bcdb0e9716fbb343f2e48

[shend, 2017-03-31 00:26:56]

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

[shend, 2017-03-31 00:26:56]

Hi Bugs, PTAL :)

[Bugs Nash, 2017-04-03 00:12:48]

Good stuff, this is better code anyways

There are more instances of m_surround members being accessed in ComputedStyle.h
and .cpp files than you've converted in this patch, is there a reason why not
all of them have been done?

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

https://codereview.chromium.org/2785343002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/style/ComputedStyle.cpp:590: if (margin() !=
other.margin() || !offsetEqual(other) ||
what methods are these margin() and padding() calling to? They don't seem to
exist in ComputedStyle

[shend, 2017-04-03 00:55:12]

Description was changed from

==========
Use getters for StyleSurroundData members inside ComputedStyle.

In ComputedStyle, we directly access StyleSurroundData members via the
m_surround pointer (e.g. m_surround->m_border), but this assumes that
these members will always be stored in StyleSurroundData. As part of
clockwork, we may change where these members are stored, so this patch
uses getter methods instead, which are independent of where these
members are stored.

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

to

==========
Use getters for StyleSurroundData members inside ComputedStyle.

In ComputedStyle, we directly access StyleSurroundData members via the
m_surround pointer (e.g. m_surround->m_border), but this assumes that
these members will always be stored in StyleSurroundData. As part of
clockwork, we may change where these members are stored, so this patch
uses getter methods instead, which are independent of where these
members are stored.

Places not affected by this patch:
- ComputedStyle.h, since this code will be eventually generated.
- When the surround pointer is used directly (e.g. for fast
  comparisons), since these are unavoidable.
- Modifying variables via m_surround.access(), since they're
  setters not getters. Setters will be refactored in a separate
  patch.

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

[shend, 2017-04-03 00:55:40]

Description was changed from

==========
Use getters for StyleSurroundData members inside ComputedStyle.

In ComputedStyle, we directly access StyleSurroundData members via the
m_surround pointer (e.g. m_surround->m_border), but this assumes that
these members will always be stored in StyleSurroundData. As part of
clockwork, we may change where these members are stored, so this patch
uses getter methods instead, which are independent of where these
members are stored.

Places not affected by this patch:
- ComputedStyle.h, since this code will be eventually generated.
- When the surround pointer is used directly (e.g. for fast
  comparisons), since these are unavoidable.
- Modifying variables via m_surround.access(), since they're
  setters not getters. Setters will be refactored in a separate
  patch.

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

to

==========
Use getters for StyleSurroundData members in ComputedStyle.cpp

In ComputedStyle, we directly access StyleSurroundData members via the
m_surround pointer (e.g. m_surround->m_border), but this assumes that
these members will always be stored in StyleSurroundData. As part of
clockwork, we may change where these members are stored, so this patch
uses getter methods instead, which are independent of where these
members are stored.

Places not affected by this patch:
- ComputedStyle.h, since this code will be eventually generated.
- When the surround pointer is used directly (e.g. for fast
  comparisons), since these are unavoidable.
- Modifying variables via m_surround.access(), since they're
  setters not getters. Setters will be refactored in a separate
  patch.

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

[shend, 2017-04-03 00:58:48]

> There are more instances of m_surround members being accessed in
ComputedStyle.h and .cpp files than you've converted in this patch, is there a
reason why not all of them have been done?

Sorry for the confusion, I've updated the CL description with a list of things
not affected by this patch and reasons. Other than those, let me know if I
missed any members :)

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

https://codereview.chromium.org/2785343002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/style/ComputedStyle.cpp:590: if (margin() !=
other.margin() || !offsetEqual(other) ||
On 2017/04/03 at 00:12:48, Bugs Nash wrote:
> what methods are these margin() and padding() calling to? They don't seem to
exist in ComputedStyle

It's being added in a parent patch (https://codereview.chromium.org/2787183002).
They just return m_surround->m_margin and m_surround->m_padding.

[Bugs Nash, 2017-04-03 03:54:37]

On 2017/04/03 at 00:58:48, shend wrote:
> > There are more instances of m_surround members being accessed in
ComputedStyle.h and .cpp files than you've converted in this patch, is there a
reason why not all of them have been done?
> 
> Sorry for the confusion, I've updated the CL description with a list of things
not affected by this patch and reasons. Other than those, let me know if I
missed any members :)
> 
>
https://codereview.chromium.org/2785343002/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/style/ComputedStyle.cpp (right):
> 
>
https://codereview.chromium.org/2785343002/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/style/ComputedStyle.cpp:590: if (margin() !=
other.margin() || !offsetEqual(other) ||
> On 2017/04/03 at 00:12:48, Bugs Nash wrote:
> > what methods are these margin() and padding() calling to? They don't seem to
exist in ComputedStyle
> 
> It's being added in a parent patch
(https://codereview.chromium.org/2787183002). They just return
m_surround->m_margin and m_surround->m_padding.

oohh sorry I missed the depends on patch there. That still doesn't seem to have
a margin() method though?

[shend, 2017-04-03 04:03:22]

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

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

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

[Bugs Nash, 2017-04-03 04:04:55]

On 2017/04/03 at 03:54:37, Bugs Nash wrote:
> On 2017/04/03 at 00:58:48, shend wrote:
> > > There are more instances of m_surround members being accessed in
ComputedStyle.h and .cpp files than you've converted in this patch, is there a
reason why not all of them have been done?
> > 
> > Sorry for the confusion, I've updated the CL description with a list of
things not affected by this patch and reasons. Other than those, let me know if
I missed any members :)
> > 
> >
https://codereview.chromium.org/2785343002/diff/1/third_party/WebKit/Source/c...
> > File third_party/WebKit/Source/core/style/ComputedStyle.cpp (right):
> > 
> >
https://codereview.chromium.org/2785343002/diff/1/third_party/WebKit/Source/c...
> > third_party/WebKit/Source/core/style/ComputedStyle.cpp:590: if (margin() !=
other.margin() || !offsetEqual(other) ||
> > On 2017/04/03 at 00:12:48, Bugs Nash wrote:
> > > what methods are these margin() and padding() calling to? They don't seem
to exist in ComputedStyle
> > 
> > It's being added in a parent patch
(https://codereview.chromium.org/2787183002). They just return
m_surround->m_margin and m_surround->m_padding.
> 
> oohh sorry I missed the depends on patch there. That still doesn't seem to
have a margin() method though?

wait, the depends on patch does add the margin() method, it just didn't say so
in the description. lgtm!

[shend, 2017-04-03 04:05:11]

On 2017/04/03 at 03:54:37, bugsnash wrote:
> On 2017/04/03 at 00:58:48, shend wrote:
> > > There are more instances of m_surround members being accessed in
ComputedStyle.h and .cpp files than you've converted in this patch, is there a
reason why not all of them have been done?
> > 
> > Sorry for the confusion, I've updated the CL description with a list of
things not affected by this patch and reasons. Other than those, let me know if
I missed any members :)
> > 
> >
https://codereview.chromium.org/2785343002/diff/1/third_party/WebKit/Source/c...
> > File third_party/WebKit/Source/core/style/ComputedStyle.cpp (right):
> > 
> >
https://codereview.chromium.org/2785343002/diff/1/third_party/WebKit/Source/c...
> > third_party/WebKit/Source/core/style/ComputedStyle.cpp:590: if (margin() !=
other.margin() || !offsetEqual(other) ||
> > On 2017/04/03 at 00:12:48, Bugs Nash wrote:
> > > what methods are these margin() and padding() calling to? They don't seem
to exist in ComputedStyle
> > 
> > It's being added in a parent patch
(https://codereview.chromium.org/2787183002). They just return
m_surround->m_margin and m_surround->m_padding.
> 
> oohh sorry I missed the depends on patch there. That still doesn't seem to
have a margin() method though?

It's hidden away in a bunch of other margin methods :P The parent patch has
landed, so you can see the method here:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/style/Com...

[commit-bot: I haz the power, 2017-04-03 05:24:20]

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

[commit-bot: I haz the power, 2017-04-03 05:24:21]

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

[shend, 2017-04-03 23:37:28]

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

[shend, 2017-04-03 23:37:37]

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

[shend, 2017-04-03 23:37:37]

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

[shend, 2017-04-03 23:37:37]

Hi Eddy, PTAL

[shend, 2017-04-03 23:37:37]

Hi Eddy, PTAL

[commit-bot: I haz the power, 2017-04-03 23:37:58]

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-04 00:39:52]

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

[commit-bot: I haz the power, 2017-04-04 00:39:53]

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

[meade_UTC10, 2017-04-04 01:06:33]

lgtm

[shend, 2017-04-04 03:07:23]

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

[commit-bot: I haz the power, 2017-04-04 03:07:47]

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-04 04:52:41]

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

[commit-bot: I haz the power, 2017-04-04 04:52:42]

Dry run: This issue passed the CQ dry run.

[shend, 2017-04-17 23:57:31]

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

[commit-bot: I haz the power, 2017-04-17 23:57:56]

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 00:03:17]

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

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

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_tsan_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...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[shend, 2017-04-18 01:16:10]

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

[shend, 2017-04-18 01:16:32]

Description was changed from

==========
Use getters for StyleSurroundData members in ComputedStyle.cpp

In ComputedStyle, we directly access StyleSurroundData members via the
m_surround pointer (e.g. m_surround->m_border), but this assumes that
these members will always be stored in StyleSurroundData. As part of
clockwork, we may change where these members are stored, so this patch
uses getter methods instead, which are independent of where these
members are stored.

Places not affected by this patch:
- ComputedStyle.h, since this code will be eventually generated.
- When the surround pointer is used directly (e.g. for fast
  comparisons), since these are unavoidable.
- Modifying variables via m_surround.access(), since they're
  setters not getters. Setters will be refactored in a separate
  patch.

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

to

==========
Use getters for StyleSurroundData members in ComputedStyle.cpp

In ComputedStyle, we directly access StyleSurroundData members via the
m_surround pointer (e.g. m_surround->m_border), but this assumes that
these members will always be stored in StyleSurroundData. As part of
clockwork, we may change where these members are stored, so this patch
uses getter methods instead, which are independent of where these
members are stored.

Places not affected by this patch:
- Getters/setters, since these will be eventually generated.
- When the surround pointer is used directly (e.g. for fast
  comparisons), since these are unavoidable.
- Modifying variables via m_surround.access(), since they're
  setters not getters. Setters will be refactored in a separate
  patch.

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

[commit-bot: I haz the power, 2017-04-18 01:16:49]

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 02:48:15]

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

[commit-bot: I haz the power, 2017-04-18 02:48:16]

Dry run: This issue passed the CQ dry run.

[shend, 2017-04-18 02:50:05]

The CQ bit was checked by shend@chromium.org

[shend, 2017-04-18 02:50:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
bugsnash@chromium.org, meade@chromium.org
Link to the patchset: https://codereview.chromium.org/2785343002/#ps60001
(title: "Rebase")

[commit-bot: I haz the power, 2017-04-18 02:50: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-18 02:53:50]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1492483805547110,
"parent_rev": "a6c1b9d6d636b72d9b88cb2ab5723be4770d006f", "commit_rev":
"20be06ee11375df67a4bcdb0e9716fbb343f2e48"}

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

Description was changed from

==========
Use getters for StyleSurroundData members in ComputedStyle.cpp

In ComputedStyle, we directly access StyleSurroundData members via the
m_surround pointer (e.g. m_surround->m_border), but this assumes that
these members will always be stored in StyleSurroundData. As part of
clockwork, we may change where these members are stored, so this patch
uses getter methods instead, which are independent of where these
members are stored.

Places not affected by this patch:
- Getters/setters, since these will be eventually generated.
- When the surround pointer is used directly (e.g. for fast
  comparisons), since these are unavoidable.
- Modifying variables via m_surround.access(), since they're
  setters not getters. Setters will be refactored in a separate
  patch.

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

to

==========
Use getters for StyleSurroundData members in ComputedStyle.cpp

In ComputedStyle, we directly access StyleSurroundData members via the
m_surround pointer (e.g. m_surround->m_border), but this assumes that
these members will always be stored in StyleSurroundData. As part of
clockwork, we may change where these members are stored, so this patch
uses getter methods instead, which are independent of where these
members are stored.

Places not affected by this patch:
- Getters/setters, since these will be eventually generated.
- When the surround pointer is used directly (e.g. for fast
  comparisons), since these are unavoidable.
- Modifying variables via m_surround.access(), since they're
  setters not getters. Setters will be refactored in a separate
  patch.

BUG=628043

Review-Url: https://codereview.chromium.org/2785343002
Cr-Commit-Position: refs/heads/master@{#465119}
Committed:
https://chromium.googlesource.com/chromium/src/+/20be06ee11375df67a4bcdb0e971...
==========

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

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/20be06ee11375df67a4bcdb0e971...