Make ComputedStyle::InheritFrom group independent.

Make ComputedStyle::InheritFrom group independent.

In ComputedStyle::InheritFrom, we assume that the UserModify() field is
always stored in RareInheritedData. After we generate the user-modify
property, we might want to change where it is stored, so UserModify may
no longer be in RareInheritedData. This patch rearranges the logic in
the function so that UserModify can be stored in any subgroup without
breaking the code. There are no changes in behaviour.

BUG=628043
Review-Url: https://codereview.chromium.org/2829773003
Cr-Commit-Position: refs/heads/master@{#471095}
Committed: https://chromium.googlesource.com/chromium/src/+/2389662b9fb0735446c5c8235bcdc1b0ff505366

[shend, 2017-04-20 04:09:45]

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

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

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 04:13:34]

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

[commit-bot: I haz the power, 2017-04-20 04:13:34]

Dry run: 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...)
  linux_chromium_chromeos_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-20 04:28:29]

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

[commit-bot: I haz the power, 2017-04-20 04:28:44]

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 07:09:51]

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

[commit-bot: I haz the power, 2017-04-20 07:09:51]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[shend, 2017-04-20 07:12:33]

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

[shend, 2017-04-20 07:12:34]

Hi Naina, PTAL

[nainar, 2017-04-20 07:16:28]

lgtm

[shend, 2017-04-24 01:00:38]

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

[commit-bot: I haz the power, 2017-04-24 01:00:52]

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-24 01:03:58]

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

[commit-bot: I haz the power, 2017-04-24 01:03:59]

Dry run: Try jobs failed on following builders:
  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-...)
  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_...)

[shend, 2017-04-25 23:52:43]

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

[commit-bot: I haz the power, 2017-04-25 23:53: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-26 00:08:47]

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

[commit-bot: I haz the power, 2017-04-26 00:08:49]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  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-...)
  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-26 00:33:08]

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

[commit-bot: I haz the power, 2017-04-26 00:33:54]

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-26 02:08:38]

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

[commit-bot: I haz the power, 2017-04-26 02:08:39]

Dry run: Try jobs failed on following builders:
  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-27 03:20:43]

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

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

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-27 05:10:01]

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

[commit-bot: I haz the power, 2017-04-27 05:10:02]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[shend, 2017-04-28 05:27:40]

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

[shend, 2017-04-28 05:27:40]

Hi Alan, PTAL

[shend, 2017-05-07 22:42:11]

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

[commit-bot: I haz the power, 2017-05-07 22:42:17]

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-05-07 22:45:06]

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

[commit-bot: I haz the power, 2017-05-07 22:45:07]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[shend, 2017-05-08 00:30:03]

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

[commit-bot: I haz the power, 2017-05-08 00:30:16]

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-05-08 01:45:37]

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

[commit-bot: I haz the power, 2017-05-08 01:45:37]

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

[shend, 2017-05-08 05:14:08]

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

[commit-bot: I haz the power, 2017-05-08 05:14:21]

Dry run: CQ is trying da patch.

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

[alancutter (OOO until 2018), 2017-05-08 07:22:55]

Does this fix a bug? Are there any changes in behaviour?
It seems like this change is in preparation for another patch that breaks the
assumption being removed. Please make it clear why we are doing this in the
description.

[commit-bot: I haz the power, 2017-05-08 07:47:07]

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

[commit-bot: I haz the power, 2017-05-08 07:47:08]

Dry run: This issue passed the CQ dry run.

[shend, 2017-05-08 22:17:38]

Description was changed from

==========
Make ComputedStyle::InheritFrom group independent.

In ComputedStyle::InheritFrom, we assume that the UserModify() field is
always stored in RareInheritedData. This patch removes this assumption
by rearranging the logic in the function.

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

to

==========
Make ComputedStyle::InheritFrom group independent.

In ComputedStyle::InheritFrom, we assume that the UserModify() field is
always stored in RareInheritedData. After we generate the user-modify
property, we might want to change where it is stored, so UserModify may
no longer be in RareInheritedData. This patch rearranges the logic in
the function so that UserModify can be stored in any subgroup without
breaking the code. There are no changes in behaviour.

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

