[webkit-reviews] review granted: [Bug 33944] [Android] bindings/v8/V8DOMWrapper.h[cpp] are missing guards for SVG, XPATH and XSLT features : [Attachment 47110] Add missing guards.

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has granted Andrei Popescu
<andreip at google.com>'s request for review:
Bug 33944: [Android] bindings/v8/V8DOMWrapper.h[cpp] are missing guards for
SVG, XPATH and XSLT features
https://bugs.webkit.org/show_bug.cgi?id=33944

Attachment 47110: Add missing guards.
https://bugs.webkit.org/attachment.cgi?id=47110&action=review

------- Additional Comments from David Levin <levin at chromium.org>
Just a few minor things to address.

> Index: WebCore/ChangeLog
> +2010-01-21  Andrei Popescu  <andreip at google.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   [Android] bindings/v8/V8DOMWrapper.h[cpp] are missing guards for
SVG, XPATH and XSLT features
> +	   https://bugs.webkit.org/show_bug.cgi?id=33944
> +
> +	   Iniside V8DOMWrapper.h[cpp], the code for SVG, XPATH and XSLT
features is not guarded

typo: Iniside

Please adjust the comment appropriately when you remove the if ENABLE(SVG) for
the SVG headers (see comment below).

> Index: WebCore/bindings/v8/V8DOMWrapper.cpp
>  #include "Notification.h"
> -#include "SVGElementInstance.h"

Please leave this here and add #include "SVGPathSeg.h" (see comments below).

>  #include "ScriptController.h"
>  #include "V8AbstractEventListener.h"
>  #include "V8Binding.h"
> @@ -63,6 +61,7 @@
>  #include "WorkerContextExecutionProxy.h"
>  
>  #if ENABLE(SVG)
> +#include "SVGElementInstance.h"

SVGElementInstance.h correctly has #if ENABLE(SVG) inside of it so this is
unnecessary. (If something is wrong with the way the header does the if ENABLE,
please fix that.)

>  #include "SVGPathSeg.h"

SVGPathSeg.h also  has #if ENABLE(SVG) inside of it. So the whole if
ENABLE(SVG) that is guarding these includes should go away and these headers
should just be sorted with the other headers (as SVGElementInstance.h was
before).


>  #endif