Bug 1337696. Fix change hint computation for table-outer frames to be more correct. r?emilio
MozReview-Commit-ID: LgRmTlWsM6o
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -125,61 +125,31 @@ ServoRestyleManager::ClearDirtyDescendan
if (n->IsElement()) {
ClearDirtyDescendantsFromSubtree(n->AsElement());
}
}
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) {
@@ -239,24 +209,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 **
@@ -7559,8 +7561,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 */