[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc

eric at webkit.org eric at webkit.org
Wed Dec 22 12:31:06 UTC 2010


The following commit has been merged in the debian/experimental branch:
commit 2094356102d85f995198fe945921c9520d0976e3
Author: eric at webkit.org <eric at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Wed Aug 25 00:40:18 2010 +0000

    2010-08-24  Eric Seidel  <eric at 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 at 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):
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@65958 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 8c747d0..5591ed4 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,19 @@
+2010-08-24  Eric Seidel  <eric at 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  Dumitru Daniliuc  <dumi at chromium.org>
 
         Unreviewed, updating a chromium-linux expectation.
diff --git a/LayoutTests/fast/css/font-face-svg-decoding-error.html b/LayoutTests/fast/css/font-face-svg-decoding-error.html
new file mode 100644
index 0000000..abd05dc
--- /dev/null
+++ b/LayoutTests/fast/css/font-face-svg-decoding-error.html
@@ -0,0 +1,9 @@
+<style>
+ at font-face {
+    font-family: 'abc'; src: url('resources/greenbox.png') format('svg')
+}
+span {
+    font-family: 'abc';
+}
+</style>
+<span>PASS - This test did not crash</span>
diff --git a/LayoutTests/fast/frames/resources/set-src-to-javascript-url.xhtml b/LayoutTests/fast/frames/resources/set-src-to-javascript-url.xhtml
new file mode 100644
index 0000000..149fa17
--- /dev/null
+++ b/LayoutTests/fast/frames/resources/set-src-to-javascript-url.xhtml
@@ -0,0 +1,15 @@
+<html xmlns="http://www.w3.org/1999/xhtml">
+<body>
+FAIL
+<script>
+    function returnStringValue() {
+        return "test";
+    }
+
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+
+    parent.document.getElementById("parent").setAttribute("src", "javascript: returnStringValue()");
+</script>
+</body>
+</html>
diff --git a/LayoutTests/fast/frames/set-parent-src-synchronously.xhtml b/LayoutTests/fast/frames/set-parent-src-synchronously.xhtml
new file mode 100644
index 0000000..5162935
--- /dev/null
+++ b/LayoutTests/fast/frames/set-parent-src-synchronously.xhtml
@@ -0,0 +1,6 @@
+<html xmlns="http://www.w3.org/1999/xhtml">
+<body>
+This test fails if we crash or any text below this is visible (the iframe should detach itself from the page).
+<iframe id="parent" src="resources/set-src-to-javascript-url.xhtml"></iframe>
+</body>
+</html>
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index f097f8e..8e7876c 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,56 @@
+2010-08-24  Eric Seidel  <eric at 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):
+
 2010-08-24  Patrick Gansterer  <paroga at paroga.com>
 
         Reviewed by Adam Roben.
diff --git a/WebCore/dom/DocumentParser.h b/WebCore/dom/DocumentParser.h
index 6b79cac..3c28856 100644
--- a/WebCore/dom/DocumentParser.h
+++ b/WebCore/dom/DocumentParser.h
@@ -60,7 +60,9 @@ public:
     // used to implements it.
     virtual bool processingData() const { return false; }
 
-    Document* document() const { return m_document; }
+    // document() will return 0 after detach() is called.
+    Document* document() const { ASSERT(m_document); return m_document; }
+    bool isDetached() const { return !m_document; }
 
     // Document is expected to detach the parser before releasing its ref.
     // After detach, m_document is cleared.  The parser will unwind its
@@ -84,6 +86,7 @@ protected:
     // FIXME: m_document = 0 could be changed to mean "parser stopped".
     bool m_parserStopped;
 
+private:
     // Every DocumentParser needs a pointer back to the document.
     // m_document will be 0 after the parser is stopped.
     Document* m_document;
diff --git a/WebCore/dom/RawDataDocumentParser.h b/WebCore/dom/RawDataDocumentParser.h
index 9eea9b6..093ddaf 100644
--- a/WebCore/dom/RawDataDocumentParser.h
+++ b/WebCore/dom/RawDataDocumentParser.h
@@ -39,8 +39,8 @@ protected:
 
     virtual void finish()
     {
-        if (!m_parserStopped)
-            m_document->finishedParsing();
+        if (!m_parserStopped && !isDetached())
+            document()->finishedParsing();
     }
 
 private:
