Changeset 65958 in webkit


Ignore:
Timestamp:
Aug 24, 2010 5:40:18 PM (14 years ago)
Author:
eric@webkit.org
Message:

2010-08-24 Eric Seidel <eric@webkit.org>

Reviewed by Adam Barth.

XMLDocumentParser needs to implement DocumentParser::detach()
https://bugs.webkit.org/show_bug.cgi?id=44533

Added a test which uses an image as an SVG font (decoding errors galore).

XML versions of one of the tests from
http://trac.webkit.org/changeset/65692

  • fast/css/font-face-svg-decoding-error.html: Added.
  • fast/frames/resources/set-src-to-javascript-url.xhtml: Added.
  • fast/frames/set-parent-src-synchronously.xhtml: Added.

2010-08-24 Eric Seidel <eric@webkit.org>

Reviewed by Adam Barth.

XMLDocumentParser needs to implement DocumentParser::detach()
https://bugs.webkit.org/show_bug.cgi?id=44533

Test: fast/frames/set-parent-src-synchronously.xhtml

In the example from the page we were accessing document()
after DocumentParser::detach() was called. To prevent this
I added an ASSERT(m_document) to document(), causing many
test cases to cover the bug shown in bug 44533.

To fix the bug, I implemented XMLDocumentParser::detach()
and had it call clearCurrentNodeStack(), thus making
it impossible for XMLDocumentParser to still have the Document
on the node stack after detach (which was what was causing this bug).

While fixing this, I noticed that XMLDocumentParser may have the
same trouble crashing that the HTMLDocumentParser did when
synchronously deleted from JS (for example by an iframe navigation).
I added a test case to cover this and protected the parser after
the two places it executes scripts.

  • dom/DocumentParser.h: (WebCore::DocumentParser::document): (WebCore::DocumentParser::isDetached):
  • dom/RawDataDocumentParser.h: (WebCore::RawDataDocumentParser::finish):
  • dom/XMLDocumentParser.cpp: (WebCore::XMLDocumentParser::append): (WebCore::XMLDocumentParser::detach): (WebCore::XMLDocumentParser::notifyFinished):
  • dom/XMLDocumentParser.h:
  • dom/XMLDocumentParserLibxml2.cpp: (WebCore::XMLDocumentParser::doWrite): (WebCore::XMLDocumentParser::endElementNs): (WebCore::XMLDocumentParser::resumeParsing):
  • html/HTMLDocumentParser.cpp: (WebCore::HTMLDocumentParser::pumpTokenizer): (WebCore::HTMLDocumentParser::willPumpLexer): (WebCore::HTMLDocumentParser::didPumpLexer): (WebCore::HTMLDocumentParser::end): (WebCore::HTMLDocumentParser::endIfDelayed): (WebCore::HTMLDocumentParser::script):
  • html/HTMLViewSourceParser.cpp: (WebCore::HTMLViewSourceParser::updateTokenizerState):
  • html/HTMLViewSourceParser.h: (WebCore::HTMLViewSourceParser::document):
  • loader/ImageDocument.cpp: (WebCore::ImageDocumentParser::document):
