[webkit-reviews] review granted: [Bug 26932] Add ENABLE(SHARED_WORKERS) flag and define SharedWorker APIs : [Attachment 32614] Removed test expectations that were incorrectly generated with non-default flags.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jul 13 00:33:23 PDT 2009


David Levin <levin at chromium.org> has granted Andrew Wilson
<atwilson at google.com>'s request for review:
Bug 26932: Add ENABLE(SHARED_WORKERS) flag and define SharedWorker APIs
https://bugs.webkit.org/show_bug.cgi?id=26932

Attachment 32614: Removed test expectations that were incorrectly generated
with non-default flags.
https://bugs.webkit.org/attachment.cgi?id=32614&action=review

------- Additional Comments from David Levin <levin at chromium.org>
> While working on a patch based on this patch, I found a bug in how
> AbstractWorker::toJS() worked, so I added a custom toJS() function in
> AbstractWorker.

It would have been nice to fix this issue while adding a layout tests that
caught the issue, but it sounds like your later patch will have a layout test
which catches this.

This looks fine.  I have a few very minor nits which I'll address while
landing.


> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> +	   (WebCore::AbstractWorker::AbstractWorker):
> +	   Common baseclass for SharedWorker and (soon) Worker. The functions
below were copied from Worker.cpp.

s/baseclass/base class/


> diff --git a/WebCore/workers/AbstractWorker.cpp
b/WebCore/workers/AbstractWorker.cpp
> +#include "AbstractWorker.h"

Typically a blank line is after the header for the cpp file.



> diff --git a/WebCore/workers/AbstractWorker.h
b/WebCore/workers/AbstractWorker.h

> +
> +#if ENABLE(SHARED_WORKERS)
> +
> +#include "AtomicStringHash.h"
> +#include "ActiveDOMObject.h"

Out of order.


More information about the webkit-reviews mailing list