Store nonAttachedStyle on NodeLayoutData instead of on HashMap in Document.

Store nonAttachedStyle on NodeLayoutData instead of on HashMap in Document.

This patch stores the ComputedStyle to be passed between recalcStyle
and rebuildLayoutTree on NodeLayoutData instead of storing it on the
HashMap on Document.

This potentially fixes the perf regression in crbug.com/700093 as it
removes the HashMap lookup in SharedStyleFinder and making it a free
operation again.

BUG=595137, 700093
Review-Url: https://codereview.chromium.org/2821193003
Cr-Commit-Position: refs/heads/master@{#467542}
Committed: https://chromium.googlesource.com/chromium/src/+/95862a52c7bc1a5846259619243289805d095b70

[nainar, 2017-04-18 03:42:04]

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

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

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

[nainar, 2017-04-18 03:48:11]

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

[nainar, 2017-04-18 03:48:12]

Elliott, 

PTAL? Thanks!

https://codereview.chromium.org/2821193003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/Node.cpp (right):

https://codereview.chromium.org/2821193003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Node.cpp:609: layout_object,
node_layout_data->GetNonAttachedStyle());
at this stage the GetNonAttachedStyle call should return a nullptr. Assign
nullptr here instead?

https://codereview.chromium.org/2821193003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Node.cpp:633: node_layout_data = new
NodeLayoutData(node_layout_data->GetLayoutObject(),
at this stage the GetLayoutObject call should return a nullptr. Assign nullptr
here instead?

[commit-bot: I haz the power, 2017-04-18 05:30:16]

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

[commit-bot: I haz the power, 2017-04-18 05:30:17]

Dry run: This issue passed the CQ dry run.

[esprehn, 2017-04-18 21:31:09]

esprehn@chromium.org changed reviewers:
+ meade@chromium.org, rune@opera.com

[esprehn, 2017-04-18 21:31:10]

This looks legit to me, we might want to think about a way to refactor that
logic to allocate a NodeLayoutData into something like
ensureMutableNodeLayoutData() and just make the constructor assign nullptr.

https://codereview.chromium.org/2821193003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/Node.cpp (right):

https://codereview.chromium.org/2821193003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Node.cpp:609: layout_object,
node_layout_data->GetNonAttachedStyle());
Yeah can we make this a DCHECK and remove the constructor argument instead? Just
initialize directly to nullptr. I don't think saving the one assignment is worth
it in general.

[nainar, 2017-04-19 01:24:21]

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

[commit-bot: I haz the power, 2017-04-19 01:24:34]

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

[nainar, 2017-04-19 01:38:34]

https://codereview.chromium.org/2821193003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/Node.cpp (right):

https://codereview.chromium.org/2821193003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Node.cpp:609: layout_object,
node_layout_data->GetNonAttachedStyle());
Done.

https://codereview.chromium.org/2821193003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Node.cpp:633: node_layout_data = new
NodeLayoutData(node_layout_data->GetLayoutObject(),
Done.

https://codereview.chromium.org/2821193003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Node.cpp (right):

https://codereview.chromium.org/2821193003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Node.cpp:610: if (HasRareData())
This can read as below instead:

  DCHECK(!node_layout_data->GetNonAttachedStyle());
  // Assign a nullptr to everything
  node_layout_data =
ensureMutableNodeLayoutData()->SetLayoutObject(layout_object);

In ensureMutableNodeLayoutData() -> 
  NodeLayoutData* data = new NodeLayoutData();
  if (HasRareData())
    data_.rare_data_->SetNodeLayoutData(data);
  else
    data_.node_layout_data_ = data;
  return data;

https://codereview.chromium.org/2821193003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Node.cpp:617: NodeLayoutData*
node_layout_data = HasRareData()
I can pull out this into a getter

[commit-bot: I haz the power, 2017-04-19 02:58:01]

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

[commit-bot: I haz the power, 2017-04-19 02:58:02]

Dry run: This issue passed the CQ dry run.

[esprehn, 2017-04-19 17:57:45]

FYI: meade@ should do a review of this too. Perhaps the two of you could go over
the patch in person so she can learn about the code?

[rune, 2017-04-19 20:34:53]

https://codereview.chromium.org/2821193003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Document.cpp (left):

https://codereview.chromium.org/2821193003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Document.cpp:2132:
non_attached_style_.Clear();
Do we retain the ComputedStyle on each node now, even after we have created the
layout objects?

Also, does that mean that if we later detach a subtree (layout tree), we will
still retain all descendant ComputedStyles in a display:none subtree? If so, is
that intentional?

https://codereview.chromium.org/2821193003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Node.h (right):

https://codereview.chromium.org/2821193003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Node.h:106: RefPtr<ComputedStyle>
non_attached_style)
Shouldn't this be a WTF_MAKE_NONCOPYABLE like e.g. NodeRareData?

