Bug 1110277 patch 3 - Look for the GenConPseudos() property on the fi…

…rst continuation/ib-split so that we can find it when looking for the ::after frame. r=bzbarsky

The change to GetAfterFrameForContent prevents the reframe that is part
of the chain of events leading to this bug, and thus fixes the bug on
its own.  The change to GetBeforeFrameForContent seems desirable for
symmetry.

Note that patch 6 also independently fixes the reported bug.

This probably needs somewhat careful review.  We should examine:

 (1) what the rules for calling nsLayoutUtils::GetBeforeFrame and
     nsLayoutUtils::GetAfterFrame are, and whether both (or neither)
     need to be patched.

 (2) What the rules are for which frame the GenConProperty() lives on,
     and whether we should adjust nsIFrame::GetGenConPseudos() to either
     do something more intelligent, or assert about callers.

(We should probably clean up some of these things in a followup bug.)

Since the symptom of this bug is (once patch 4 is in the tree) only
causing extra reframes, it can only be tested using the new API (from
bug 1115691) for observing reframes.  I confirmed that the test for this
bug fails without the patch and passes with the patch (as noted by the
removal of its todo annotation).

This patch fixes the assertion on layout/generic/crashtests/600100.xhtml,
though I haven't investigated why.

UltraBlame original commit: 1bbebe9fec17e88234845d8da22c8b44f394121b

layout/base/nsCSSFrameConstructor.cpp

@@ -5789,6 +5789,9 @@ nsCSSFrameConstructor::AddFrameConstructionItemsInternal(nsFrameConstructorState
 static void
 AddGenConPseudoToFrame(nsIFrame* aOwnerFrame, nsIContent* aContent)
 {
+  NS_ASSERTION(nsLayoutUtils::IsFirstContinuationOrIBSplitSibling(aOwnerFrame),
+               "property should only be set on first continuation/ib-sibling");
+
   typedef nsAutoTArray<nsIContent*, 2> T;
   const FramePropertyDescriptor* prop = nsIFrame::GenConProperty();
   FrameProperties props = aOwnerFrame->Properties();

layout/base/nsLayoutUtils.cpp

@@ -1171,7 +1171,10 @@ nsLayoutUtils::GetChildListNameFor(nsIFrame* aChildFrame)
 nsLayoutUtils::GetBeforeFrameForContent(nsIFrame* aFrame,
                                         nsIContent* aContent)
 {
-  nsContainerFrame* genConParentFrame = aFrame->GetContentInsertionFrame();
+
+
+  nsContainerFrame* genConParentFrame =
+    FirstContinuationOrIBSplitSibling(aFrame)->GetContentInsertionFrame();
   if (!genConParentFrame) {
     return nullptr;
   }
@@ -1207,7 +1210,10 @@ nsLayoutUtils::GetBeforeFrame(nsIFrame* aFrame)
 nsLayoutUtils::GetAfterFrameForContent(nsIFrame* aFrame,
                                        nsIContent* aContent)
 {
-  nsContainerFrame* genConParentFrame = aFrame->GetContentInsertionFrame();
+
+
+  nsContainerFrame* genConParentFrame =
+    FirstContinuationOrIBSplitSibling(aFrame)->GetContentInsertionFrame();
   if (!genConParentFrame) {
     return nullptr;
   }
@@ -1224,8 +1230,13 @@ nsLayoutUtils::GetAfterFrameForContent(nsIFrame* aFrame,
 
 
 
+  genConParentFrame = aFrame->GetContentInsertionFrame();
+  if (!genConParentFrame) {
+    return nullptr;
+  }
   nsIFrame* lastParentContinuation =
-    nsLayoutUtils::LastContinuationWithChild(genConParentFrame);
+    LastContinuationWithChild(static_cast<nsContainerFrame*>(
+      LastContinuationOrIBSplitSibling(genConParentFrame)));
   nsIFrame* childFrame =
     lastParentContinuation->GetLastChild(nsIFrame::kPrincipalList);
   if (childFrame &&

layout/base/tests/test_frame_reconstruction_for_pseudo_elements.html

@@ -35,12 +35,12 @@
   SimpleTest.waitForExplicitFinish();
 
   function run() {
-    runtest("first line test", "#firstlinetest > .testspan", {});
-    runtest("after test", "#aftertest > .testspan", { todo: true });
+    runtest("first line test", "#firstlinetest > .testspan");
+    runtest("after test", "#aftertest > .testspan");
     SimpleTest.finish();
   }
 
-  function runtest(description, selector, flags) {
+  function runtest(description, selector) {
     var utils = SpecialPowers.getDOMWindowUtils(window);
     var span = document.querySelector(selector);
     var cs = getComputedStyle(span, "");
@@ -54,7 +54,7 @@
     var endcolor = cs.color;
     var endcount = utils.framesConstructed;
     is(endcolor, "rgb(0, 0, 255)", description + ": final color");
-    (flags.todo ? todo_is : is)(endcount, startcount,
+    is(endcount, startcount,
        description + ": should not do frame construction")
   }
 

layout/generic/crashtests/crashtests.list

@@ -425,7 +425,7 @@ asserts(0-1) load 592118.html
 load 594808-1.html
 load 595435-1.xhtml
 load 595740-1.html
-pref(layout.float-fragments-inside-column.enabled,true) asserts(1) load 600100.xhtml # bug 866955
+pref(layout.float-fragments-inside-column.enabled,true) load 600100.xhtml
 pref(layout.float-fragments-inside-column.enabled,false) load 600100.xhtml
 load 603490-1.html
 load 603510-1.html

layout/generic/nsIFrame.h

@@ -871,6 +871,8 @@ class nsIFrame : public nsQueryFrame
   NS_DECLARE_FRAME_PROPERTY(GenConProperty, DestroyContentArray)
 
   nsTArray<nsIContent*>* GetGenConPseudos() {
+    NS_ASSERTION(nsLayoutUtils::IsFirstContinuationOrIBSplitSibling(this),
+                 "should only call on first continuation/ib-sibling");
     const FramePropertyDescriptor* prop = GenConProperty();
     return static_cast<nsTArray<nsIContent*>*>(Properties().Get(prop));
   }