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

[bugzilla-daemon at webkit.org]

Joseph Pecoraro <joepeck at webkit.org> has asked	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 Joseph Pecoraro <joepeck at webkit.org>
> No outer parenthesis needed.

Arg, I attached the same patch for patch 3 instead of the one fixing style
issues. Thanks.


> This seems like it should be an asHTMLTokenzer method of m_tokenizer.  Does
> this pattern occur elsewhere in this file?  RTTI is usually a bad idea.

I'm not sure how useful this will be. I still need to do the same thing I am
doing, checking if it is an HTMLTokenizer and casting it. But I haven't had
much experience using this idiom.

Document.cpp uses the following pattern a lot, which is similar:

    if (node->hasLocalName(someTag))
	sheet = static_cast<HTMLSomeElement*>(n)->...

There is one other use of `isHTMLTokenizer()`, in HTMLFormControlElement.cpp.


> WebCore/dom/Document.cpp:1972
>  +	      htmlTokenizer->setForceSynchronous(savedForceSynchronous);
> Can we do this with a RAII class?  The implementation you have is error
prone.

Added this in the recent patch. I'd appreciate feedback on the name to make it
clearer what this is doing.

I'm still worried about this test being too long. It takes ~7 seconds on my
machine. If anyone else thinks that is too long I'll gladly make this a manual
test.