[webkit-reviews] review granted: [Bug 38146] document.write is not synchronous after page load : [Attachment 55169] [PATCH] Fixed Comments - Added RAII Class

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Joseph Pecoraro
<joepeck at webkit.org>'s request for review:
Bug 38146: document.write is not synchronous after page load
https://bugs.webkit.org/show_bug.cgi?id=38146

Attachment 55169: [PATCH] Fixed Comments - Added RAII Class
https://bugs.webkit.org/attachment.cgi?id=55169&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
Looks good.  A 7 second test is somewhat worrying.  Please address the comments
below before landing.

LayoutTests/fast/dom/script-tests/document-write-synchronous.js:3
 +  var immediateElementCount, delayedElementCount;
I'm not sure this does what you think it does.

WebCore/dom/Document.cpp:217
 +	    m_htmlTokenizer = asHTMLTokenizer(tokenizer);
Oh, I meant to add asHTMLTokenizer as a virtual function on Tokenizer to avoid
the static_cast.

WebCore/html/HTMLTokenizer.h:436
 +	ASSERT(tokenizer->isHTMLTokenizer());
Because write only works on HTML documents?  I think it would be better to make
this a virtual function and get rid of the static cast.

WebCore/dom/Document.cpp:1984
 +	SynchronousHTMLTokenizerGuard tokenizerGuard(m_tokenizer.get());
Do we want to use  { } to control the scope of this explicitly?