[nainar, 2017-04-19 23:42:46]

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

[commit-bot: I haz the power, 2017-04-19 23:43:24]

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

[nainar, 2017-04-19 23:43:41]

New patch in response to rune@'s comments.esprehn@ will run meade@ through the
code and look for their lgtm in that case :) Thank you!

https://codereview.chromium.org/2821193003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Document.cpp (left):

https://codereview.chromium.org/2821193003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Document.cpp:2132:
non_attached_style_.Clear();
On 2017/04/19 at 20:34:52, rune wrote:
> Do we retain the ComputedStyle on each node now, even after we have created
the layout objects?
> 
> Also, does that mean that if we later detach a subtree (layout tree), we will
still retain all descendant ComputedStyles in a display:none subtree? If so, is
that intentional?

For the needs of this patch I should delete the computedStyle once we get the
info in rebuildLayoutTree but in a future patch I do want to retain
ComputedStyle for even display:non elements. 

I don't know what is the best place to clear the non attached ComputedStyle.
Can't do it in Element::rebuildLayoutTree because of the SharedStyleFinder which
may get called in LayoutTreeBuilder. And clearing it in LayoutTreeBuilder::style
would mean that elements that don't get a LayoutObject will still retain this
information.

https://codereview.chromium.org/2821193003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Node.h (right):

https://codereview.chromium.org/2821193003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Node.h:106: RefPtr<ComputedStyle>
non_attached_style)
On 2017/04/19 at 20:34:52, rune wrote:
> Shouldn't this be a WTF_MAKE_NONCOPYABLE like e.g. NodeRareData?

You are right. Done.

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

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

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

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[meade_UTC10, 2017-04-20 03:12:11]

lgtm

[nainar, 2017-04-20 06:48:16]

Thanks for the review meade@, will wait on rune@ to voice off on the concern
expressed in an earlier patch before landing. :)

[rune, 2017-04-20 08:56:39]

https://codereview.chromium.org/2821193003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Document.cpp (left):

https://codereview.chromium.org/2821193003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Document.cpp:2132:
non_attached_style_.Clear();
On 2017/04/19 23:43:41, nainar wrote:
> On 2017/04/19 at 20:34:52, rune wrote:
> > Do we retain the ComputedStyle on each node now, even after we have created
> the layout objects?
> > 
> > Also, does that mean that if we later detach a subtree (layout tree), we
will
> still retain all descendant ComputedStyles in a display:none subtree? If so,
is
> that intentional?
> 
> For the needs of this patch I should delete the computedStyle once we get the
> info in rebuildLayoutTree but in a future patch I do want to retain
> ComputedStyle for even display:non elements. 

So, if we have a subtree of N elements which is initially rendered, and make the
root of that subtree display:none and never display it again, should we retain
all N ComputedStyles for the lifetime of the subtree?

Later, it would make sense to unify non-attached style with the computed_style_
in ElementRareData?

> I don't know what is the best place to clear the non attached ComputedStyle.
> Can't do it in Element::rebuildLayoutTree because of the SharedStyleFinder
which
> may get called in LayoutTreeBuilder. And clearing it in
LayoutTreeBuilder::style
> would mean that elements that don't get a LayoutObject will still retain this
> information.

It looks like we currently only store non-attached style for the root of a
layout tree which needs rebuilding (I know this is not the end goal). Can't you
clear the non-attached style after calling ReattachLayoutTree() in
Element::RebuildLayoutTree()?