Location:
trunk
Files:
3 added
11 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r65956 r65958  
     12010-08-24  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by Adam Barth.
     4
     5        XMLDocumentParser needs to implement DocumentParser::detach()
     6        https://bugs.webkit.org/show_bug.cgi?id=44533
     7
     8        Added a test which uses an image as an SVG font (decoding errors galore).
     9
     10        XML versions of one of the tests from
     11        http://trac.webkit.org/changeset/65692
     12
     13        * fast/css/font-face-svg-decoding-error.html: Added.
     14        * fast/frames/resources/set-src-to-javascript-url.xhtml: Added.
     15        * fast/frames/set-parent-src-synchronously.xhtml: Added.
     16
    1172010-08-24  Dumitru Daniliuc  <dumi@chromium.org>
    218
  • trunk/WebCore/ChangeLog

    r65957 r65958  
     12010-08-24  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by Adam Barth.
     4
     5        XMLDocumentParser needs to implement DocumentParser::detach()
     6        https://bugs.webkit.org/show_bug.cgi?id=44533
     7
     8        Test: fast/frames/set-parent-src-synchronously.xhtml
     9
     10        In the example from the page we were accessing document()
     11        after DocumentParser::detach() was called.  To prevent this
     12        I added an ASSERT(m_document) to document(), causing many
     13        test cases to cover the bug shown in bug 44533.
     14
     15        To fix the bug, I implemented XMLDocumentParser::detach()
     16        and had it call clearCurrentNodeStack(), thus making
     17        it impossible for XMLDocumentParser to still have the Document
     18        on the node stack after detach (which was what was causing this bug).
     19
     20        While fixing this, I noticed that XMLDocumentParser may have the
     21        same trouble crashing that the HTMLDocumentParser did when
     22        synchronously deleted from JS (for example by an iframe navigation).
     23        I added a test case to cover this and protected the parser after
     24        the two places it executes scripts.
     25
     26        * dom/DocumentParser.h:
     27        (WebCore::DocumentParser::document):
     28        (WebCore::DocumentParser::isDetached):
     29        * dom/RawDataDocumentParser.h:
     30        (WebCore::RawDataDocumentParser::finish):
     31        * dom/XMLDocumentParser.cpp:
     32        (WebCore::XMLDocumentParser::append):
     33        (WebCore::XMLDocumentParser::detach):
     34        (WebCore::XMLDocumentParser::notifyFinished):
     35        * dom/XMLDocumentParser.h:
     36        * dom/XMLDocumentParserLibxml2.cpp:
     37        (WebCore::XMLDocumentParser::doWrite):
     38        (WebCore::XMLDocumentParser::endElementNs):
     39        (WebCore::XMLDocumentParser::resumeParsing):
     40        * html/HTMLDocumentParser.cpp:
     41        (WebCore::HTMLDocumentParser::pumpTokenizer):
     42        (WebCore::HTMLDocumentParser::willPumpLexer):
     43        (WebCore::HTMLDocumentParser::didPumpLexer):
     44        (WebCore::HTMLDocumentParser::end):
     45        (WebCore::HTMLDocumentParser::endIfDelayed):
     46        (WebCore::HTMLDocumentParser::script):
     47        * html/HTMLViewSourceParser.cpp:
     48        (WebCore::HTMLViewSourceParser::updateTokenizerState):
     49        * html/HTMLViewSourceParser.h:
     50        (WebCore::HTMLViewSourceParser::document):
     51        * loader/ImageDocument.cpp:
     52        (WebCore::ImageDocumentParser::document):
     53
    1542010-08-24  Patrick Gansterer  <paroga@paroga.com>
    255
  • trunk/WebCore/dom/DocumentParser.h

    r65941 r65958  
    6161    virtual bool processingData() const { return false; }
    6262
    63     Document* document() const { return m_document; }
     63    // document() will return 0 after detach() is called.
     64    Document* document() const { ASSERT(m_document); return m_document; }
     65    bool isDetached() const { return !m_document; }
    6466
    6567    // Document is expected to detach the parser before releasing its ref.
     
    8587    bool m_parserStopped;
    8688
     89private:
    8790    // Every DocumentParser needs a pointer back to the document.
    8891    // m_document will be 0 after the parser is stopped.
  • trunk/WebCore/dom/RawDataDocumentParser.h

    r65692 r65958  
    4040    virtual void finish()
    4141    {
    42         if (!m_parserStopped)
    43             m_document->finishedParsing();
     42        if (!m_parserStopped && !isDetached())
     43            document()->finishedParsing();
    4444    }
    4545
  • trunk/WebCore/dom/XMLDocumentParser.cpp

    r65842 r65958  
    133133        m_originalSourceForTransform += parseString;
    134134
    135     if (m_parserStopped || m_sawXSLTransform)
     135    if (isDetached() || m_parserStopped || m_sawXSLTransform)
    136136        return;
    137137
     
    210210
    211211    popCurrentNode();
     212}
     213
     214void XMLDocumentParser::detach()
     215{
     216    clearCurrentNodeStack();
     217    ScriptableDocumentParser::detach();
    212218}
    213219
     
    347353    ASSERT(scriptElement);
    348354
     355    // JavaScript can detach this parser, make sure it's kept alive even if detached.
     356    RefPtr<XMLDocumentParser> protect(this);
     357   
    349358    if (errorOccurred)
    350359        scriptElement->dispatchErrorEvent();
     
    356365    m_scriptElement = 0;
    357366
    358     if (!m_requestingScript)
     367    if (!isDetached() && !m_requestingScript)
    359368        resumeParsing();
    360369}
  • trunk/WebCore/dom/XMLDocumentParser.h

    r65878 r65958  
    118118        virtual bool isWaitingForScripts() const;
    119119        virtual void stopParsing();
     120        virtual void detach();
    120121
    121122        // from CachedResourceClient
  • trunk/WebCore/dom/XMLDocumentParserLibxml2.cpp

    r65878 r65958  
    622622XMLDocumentParser::~XMLDocumentParser()
    623623{
    624     clearCurrentNodeStack();
     624    // The XMLDocumentParser will always be detached before being destroyed.
     625    ASSERT(m_currentNodeStack.isEmpty());
     626    ASSERT(!m_currentNode);
     627
     628    // FIXME: m_pendingScript handling should be moved into XMLDocumentParser.cpp!
    625629    if (m_pendingScript)
    626630        m_pendingScript->removeClient(this);
     
    629633void XMLDocumentParser::doWrite(const String& parseString)
    630634{
     635    ASSERT(!isDetached());
    631636    if (!m_context)
    632637        initializeParserContext();
     
    637642    // libXML throws an error if you try to switch the encoding for an empty string.
    638643    if (parseString.length()) {
     644        // JavaScript may cause the parser to detach during xmlParseChunk
     645        // keep this alive until this function is done.
     646        RefPtr<XMLDocumentParser> protect(this);
     647
    639648        // Hack around libxml2's lack of encoding overide support by manually
    640649        // resetting the encoding to UTF-16 before every chunk.  Otherwise libxml
     
    647656        XMLDocumentParserScope scope(document()->docLoader());
    648657        xmlParseChunk(context->context(), reinterpret_cast<const char*>(parseString.characters()), sizeof(UChar) * parseString.length(), 0);
    649     }
    650 
     658
     659        // JavaScript (which may be run under the xmlParseChunk callstack) may
     660        // cause the parser to be stopped or detached.
     661        if (isDetached() || m_parserStopped)
     662            return;
     663    }
     664
     665    // FIXME: Why is this here?  And why is it after we process the passed source?
    651666    if (document()->decoder() && document()->decoder()->sawError()) {
    652667        // If the decoder saw an error, report it as fatal (stops parsing)
    653668        handleError(fatal, "Encoding error", context->context()->input->line, context->context()->input->col);
    654669    }
    655 
    656     return;
    657670}
    658671
     
    861874#endif
    862875    {
     876        // FIXME: Script execution should be shared should be shared between
     877        // the libxml2 and Qt XMLDocumentParser implementations.
     878
     879        // JavaScript can detach the parser.  Make sure this is not released
     880        // before the end of this method.
     881        RefPtr<XMLDocumentParser> protect(this);
     882
    863883        String scriptHref = scriptElement->sourceAttributeValue();
    864884        if (!scriptHref.isEmpty()) {
     
    877897        } else
    878898            m_view->frame()->script()->executeScript(ScriptSourceCode(scriptElement->scriptContent(), document()->url(), m_scriptStartLine));
     899
     900        // JavaScript may have detached the parser
     901        if (isDetached())
     902            return;
    879903    }
    880904    m_requestingScript = false;
     
    13551379void XMLDocumentParser::resumeParsing()
    13561380{
     1381    ASSERT(!isDetached());
    13571382    ASSERT(m_parserPaused);
    13581383
  • trunk/WebCore/html/HTMLDocumentParser.cpp

    r65932 r65958  
    194194void HTMLDocumentParser::pumpTokenizer(SynchronousMode mode)
    195195{
    196     ASSERT(m_document);
     196    ASSERT(!isDetached());
    197197    ASSERT(!m_parserStopped);
    198198    ASSERT(!m_treeBuilder->isPaused());
     
    215215
    216216        // JavaScript may have stopped or detached the parser.
    217         if (!m_document || m_parserStopped)
     217        if (isDetached() || m_parserStopped)
    218218            return;
    219219
     
    227227
    228228        // JavaScript may have stopped or detached the parser.
    229         if (!m_document || m_parserStopped)
     229        if (isDetached() || m_parserStopped)
    230230            return;
    231231
     
    241241        ASSERT(m_tokenizer->state() == HTMLTokenizer::DataState);
    242242        if (!m_preloadScanner) {
    243             m_preloadScanner.set(new HTMLPreloadScanner(m_document));
     243            m_preloadScanner.set(new HTMLPreloadScanner(document()));
    244244            m_preloadScanner->appendToEnd(m_input.current());
    245245        }
     
    256256    // end up parsing the whole buffer in this pump.  We should pass how
    257257    // much we parsed as part of didWriteHTML instead of willWriteHTML.
    258     if (InspectorTimelineAgent* timelineAgent = m_document->inspectorTimelineAgent())
     258    if (InspectorTimelineAgent* timelineAgent = document()->inspectorTimelineAgent())
    259259        timelineAgent->willWriteHTML(m_input.current().length(), m_tokenizer->lineNumber());
    260260#endif
     
    264264{
    265265#if ENABLE(INSPECTOR)
    266     if (InspectorTimelineAgent* timelineAgent = m_document->inspectorTimelineAgent())
     266    if (InspectorTimelineAgent* timelineAgent = document()->inspectorTimelineAgent())
    267267        timelineAgent->didWriteHTML(m_tokenizer->lineNumber());
    268268#endif
     
    326326void HTMLDocumentParser::end()
    327327{
    328     ASSERT(m_document);
     328    ASSERT(!isDetached());
    329329    ASSERT(!isScheduledForResume());
    330330
     
    356356{
    357357    // If we've already been detached, don't bother ending.
    358     if (!m_document)
     358    if (isDetached())
    359359        return;
    360360
     
    492492ScriptController* HTMLDocumentParser::script() const
    493493{
    494     return m_document->frame() ? m_document->frame()->script() : 0;
     494    return document()->frame() ? document()->frame()->script() : 0;
    495495}
    496496
  • trunk/WebCore/html/HTMLViewSourceParser.cpp

    r65932 r65958  
    8888
    8989    AtomicString tagName(m_token.name().data(), m_token.name().size());
    90     m_tokenizer->setState(HTMLTreeBuilder::adjustedLexerState(m_tokenizer->state(), tagName, m_document->frame()));
     90    m_tokenizer->setState(HTMLTreeBuilder::adjustedLexerState(m_tokenizer->state(), tagName, document()->frame()));
    9191    if (tagName == HTMLNames::scriptTag) {
    9292        // The tree builder handles scriptTag separately from the other tokenizer
  • trunk/WebCore/html/HTMLViewSourceParser.h

    r65941 r65958  
    6060    virtual bool finishWasCalled();
    6161
    62     HTMLViewSourceDocument* document() const { return static_cast<HTMLViewSourceDocument*>(m_document); }
     62    HTMLViewSourceDocument* document() const { return static_cast<HTMLViewSourceDocument*>(DecodedDataDocumentParser::document()); }
    6363
    6464    void pumpTokenizer();
  • trunk/WebCore/loader/ImageDocument.cpp

    r65692 r65958  
    8181    ImageDocument* document() const
    8282    {
    83         return static_cast<ImageDocument*>(m_document);
     83        return static_cast<ImageDocument*>(RawDataDocumentParser::document());
    8484    }
    8585   
Note: See TracChangeset for help on using the changeset viewer.