Bug 120358

Summary: Remove branch from DescendantIteratorAdapter::begin() when invoking for ContainerNode
Product: WebKit Reporter: Antti Koivisto <koivisto>
Component: DOMAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, esprehn+autocc, kangil.han, kling
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
kling: review+
Simpler patch kling: review+

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.