[webkit-reviews] review denied: [Bug 47035] Application cache selection algorithm should only be invoked after navigation : [Attachment 69565] proposed fix

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Oct 2 15:32:41 PDT 2010


Adam Barth <abarth at webkit.org> has denied Alexey Proskuryakov <ap at webkit.org>'s
request for review:
Bug 47035: Application cache selection algorithm should only be invoked after
navigation
https://bugs.webkit.org/show_bug.cgi?id=47035

Attachment 69565: proposed fix
https://bugs.webkit.org/attachment.cgi?id=69565&action=review

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

Please add a test case for an HTMLHtmlElement created by createElement being
added to the document while the parser is running.

> WebCore/dom/DocumentParser.h:92
> +    void setWasLoadedAsPartOfNavigation() { m_wasLoadedAsPartOfNavigation =
true; }
> +    bool wasLoadedAsPartOfNavigation() const { return
m_wasLoadedAsPartOfNavigation; }

Can we make this an argument to the constructor?  This API is much wider than
we need.  Also, the getter can be protected if we the parser itself make the
check.	Adding it to the constructor will also let us scope the bit to the
HTMLDocumentParser, where it seems to belong according to the spec.

> WebCore/html/HTMLHtmlElement.cpp:65
> -    if (!document()->parsing())
> +    if (!document()->parser() ||
!document()->parser()->wasLoadedAsPartOfNavigation())
>	   return;

This code looks wrong.	It only supposed to trigger for HTMLHtmlElement created
by the parser.	It's entirely possible to createElement('html') and insert it
into the document while the document has a parser that was loaded as part of
navigation.

Instead of putting this code in insertedIntoDocument, we should put the code in
a new method of HTMLHtmlElement that the parser calls explicitly in the proper
state.

> WebCore/loader/FrameLoader.cpp:614
> +    m_frame->document()->parser()->setWasLoadedAsPartOfNavigation();

This works should be done by the DocumentWriter.  It's responsible for
interacting with the parser.


More information about the webkit-reviews mailing list