[nainar, 2017-04-21 00:29:59]

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

[commit-bot: I haz the power, 2017-04-21 00:30:25]

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 00:32:40]

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

[commit-bot: I haz the power, 2017-04-21 00:32:41]

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_...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[nainar, 2017-04-21 00:51:30]

rune@, 

responded to your comments inline. PTAL? Thanks!

https://codereview.chromium.org/2821193003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Document.cpp (left):

https://codereview.chromium.org/2821193003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Document.cpp:2132:
non_attached_style_.Clear();
On 2017/04/20 at 08:56:39, rune wrote:
> On 2017/04/19 23:43:41, nainar wrote:
> > On 2017/04/19 at 20:34:52, rune wrote:
> > > Do we retain the ComputedStyle on each node now, even after we have
created
> > the layout objects?
> > > 
> > > Also, does that mean that if we later detach a subtree (layout tree), we
will
> > still retain all descendant ComputedStyles in a display:none subtree? If so,
is
> > that intentional?
> > 
> > For the needs of this patch I should delete the computedStyle once we get
the
> > info in rebuildLayoutTree but in a future patch I do want to retain
> > ComputedStyle for even display:non elements. 
> 
> So, if we have a subtree of N elements which is initially rendered, and make
the root of that subtree display:none and never display it again, should we
retain all N ComputedStyles for the lifetime of the subtree?
> 
> Later, it would make sense to unify non-attached style with the
computed_style_ in ElementRareData?

We shouldn't be retaining the nonAttachedStyle for those nodes at this stage. I
have the same intent to unify it with the ComputedStyle stored on
ElementRareData. Iterating over all descendants clearing their nonAttachedStyle
when the root is set to display:none sounds liek a new perf regression to me
though. Am I wrong?

> > I don't know what is the best place to clear the non attached ComputedStyle.
> > Can't do it in Element::rebuildLayoutTree because of the SharedStyleFinder
which
> > may get called in LayoutTreeBuilder. And clearing it in
LayoutTreeBuilder::style
> > would mean that elements that don't get a LayoutObject will still retain
this
> > information.
> 
> It looks like we currently only store non-attached style for the root of a
layout tree which needs rebuilding (I know this is not the end goal). Can't you
clear the non-attached style after calling ReattachLayoutTree() in
Element::RebuildLayoutTree()?

Done.

[rune, 2017-04-21 07:41:46]

lgtm

https://codereview.chromium.org/2821193003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Document.cpp (left):

https://codereview.chromium.org/2821193003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Document.cpp:2132:
non_attached_style_.Clear();
On 2017/04/21 00:51:30, nainar wrote:
> On 2017/04/20 at 08:56:39, rune wrote:
> > On 2017/04/19 23:43:41, nainar wrote:
> > > On 2017/04/19 at 20:34:52, rune wrote:
> > > > Do we retain the ComputedStyle on each node now, even after we have
> created
> > > the layout objects?
> > > > 
> > > > Also, does that mean that if we later detach a subtree (layout tree), we
> will
> > > still retain all descendant ComputedStyles in a display:none subtree? If
so,
> is
> > > that intentional?
> > > 
> > > For the needs of this patch I should delete the computedStyle once we get
> the
> > > info in rebuildLayoutTree but in a future patch I do want to retain
> > > ComputedStyle for even display:non elements. 
> > 
> > So, if we have a subtree of N elements which is initially rendered, and make
> the root of that subtree display:none and never display it again, should we
> retain all N ComputedStyles for the lifetime of the subtree?
> > 
> > Later, it would make sense to unify non-attached style with the
> computed_style_ in ElementRareData?
> 
> We shouldn't be retaining the nonAttachedStyle for those nodes at this stage.
I

No, this was more of a future concern.

> have the same intent to unify it with the ComputedStyle stored on
> ElementRareData. Iterating over all descendants clearing their
nonAttachedStyle
> when the root is set to display:none sounds liek a new perf regression to me
> though. Am I wrong?

Detach will traverse over displayed descendants of the root and the rare-data
computed style is already cleared in DetachLayoutTree or AttachLayoutTree today:

https://codesearch.chromium.org/chromium/src/third_party/WebKit/Source/core/d...

