Changeset 62033 in webkit

Timestamp:

Jun 28, 2010 1:30:32 PM (14 years ago)

Author:

abarth@webkit.org

Message:

2010-06-28 Adam Barth <​abarth@webkit.org>

Reviewed by Eric Seidel.

HTML5 Regression: Crash in insert()
​https://bugs.webkit.org/show_bug.cgi?id=41281

We need to call endIfDelayed() outside of the script nesting block
because endIfDelayed() might call end(), which deletes the
HTMLDocumentParser. If we try to exit the script nesting block after
the HTMLDocumentParser has been deleted, we'll decrement unallocated
memory, which is bad times.

Moving endIfDelayed outside of the script nesting block also lets us
avoid ending if inWrite() is true. If we're inWrite(), then there's
folks above us on the stack who will crash of the HTMLDocumentParser is
deallocated. Adding this check matches the LegacyHTMLDocumentParser
and the logic in attemptToEnd, facilitating a small refactoring of the
common logic for improved readability.

I don't know of any test case that changes in behavior because of this
patch, but this bug exists on the same line of code that the reliablity
tests crashed. I'm not sure whether this patch will fix that crash,
but removing bugs (even theoretical ones) seems like a good idea.

• html/HTMLDocumentParser.cpp: (WebCore::HTMLDocumentParser::insert): (WebCore::HTMLDocumentParser::append): (WebCore::HTMLDocumentParser::attemptToEnd): (WebCore::HTMLDocumentParser::endIfDelayed): (WebCore::HTMLDocumentParser::resumeParsingAfterScriptExecution):
• html/HTMLDocumentParser.h: (WebCore::HTMLDocumentParser::shouldDelayEnd):

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• html/HTMLDocumentParser.cpp (modified) (5 diffs)
• html/HTMLDocumentParser.h (modified) (1 diff)

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-06-28  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        HTML5 Regression: Crash in insert()
+        https://bugs.webkit.org/show_bug.cgi?id=41281
+
+        We need to call endIfDelayed() outside of the script nesting block
+        because endIfDelayed() might call end(), which deletes the
+        HTMLDocumentParser.  If we try to exit the script nesting block after
+        the HTMLDocumentParser has been deleted, we'll decrement unallocated
+        memory, which is bad times.
+
+        Moving endIfDelayed outside of the script nesting block also lets us
+        avoid ending if inWrite() is true.  If we're inWrite(), then there's
+        folks above us on the stack who will crash of the HTMLDocumentParser is
+        deallocated.  Adding this check matches the LegacyHTMLDocumentParser
+        and the logic in attemptToEnd, facilitating a small refactoring of the
+        common logic for improved readability.
+
+        I don't know of any test case that changes in behavior because of this
+        patch, but this bug exists on the same line of code that the reliablity
+        tests crashed.  I'm not sure whether this patch will fix that crash,
+        but removing bugs (even theoretical ones) seems like a good idea.
+
+        * html/HTMLDocumentParser.cpp:
+        (WebCore::HTMLDocumentParser::insert):
+        (WebCore::HTMLDocumentParser::append):
+        (WebCore::HTMLDocumentParser::attemptToEnd):
+        (WebCore::HTMLDocumentParser::endIfDelayed):
+        (WebCore::HTMLDocumentParser::resumeParsingAfterScriptExecution):
+        * html/HTMLDocumentParser.h:
+        (WebCore::HTMLDocumentParser::shouldDelayEnd):
+
 2010-06-28  Eric Carlson  <eric.carlson@apple.com>
 

trunk/WebCore/html/HTMLDocumentParser.cpp

@@ -219 +219 @@
         return;
 
-    NestingLevelIncrementer nestingLevelIncrementer(m_writeNestingLevel);
-
-    SegmentedString excludedLineNumberSource(source);
-    excludedLineNumberSource.setExcludeLineNumbers();
-    m_input.insertAtCurrentInsertionPoint(excludedLineNumberSource);
-    pumpTokenizerIfPossible(ForceSynchronous);
+    {
+        NestingLevelIncrementer nestingLevelIncrementer(m_writeNestingLevel);
+
+        SegmentedString excludedLineNumberSource(source);
+        excludedLineNumberSource.setExcludeLineNumbers();
+        m_input.insertAtCurrentInsertionPoint(excludedLineNumberSource);
+        pumpTokenizerIfPossible(ForceSynchronous);
+    }
+
     endIfDelayed();
 }
@@ -233 +236 @@
         return;
 
-    NestingLevelIncrementer nestingLevelIncrementer(m_writeNestingLevel);
-
-    m_input.appendToEnd(source);
-    if (m_preloadScanner)
-        m_preloadScanner->appendToEnd(source);
-
-    if (m_writeNestingLevel > 1) {
-        // We've gotten data off the network in a nested write.
-        // We don't want to consume any more of the input stream now.  Do
-        // not worry.  We'll consume this data in a less-nested write().
-        return;
-    }
-
-    pumpTokenizerIfPossible(AllowYield);
+    {
+        NestingLevelIncrementer nestingLevelIncrementer(m_writeNestingLevel);
+
+        m_input.appendToEnd(source);
+        if (m_preloadScanner)
+            m_preloadScanner->appendToEnd(source);
+
+        if (m_writeNestingLevel > 1) {
+            // We've gotten data off the network in a nested write.
+            // We don't want to consume any more of the input stream now.  Do
+            // not worry.  We'll consume this data in a less-nested write().
+            return;
+        }
+
+        pumpTokenizerIfPossible(AllowYield);
+    }
+
     endIfDelayed();
 }
@@ -266 +272 @@
     // an external script to load, we can't finish parsing quite yet.
 
-    if (inWrite() || isWaitingForScripts() || inScriptExecution() || isScheduledForResume()) {
+    if (shouldDelayEnd()) {
         m_endWasDelayed = true;
         return;
@@ -275 +281 @@
 void HTMLDocumentParser::endIfDelayed()
 {
-    // We don't check inWrite() here since inWrite() will be true if this was
-    // called from write().
-    if (!m_endWasDelayed || isWaitingForScripts() || inScriptExecution() || isScheduledForResume())
+    if (!m_endWasDelayed || shouldDelayEnd())
         return;
 
@@ -337 +341 @@
 
     pumpTokenizerIfPossible(AllowYield);
-
-    // The document already finished parsing we were just waiting on scripts when finished() was called.
     endIfDelayed();
 }

trunk/WebCore/html/HTMLDocumentParser.h

@@ -110 +110 @@
     bool inScriptExecution() const;
     bool inWrite() const { return m_writeNestingLevel > 0; }
+    bool shouldDelayEnd() const { return inWrite() || isWaitingForScripts() || inScriptExecution() || isScheduledForResume(); }
 
     ScriptController* script() const;