Bug 1411754: Rename PresShell::DestroyFramesFor to DestroyFramesForAndRestyle. r?mats
I'm drive-by removing the comment about the frame tree state because I looked
into it, and the answer is: we properly restore it.
The gotcha is that we retain it too much, indeed, we retain it enough that it
can leak. See
bug 1397239.
MozReview-Commit-ID: LP6bXkduEZ4
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -1164,17 +1164,17 @@ Element::CreateShadowRoot(ErrorResult& a
docInfo, nullptr, true);
if (aError.Failed()) {
delete protoBinding;
return nullptr;
}
if (nsIDocument* doc = GetComposedDoc()) {
if (nsIPresShell* shell = doc->GetShell()) {
- shell->DestroyFramesFor(this);
+ shell->DestroyFramesForAndRestyle(this);
MOZ_ASSERT(!shell->FrameManager()->GetDisplayContentsStyleFor(this));
}
}
MOZ_ASSERT(!GetPrimaryFrame());
// Unlike for XBL, false is the default for inheriting style.
protoBinding->SetInheritsStyle(false);
--- a/dom/base/ShadowRoot.cpp
+++ b/dom/base/ShadowRoot.cpp
@@ -262,18 +262,17 @@ ShadowRoot::DistributionChanged()
return;
}
auto* shell = OwnerDoc()->GetShell();
if (!shell) {
return;
}
- // FIXME(emilio): Rename this to DestroyFramesForAndRestyle?
- shell->DestroyFramesFor(host);
+ shell->DestroyFramesForAndRestyle(host);
}
const HTMLContentElement*
ShadowRoot::DistributeSingleNode(nsIContent* aContent)
{
// Find the insertion point to which the content belongs.
HTMLContentElement* foundInsertionPoint = nullptr;
for (HTMLContentElement* insertionPoint : mInsertionPoints) {
@@ -508,17 +507,17 @@ ShadowRoot::AttributeChanged(nsIDocument
return;
}
auto* shell = OwnerDoc()->GetShell();
if (!shell) {
return;
}
- shell->DestroyFramesFor(aElement);
+ shell->DestroyFramesForAndRestyle(aElement);
}
bool
ShadowRoot::RedistributeElement(Element* aElement)
{
auto* oldInsertionPoint = RemoveDistributedNode(aElement);
auto* newInsertionPoint = DistributeSingleNode(aElement);
--- a/dom/xbl/nsXBLService.cpp
+++ b/dom/xbl/nsXBLService.cpp
@@ -121,17 +121,17 @@ public:
nsXBLService::GetInstance()->BindingReady(mBoundElement, mBindingURI, &ready);
if (!ready)
return;
// Destroy the frames for mBoundElement. Do this after getting the binding,
// since if the binding fetch fails then we don't want to destroy the
// frames.
if (nsIPresShell* shell = doc->GetShell()) {
- shell->DestroyFramesFor(mBoundElement->AsElement());
+ shell->DestroyFramesForAndRestyle(mBoundElement->AsElement());
}
MOZ_ASSERT(!mBoundElement->GetPrimaryFrame());
}
nsXBLBindingRequest(nsIURI* aURI, nsIContent* aBoundElement)
: mBindingURI(aURI),
mBoundElement(aBoundElement)
{
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -2917,17 +2917,17 @@ PresShell::CancelAllPendingReflows()
GetPresContext()->RefreshDriver()->RemoveLayoutFlushObserver(this);
mObservingLayoutFlushes = false;
}
ASSERT_REFLOW_SCHEDULED_STATE();
}
void
-PresShell::DestroyFramesFor(Element* aElement)
+PresShell::DestroyFramesForAndRestyle(Element* aElement)
{
MOZ_ASSERT(aElement);
NS_ENSURE_TRUE_VOID(mPresContext);
if (!mDidInitialize) {
return;
}
nsAutoScriptBlocker scriptBlocker;
@@ -2935,31 +2935,25 @@ PresShell::DestroyFramesFor(Element* aEl
// Mark ourselves as not safe to flush while we're doing frame destruction.
++mChangeNestCount;
nsCSSFrameConstructor* fc = FrameConstructor();
fc->BeginUpdate();
bool didReconstruct = fc->DestroyFramesFor(aElement);
fc->EndUpdate();
- // XXXmats doesn't frame state need to be restored in this case?
- if (!didReconstruct) {
- PostRecreateFramesFor(aElement);
- }
-
- // NOTE(emilio): This is needed to force also a full subtree restyle for the
- // content (in Stylo, where the existence of frames != the existence of
- // styles).
- //
- // It's a bit out of place in a function called DestroyFramesFor,
- // however the two only callers of this code really need this (given they
- // shuffle the flattened tree around), and this avoids exposing additional
- // APIs on the pres shell.
+ auto changeHint = didReconstruct
+ ? nsChangeHint(0)
+ : nsChangeHint_ReconstructFrame;
+
+ // NOTE(emilio): eRestyle_Subtree is needed to force also a full subtree
+ // restyle for the content (in Stylo, where the existence of frames != the
+ // existence of styles).
mPresContext->RestyleManager()->PostRestyleEvent(
- aElement, eRestyle_Subtree, nsChangeHint(0));
+ aElement, eRestyle_Subtree, changeHint);
--mChangeNestCount;
}
void
nsIPresShell::PostRecreateFramesFor(Element* aElement)
{
if (MOZ_UNLIKELY(!mDidInitialize)) {
--- a/layout/base/PresShell.h
+++ b/layout/base/PresShell.h
@@ -126,17 +126,17 @@ public:
nsFrameState aBitToAdd,
ReflowRootHandling aRootHandling =
eInferFromBitToAdd) override;
virtual void FrameNeedsToContinueReflow(nsIFrame *aFrame) override;
virtual void CancelAllPendingReflows() override;
virtual bool IsSafeToFlush() const override;
virtual void DoFlushPendingNotifications(mozilla::FlushType aType) override;
virtual void DoFlushPendingNotifications(mozilla::ChangesToFlush aType) override;
- virtual void DestroyFramesFor(mozilla::dom::Element* aElement) override;
+ virtual void DestroyFramesForAndRestyle(mozilla::dom::Element* aElement) override;
/**
* Post a callback that should be handled after reflow has finished.
*/
virtual nsresult PostReflowCallback(nsIReflowCallback* aCallback) override;
virtual void CancelReflowCallback(nsIReflowCallback* aCallback) override;
virtual void ClearFrameRefs(nsIFrame* aFrame) override;
--- a/layout/base/nsIPresShell.h
+++ b/layout/base/nsIPresShell.h
@@ -523,17 +523,17 @@ public:
virtual void NotifyCounterStylesAreDirty() = 0;
/**
* Destroy the frames for aElement, and reconstruct them asynchronously if
* needed.
*
* Note that this may destroy frames for an ancestor instead.
*/
- virtual void DestroyFramesFor(mozilla::dom::Element* aElement) = 0;
+ virtual void DestroyFramesForAndRestyle(mozilla::dom::Element* aElement) = 0;
void PostRecreateFramesFor(mozilla::dom::Element* aElement);
void RestyleForAnimation(mozilla::dom::Element* aElement,
nsRestyleHint aHint);
// ShadowRoot has APIs that can change styles so we only
// want to restyle elements in the ShadowRoot and not the whole
// document.