Bug 438987, backing out

layout/base/nsCSSFrameConstructor.cpp

@@ -1794,7 +1794,6 @@ nsCSSFrameConstructor::nsCSSFrameConstructor(nsIDocument *aDocument,
   : mDocument(aDocument)
   , mPresShell(aPresShell)
   , mInitialContainingBlock(nsnull)
-  , mRootElementStyleFrame(nsnull)
   , mFixedContainingBlock(nsnull)
   , mDocElementContainingBlock(nsnull)
   , mGfxScrollFrame(nsnull)
@@ -4381,16 +4380,6 @@ nsCSSFrameConstructor::ConstructDocElementFrame(nsFrameConstructorState& aState,
   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
@@ -9471,7 +9460,7 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer,
 
     if (mInitialContainingBlock == childFrame) {
       mInitialContainingBlock = nsnull;
-      mRootElementStyleFrame = nsnull;
+      mInitialContainingBlockIsAbsPosContainer = PR_FALSE;
     }
 
     if (haveFLS && mInitialContainingBlock) {

layout/base/nsCSSFrameConstructor.h

@@ -248,13 +248,7 @@ public:
 
   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:
@@ -1150,15 +1144,8 @@ private:
   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;

layout/base/nsCSSRendering.cpp

@@ -78,7 +78,6 @@
 #include "gfxPlatform.h"
 #include "gfxImageSurface.h"
 #include "nsStyleStructInlines.h"
-#include "nsCSSFrameConstructor.h"
 
 #include "nsCSSRenderingBorders.h"
 
@@ -1008,52 +1007,80 @@ nsCSSRendering::FindNonTransparentBackground(nsStyleContext* aContext,
  * 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 ||
+  if (frameType == nsGkAtoms::canvasFrame ||
-         frameType == nsGkAtoms::rootFrame ||
+      frameType == nsGkAtoms::rootFrame ||
-         frameType == nsGkAtoms::pageFrame ||
+      frameType == nsGkAtoms::pageFrame ||
-         frameType == nsGkAtoms::pageContentFrame ||
+      frameType == nsGkAtoms::pageContentFrame) {
-         frameType == nsGkAtoms::viewportFrame;
+    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) {
+  // XXXldb What if the root element is positioned, etc.?  (We don't
-    const nsStyleBackground* result = aRootElementFrame->GetStyleBackground();
+  // 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();
+      nsIContent* content = topFrame->GetContent();
-      // The root element content can't be null. We wouldn't know what
+      if (content) {
-      // frame to create for aRootElementFrame.
+        // Use |GetOwnerDoc| so it works during destruction.
-      // Use |GetOwnerDoc| so it works during destruction.
+        nsIDocument* document = content->GetOwnerDoc();
-      nsIDocument* document = content->GetOwnerDoc();
+        nsCOMPtr<nsIHTMLDocument> htmlDoc = do_QueryInterface(document);
-      nsCOMPtr<nsIHTMLDocument> htmlDoc = do_QueryInterface(document);
+        if (htmlDoc) {
-      if (htmlDoc) {
+          nsIContent* bodyContent = htmlDoc->GetBodyContentExternal();
-        nsIContent* bodyContent = htmlDoc->GetBodyContentExternal();
+          // We need to null check the body node (bug 118829) since
-        // We need to null check the body node (bug 118829) since
+          // there are cases, thanks to the fix for bug 5569, where we
-        // there are cases, thanks to the fix for bug 5569, where we
+          // will reflow a document with no body.  In particular, if a
-        // will reflow a document with no body.  In particular, if a
+          // SCRIPT element in the head blocks the parser and then has a
-        // SCRIPT element in the head blocks the parser and then has a
+          // SCRIPT that does "document.location.href = 'foo'", then
-        // SCRIPT that does "document.location.href = 'foo'", then
+          // nsParser::Terminate will call |DidBuildModel| methods
-        // nsParser::Terminate will call |DidBuildModel| methods
+          // through to the content sink, which will call |StartLayout|
-        // through to the content sink, which will call |StartLayout|
+          // and thus |InitialReflow| on the pres shell.  See bug 119351
-        // and thus |InitialReflow| on the pres shell.  See bug 119351
+          // for the ugly details.
-        // for the ugly details.
+          if (bodyContent) {
-        if (bodyContent) {
+            nsIFrame *bodyFrame = aForFrame->PresContext()->GetPresShell()->
-          nsIFrame *bodyFrame = aForFrame->PresContext()->GetPresShell()->
+              GetPrimaryFrameFor(bodyContent);
-            GetPrimaryFrameFor(bodyContent);
+            if (bodyFrame)
-          if (bodyFrame)
+              result = bodyFrame->GetStyleBackground();
-            result = bodyFrame->GetStyleBackground();
+          }
         }
       }
     }
@@ -1070,12 +1097,16 @@ FindCanvasBackground(nsIFrame* aForFrame, nsIFrame* aRootElementFrame,
 }
 
 inline PRBool
-FindElementBackground(nsIFrame* aForFrame, nsIFrame* aRootElementFrame,
+FindElementBackground(nsIFrame* aForFrame,
                       const nsStyleBackground** aBackground)
 {
-  if (aForFrame == aRootElementFrame) {
+  nsIFrame *parentFrame = aForFrame->GetParent();
-    // We must have propagated our background to the viewport or canvas. Abort.
+  // XXXldb We shouldn't have to null-check |parentFrame| here.
-    return PR_FALSE;
+  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();
@@ -1083,15 +1114,19 @@ FindElementBackground(nsIFrame* aForFrame, nsIFrame* aRootElementFrame,
   // 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);
@@ -1102,13 +1137,7 @@ FindElementBackground(nsIFrame* aForFrame, nsIFrame* aRootElementFrame,
   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
+  const nsStyleBackground* htmlBG = parentFrame->GetStyleBackground();
-  // construction, via nsLayoutUtils::FrameHasTransparency and
-  // nsContainerFrame::SyncFrameViewProperties.
-  if (!aRootElementFrame)
-    return PR_TRUE;
-
-  const nsStyleBackground* htmlBG = aRootElementFrame->GetStyleBackground();
   return !htmlBG->IsTransparent();
 }
 
@@ -1118,13 +1147,11 @@ nsCSSRendering::FindBackground(nsPresContext* aPresContext,
                                const nsStyleBackground** aBackground,
                                PRBool* aIsCanvas)
 {
-  nsIFrame* rootElementFrame =
+  nsIFrame* canvasFrame = IsCanvasFrame(aForFrame);
-    aPresContext->PresShell()->FrameConstructor()->GetRootElementStyleFrame();
+  *aIsCanvas = canvasFrame != nsnull;
-  PRBool isCanvasFrame = IsCanvasFrame(aForFrame);
+  return canvasFrame
-  *aIsCanvas = isCanvasFrame;
+      ? FindCanvasBackground(canvasFrame, aBackground)
-  return isCanvasFrame
+      : FindElementBackground(aForFrame, aBackground);
-      ? FindCanvasBackground(aForFrame, rootElementFrame, aBackground)
-      : FindElementBackground(aForFrame, rootElementFrame, aBackground);
 }
 
 void

layout/reftests/bugs/reftest.list

@@ -861,11 +861,6 @@ random == 429849-1.html 429849-1-ref.html # bug 432288
 == 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