Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(35)

Issue 128823002: Get rid of the overloads on templated traversal methods for clarity and safety (Closed)

Created:
6 years, 11 months ago by atreat
Modified:
6 years, 11 months ago
Reviewers:
esprehn, Inactive, eseidel
CC:
blink-reviews, loislo+blink_chromium.org, sof, eae+blinkwatch, yurys+blink_chromium.org, abarth-chromium, dglazkov+blink, adamk+blink_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Get rid of the overloads on templated traversal methods for clarity and safety After careful thought, these overloads decrease clarity and safety for marginal benefit. Because we have the dichotomy between Node and ContainerNode we do not want to call the hidden method Node::firstChild when we pass a ContainerNode to the traversal method and thus incur the cost of the extra check. It is best to be explicit to decrease the likelyhood that someone will inadvertantly use the slower hidden method when traversing. BUG=331880 R=ch.dumez@samsung.com, esprehn@chromium.org Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=164795

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+11 lines, -39 lines) Patch
M Source/core/dom/NodeTraversal.h View 1 chunk +8 lines, -8 lines 0 comments Download
M Source/core/dom/StyleEngine.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/wtf/TreeNode.h View 7 chunks +0 lines, -28 lines 0 comments Download
M Source/wtf/TreeNodeTest.cpp View 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
atreat
The one linux bot failure is the gclient revert issue(s) that have been popping up. ...
6 years, 11 months ago (2014-01-08 18:37:13 UTC) #1
Inactive
lgtm for core/
6 years, 11 months ago (2014-01-08 19:15:29 UTC) #2
eseidel
There was a perf regression filed this morning from these traversal changes. I'd like to ...
6 years, 11 months ago (2014-01-09 00:42:14 UTC) #3
Inactive
On 2014/01/09 00:42:14, eseidel wrote: > There was a perf regression filed this morning from ...
6 years, 11 months ago (2014-01-09 00:49:33 UTC) #4
atreat
On 2014/01/09 00:49:33, Chris Dumez wrote: > On 2014/01/09 00:42:14, eseidel wrote: > > There ...
6 years, 11 months ago (2014-01-09 15:00:31 UTC) #5
eseidel
lgtm.
6 years, 11 months ago (2014-01-09 16:26:14 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/adam.treat@samsung.com/128823002/1
6 years, 11 months ago (2014-01-09 17:07:36 UTC) #7
commit-bot: I haz the power
6 years, 11 months ago (2014-01-09 18:55:30 UTC) #8
Message was sent while issue was closed.
Change committed as 164795

Powered by Google App Engine
This is Rietveld 408576698