Using ElementTraversal::firstAncestorOrSelf API

Using ElementTraversal::firstAncestorOrSelf API

Intial code did the same thing as done by the
ElementTraversal::firstAncestorOrSelf API.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=171324

[deepak.sa, 2014-04-07 16:51:09]

Please have a look.

[Inactive, 2014-04-08 16:49:12]

https://codereview.chromium.org/227123004/diff/1/Source/core/rendering/Render...
File Source/core/rendering/RenderObject.cpp (right):

https://codereview.chromium.org/227123004/diff/1/Source/core/rendering/Render...
Source/core/rendering/RenderObject.cpp:2991: Element* element =
Traversal<HTMLElement>::firstAncestorOrSelf(*node());
If I remember correctly, my initial patch did this (or similar) but esprehn did
not like it so I'll let esprehn review this.

[Inactive, 2014-04-08 16:54:29]

not lgtm

https://codereview.chromium.org/227123004/diff/1/Source/core/rendering/Render...
File Source/core/rendering/RenderObject.cpp (right):

https://codereview.chromium.org/227123004/diff/1/Source/core/rendering/Render...
Source/core/rendering/RenderObject.cpp:2991: Element* element =
Traversal<HTMLElement>::firstAncestorOrSelf(*node());
On 2014/04/08 16:49:12, Chris Dumez (away) wrote:
> If I remember correctly, my initial patch did this (or similar) but esprehn
did
> not like it so I'll let esprehn review this.

Oh and using Traversal<HTMLElement> seems really wrong here. We would want
ElementTraversal instead as we are interested in the first parent of *Element*
type (not HTMLElement).

[esprehn, 2014-04-09 20:34:57]

Did you fix what Chris Dumez didn't like?

[Inactive, 2014-04-09 21:34:31]

On 2014/04/09 20:34:57, esprehn wrote:
> Did you fix what Chris Dumez didn't like?

Yes, the new version is correct.

[Inactive, 2014-04-09 21:45:33]

On 2014/04/09 21:34:31, Chris Dumez (away) wrote:
> On 2014/04/09 20:34:57, esprehn wrote:
> > Did you fix what Chris Dumez didn't like?
> 
> Yes, the new version is correct.

That said, I am letting you review because I believe I proposed this change
already when I implemented firstAncestorOrSelf() and I think you rejected it
saying it would be slower?

[esprehn, 2014-04-10 22:27:29]

This seems okay, it's all kind of silly since you only ever need to go one level
from a non-Element to an Element so this is either node() or
node()->parentElement() but I guess this is the nicest we have right now.

lgtm

[deepak.sa, 2014-04-11 05:11:00]

The CQ bit was checked by deepak.sa@samsung.com

[commit-bot: I haz the power, 2014-04-11 05:11:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/deepak.sa@samsung.com/227123004/20001

[commit-bot: I haz the power, 2014-04-11 08:39:25]

Change committed as 171324