[shend, 2017-05-08 22:17:59]

On 2017/05/08 at 07:22:55, alancutter wrote:
> Does this fix a bug? Are there any changes in behaviour?
> It seems like this change is in preparation for another patch that breaks the
assumption being removed. Please make it clear why we are doing this in the
description.

Oops yeah that was a bad CL description. PTAL again?

[alancutter (OOO until 2018), 2017-05-09 00:09:18]

lgtm

https://codereview.chromium.org/2829773003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/style/ComputedStyle.cpp (right):

https://codereview.chromium.org/2829773003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/style/ComputedStyle.cpp:317:
ComputedStyleBase::InheritFrom(inherit_parent, is_at_shadow_boundary);
Unrelated to this patch: Why does this function take is_at_shadow_boundary? It
doesn't appear to use it.

[shend, 2017-05-09 00:16:02]

https://codereview.chromium.org/2829773003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/style/ComputedStyle.cpp (right):

https://codereview.chromium.org/2829773003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/style/ComputedStyle.cpp:317:
ComputedStyleBase::InheritFrom(inherit_parent, is_at_shadow_boundary);
On 2017/05/09 at 00:09:18, alancutter wrote:
> Unrelated to this patch: Why does this function take is_at_shadow_boundary? It
doesn't appear to use it.

I think ComputedStyleBase::InheritFrom was created as a direct copy of
ComputedStyle::InheritFrom, in case we want to replace ComputedStyle with
ComputedStyleBase. IMO, such an endeavour is very complicated and I don't think
it's worth. So in the near future, no, is_at_shadow_boundary does not need to be
in ComputedStyleBase::InheritFrom.

[shend, 2017-05-09 04:01:32]

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

[commit-bot: I haz the power, 2017-05-09 04:02: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-05-09 07:21:03]

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

[commit-bot: I haz the power, 2017-05-09 07:21:04]

Dry run: This issue passed the CQ dry run.

[shend, 2017-05-11 02:44:28]

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

[commit-bot: I haz the power, 2017-05-11 02:46:13]

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-05-11 04:45:16]

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

[commit-bot: I haz the power, 2017-05-11 04:45:17]

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-05-11 08:17:26]

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

[commit-bot: I haz the power, 2017-05-11 08:17:43]

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-05-11 11:49:13]

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

[commit-bot: I haz the power, 2017-05-11 11:49:14]

Dry run: This issue passed the CQ dry run.

[shend, 2017-05-11 22:18:47]

The CQ bit was checked by shend@chromium.org

[shend, 2017-05-11 22:18:48]

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

[commit-bot: I haz the power, 2017-05-11 22:19:57]

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-05-11 22:36:07]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1494541127887860,
"parent_rev": "2a03a77423e306ac78fcd89eaf09964153d7599a", "commit_rev":
"2389662b9fb0735446c5c8235bcdc1b0ff505366"}

[commit-bot: I haz the power, 2017-05-11 22:36:23]

Description was changed from

==========
Make ComputedStyle::InheritFrom group independent.

In ComputedStyle::InheritFrom, we assume that the UserModify() field is
always stored in RareInheritedData. After we generate the user-modify
property, we might want to change where it is stored, so UserModify may
no longer be in RareInheritedData. This patch rearranges the logic in
the function so that UserModify can be stored in any subgroup without
breaking the code. There are no changes in behaviour.

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

to

==========
Make ComputedStyle::InheritFrom group independent.

In ComputedStyle::InheritFrom, we assume that the UserModify() field is
always stored in RareInheritedData. After we generate the user-modify
property, we might want to change where it is stored, so UserModify may
no longer be in RareInheritedData. This patch rearranges the logic in
the function so that UserModify can be stored in any subgroup without
breaking the code. There are no changes in behaviour.

BUG=628043

Review-Url: https://codereview.chromium.org/2829773003
Cr-Commit-Position: refs/heads/master@{#471095}
Committed:
https://chromium.googlesource.com/chromium/src/+/2389662b9fb0735446c5c8235bcd...
==========

[commit-bot: I haz the power, 2017-05-11 22:36:25]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/2389662b9fb0735446c5c8235bcd...