Issue 482903002: Get rid of loops looking for the first Element ancestor

Issue 482903002: Get rid of loops looking for the first Element ancestor (Closed)

Created:
6 years, 4 months ago by Inactive

Modified:
6 years, 4 months ago

Reviewers:
esprehn

CC:
aboxhall, blink-reviews, blink-reviews-dom_chromium.org, blink-reviews-rendering, dglazkov+blink, dmazzoni, eae+blinkwatch, jchaffraix+rendering, leviw+renderwatch, pdr., rwlbuis, rune+blink, sof, zoltan1

Base URL:
svn://svn.chromium.org/blink/trunk

Project:
blink

Visibility:
Public.

More Reviews

Description

Get rid of loops looking for the first Element ancestor Get rid of loops looking for the first Element ancestor and use Node::parentElement() instead. There is no point going up more than one level to find an Element ancestor. Only non-Element nodes can have children: Document, DocumentFragment and Attr. However, none of these can have parents. This CL also uses Node::parentElement() instead of Node::parentNode() + Node::isElementNode() in a couple of instances to simplify the code a bit. R=esprehn@chromium.org Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=180552

Patch Set 1 #

Patch Set 2 : Update one more instance #

Total comments: 4

Patch Set 3 : Fix nits #

Created: 6 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+17 lines, -17 lines)			Patch
M	Source/core/accessibility/AXNodeObject.cpp	View		1 chunk	+2 lines, -2 lines	0 comments	Download
M	Source/core/dom/Element.cpp	View		1 chunk	+3 lines, -3 lines	0 comments	Download
M	Source/core/dom/Position.cpp	View	1 2	1 chunk	+4 lines, -4 lines	0 comments	Download
M	Source/core/editing/htmlediting.cpp	View	1	1 chunk	+2 lines, -1 line	0 comments	Download
M	Source/core/rendering/TextAutosizer.cpp	View	1 2	1 chunk	+3 lines, -4 lines	0 comments	Download
M	Source/core/xml/parser/XMLDocumentParser.cpp	View		1 chunk	+3 lines, -3 lines	0 comments	Download

Messages

Total messages: 6 (0 generated)

Expand Messages | Collapse Messages

esprehn

lgtm w/ nits https://codereview.chromium.org/482903002/diff/20001/Source/core/dom/Position.cpp File Source/core/dom/Position.cpp (right): https://codereview.chromium.org/482903002/diff/20001/Source/core/dom/Position.cpp#newcode284 Source/core/dom/Position.cpp:284: return n->parentElement(); n -> node https://codereview.chromium.org/482903002/diff/20001/Source/core/rendering/TextAutosizer.cpp ...

6 years, 4 months ago (2014-08-19 01:02:45 UTC) #2

Inactive

https://codereview.chromium.org/482903002/diff/20001/Source/core/dom/Position.cpp File Source/core/dom/Position.cpp (right): https://codereview.chromium.org/482903002/diff/20001/Source/core/dom/Position.cpp#newcode284 Source/core/dom/Position.cpp:284: return n->parentElement(); On 2014/08/19 01:02:45, esprehn wrote: > n ...

6 years, 4 months ago (2014-08-19 12:51:46 UTC) #3

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ch.dumez@samsung.com/482903002/40001

6 years, 4 months ago (2014-08-19 12:52:22 UTC) #5

commit-bot: I haz the power

6 years, 4 months ago (2014-08-19 13:51:24 UTC) #6

Message was sent while issue was closed.

Committed patchset #3 (40001) as 180552

Expand Messages | Collapse Messages