[webkit-reviews] review granted: [Bug 100316] [V8] We can merge DOMDataStore, ScopedDOMDataStore, and StaticDOMDataStore into one class : [Attachment 170552] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Adam Barth <abarth at webkit.org>'s
request for review:
Bug 100316: [V8] We can merge DOMDataStore, ScopedDOMDataStore, and
StaticDOMDataStore into one class
https://bugs.webkit.org/show_bug.cgi?id=100316

Attachment 170552: Patch
https://bugs.webkit.org/attachment.cgi?id=170552&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=170552&action=review


LGTM.

> Source/WebCore/ChangeLog:11
> +	   unifies all these classes into DOMDataStore itself. This makese this


makese!

> Source/WebCore/bindings/v8/DOMDataStore.cpp:57
> +	   m_domObjectMap = adoptPtr(new DOMWrapperHashMap<void>);
> +	   m_activeDomObjectMap = adoptPtr(new DOMWrapperHashMap<void>);

These are always the same for all 3, no?  It seems this whole "Switch" is
clearer as an if.  But presumably you wanted the "forgot to add handling of the
new enum value" help from the compiler?