[nainar, 2017-04-26 02:54:16]

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

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

Dry run: CQ is trying da patch.

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

[nainar, 2017-04-26 02:55:16]

rune@, 

Made the changes you suggested. PTAL? Thanks!

https://codereview.chromium.org/2821193003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Document.cpp (left):

https://codereview.chromium.org/2821193003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Document.cpp:2132:
non_attached_style_.Clear();
On 2017/04/21 at 07:41:46, rune wrote:
> On 2017/04/21 00:51:30, nainar wrote:
> > On 2017/04/20 at 08:56:39, rune wrote:
> > > On 2017/04/19 23:43:41, nainar wrote:
> > > > On 2017/04/19 at 20:34:52, rune wrote:
> > > > > Do we retain the ComputedStyle on each node now, even after we have
> > created
> > > > the layout objects?
> > > > > 
> > > > > Also, does that mean that if we later detach a subtree (layout tree),
we
> > will
> > > > still retain all descendant ComputedStyles in a display:none subtree? If
so,
> > is
> > > > that intentional?
> > > > 
> > > > For the needs of this patch I should delete the computedStyle once we
get
> > the
> > > > info in rebuildLayoutTree but in a future patch I do want to retain
> > > > ComputedStyle for even display:non elements. 
> > > 
> > > So, if we have a subtree of N elements which is initially rendered, and
make
> > the root of that subtree display:none and never display it again, should we
> > retain all N ComputedStyles for the lifetime of the subtree?
> > > 
> > > Later, it would make sense to unify non-attached style with the
> > computed_style_ in ElementRareData?
> > 
> > We shouldn't be retaining the nonAttachedStyle for those nodes at this
stage. I
> 
> No, this was more of a future concern.
> 
> > have the same intent to unify it with the ComputedStyle stored on
> > ElementRareData. Iterating over all descendants clearing their
nonAttachedStyle
> > when the root is set to display:none sounds liek a new perf regression to me
> > though. Am I wrong?
> 
> Detach will traverse over displayed descendants of the root and the rare-data
computed style is already cleared in DetachLayoutTree or AttachLayoutTree today:
> 
>
https://codesearch.chromium.org/chromium/src/third_party/WebKit/Source/core/d...

Done. 

Had to change Node::setNonAttachedStyle() to incorporate the ability to set
nonAttachedStyle to null

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

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

[commit-bot: I haz the power, 2017-04-26 03:15:08]

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...)

[rune, 2017-04-26 08:47:28]

https://codereview.chromium.org/2821193003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Document.cpp (left):

https://codereview.chromium.org/2821193003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Document.cpp:2132:
non_attached_style_.Clear();
On 2017/04/26 02:55:16, nainar wrote:
> On 2017/04/21 at 07:41:46, rune wrote:
> > On 2017/04/21 00:51:30, nainar wrote:
> > > On 2017/04/20 at 08:56:39, rune wrote:
> > > > On 2017/04/19 23:43:41, nainar wrote:
> > > > > On 2017/04/19 at 20:34:52, rune wrote:
> > > > > > Do we retain the ComputedStyle on each node now, even after we have
> > > created
> > > > > the layout objects?
> > > > > > 
> > > > > > Also, does that mean that if we later detach a subtree (layout
tree),
> we
> > > will
> > > > > still retain all descendant ComputedStyles in a display:none subtree?
If
> so,
> > > is
> > > > > that intentional?
> > > > > 
> > > > > For the needs of this patch I should delete the computedStyle once we
> get
> > > the
> > > > > info in rebuildLayoutTree but in a future patch I do want to retain
> > > > > ComputedStyle for even display:non elements. 
> > > > 
> > > > So, if we have a subtree of N elements which is initially rendered, and
> make
> > > the root of that subtree display:none and never display it again, should
we
> > > retain all N ComputedStyles for the lifetime of the subtree?
> > > > 
> > > > Later, it would make sense to unify non-attached style with the
> > > computed_style_ in ElementRareData?
> > > 
> > > We shouldn't be retaining the nonAttachedStyle for those nodes at this
> stage. I
> > 
> > No, this was more of a future concern.
> > 
> > > have the same intent to unify it with the ComputedStyle stored on
> > > ElementRareData. Iterating over all descendants clearing their
> nonAttachedStyle
> > > when the root is set to display:none sounds liek a new perf regression to
me
> > > though. Am I wrong?
> > 
> > Detach will traverse over displayed descendants of the root and the
rare-data
> computed style is already cleared in DetachLayoutTree or AttachLayoutTree
today:
> > 
> >
>
https://codesearch.chromium.org/chromium/src/third_party/WebKit/Source/core/d...
> 
> Done. 
> 
> Had to change Node::setNonAttachedStyle() to incorporate the ability to set
> nonAttachedStyle to null

