Merge LayoutInline::mapLocalToAncestor() into LayoutObject.

Merge LayoutInline::mapLocalToAncestor() into LayoutObject.

Avoids code duplication and maybe also makes the LayoutObject implementation
more correct. It was typically only LayoutText objects that used the
LayoutObject implementation of mapLocalToAncestor() prior to this change.

Some extra care was needed when calling style() members, since text nodes just
copy their parent's computed style instead of just inheriting the inheritable
ones. Apart from that, it's basically just about replacing the LayoutObject
implementation with that of LayoutInline.

BUG=568492
R=leviw@chromium.org,ojan@chromium.org

Committed: https://crrev.com/f54369c41aab496af8ed8133395d40625266a839
Cr-Commit-Position: refs/heads/master@{#375269}

[mstensho (USE GERRIT), 2015-12-10 21:22:01]

This is a reboot of https://codereview.chromium.org/1181953005/ , which I
closed, because I had more important things to do back then. Now it turns out
that this may be key to fixing bug 527709 (a serious paint bug in multicol), so
I'm more keen to fix it than ever. I've also found a bug because of poor code
duplication in two offsetFromContainer() implementations. But more on that in an
upcoming CL. Hope you're as excited as I am. :)

[mstensho (USE GERRIT), 2016-01-04 11:31:08]

mstensho@opera.com changed reviewers:
+ eae@chromium.org

[ojan, 2016-01-22 01:54:17]

Levi, I think you're the best reviewer for this?

[mstensho (USE GERRIT), 2016-01-29 08:07:17]

On 2016/01/22 01:54:17, ojan wrote:
> Levi, I think you're the best reviewer for this?

Levi is great! Levi is the best! But where IS Levi? :)

[leviw_travelin_and_unemployed, 2016-01-29 21:58:33]

Sorry, I've been a bit behind and was hoping Ojan would lend his reviewing hand.

Can I hang a carrot in front of this cl? How much øl does it take to get you to
write a unit test for this method? It'd make me feel a tad better about hacking
on it. WDYT?

From what I can tell, this looks fine. I'll give it a more thorough look after
the weekend (I'm currently in Sydney for the Houdini TF).

[mstensho (USE GERRIT), 2016-02-01 13:32:13]

Description was changed from

==========
Merge LayoutInline::mapLocalToContainer() into LayoutObject.

Avoids code duplication and maybe also makes the LayoutObject implementation
more correct. It was typically only LayoutText objects that used the
LayoutObject implementation of mapLocalToContainer() prior to this change.

Some extra care was needed when calling style() members, since text nodes just
copy their parent's computed style instead of just inheriting the inheritable
ones. Apart from that, it's basically just about replacing the LayoutObject
implementation with that of LayoutInline.

BUG=568492
R=leviw@chromium.org,ojan@chromium.org
==========

to

==========
Merge LayoutInline::mapLocalToAncestor() into LayoutObject.

Avoids code duplication and maybe also makes the LayoutObject implementation
more correct. It was typically only LayoutText objects that used the
LayoutObject implementation of mapLocalToAncestor() prior to this change.

Some extra care was needed when calling style() members, since text nodes just
copy their parent's computed style instead of just inheriting the inheritable
ones. Apart from that, it's basically just about replacing the LayoutObject
implementation with that of LayoutInline.

BUG=568492
R=leviw@chromium.org,ojan@chromium.org
==========

[mstensho (USE GERRIT), 2016-02-11 12:18:29]

On 2016/01/29 21:58:33, leviw wrote:
> Can I hang a carrot in front of this cl? How much øl does it take to get you
to
> write a unit test for this method? It'd make me feel a tad better about
hacking
> on it. WDYT?

I, for one, never say no to carrots and beer. :)

Did it as a separate CL: 
https://codereview.chromium.org/1681403004/

[mstensho (USE GERRIT), 2016-02-12 13:08:53]

On 2016/02/11 12:18:29, mstensho wrote:
> On 2016/01/29 21:58:33, leviw wrote:
> > Can I hang a carrot in front of this cl? How much øl does it take to get you
> to
> > write a unit test for this method? It'd make me feel a tad better about
> hacking
> > on it. WDYT?
> 
> I, for one, never say no to carrots and beer. :)
> 
> Did it as a separate CL: 
> https://codereview.chromium.org/1681403004/

Passes all new unit tests. Levi? :)

[eae, 2016-02-12 17:31:55]

LGTM

