Bug 1110277 patch 3 - Look for the GenConPseudos() property on the first 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.

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();
+  // We need to call GetGenConPseudos() on the first continuation/ib-split.
+  // Find it, for symmetry with GetAfterFrameForContent.
+  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();
+  // We need to call GetGenConPseudos() on the first continuation,
+  // but callers are likely to pass the last.
+  nsContainerFrame* genConParentFrame =
+    FirstContinuationOrIBSplitSibling(aFrame)->GetContentInsertionFrame();
   if (!genConParentFrame) {
     return nullptr;
   }
@@ -1224,8 +1230,13 @@ nsLayoutUtils::GetAfterFrameForContent(nsIFrame* aFrame,
   // If the last child frame is a pseudo-frame, then try that.
   // Note that the frame we create for the generated content is also a
   // pseudo-frame and so don't drill down in that case.
+  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 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=1110277
   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 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=1110277
     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 @@ public:
   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));
   }