Bug 438987, backing out
authorRobert O'Callahan <robert@ocallahan.org>
Fri, 08 Aug 2008 15:34:43 +1200
changeset 16503 d8140e40fbc31bcfb361b4da25d7a118ef4d24e6
parent 16502 9628f4628ed46cc75bf48550086e036aea6e34a4
child 16504 684f482c363a0dd3bf316770eb5084d9912829a6
push id1086
push userrocallahan@mozilla.com
push dateFri, 08 Aug 2008 03:35:02 +0000
treeherdermozilla-central@d8140e40fbc3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs438987
milestone1.9.1a2pre
Bug 438987, backing out
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsCSSFrameConstructor.h
layout/base/nsCSSRendering.cpp
layout/reftests/bugs/reftest.list
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -1789,17 +1789,16 @@ GetChildListNameFor(nsIFrame*       aChi
 
 //----------------------------------------------------------------------
 
 nsCSSFrameConstructor::nsCSSFrameConstructor(nsIDocument *aDocument,
                                              nsIPresShell *aPresShell)
   : mDocument(aDocument)
   , mPresShell(aPresShell)
   , mInitialContainingBlock(nsnull)
-  , mRootElementStyleFrame(nsnull)
   , mFixedContainingBlock(nsnull)
   , mDocElementContainingBlock(nsnull)
   , mGfxScrollFrame(nsnull)
   , mPageSequenceFrame(nsnull)
   , mUpdateCount(0)
   , mQuotesDirty(PR_FALSE)
   , mCountersDirty(PR_FALSE)
   , mInitialContainingBlockIsAbsPosContainer(PR_FALSE)
@@ -4376,26 +4375,16 @@ nsCSSFrameConstructor::ConstructDocEleme
   // set the primary frame
   aState.mFrameManager->SetPrimaryFrameFor(aDocElement, contentFrame);
 
   *aNewFrame = contentFrame;
 
   mInitialContainingBlock = contentFrame;
   mInitialContainingBlockIsAbsPosContainer = PR_FALSE;
 
-  // Figure out which frame has the main style for the document element,
-  // assigning it to mRootElementStyleFrame.
-  // Backgrounds should be propagated from that frame to the viewport.
-  PRBool isChild;
-  contentFrame->GetParentStyleContextFrame(aState.mPresContext,
-          &mRootElementStyleFrame, &isChild);
-  if (!isChild) {
-    mRootElementStyleFrame = mInitialContainingBlock;
-  }
-
   // if it was a table then we don't need to process our children.
   if (!docElemIsTable) {
     // Process the child content
     nsFrameConstructorSaveState absoluteSaveState;
     nsFrameConstructorSaveState floatSaveState;
     nsFrameItems                childItems;
 
     if (isBlockFrame) {
@@ -9466,17 +9455,17 @@ nsCSSFrameConstructor::ContentRemoved(ns
       }
       else {
         rv = frameManager->RemoveFrame(parentFrame, nsnull, childFrame);
       }
     }
 
     if (mInitialContainingBlock == childFrame) {
       mInitialContainingBlock = nsnull;
-      mRootElementStyleFrame = nsnull;
+      mInitialContainingBlockIsAbsPosContainer = PR_FALSE;
     }
 
     if (haveFLS && mInitialContainingBlock) {
       NS_ASSERTION(containingBlock == GetFloatContainingBlock(parentFrame),
                    "What happened here?");
       nsFrameConstructorState state(mPresShell, mFixedContainingBlock,
                                     GetAbsoluteContainingBlock(parentFrame),
                                     containingBlock);
--- a/layout/base/nsCSSFrameConstructor.h
+++ b/layout/base/nsCSSFrameConstructor.h
@@ -243,23 +243,17 @@ public:
                                 nsIContent*     aChild,
                                 nsIFrame**      aResult,
                                 PRBool          aIsAppend,
                                 PRBool          aIsScrollbar,
                                 nsILayoutHistoryState* aFrameState);
 
   nsresult RemoveMappingsForFrameSubtree(nsIFrame* aRemovedFrame);
 
-  // This is misnamed! This returns the outermost frame for the root element
   nsIFrame* GetInitialContainingBlock() { return mInitialContainingBlock; }
-  // This returns the outermost frame for the root element
-  nsIFrame* GetRootElementFrame() { return mInitialContainingBlock; }
-  // This returns the frame for the root element that does not
-  // have a psuedo-element style
-  nsIFrame* GetRootElementStyleFrame() { return mRootElementStyleFrame; }
   nsIFrame* GetPageSequenceFrame() { return mPageSequenceFrame; }
 
 private:
 
   nsresult ReconstructDocElementHierarchyInternal();
 
   nsresult ReinsertContent(nsIContent*    aContainer,
                            nsIContent*    aChild);
@@ -1145,25 +1139,18 @@ private:
     nsCOMPtr<nsIPresShell> mPresShell;
     nsLazyFrameConstructionCallback* mCallback;
     void* mArg;
   };
 
   nsIDocument*        mDocument;  // Weak ref
   nsIPresShell*       mPresShell; // Weak ref
 
-  // This is not the real CSS 2.1 "initial containing block"! It is just
-  // the outermost frame for the root element.
   nsIFrame*           mInitialContainingBlock;
-  // This is the frame for the root element that has no pseudo-element style.
-  nsIFrame*           mRootElementStyleFrame;
-  // This is the containing block for fixed-pos frames --- the viewport
   nsIFrame*           mFixedContainingBlock;
-  // This is the containing block that contains the root element ---
-  // the real "initial containing block" according to CSS 2.1.
   nsIFrame*           mDocElementContainingBlock;
   nsIFrame*           mGfxScrollFrame;
   nsIFrame*           mPageSequenceFrame;
   nsQuoteList         mQuoteList;
   nsCounterManager    mCounterManager;
   PRUint16            mUpdateCount;
   PRPackedBool        mQuotesDirty : 1;
   PRPackedBool        mCountersDirty : 1;
--- a/layout/base/nsCSSRendering.cpp
+++ b/layout/base/nsCSSRendering.cpp
@@ -73,17 +73,16 @@
 #include "nsLayoutUtils.h"
 #include "nsINameSpaceManager.h"
 #include "nsBlockFrame.h"
 #include "gfxContext.h"
 #include "nsIInterfaceRequestorUtils.h"
 #include "gfxPlatform.h"
 #include "gfxImageSurface.h"
 #include "nsStyleStructInlines.h"
-#include "nsCSSFrameConstructor.h"
 
 #include "nsCSSRenderingBorders.h"
 
 // To avoid storing this data on nsInlineFrame (bloat) and to avoid
 // recalculating this for each frame in a continuation (perf), hold
 // a cache of various coordinate information that we need in order
 // to paint inline backgrounds.
 struct InlineBackgroundData
@@ -1003,133 +1002,161 @@ nsCSSRendering::FindNonTransparentBackgr
  * |FindBackground| returns true if a background should be painted, and
  * the resulting style context to use for the background information
  * will be filled in to |aBackground|.  It fills in a boolean indicating
  * whether the frame is the canvas frame to allow PaintBackground to
  * ensure that it always paints something non-transparent for the
  * canvas.
  */
 
-// Returns true if aFrame is a canvas frame.
+// Returns nsnull if aFrame is not a canvas frame.
+// Otherwise, it returns the frame we should look for the background on.
+// This is normally aFrame but if aFrame is the viewport, we need to
+// look for the background starting at the scroll root (which shares
+// style context with the document root) or the document root itself.
 // We need to treat the viewport as canvas because, even though
 // it does not actually paint a background, we need to get the right
 // background style so we correctly detect transparent documents.
-inline PRBool
+inline nsIFrame*
 IsCanvasFrame(nsIFrame *aFrame)
 {
   nsIAtom* frameType = aFrame->GetType();
-  return frameType == nsGkAtoms::canvasFrame ||
-         frameType == nsGkAtoms::rootFrame ||
-         frameType == nsGkAtoms::pageFrame ||
-         frameType == nsGkAtoms::pageContentFrame ||
-         frameType == nsGkAtoms::viewportFrame;
+  if (frameType == nsGkAtoms::canvasFrame ||
+      frameType == nsGkAtoms::rootFrame ||
+      frameType == nsGkAtoms::pageFrame ||
+      frameType == nsGkAtoms::pageContentFrame) {
+    return aFrame;
+  } else if (frameType == nsGkAtoms::viewportFrame) {
+    nsIFrame* firstChild = aFrame->GetFirstChild(nsnull);
+    if (firstChild) {
+      return firstChild;
+    }
+  }
+  
+  return nsnull;
 }
 
 inline PRBool
-FindCanvasBackground(nsIFrame* aForFrame, nsIFrame* aRootElementFrame,
+FindCanvasBackground(nsIFrame* aForFrame,
                      const nsStyleBackground** aBackground)
 {
-  if (aRootElementFrame) {
-    const nsStyleBackground* result = aRootElementFrame->GetStyleBackground();
+  // XXXldb What if the root element is positioned, etc.?  (We don't
+  // allow that yet, do we?)
+  nsIFrame *firstChild = aForFrame->GetFirstChild(nsnull);
+  if (firstChild) {
+    const nsStyleBackground* result = firstChild->GetStyleBackground();
+    nsIFrame* topFrame = aForFrame;
+
+    if (firstChild->GetType() == nsGkAtoms::pageContentFrame) {
+      topFrame = firstChild->GetFirstChild(nsnull);
+      NS_ASSERTION(topFrame,
+                   "nsPageContentFrame is missing a normal flow child");
+      if (!topFrame) {
+        return PR_FALSE;
+      }
+      NS_ASSERTION(topFrame->GetContent(),
+                   "nsPageContentFrame child without content");
+      result = topFrame->GetStyleBackground();
+    }
 
     // Check if we need to do propagation from BODY rather than HTML.
     if (result->IsTransparent()) {
-      nsIContent* content = aRootElementFrame->GetContent();
-      // The root element content can't be null. We wouldn't know what
-      // frame to create for aRootElementFrame.
-      // Use |GetOwnerDoc| so it works during destruction.
-      nsIDocument* document = content->GetOwnerDoc();
-      nsCOMPtr<nsIHTMLDocument> htmlDoc = do_QueryInterface(document);
-      if (htmlDoc) {
-        nsIContent* bodyContent = htmlDoc->GetBodyContentExternal();
-        // We need to null check the body node (bug 118829) since
-        // there are cases, thanks to the fix for bug 5569, where we
-        // will reflow a document with no body.  In particular, if a
-        // SCRIPT element in the head blocks the parser and then has a
-        // SCRIPT that does "document.location.href = 'foo'", then
-        // nsParser::Terminate will call |DidBuildModel| methods
-        // through to the content sink, which will call |StartLayout|
-        // and thus |InitialReflow| on the pres shell.  See bug 119351
-        // for the ugly details.
-        if (bodyContent) {
-          nsIFrame *bodyFrame = aForFrame->PresContext()->GetPresShell()->
-            GetPrimaryFrameFor(bodyContent);
-          if (bodyFrame)
-            result = bodyFrame->GetStyleBackground();
+      nsIContent* content = topFrame->GetContent();
+      if (content) {
+        // Use |GetOwnerDoc| so it works during destruction.
+        nsIDocument* document = content->GetOwnerDoc();
+        nsCOMPtr<nsIHTMLDocument> htmlDoc = do_QueryInterface(document);
+        if (htmlDoc) {
+          nsIContent* bodyContent = htmlDoc->GetBodyContentExternal();
+          // We need to null check the body node (bug 118829) since
+          // there are cases, thanks to the fix for bug 5569, where we
+          // will reflow a document with no body.  In particular, if a
+          // SCRIPT element in the head blocks the parser and then has a
+          // SCRIPT that does "document.location.href = 'foo'", then
+          // nsParser::Terminate will call |DidBuildModel| methods
+          // through to the content sink, which will call |StartLayout|
+          // and thus |InitialReflow| on the pres shell.  See bug 119351
+          // for the ugly details.
+          if (bodyContent) {
+            nsIFrame *bodyFrame = aForFrame->PresContext()->GetPresShell()->
+              GetPrimaryFrameFor(bodyContent);
+            if (bodyFrame)
+              result = bodyFrame->GetStyleBackground();
+          }
         }
       }
     }
 
     *aBackground = result;
   } else {
     // This should always give transparent, so we'll fill it in with the
     // default color if needed.  This seems to happen a bit while a page is
     // being loaded.
     *aBackground = aForFrame->GetStyleBackground();
   }
   
   return PR_TRUE;
 }
 
 inline PRBool
-FindElementBackground(nsIFrame* aForFrame, nsIFrame* aRootElementFrame,
+FindElementBackground(nsIFrame* aForFrame,
                       const nsStyleBackground** aBackground)
 {
-  if (aForFrame == aRootElementFrame) {
-    // We must have propagated our background to the viewport or canvas. Abort.
-    return PR_FALSE;
+  nsIFrame *parentFrame = aForFrame->GetParent();
+  // XXXldb We shouldn't have to null-check |parentFrame| here.
+  if (parentFrame && IsCanvasFrame(parentFrame) == parentFrame) {
+    // Check that we're really the root (rather than in another child list).
+    nsIFrame *childFrame = parentFrame->GetFirstChild(nsnull);
+    if (childFrame == aForFrame)
+      return PR_FALSE; // Background was already drawn for the canvas.
   }
 
   *aBackground = aForFrame->GetStyleBackground();
 
   // Return true unless the frame is for a BODY element whose background
   // was propagated to the viewport.
 
-  nsIContent* content = aForFrame->GetContent();
-  if (!content || content->Tag() != nsGkAtoms::body)
-    return PR_TRUE; // not frame for a "body" element
-  // It could be a non-HTML "body" element but that's OK, we'd fail the
-  // bodyContent check below
-
   if (aForFrame->GetStyleContext()->GetPseudoType())
     return PR_TRUE; // A pseudo-element frame.
 
+  nsIContent* content = aForFrame->GetContent();
+  if (!content || !content->IsNodeOfType(nsINode::eHTML))
+    return PR_TRUE;  // not frame for an HTML element
+
+  if (!parentFrame)
+    return PR_TRUE; // no parent to look at
+
+  if (content->Tag() != nsGkAtoms::body)
+    return PR_TRUE; // not frame for <BODY> element
+
   // We should only look at the <html> background if we're in an HTML document
   nsIDocument* document = content->GetOwnerDoc();
   nsCOMPtr<nsIHTMLDocument> htmlDoc = do_QueryInterface(document);
   if (!htmlDoc)
     return PR_TRUE;
 
   nsIContent* bodyContent = htmlDoc->GetBodyContentExternal();
   if (bodyContent != content)
     return PR_TRUE; // this wasn't the background that was propagated
 
-  // This can be called even when there's no root element yet, during frame
-  // construction, via nsLayoutUtils::FrameHasTransparency and
-  // nsContainerFrame::SyncFrameViewProperties.
-  if (!aRootElementFrame)
-    return PR_TRUE;
-
-  const nsStyleBackground* htmlBG = aRootElementFrame->GetStyleBackground();
+  const nsStyleBackground* htmlBG = parentFrame->GetStyleBackground();
   return !htmlBG->IsTransparent();
 }
 
 PRBool
 nsCSSRendering::FindBackground(nsPresContext* aPresContext,
                                nsIFrame* aForFrame,
                                const nsStyleBackground** aBackground,
                                PRBool* aIsCanvas)
 {
-  nsIFrame* rootElementFrame =
-    aPresContext->PresShell()->FrameConstructor()->GetRootElementStyleFrame();
-  PRBool isCanvasFrame = IsCanvasFrame(aForFrame);
-  *aIsCanvas = isCanvasFrame;
-  return isCanvasFrame
-      ? FindCanvasBackground(aForFrame, rootElementFrame, aBackground)
-      : FindElementBackground(aForFrame, rootElementFrame, aBackground);
+  nsIFrame* canvasFrame = IsCanvasFrame(aForFrame);
+  *aIsCanvas = canvasFrame != nsnull;
+  return canvasFrame
+      ? FindCanvasBackground(canvasFrame, aBackground)
+      : FindElementBackground(aForFrame, aBackground);
 }
 
 void
 nsCSSRendering::DidPaint()
 {
   gInlineBGData->Reset();
 }
 
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -856,16 +856,11 @@ random == 429849-1.html 429849-1-ref.htm
 == 431341-2.html 431341-2-ref.html
 == 431520-1.html 431520-1-ref.html
 == 440112.html 440112-ref.html
 == 433640-1.html 433640-1-ref.html
 == 433700.html 433700-ref.html
 == 436356-1.html 436356-1-ref.html
 == 436356-2.html 436356-2-ref.html
 == 438981-1.xhtml about:blank
-== 438987-1.html 438987-1-ref.html
-== 438987-2a.html 438987-2-ref.html
-== 438987-2b.html 438987-2-ref.html
-== 438987-2c.html 438987-2-ref.html
-!= about:blank 438987-2-ref.html # check that backgrounds work at all 
 == 439004-1.html 439004-1-ref.html
 == 439910.html 439910-ref.html
 # == 448987.html 448987-ref.html  # Disabled for now - it needs privileges