[webkit-reviews] review granted: [Bug 40934] Support <script defer> as specified by HTML5 : [Attachment 66162] Fix Qt build

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Tony Gentilcore
<tonyg at chromium.org>'s request for review:
Bug 40934: Support <script defer> as specified by HTML5
https://bugs.webkit.org/show_bug.cgi?id=40934

Attachment 66162: Fix Qt build
https://bugs.webkit.org/attachment.cgi?id=66162&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context:
https://bugs.webkit.org/attachment.cgi?id=66162&action=prettypatch

This looks great.  A few minor comments below.	The bitfield one is the main
thing.

> WebCore/ChangeLog:16
> +2010-08-31  Tony Gentilcore	<tonyg at chromium.org>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   Support <script defer> as specified by HTML5
> +	   https://bugs.webkit.org/show_bug.cgi?id=40934
> +
> +	   Tests: fast/dom/HTMLScriptElement/defer-double-defer-write.html
> +		  fast/dom/HTMLScriptElement/defer-double-write.html
> +		  fast/dom/HTMLScriptElement/defer-inline-script.html
> +		  fast/dom/HTMLScriptElement/defer-onbeforeload.html
> +		  fast/dom/HTMLScriptElement/defer-script-invalid-url.html
> +		  fast/dom/HTMLScriptElement/defer-write.html
> +		  fast/dom/HTMLScriptElement/two-defer-writes.html
> +		  http/tests/misc/script-defer-after-slow-stylesheet.html
> +		  http/tests/misc/script-defer.html
So much boilerplate in the ChangeLog but no actual text.  :)

> WebCore/dom/DocumentParser.h:69
> +    bool isParsing() const { return m_state == ParsingState; }
> +    bool isActive() const { return m_state & (ParsingState | StoppingState);
}
> +    bool isStopping() const { return m_state == StoppingState; }
> +    bool isDetached() const { return m_state == DetachedState; }
Can you ASSERT m_document (as appropriate) in these states?

> WebCore/dom/DocumentParser.h:100
> +    enum ParserState {
> +	   ParsingState = 0x1,
> +	   StoppingState = 0x2,
> +	   StoppedState = 0x4,
> +	   DetachedState = 0x8
> +    };
Woah.  Do we really need a bit field here?  It seems like an enum should be
enough.