[webkit-reviews] review denied: [Bug 64391] [Qt] Make QtWebkit2.2 compile and run on S60 Emulator : [Attachment 100842] Patch with the changes made based on Laszlo's comments

[bugzilla-daemon at webkit.org]

Laszlo Gombos <laszlo.1.gombos at nokia.com> has denied Hui Huang
<hui_huang at yahoo.com>'s request for review:
Bug 64391: [Qt] Make QtWebkit2.2 compile and run on S60 Emulator
https://bugs.webkit.org/show_bug.cgi?id=64391

Attachment 100842: Patch with the changes made based on Laszlo's comments
https://bugs.webkit.org/attachment.cgi?id=100842&action=review

------- Additional Comments from Laszlo Gombos <laszlo.1.gombos at nokia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=100842&action=review


r- for the likely non-WINSCW build break.

> Source/JavaScriptCore/wtf/PageAllocatorSymbian.h:71
> +// const size_t largeReservationSize = 96*1024*1024;

Please remove this line instead of commenting it out !

> Source/JavaScriptCore/wtf/text/AtomicString.h:176
> +    extern const JS_EXPORTDATA AtomicString xmlnsAtom1;

No need for the extra spaces here, please remove them !

> Source/JavaScriptCore/wtf/text/AtomicString.h:192
> +    extern const JS_EXPORTDATA AtomicString xmlnsAtom;

Ditto.

> Source/WebCore/css/CSSStyleSelector.h:212
> +	   template <bool applyFirst>

Is this really needed ?

> Source/WebCore/page/PrintContext.cpp:57
>  

COMPILER(WINSCW) guard is missing from here. I think this would break
non-WINSCW builds (including Symbian RVCT builds). As this patch no longer
applies to trunk, I have no way telling if this patch breaks the build on e.g.
Linux. Have you tried any non-WINSCW build with this patch ?