diff --git a/WebCore/dom/XMLDocumentParser.cpp b/WebCore/dom/XMLDocumentParser.cpp
index 054e4e3..117a454 100644
--- a/WebCore/dom/XMLDocumentParser.cpp
+++ b/WebCore/dom/XMLDocumentParser.cpp
@@ -132,7 +132,7 @@ void XMLDocumentParser::append(const SegmentedString& s)
     if (m_sawXSLTransform || !m_sawFirstElement)
         m_originalSourceForTransform += parseString;
 
-    if (m_parserStopped || m_sawXSLTransform)
+    if (isDetached() || m_parserStopped || m_sawXSLTransform)
         return;
 
     if (m_parserPaused) {
@@ -211,6 +211,12 @@ void XMLDocumentParser::exitText()
     popCurrentNode();
 }
 
+void XMLDocumentParser::detach()
+{
+    clearCurrentNodeStack();
+    ScriptableDocumentParser::detach();
+}
+
 void XMLDocumentParser::end()
 {
     // XMLDocumentParserLibxml2 will do bad things to the document if doEnd() is called.
@@ -346,6 +352,9 @@ void XMLDocumentParser::notifyFinished(CachedResource* unusedResource)
     ScriptElement* scriptElement = toScriptElement(e.get());
     ASSERT(scriptElement);
 
+    // JavaScript can detach this parser, make sure it's kept alive even if detached.
+    RefPtr<XMLDocumentParser> protect(this);
+    
     if (errorOccurred)
         scriptElement->dispatchErrorEvent();
     else {
@@ -355,7 +364,7 @@ void XMLDocumentParser::notifyFinished(CachedResource* unusedResource)
 
     m_scriptElement = 0;
 
-    if (!m_requestingScript)
+    if (!isDetached() && !m_requestingScript)
         resumeParsing();
 }
 
diff --git a/WebCore/dom/XMLDocumentParser.h b/WebCore/dom/XMLDocumentParser.h
index bebaf7c..a93fda7 100644
--- a/WebCore/dom/XMLDocumentParser.h
+++ b/WebCore/dom/XMLDocumentParser.h
@@ -117,6 +117,7 @@ namespace WebCore {
         virtual bool finishWasCalled();
         virtual bool isWaitingForScripts() const;
         virtual void stopParsing();
+        virtual void detach();
 
         // from CachedResourceClient
         virtual void notifyFinished(CachedResource*);
diff --git a/WebCore/dom/XMLDocumentParserLibxml2.cpp b/WebCore/dom/XMLDocumentParserLibxml2.cpp
index ce25905..4022db5 100644
--- a/WebCore/dom/XMLDocumentParserLibxml2.cpp
+++ b/WebCore/dom/XMLDocumentParserLibxml2.cpp
@@ -621,13 +621,18 @@ XMLParserContext::~XMLParserContext()
 
 XMLDocumentParser::~XMLDocumentParser()
 {
-    clearCurrentNodeStack();
+    // The XMLDocumentParser will always be detached before being destroyed.
+    ASSERT(m_currentNodeStack.isEmpty());
+    ASSERT(!m_currentNode);
+
+    // FIXME: m_pendingScript handling should be moved into XMLDocumentParser.cpp!
     if (m_pendingScript)
         m_pendingScript->removeClient(this);
 }
 
 void XMLDocumentParser::doWrite(const String& parseString)
 {
+    ASSERT(!isDetached());
     if (!m_context)
         initializeParserContext();
 
@@ -636,6 +641,10 @@ void XMLDocumentParser::doWrite(const String& parseString)
 
     // libXML throws an error if you try to switch the encoding for an empty string.
     if (parseString.length()) {
+        // JavaScript may cause the parser to detach during xmlParseChunk
+        // keep this alive until this function is done.
+        RefPtr<XMLDocumentParser> protect(this);
+
         // Hack around libxml2's lack of encoding overide support by manually
         // resetting the encoding to UTF-16 before every chunk.  Otherwise libxml
         // will detect <?xml version="1.0" encoding="<encoding name>"?> blocks
@@ -646,14 +655,18 @@ void XMLDocumentParser::doWrite(const String& parseString)
 
         XMLDocumentParserScope scope(document()->docLoader());
         xmlParseChunk(context->context(), reinterpret_cast<const char*>(parseString.characters()), sizeof(UChar) * parseString.length(), 0);
+
+        // JavaScript (which may be run under the xmlParseChunk callstack) may
+        // cause the parser to be stopped or detached.
+        if (isDetached() || m_parserStopped)
+            return;
     }
 
+    // FIXME: Why is this here?  And why is it after we process the passed source?
     if (document()->decoder() && document()->decoder()->sawError()) {
         // If the decoder saw an error, report it as fatal (stops parsing)
         handleError(fatal, "Encoding error", context->context()->input->line, context->context()->input->col);
     }
-
-    return;
 }
 
 static inline String toString(const xmlChar* str, unsigned len)
@@ -860,6 +873,13 @@ void XMLDocumentParser::endElementNs()
     else
 #endif
     {
+        // FIXME: Script execution should be shared should be shared between
+        // the libxml2 and Qt XMLDocumentParser implementations.
+
+        // JavaScript can detach the parser.  Make sure this is not released
+        // before the end of this method.
+        RefPtr<XMLDocumentParser> protect(this);
+
         String scriptHref = scriptElement->sourceAttributeValue();
         if (!scriptHref.isEmpty()) {
             // we have a src attribute
@@ -876,6 +896,10 @@ void XMLDocumentParser::endElementNs()
                 m_scriptElement = 0;
         } else
             m_view->frame()->script()->executeScript(ScriptSourceCode(scriptElement->scriptContent(), document()->url(), m_scriptStartLine));
+
+        // JavaScript may have detached the parser
+        if (isDetached())
+            return;
     }
     m_requestingScript = false;
     popCurrentNode();
@@ -1354,6 +1378,7 @@ void XMLDocumentParser::stopParsing()
 
 void XMLDocumentParser::resumeParsing()
 {
+    ASSERT(!isDetached());
     ASSERT(m_parserPaused);
 
     m_parserPaused = false;
diff --git a/WebCore/html/HTMLDocumentParser.cpp b/WebCore/html/HTMLDocumentParser.cpp
index 8bdaf5c..e6e9488 100644
--- a/WebCore/html/HTMLDocumentParser.cpp
+++ b/WebCore/html/HTMLDocumentParser.cpp
@@ -193,7 +193,7 @@ bool HTMLDocumentParser::runScriptsForPausedTreeBuilder()
 
 void HTMLDocumentParser::pumpTokenizer(SynchronousMode mode)
 {
-    ASSERT(m_document);
+    ASSERT(!isDetached());
     ASSERT(!m_parserStopped);
     ASSERT(!m_treeBuilder->isPaused());
     ASSERT(!isScheduledForResume());
@@ -214,7 +214,7 @@ void HTMLDocumentParser::pumpTokenizer(SynchronousMode mode)
         m_token.clear();
 
         // JavaScript may have stopped or detached the parser.
-        if (!m_document || m_parserStopped)
+        if (isDetached() || m_parserStopped)
             return;
 
         // The parser will pause itself when waiting on a script to load or run.
@@ -226,7 +226,7 @@ void HTMLDocumentParser::pumpTokenizer(SynchronousMode mode)
         m_treeBuilder->setPaused(!shouldContinueParsing);
 
         // JavaScript may have stopped or detached the parser.
-        if (!m_document || m_parserStopped)
+        if (isDetached() || m_parserStopped)
             return;
 
         if (!shouldContinueParsing)
@@ -240,7 +240,7 @@ void HTMLDocumentParser::pumpTokenizer(SynchronousMode mode)
     if (isWaitingForScripts()) {
         ASSERT(m_tokenizer->state() == HTMLTokenizer::DataState);
         if (!m_preloadScanner) {
-            m_preloadScanner.set(new HTMLPreloadScanner(m_document));
+            m_preloadScanner.set(new HTMLPreloadScanner(document()));
             m_preloadScanner->appendToEnd(m_input.current());
         }
         m_preloadScanner->scan();
@@ -255,7 +255,7 @@ void HTMLDocumentParser::willPumpLexer()
     // FIXME: m_input.current().length() is only accurate if we
     // end up parsing the whole buffer in this pump.  We should pass how
     // much we parsed as part of didWriteHTML instead of willWriteHTML.
-    if (InspectorTimelineAgent* timelineAgent = m_document->inspectorTimelineAgent())
+    if (InspectorTimelineAgent* timelineAgent = document()->inspectorTimelineAgent())
         timelineAgent->willWriteHTML(m_input.current().length(), m_tokenizer->lineNumber());
 #endif
 }
@@ -263,7 +263,7 @@ void HTMLDocumentParser::willPumpLexer()
 void HTMLDocumentParser::didPumpLexer()
 {
 #if ENABLE(INSPECTOR)
-    if (InspectorTimelineAgent* timelineAgent = m_document->inspectorTimelineAgent())
+    if (InspectorTimelineAgent* timelineAgent = document()->inspectorTimelineAgent())
         timelineAgent->didWriteHTML(m_tokenizer->lineNumber());
 #endif
 }
@@ -325,7 +325,7 @@ void HTMLDocumentParser::append(const SegmentedString& source)
 
 void HTMLDocumentParser::end()
 {
-    ASSERT(m_document);
+    ASSERT(!isDetached());
     ASSERT(!isScheduledForResume());
 
     // pumpTokenizer can cause this parser to be detached from the Document,
@@ -355,7 +355,7 @@ void HTMLDocumentParser::attemptToEnd()
 void HTMLDocumentParser::endIfDelayed()
 {
     // If we've already been detached, don't bother ending.
-    if (!m_document)
+    if (isDetached())
         return;
 
     if (!m_endWasDelayed || shouldDelayEnd())
@@ -491,7 +491,7 @@ void HTMLDocumentParser::executeScriptsWaitingForStylesheets()
 
 ScriptController* HTMLDocumentParser::script() const
 {
-    return m_document->frame() ? m_document->frame()->script() : 0;
+    return document()->frame() ? document()->frame()->script() : 0;
 }
 
 void HTMLDocumentParser::parseDocumentFragment(const String& source, DocumentFragment* fragment, Element* contextElement, FragmentScriptingPermission scriptingPermission)
diff --git a/WebCore/html/HTMLViewSourceParser.cpp b/WebCore/html/HTMLViewSourceParser.cpp
index 79aecef..8a7984d 100644
--- a/WebCore/html/HTMLViewSourceParser.cpp
+++ b/WebCore/html/HTMLViewSourceParser.cpp
@@ -87,7 +87,7 @@ void HTMLViewSourceParser::updateTokenizerState()
         return;
 
     AtomicString tagName(m_token.name().data(), m_token.name().size());
-    m_tokenizer->setState(HTMLTreeBuilder::adjustedLexerState(m_tokenizer->state(), tagName, m_document->frame()));
+    m_tokenizer->setState(HTMLTreeBuilder::adjustedLexerState(m_tokenizer->state(), tagName, document()->frame()));
     if (tagName == HTMLNames::scriptTag) {
         // The tree builder handles scriptTag separately from the other tokenizer
         // state adjustments, so we need to handle it separately too.
diff --git a/WebCore/html/HTMLViewSourceParser.h b/WebCore/html/HTMLViewSourceParser.h
index 763d7d3..34caf43 100644
--- a/WebCore/html/HTMLViewSourceParser.h
+++ b/WebCore/html/HTMLViewSourceParser.h
@@ -59,7 +59,7 @@ private:
     virtual void finish();
     virtual bool finishWasCalled();
 
-    HTMLViewSourceDocument* document() const { return static_cast<HTMLViewSourceDocument*>(m_document); }
+    HTMLViewSourceDocument* document() const { return static_cast<HTMLViewSourceDocument*>(DecodedDataDocumentParser::document()); }
 
     void pumpTokenizer();
     String sourceForToken();
diff --git a/WebCore/loader/ImageDocument.cpp b/WebCore/loader/ImageDocument.cpp
index e5e70c7..a1a9f80 100644
--- a/WebCore/loader/ImageDocument.cpp
+++ b/WebCore/loader/ImageDocument.cpp
@@ -80,7 +80,7 @@ public:
 
     ImageDocument* document() const
     {
-        return static_cast<ImageDocument*>(m_document);
+        return static_cast<ImageDocument*>(RawDataDocumentParser::document());
     }
     
 private:

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list