Comment on attachment 335973 FlushTags in HTMLContentSink::EndContext >

diff --git a/content/html/document/crashtests/448564.html b/content/html/document/crashtests/448564.html
>new file mode 100644
>--- /dev/null
>+++ b/content/html/document/crashtests/448564.html
>@@ -0,0 +1,7 @@
>+
>+a
>+
>+
>+
>+
>+
>\ No newline at end of file
>diff --git a/content/html/document/crashtests/crashtests.list b/content/html/document/crashtests/crashtests.list
>--- a/content/html/document/crashtests/crashtests.list
>+++ b/content/html/document/crashtests/crashtests.list
>@@ -1,3 +1,4 @@ load 388183-1.html
> load 388183-1.html
> load 395340-1.html
> load 407053.html
>+load 448564.html
>diff --git a/content/html/document/reftests/bug448564-1_malformed.html b/content/html/document/reftests/bug448564-1_malformed.html
>new file mode 100644
>--- /dev/null
>+++ b/content/html/document/reftests/bug448564-1_malformed.html
>@@ -0,0 +1,11 @@
>+
>+
>+  
>+  
>+    a
>+    b
>+    

>+    
>+  
>+
>+
>\ No newline at end of file
>diff --git a/content/html/document/reftests/bug448564-1_well-formed.html b/content/html/document/reftests/bug448564-1_well-formed.html
>new file mode 100644
>--- /dev/null
>+++ b/content/html/document/reftests/bug448564-1_well-formed.html
>@@ -0,0 +1,13 @@
>+
>+
>+  a
>+  
>+    
>+  
>+  
>+    b
>+  
>+
>+
>\ No newline at end of file
>diff --git a/content/html/document/reftests/bug448564-2_malformed.html b/content/html/document/reftests/bug448564-2_malformed.html
>new file mode 100644
>--- /dev/null
>+++ b/content/html/document/reftests/bug448564-2_malformed.html
>@@ -0,0 +1,11 @@
>+
>+
>+  
>+  a
>+  
>+  
>+  
>+  

>+  
>+
>+
>\ No newline at end of file
>diff --git a/content/html/document/reftests/bug448564-2_well-formed.html b/content/html/document/reftests/bug448564-2_well-formed.html
>new file mode 100644
>--- /dev/null
>+++ b/content/html/document/reftests/bug448564-2_well-formed.html
>@@ -0,0 +1,15 @@
>+
>+
>+  
>+    
>+  
>+  
>+    
>+      a
>+      
>+    
>+  
>+
>+
>\ No newline at end of file
>diff --git a/content/html/document/reftests/bug448564-3_malformed.html b/content/html/document/reftests/bug448564-3_malformed.html
>new file mode 100644
>--- /dev/null
>+++ b/content/html/document/reftests/bug448564-3_malformed.html
>@@ -0,0 +1,4 @@
>+

>+  
>+  b
>+head

