[webkit-reviews] review granted: [Bug 23223] Teach ScheduledAction how to execute script in WorkerContext, in addition to a Document. : [Attachment 26590] Updated patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Dmitry Titov <dimich at chromium.org>'s
request for review:
Bug 23223: Teach ScheduledAction how to execute script in WorkerContext, in
addition to a Document.
https://bugs.webkit.org/show_bug.cgi?id=23223

Attachment 26590: Updated patch
https://bugs.webkit.org/attachment.cgi?id=26590&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +    if (context->isDocument())
> +	   execute(static_cast<Document*>(context));
> +#if ENABLE(WORKERS)
> +    else {
> +	   ASSERT(context->isWorkerContext());
> +	   execute(static_cast<WorkerContext*>(context));
> +    }
> +#else
>      ASSERT(context->isDocument());
> +#endif

I think the code above is unnecessarily twisted. How about this?

    #if !ENABLE(WORKERS)
	ASSERT(context->isDocument());
	execute(static_cast<Document*>(context));
    #else
	if (context->isDocument())
	    execute(static_cast<Document*>(context));
	else {
	    ASSERT(context->isWorkerContext());
	    execute(static_cast<WorkerContext*>(context));
	}
    #endif

I think Alexey's other original comments are definitely worth considering too.
But this patch seems OK to land as-is.

r=me