[webkit-reviews] review granted: [Bug 57689] Extract scoping functionality from Document : [Attachment 88124] Patch

[bugzilla-daemon at webkit.org]

Dimitri Glazkov (Google) <dglazkov at chromium.org> has granted Roland Steiner
<rolandsteiner at chromium.org>'s request for review:
Bug 57689: Extract scoping functionality from Document
https://bugs.webkit.org/show_bug.cgi?id=57689

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

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=88124&action=review

This is actually quite cool.

> Source/WebCore/ChangeLog:55
> +	   (WebCore::TreeScope::DOMTreeScope):
> +	   (WebCore::TreeScope::removedLastRef):
> +	   (WebCore::TreeScope::destroyScope):
> +	   (WebCore::TreeScope::getElementById):
> +	   (WebCore::TreeScope::addElementById):
> +	   (WebCore::TreeScope::removeElementById):
> +	   (WebCore::TreeScope::getElementByAccessKey):
> +	   (WebCore::TreeScope::invalidateAccessKeyMap):
> +	   (WebCore::TreeScope::addImageMap):
> +	   (WebCore::TreeScope::removeImageMap):
> +	   (WebCore::TreeScope::getImageMap):
> +	   (WebCore::TreeScope::findAnchor):

You can remove these lines from the ChangeLog for brevity.

> Source/WebCore/ChangeLog:60
> +	   (WebCore::TreeScope::guardRef):
> +	   (WebCore::TreeScope::guardDeref):
> +	   (WebCore::TreeScope::hasElementWithId):
> +	   (WebCore::TreeScope::containsMultipleElementsWithId):

Ditto.

> Source/WebCore/WebCore.vcproj/WebCore.vcproj:47224
> +				RelativePath="..\dom\TreeScope.cpp"
> +				>
> +			</File>
> +			<File
> +				RelativePath="..\dom\TreeScope.h"
> +				>
> +			</File>
> +			<File

This is incorrect. The .cpp file should be set to not build (see nearby .cpp in
..\dom\ for an example) and then it should be added to DOMAllInOne.cpp file.

> Source/WebCore/dom/Node.h:368
> +    

Extra spaces here?

> Source/WebCore/dom/TreeScope.h:64
> +    virtual void removedLastRef();
> +    virtual void destroyScope();

Can we make these protected?