Bug 1337696. Fix change hint computation for table-outer frames to be more correct. r=emilio

MozReview-Commit-ID: LgRmTlWsM6o

--HG--
extra : rebase_source : dc18a9ca923eedbb2aaba80d515c70bd80a90d4e

layout/base/ServoRestyleManager.cpp

@@ -129,20 +129,6 @@ ClearRestyleStateFromSubtree(Element* aElement)
   aElement->UnsetHasDirtyDescendantsForServo();
 }
 
-static void
-UpdateStyleContextForTableWrapper(nsIFrame* aTableWrapper,
-                                  nsStyleContext* aTableStyleContext,
-                                  ServoStyleSet* aStyleSet)
-{
-  MOZ_ASSERT(aTableWrapper->GetType() == nsGkAtoms::tableWrapperFrame);
-  MOZ_ASSERT(aTableWrapper->StyleContext()->GetPseudo() ==
-             nsCSSAnonBoxes::tableWrapper);
-  RefPtr<nsStyleContext> newContext =
-    aStyleSet->ResolveAnonymousBoxStyle(nsCSSAnonBoxes::tableWrapper,
-                                        aTableStyleContext);
-  aTableWrapper->SetStyleContext(newContext);
-}
-
 void
 ServoRestyleManager::RecreateStyleContexts(Element* aElement,
                                            nsStyleContext* aParentContext,
@@ -150,7 +136,6 @@ ServoRestyleManager::RecreateStyleContexts(Element* aElement,
                                            nsStyleChangeList& aChangeListToProcess)
 {
   nsIFrame* styleFrame = nsLayoutUtils::GetStyleFrame(aElement);
-  bool isTable = styleFrame != aElement->GetPrimaryFrame();
 
   nsChangeHint changeHint = Servo_TakeChangeHint(aElement);
   // Although we shouldn't generate non-ReconstructFrame hints for elements with
@@ -158,22 +143,7 @@ ServoRestyleManager::RecreateStyleContexts(Element* aElement,
   // PostRestyleEvent, such as a RepaintFrame hint when a :link changes to be
   // :visited.  Skip processing these hints if there is no frame.
   if ((styleFrame || (changeHint & nsChangeHint_ReconstructFrame)) && changeHint) {
-    if (isTable) {
-      // This part is a bit annoying: when isTable, that means the style frame
-      // is actually a _child_ of the primary frame.  In that situation, we want
-      // to go ahead and append the changeHint for the _parent_ but also append
-      // all the parts of it not handled for descendants for the _child_.
-      MOZ_ASSERT(styleFrame, "Or else GetPrimaryFrame() would be null too");
-      MOZ_ASSERT(styleFrame->GetParent() == aElement->GetPrimaryFrame(),
-                 "How did that happen?");
-      aChangeListToProcess.AppendChange(aElement->GetPrimaryFrame(), aElement,
-                                        changeHint);
-      // We may be able to do better here.
-      // https://bugzilla.mozilla.org/show_bug.cgi?id=1340717 tracks that.
-      aChangeListToProcess.AppendChange(styleFrame, aElement, changeHint);
-    } else {
-      aChangeListToProcess.AppendChange(styleFrame, aElement, changeHint);
-    }
+    aChangeListToProcess.AppendChange(styleFrame, aElement, changeHint);
   }
 
   // If our change hint is reconstruct, we delegate to the frame constructor,
@@ -243,14 +213,6 @@ ServoRestyleManager::RecreateStyleContexts(Element* aElement,
       f->SetStyleContext(newContext);
     }
 
-    if (isTable) {
-      nsIFrame* primaryFrame = aElement->GetPrimaryFrame();
-      MOZ_ASSERT(primaryFrame->StyleContext()->GetPseudo() ==
-                   nsCSSAnonBoxes::tableWrapper,
-                 "What sort of frame is this?");
-      UpdateStyleContextForTableWrapper(primaryFrame, newContext, aStyleSet);
-    }
-
     if (MOZ_UNLIKELY(displayContentsNode)) {
       MOZ_ASSERT(!styleFrame);
       displayContentsNode->mStyle = newContext;

layout/base/crashtests/crashtests.list

@@ -289,7 +289,7 @@ load 468578-1.xhtml
 # browser does when setting up print preview, but adding them anyway.
 load 468645-1.xhtml
 load 468645-2.xhtml
-asserts-if(stylo,1) load 468645-3.xhtml # bug 1337696
+load 468645-3.xhtml
 load 469861-1.xhtml
 load 469861-2.xhtml
 load 470851-1.xhtml

layout/base/nsCSSFrameConstructor.cpp

@@ -2102,6 +2102,7 @@ nsCSSFrameConstructor::ConstructTable(nsFrameConstructorState& aState,
     innerFrame = NS_NewTableFrame(mPresShell, styleContext);
 
   InitAndRestoreFrame(aState, content, newFrame, innerFrame);
+  innerFrame->AddStateBits(NS_FRAME_OWNS_ANON_BOXES);
 
   // Put the newly created frames into the right child list
   SetInitialSingleChild(newFrame, innerFrame);

layout/tables/nsTableFrame.cpp

@@ -40,12 +40,14 @@
 #include "nsFrameManager.h"
 #include "nsError.h"
 #include "nsCSSFrameConstructor.h"
+#include "mozilla/ServoStyleSet.h"
 #include "mozilla/StyleSetHandle.h"
 #include "mozilla/StyleSetHandleInlines.h"
 #include "nsDisplayList.h"
 #include "nsIScrollableFrame.h"
 #include "nsCSSProps.h"
 #include "RestyleTracker.h"
+#include "nsStyleChangeList.h"
 #include <algorithm>
 
 using namespace mozilla;
@@ -7677,3 +7679,43 @@ nsTableFrame::InvalidateTableFrame(nsIFrame* aFrame,
     parent->InvalidateFrame();
   }
 }
+
+void
+nsTableFrame::DoUpdateStyleOfOwnedAnonBoxes(ServoStyleSet& aStyleSet,
+                                            nsStyleChangeList& aChangeList,
+                                            nsChangeHint aHintForThisFrame)
+{
+  nsIFrame* wrapper = GetParent();
+
+  MOZ_ASSERT(wrapper->StyleContext()->GetPseudo() ==
+               nsCSSAnonBoxes::tableWrapper,
+             "What happened to our parent?");
+
+  RefPtr<nsStyleContext> newContext =
+    aStyleSet.ResolveAnonymousBoxStyle(nsCSSAnonBoxes::tableWrapper,
+                                       StyleContext());
+
+  // Figure out whether we have an actual change.  It's important that we do
+  // this, even though all the wrapper's changes are due to properties it
+  // inherits from us, because it's possible that no one ever asked us for those
+  // style structs and hence changes to them aren't reflected in
+  // aHintForThisFrame at all.
+  uint32_t equalStructs, samePointerStructs; // Not used, actually.
+  nsChangeHint wrapperHint = wrapper->StyleContext()->CalcStyleDifference(
+    newContext,
+    // The wrapper is not our descendant, so just be pessimistic about which
+    // things it needs to care about.
+    nsChangeHint_Hints_NotHandledForDescendants,
+    &equalStructs,
+    &samePointerStructs);
+  if (wrapperHint) {
+    aChangeList.AppendChange(wrapper, wrapper->GetContent(), wrapperHint);
+  }
+
+  for (nsIFrame* cur = wrapper; cur; cur = cur->GetNextContinuation()) {
+    cur->SetStyleContext(newContext);
+  }
+
+  MOZ_ASSERT(!(wrapper->GetStateBits() & NS_FRAME_OWNS_ANON_BOXES),
+             "Wrapper frame doesn't have any anon boxes of its own!");
+}

layout/tables/nsTableFrame.h

@@ -601,6 +601,11 @@ public:
 
   virtual bool ComputeCustomOverflow(nsOverflowAreas& aOverflowAreas) override;
 
+  // Update the style of our table wrapper frame.
+  virtual void DoUpdateStyleOfOwnedAnonBoxes(
+    mozilla::ServoStyleSet& aStyleSet,
+    nsStyleChangeList& aChangeList,
+    nsChangeHint aHintForThisFrame) override;
 protected:
 
   /** protected constructor.