Bug 1337696. Fix change hint computation for table-outer frames to be more correct. r=emilio
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 03 Mar 2017 15:54:47 -0500
changeset 394883 53a53f671871e4d7a854bd2c9d04471dd307b533
parent 394882 8335b277da3fb5c8407d694d87203830e2475bc4
child 394884 a1a760aee967f10e29592efe0ce8de63a12a3c28
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1337696
milestone54.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1337696. Fix change hint computation for table-outer frames to be more correct. r=emilio MozReview-Commit-ID: LgRmTlWsM6o
layout/base/ServoRestyleManager.cpp
layout/base/crashtests/crashtests.list
layout/base/nsCSSFrameConstructor.cpp
layout/tables/nsTableFrame.cpp
layout/tables/nsTableFrame.h
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -124,61 +124,31 @@ ClearRestyleStateFromSubtree(Element* aE
       }
     }
   }
 
   Unused << Servo_TakeChangeHint(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,
                                            ServoStyleSet* aStyleSet,
                                            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
   // no frames, we can still get them here if they were explicitly posted by
   // 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,
   // which consumes the new style and expects the old style to be on the frame.
   //
   // XXXbholley: We should teach the frame constructor how to clear the dirty
   // descendants bit to avoid the traversal here.
   if (changeHint & nsChangeHint_ReconstructFrame) {
@@ -238,24 +208,16 @@ ServoRestyleManager::RecreateStyleContex
     // XXXbz I think the UpdateStyleOfOwnedAnonBoxes call below handles _that_
     // right, but not other cases where we happen to have different styles on
     // different continuations... (e.g. first-line).
     for (nsIFrame* f = styleFrame; f;
          f = GetNextContinuationWithSameStyle(f, oldStyleContext)) {
       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;
     }
 
     if (styleFrame) {
       styleFrame->UpdateStyleOfOwnedAnonBoxes(*aStyleSet, aChangeListToProcess,
                                               changeHint);
--- a/layout/base/crashtests/crashtests.list
+++ b/layout/base/crashtests/crashtests.list
@@ -284,17 +284,17 @@ load 468491-1.html
 load 468546-1.xhtml
 load 468555-1.xhtml
 load 468563-1.html
 load 468578-1.xhtml
 # These three didn't actually crash without the resizing that the
 # 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
 load 471594-1.xhtml
 asserts-if(Android&&!asyncPan,1-2) load 473042.xhtml # bug 1034369 (may also cause a few assertions to be registered on the next test)
 asserts(0-5) load 474075.html # bug 847368
 load 477333-1.xhtml
 load 477731-1.html
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -2097,16 +2097,17 @@ nsCSSFrameConstructor::ConstructTable(ns
   // Create the inner table frame
   nsContainerFrame* innerFrame;
   if (kNameSpaceID_MathML == nameSpaceID)
     innerFrame = NS_NewMathMLmtableFrame(mPresShell, styleContext);
   else
     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);
 
   aState.AddChild(newFrame, aFrameItems, content, styleContext, aParentFrame);
 
   if (!mRootElementFrame) {
     // The frame we're constructing will be the root element frame.
--- a/layout/tables/nsTableFrame.cpp
+++ b/layout/tables/nsTableFrame.cpp
@@ -35,22 +35,24 @@
 #include "nsCSSAnonBoxes.h"
 #include "nsIPresShell.h"
 #include "nsIDOMElement.h"
 #include "nsIDOMHTMLElement.h"
 #include "nsIScriptError.h"
 #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;
 using namespace mozilla::image;
 using namespace mozilla::layout;
 
 /********************************************************************************
  ** TableReflowInput                                                         **
@@ -7672,8 +7674,48 @@ nsTableFrame::InvalidateTableFrame(nsIFr
   } else if (aOrigRect.Size() != aFrame->GetSize() ||
              aOrigVisualOverflow.Size() != visualOverflow.Size()){
     aFrame->InvalidateFrameWithRect(aOrigVisualOverflow);
     aFrame->InvalidateFrame();
     parent->InvalidateFrameWithRect(aOrigRect);
     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!");
+}
--- a/layout/tables/nsTableFrame.h
+++ b/layout/tables/nsTableFrame.h
@@ -596,16 +596,21 @@ public:
    */
   static void InvalidateTableFrame(nsIFrame* aFrame,
                                    const nsRect& aOrigRect,
                                    const nsRect& aOrigVisualOverflow,
                                    bool aIsFirstReflow);
 
   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.
     * @see NewFrame
     */
   explicit nsTableFrame(nsStyleContext* aContext);
 
   /** destructor, responsible for mColumnLayoutData */