Sorry for being unclear, but I think the patch set at the time I lgtm'ed was
ready for landing (modulo any SetNonAttachedStyle() you had to change).

I was just thinking ahead about a future CL where we unify non-attached style
with non-layout-object-computed-style. I think the non-attached style is only
set for the root node of a subtree being re-attached as RecalcStyle is currently
skipping descendants when we get a kReattach change.

https://codereview.chromium.org/2821193003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Node.cpp (right):

https://codereview.chromium.org/2821193003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Node.cpp:633: data_.node_layout_data_ =
&NodeLayoutData::SharedEmptyData();
Won't this leak the previously set node_layout_data_?

Could we just keep the node_layout_data_ like we do for rare data?

[nainar, 2017-04-26 09:07:52]

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

[commit-bot: I haz the power, 2017-04-26 09:08:06]

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 13:19:32]

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

[commit-bot: I haz the power, 2017-04-26 13:19:33]

Dry run: This issue passed the CQ dry run.

[nainar, 2017-04-27 01:38:42]

The CQ bit was checked by nainar@chromium.org

[nainar, 2017-04-27 01:38:43]

The patchset sent to the CQ was uploaded after l-g-t-m from meade@chromium.org,
rune@opera.com
Link to the patchset: https://codereview.chromium.org/2821193003/#ps100001
(title: "Revert back to rune@'s lgtmed patch")

[commit-bot: I haz the power, 2017-04-27 01:39:13]

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 01:44:21]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1493257122513790,
"parent_rev": "c5f7e1d59850173697e0596155e0c9ab54fbd649", "commit_rev":
"95862a52c7bc1a5846259619243289805d095b70"}

[commit-bot: I haz the power, 2017-04-27 01:45:22]

Description was changed from

==========
Store nonAttachedStyle on NodeLayoutData instead of on HashMap in Document.

This patch stores the ComputedStyle to be passed between recalcStyle
and rebuildLayoutTree on NodeLayoutData instead of storing it on the
HashMap on Document.

This potentially fixes the perf regression in crbug.com/700093 as it
removes the HashMap lookup in SharedStyleFinder and making it a free
operation again.

BUG=595137, 700093
==========

to

==========
Store nonAttachedStyle on NodeLayoutData instead of on HashMap in Document.

This patch stores the ComputedStyle to be passed between recalcStyle
and rebuildLayoutTree on NodeLayoutData instead of storing it on the
HashMap on Document.

This potentially fixes the perf regression in crbug.com/700093 as it
removes the HashMap lookup in SharedStyleFinder and making it a free
operation again.

BUG=595137, 700093

Review-Url: https://codereview.chromium.org/2821193003
Cr-Commit-Position: refs/heads/master@{#467542}
Committed:
https://chromium.googlesource.com/chromium/src/+/95862a52c7bc1a58462596192432...
==========

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

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

[nainar, 2017-04-27 01:50:06]

Should have sent this before landing.

https://codereview.chromium.org/2821193003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Node.cpp (right):

https://codereview.chromium.org/2821193003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Node.cpp:633: data_.node_layout_data_ =
&NodeLayoutData::SharedEmptyData();
On 2017/04/26 at 08:47:28, rune wrote:
> Won't this leak the previously set node_layout_data_?
> 
> Could we just keep the node_layout_data_ like we do for rare data?

Yup, I see what you mean. I have reverted the CL back to the patch where I had
your LGTM last. Sorry about that I thought it was an lgtm with the suggested
change. 

I will take another stab at this when I try to unify the two ComputedStyles.