[webkit-reviews] review granted: [Bug 73964] Use HashMap<Node*, OwnPtr<...>> in ChildListMutationScope : [Attachment 118423] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Dec 8 14:52:01 PST 2011

Darin Adler <darin at apple.com> has granted Adam Klein <adamk at chromium.org>'s
request for review:
Bug 73964: Use HashMap<Node*, OwnPtr<...>> in ChildListMutationScope

Attachment 118423: Patch

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=118423&action=review

> Source/JavaScriptCore/ChangeLog:8
> +	   * wtf/HashTraits.h: Add passOut(std::nullptr_t) to allow callers to
use HashMap::take on an entry whose value is null.

This is an incorrect comment. The code is used when take is called and there is
no entry, not when the entry’s value is null. But even if we never do that, the
reason we have to add passOut is to make HashMake::take compile the code for
that case.

> Source/WebCore/dom/ChildListMutationScope.cpp:245
> -	   m_accumulations.set(target, 0);
> +	   m_accumulations.set(target, nullptr);

Maybe we should do a remove here instead of a set.

> Source/WebCore/dom/ChildListMutationScope.cpp:261
> -    RefPtr<ChildListMutationAccumulator> record =
> +    OwnPtr<ChildListMutationAccumulator>

Why not use "="? I find the other style harder to read.

More information about the webkit-reviews mailing list