>\ No newline at end of file
>diff --git a/content/html/document/reftests/bug448564-3_well-formed.html b/content/html/document/reftests/bug448564-3_well-formed.html
>new file mode 100644
>--- /dev/null
>+++ b/content/html/document/reftests/bug448564-3_well-formed.html
>@@ -0,0 +1,8 @@
>+
>+  
>+
>+
>+  
>+head
>\ No newline at end of file
>diff --git a/content/html/document/reftests/reftests.list b/content/html/document/reftests/reftests.list
>new file mode 100644
>--- /dev/null
>+++ b/content/html/document/reftests/reftests.list
>@@ -0,0 +1,3 @@
>+== bug448564-1_well-formed.html bug448564-1_malformed.html
>+== bug448564-2_well-formed.html bug448564-2_malformed.html
>+== bug448564-3_well-formed.html bug448564-3_malformed.html
>diff --git a/content/html/document/src/nsHTMLContentSink.cpp b/content/html/document/src/nsHTMLContentSink.cpp
>--- a/content/html/document/src/nsHTMLContentSink.cpp
>+++ b/content/html/document/src/nsHTMLContentSink.cpp
>@@ -368,16 +368,18 @@ public:
>   nsCOMPtr mLastTextNode;
>   PRInt32 mLastTextNodeSize;
> 
>   struct Node {
>     nsHTMLTag mType;
>     nsGenericHTMLElement* mContent;
>     PRUint32 mNumFlushed;
>     PRInt32 mInsertionPoint;
>+    
>+    nsIContent *Add(nsIContent *child);
>   };
> 
>   Node* mStack;
>   PRInt32 mStackSize;
>   PRInt32 mStackPos;
> 
>   PRUnichar* mText;
>   PRInt32 mTextLength;
>@@ -725,20 +727,22 @@ SinkContext::DidAddContent(nsIContent* a
>       SINK_TRACE(gSinkLogModuleInfo, SINK_TRACE_REFLOW,
>                  ("SinkContext::DidAddContent: Insertion notification for "
>                   "parent=%s at position=%d and stackPos=%d",
>                   str.get(), mStack[mStackPos - 1].mInsertionPoint - 1,
>                   mStackPos - 1));
>     }
> #endif
> 
>-    mSink->NotifyInsert(parent, aContent,
>-                        mStack[mStackPos - 1].mInsertionPoint - 1);
>+    PRInt32 childIndex = mStack[mStackPos - 1].mInsertionPoint - 1;
>+    NS_ASSERTION(parent->GetChildAt(childIndex) == aContent,
>+                 "Flushing the wrong child.");
>+    mSink->NotifyInsert(parent, aContent, childIndex);
>     mStack[mStackPos - 1].mNumFlushed = parent->GetChildCount();
>-  } else if (mSink->IsTimeToNotify()) {
>+  } else if (false && mSink->IsTimeToNotify()) {
>     SINK_TRACE(gSinkLogModuleInfo, SINK_TRACE_REFLOW,
>                ("SinkContext::DidAddContent: Notification as a result of the "
>                 "interval expiring; backoff count: %d", mSink->mBackoffCount));
>     FlushTags();
>   }
> }
> 
> nsresult
>@@ -830,25 +834,17 @@ SinkContext::OpenContainer(const nsIPars
>       break;
>     default:
>       break;    
>   }
>   
>   rv = mSink->AddAttributes(aNode, content);
>   MaybeSetForm(content, nodeType, mSink);
> 
>-  nsGenericHTMLElement* parent = mStack[mStackPos - 2].mContent;
>-
>-  if (mStack[mStackPos - 2].mInsertionPoint != -1) {
>-    parent->InsertChildAt(content,
>-                          mStack[mStackPos - 2].mInsertionPoint++,
>-                          PR_FALSE);
>-  } else {
>-    parent->AppendChildTo(content, PR_FALSE);
>-  }
>+  mStack[mStackPos - 2].Add(content);
> 
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>   if (mSink->IsMonolithicContainer(nodeType)) {
>     mSink->mInMonolithicContainer++;
>   }
> 
>   // Special handling for certain tags
>@@ -895,16 +891,29 @@ SinkContext::HaveNotifiedForCurrentConte
> SinkContext::HaveNotifiedForCurrentContent() const
> {
>   if (0 < mStackPos) {
>     nsIContent* parent = mStack[mStackPos - 1].mContent;
>     return mStack[mStackPos-1].mNumFlushed == parent->GetChildCount();
>   }
> 
>   return PR_TRUE;
>+}
>+
>+nsIContent *
>+SinkContext::Node::Add(nsIContent *child)
>+{
>+  if (mInsertionPoint != -1) {
>+    NS_ASSERTION(mNumFlushed == mContent->GetChildCount(),
>+                 "Inserting multiple children without flushing.");
>+    mContent->InsertChildAt(child, mInsertionPoint++, PR_FALSE);
>+  } else {
>+    mContent->AppendChildTo(child, PR_FALSE);
>+  }
>+  return child;
> }
> 
> nsresult
> SinkContext::CloseContainer(const nsHTMLTag aTag, PRBool aMalformed)
> {
>   nsresult result = NS_OK;
> 
>   // Flush any collected text content. Release the last text
>@@ -1148,29 +1157,18 @@ SinkContext::AddLeaf(const nsIParserNode
> 
> nsresult
> SinkContext::AddLeaf(nsGenericHTMLElement* aContent)
> {
>   NS_ASSERTION(mStackPos > 0, "leaf w/o container");
>   if (mStackPos <= 0) {
>     return NS_ERROR_FAILURE;
>   }
>-
>-  nsGenericHTMLElement* parent = mStack[mStackPos - 1].mContent;
>-
>-  // If the parent has an insertion point, insert rather than append.
>-  if (mStack[mStackPos - 1].mInsertionPoint != -1) {
>-    parent->InsertChildAt(aContent,
>-                          mStack[mStackPos - 1].mInsertionPoint++,
>-                          PR_FALSE);
>-  } else {
>-    parent->AppendChildTo(aContent, PR_FALSE);
>-  }
>-
>-  DidAddContent(aContent);
>+  
>+  DidAddContent(mStack[mStackPos - 1].Add(aContent));
> 
> #ifdef DEBUG
>   if (SINK_LOG_TEST(gSinkLogModuleInfo, SINK_ALWAYS_REFLOW)) {
>     mSink->ForceReflow();
>   }
> #endif
> 
>   return NS_OK;
>@@ -1196,35 +1194,26 @@ SinkContext::AddComment(const nsIParserN
> 
>   comment->SetText(aNode.GetText(), PR_FALSE);
> 
>   NS_ASSERTION(mStackPos > 0, "stack out of bounds");
>   if (mStackPos <= 0) {
>     return NS_ERROR_FAILURE;
>   }
> 
>-  nsGenericHTMLElement* parent;
>-  if (!mSink->mBody && !mSink->mFrameset && mSink->mHead) {
>-    // XXXbz but this will make DidAddContent use the wrong parent for
>-    // the notification!  That seems so bogus it's not even funny.
>-    parent = mSink->mHead;
>-  } else {
>-    parent = mStack[mStackPos - 1].mContent;
>+  {
>+    Node &parentNode = mStack[mStackPos - 1];
>+    nsGenericHTMLElement *parent = parentNode.mContent;
>+    if (!mSink->mBody && !mSink->mFrameset && mSink->mHead)
>+      // XXXbz but this will make DidAddContent use the wrong parent for
>+      // the notification!  That seems so bogus it's not even funny.
>+      parentNode.mContent = mSink->mHead;
>+    DidAddContent(parentNode.Add(comment));
>+    parentNode.mContent = parent;
>   }
>-
>-  // If the parent has an insertion point, insert rather than append.
>-  if (mStack[mStackPos - 1].mInsertionPoint != -1) {
>-    parent->InsertChildAt(comment,
>-                          mStack[mStackPos - 1].mInsertionPoint++,
>-                          PR_FALSE);
>-  } else {
>-    parent->AppendChildTo(comment, PR_FALSE);
>-  }
>-
>-  DidAddContent(comment);
> 
> #ifdef DEBUG
>   if (SINK_LOG_TEST(gSinkLogModuleInfo, SINK_ALWAYS_REFLOW)) {
>     mSink->ForceReflow();
>   }
> #endif
> 
>   return rv;
>@@ -1380,20 +1369,21 @@ SinkContext::FlushTags()
>           SINK_TRACE(gSinkLogModuleInfo, SINK_TRACE_REFLOW,
>                      ("SinkContext::FlushTags: tag=%s from newindex=%d at "
>                       "stackPos=%d", tagStr,
>                       mStack[stackPos].mNumFlushed, stackPos));
>         }
> #endif
>         if ((mStack[stackPos].mInsertionPoint != -1) &&
>             (mStackPos > (stackPos + 1))) {
>+          PRInt32 childIndex = mStack[stackPos].mInsertionPoint - 1;
>           nsIContent* child = mStack[stackPos + 1].mContent;
>-          mSink->NotifyInsert(content,
>-                              child,
>-                              mStack[stackPos].mInsertionPoint - 1);
>+          NS_ASSERTION(content->GetChildAt(childIndex) == child,
>+                       "Flushing the wrong child.");
>+          mSink->NotifyInsert(content, child, childIndex);
>         } else {
>           mSink->NotifyAppend(content, mStack[stackPos].mNumFlushed);
>         }
> 
>         flushed = PR_TRUE;
>       }
> 
>       mStack[stackPos].mNumFlushed = childCount;
>@@ -1484,28 +1474,19 @@ SinkContext::FlushText(PRBool* aDidFlush
>       mTextLength = 0;
> 
>       // Add text to its parent
>       NS_ASSERTION(mStackPos > 0, "leaf w/o container");
>       if (mStackPos <= 0) {
>         return NS_ERROR_FAILURE;
>       }
> 
>-      nsGenericHTMLElement* parent = mStack[mStackPos - 1].mContent;
>-      if (mStack[mStackPos - 1].mInsertionPoint != -1) {
>-        parent->InsertChildAt(mLastTextNode,
>-                              mStack[mStackPos - 1].mInsertionPoint++,
>-                              PR_FALSE);
>-      } else {
>-        parent->AppendChildTo(mLastTextNode, PR_FALSE);
>-      }
>+      DidAddContent(mStack[mStackPos - 1].Add(mLastTextNode));
> 
>       didFlush = PR_TRUE;
>-
>-      DidAddContent(mLastTextNode);
>     }
>   }
> 
>   if (aDidFlush) {
>     *aDidFlush = didFlush;
>   }
> 
>   if (aReleaseLast) {
>@@ -1939,18 +1920,20 @@ HTMLContentSink::EndContext(PRInt32 aPos
> 
>   PRInt32 n = mContextStack.Count() - 1;
>   SinkContext* sc = (SinkContext*) mContextStack.ElementAt(n);
> 
>   NS_ASSERTION(sc->mStack[aPosition].mType == mCurrentContext->mStack[0].mType,
>                "ending a wrong context");
> 
>   mCurrentContext->FlushTextAndRelease();
>+  mCurrentContext->FlushTags();
> 
>   sc->mStack[aPosition].mNumFlushed = mCurrentContext->mStack[0].mNumFlushed;
>+  sc->mStack[aPosition].mInsertionPoint = mCurrentContext->mStack[0].mInsertionPoint;
> 
>   for (PRInt32 i = 0; imStackPos; i++) {
>     NS_IF_RELEASE(mCurrentContext->mStack[i].mContent);
>   }
> 
>   delete [] mCurrentContext->mStack;
> 
>   mCurrentContext->mStack      = nsnull;
>diff --git a/layout/reftests/reftest.list b/layout/reftests/reftest.list
>--- a/layout/reftests/reftest.list
>+++ b/layout/reftests/reftest.list
>@@ -105,8 +105,11 @@ include text-transform/reftest.list
> # xul-document-load/
> include xul-document-load/reftest.list
> 
> # xul grid
> include ../xul/base/src/grid/reftests/reftest.list
> 
> # z-index/
> include z-index/reftest.list
>+
>+# reftest(s) to verify content bugfixes
>+include ../../content/html/document/reftests/reftests.list