https://codereview.chromium.org/1516003003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right):

https://codereview.chromium.org/1516003003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:2209: if (PaintLayer*
layer = style()->hasInFlowPosition() && hasLayer() ?
toLayoutBoxModelObject(this)->layer() : nullptr)
Can we make this "const PaintLayer*"? 
(PaintLayer::offsetForInFlowPosition is marked const)

[leviw_travelin_and_unemployed, 2016-02-12 19:20:12]

LGTM 2 when eae's nit is addressed.

Thanks for jumping through the hoops, Morten!

[mstensho (USE GERRIT), 2016-02-12 20:12:49]

https://codereview.chromium.org/1516003003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right):

https://codereview.chromium.org/1516003003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:2209: if (PaintLayer*
layer = style()->hasInFlowPosition() && hasLayer() ?
toLayoutBoxModelObject(this)->layer() : nullptr)
On 2016/02/12 17:31:55, eae wrote:
> Can we make this "const PaintLayer*"? 
> (PaintLayer::offsetForInFlowPosition is marked const)

Done.

[mstensho (USE GERRIT), 2016-02-12 20:12:59]

The CQ bit was checked by mstensho@opera.com

[mstensho (USE GERRIT), 2016-02-12 20:13:00]

The patchset sent to the CQ was uploaded after l-g-t-m from leviw@chromium.org,
eae@chromium.org
Link to the patchset: https://codereview.chromium.org/1516003003/#ps80001
(title: "review constness + bonus constness")

[commit-bot: I haz the power, 2016-02-12 20:13:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1516003003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1516003003/80001

[commit-bot: I haz the power, 2016-02-12 22:05:10]

Description was changed from

==========
Merge LayoutInline::mapLocalToAncestor() into LayoutObject.

Avoids code duplication and maybe also makes the LayoutObject implementation
more correct. It was typically only LayoutText objects that used the
LayoutObject implementation of mapLocalToAncestor() prior to this change.

Some extra care was needed when calling style() members, since text nodes just
copy their parent's computed style instead of just inheriting the inheritable
ones. Apart from that, it's basically just about replacing the LayoutObject
implementation with that of LayoutInline.

BUG=568492
R=leviw@chromium.org,ojan@chromium.org
==========

to

==========
Merge LayoutInline::mapLocalToAncestor() into LayoutObject.

Avoids code duplication and maybe also makes the LayoutObject implementation
more correct. It was typically only LayoutText objects that used the
LayoutObject implementation of mapLocalToAncestor() prior to this change.

Some extra care was needed when calling style() members, since text nodes just
copy their parent's computed style instead of just inheriting the inheritable
ones. Apart from that, it's basically just about replacing the LayoutObject
implementation with that of LayoutInline.

BUG=568492
R=leviw@chromium.org,ojan@chromium.org
==========

[commit-bot: I haz the power, 2016-02-12 22:05:10]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-02-16 22:45:28]

Description was changed from

==========
Merge LayoutInline::mapLocalToAncestor() into LayoutObject.

Avoids code duplication and maybe also makes the LayoutObject implementation
more correct. It was typically only LayoutText objects that used the
LayoutObject implementation of mapLocalToAncestor() prior to this change.

Some extra care was needed when calling style() members, since text nodes just
copy their parent's computed style instead of just inheriting the inheritable
ones. Apart from that, it's basically just about replacing the LayoutObject
implementation with that of LayoutInline.

BUG=568492
R=leviw@chromium.org,ojan@chromium.org
==========

to

==========
Merge LayoutInline::mapLocalToAncestor() into LayoutObject.

Avoids code duplication and maybe also makes the LayoutObject implementation
more correct. It was typically only LayoutText objects that used the
LayoutObject implementation of mapLocalToAncestor() prior to this change.

Some extra care was needed when calling style() members, since text nodes just
copy their parent's computed style instead of just inheriting the inheritable
ones. Apart from that, it's basically just about replacing the LayoutObject
implementation with that of LayoutInline.

BUG=568492
R=leviw@chromium.org,ojan@chromium.org

Committed: https://crrev.com/f54369c41aab496af8ed8133395d40625266a839
Cr-Commit-Position: refs/heads/master@{#375269}
==========

[commit-bot: I haz the power, 2016-02-16 22:45:29]

Patchset 5 (id:??) landed as
https://crrev.com/f54369c41aab496af8ed8133395d40625266a839
Cr-Commit-Position: refs/heads/master@{#375269}