RESOLVED FIXED 120358
Remove branch from DescendantIteratorAdapter::begin() when invoking for ContainerNode 
https://bugs.webkit.org/show_bug.cgi?id=120358
Summary Remove branch from DescendantIteratorAdapter::begin() when invoking for Conta...
Antti Koivisto
Reported 2013-08-27 06:34:32 PDT
Currently we have an unnecessary branch in begin().
Attachments
patch (4.38 KB, patch)
2013-08-27 06:41 PDT, Antti Koivisto 		kling: review+
DetailsFormatted DiffDiff
Simpler patch (3.85 KB, patch)
2013-08-27 07:34 PDT, Antti Koivisto 		kling: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Antti Koivisto
Comment 1 2013-08-27 06:41:17 PDT
Created attachment 209764 [details] patch
Andreas Kling
Comment 2 2013-08-27 06:58:02 PDT
Comment on attachment 209764 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=209764&action=review r=me I guess. > Source/WebCore/dom/ContainerNode.h:150 > + bool isContainerNode() WTF_DELETED_FUNCTION; Does this do the right thing? Node::isContainerNode() is a const function.
Antti Koivisto
Comment 3 2013-08-27 07:34:00 PDT
Created attachment 209771 [details] Simpler patch
Antti Koivisto
Comment 4 2013-08-27 07:39:00 PDT
https://trac.webkit.org/r154682
Darin Adler
Comment 5 2013-08-27 09:25:41 PDT
Comment on attachment 209771 [details] Simpler patch View in context: https://bugs.webkit.org/attachment.cgi?id=209771&action=review > Source/WebCore/dom/DescendantIterator.h:41 > - DescendantIterator(const ContainerNode* root); > - DescendantIterator(const ContainerNode* root, ElementType* current); > + DescendantIterator(const Node* root); > + DescendantIterator(const Node* root, ElementType* current); Why is this a good change? I understand it’s only used for equality comparison, but are there really callers that need to use this with a Node*?
Antti Koivisto
Comment 6 2013-08-27 23:15:34 PDT
(In reply to comment #5) > Why is this a good change? I understand it’s only used for equality comparison, but are there really callers that need to use this with a Node*? Yeah, for now. But it is true that it might be better to just fix the call sites.
Note You need to log in before you can comment on or make changes to this bug.