From 79ab63aee9a2d6a6e41cd8c1578d84920c181441 Mon Sep 17 00:00:00 2001 From: Brad Werth Date: Tue, 21 Feb 2023 04:06:37 +0000 Subject: [PATCH] Bug 1815140 Part 1: Make Grid dom object an idempotent property of its frame. r=emilio This ensures that repeated calls to Element::GetGridFragments will return an array of idempotent Grid objects for each fragment. This is accomplished by making the Grid object hold a WeakFrame back to its originating frame, and updating a property on construction and destruction. Differential Revision: https://phabricator.services.mozilla.com/D169724 --- dom/base/Element.cpp | 11 ++++++++++- dom/grid/Grid.cpp | 12 +++++++++++- dom/grid/Grid.h | 1 + layout/generic/nsGridContainerFrame.cpp | 5 +++++ layout/generic/nsGridContainerFrame.h | 15 +++++++++++++++ 5 files changed, 42 insertions(+), 2 deletions(-) diff --git a/dom/base/Element.cpp b/dom/base/Element.cpp index afde234c683a..3ca462e439cd 100644 --- a/dom/base/Element.cpp +++ b/dom/base/Element.cpp @@ -3563,7 +3563,16 @@ void Element::GetGridFragments(nsTArray>& aResult) { // If we get a nsGridContainerFrame from the prior call, // all the next-in-flow frames will also be nsGridContainerFrames. while (frame) { - aResult.AppendElement(new Grid(this, frame)); + // Get the existing Grid object, if it exists. This object is + // guaranteed to be up-to-date because GetGridFrameWithComputedInfo + // will delete an existing one when regenerating grid info. + Grid* gridFragment = frame->GetGridFragmentInfo(); + if (!gridFragment) { + // Grid constructor will add itself as a property to frame, and the + // destructor will remove itself if the frame still exists. + gridFragment = new Grid(this, frame); + } + aResult.AppendElement(gridFragment); frame = static_cast(frame->GetNextInFlow()); } } diff --git a/dom/grid/Grid.cpp b/dom/grid/Grid.cpp index fd8eb0817a16..f31503017fab 100644 --- a/dom/grid/Grid.cpp +++ b/dom/grid/Grid.cpp @@ -25,11 +25,14 @@ NS_INTERFACE_MAP_END Grid::Grid(nsISupports* aParent, nsGridContainerFrame* aFrame) : mParent(do_QueryInterface(aParent)), + mFrame(aFrame), mRows(new GridDimension(this)), mCols(new GridDimension(this)) { MOZ_ASSERT(aFrame, "Should never be instantiated with a null nsGridContainerFrame"); + aFrame->SetProperty(nsGridContainerFrame::GridFragmentInfo(), this); + // Construct areas first, because lines may need to reference them // to extract additional names for boundary lines. @@ -80,7 +83,14 @@ Grid::Grid(nsISupports* aParent, nsGridContainerFrame* aFrame) mCols->SetLineInfo(columnTrackInfo, columnLineInfo, mAreas, false); } -Grid::~Grid() = default; +Grid::~Grid() { + // If mFrame is still alive, clear the property that points back to us. + if (mFrame.IsAlive()) { + mFrame->RemoveProperty(nsGridContainerFrame::GridFragmentInfo()); + } + + // Our member variables are correctly handled by default destruction. +} JSObject* Grid::WrapObject(JSContext* aCx, JS::Handle aGivenProto) { return Grid_Binding::Wrap(aCx, this, aGivenProto); diff --git a/dom/grid/Grid.h b/dom/grid/Grid.h index 57f13e404436..0053ea0c22e3 100644 --- a/dom/grid/Grid.h +++ b/dom/grid/Grid.h @@ -38,6 +38,7 @@ class Grid : public nsISupports, public nsWrapperCache { protected: nsCOMPtr mParent; + WeakFrame mFrame; RefPtr mRows; RefPtr mCols; nsTArray> mAreas; diff --git a/layout/generic/nsGridContainerFrame.cpp b/layout/generic/nsGridContainerFrame.cpp index 9332314f77ad..640124cf2baa 100644 --- a/layout/generic/nsGridContainerFrame.cpp +++ b/layout/generic/nsGridContainerFrame.cpp @@ -8919,6 +8919,11 @@ void nsGridContainerFrame::Reflow(nsPresContext* aPresContext, // since this bit is only set by accessing a ChromeOnly property, and // therefore can't unduly slow down normal web browsing. + // Clear our GridFragmentInfo property, which might be holding a stale + // dom::Grid object built from previously-computed info. This will + // ensure that the next call to GetGridFragments will create a new one. + RemoveProperty(GridFragmentInfo()); + // Now that we know column and row sizes and positions, set // the ComputedGridTrackInfo and related properties diff --git a/layout/generic/nsGridContainerFrame.h b/layout/generic/nsGridContainerFrame.h index 09ace39f2e1e..1a7322a21958 100644 --- a/layout/generic/nsGridContainerFrame.h +++ b/layout/generic/nsGridContainerFrame.h @@ -18,6 +18,9 @@ namespace mozilla { class PresShell; +namespace dom { +class Grid; +} } // namespace mozilla /** @@ -214,6 +217,18 @@ class nsGridContainerFrame final : public nsContainerFrame, return info; } + /** + * This property is set by the creation of a dom::Grid object, and cleared by + * its destructor. Since the Grid object manages the lifecycle, the property + * itself is set without a destructor. The property is also cleared whenever + * new grid computed info is generated during reflow, ensuring that we aren't + * holding a stale dom::Grid object. + */ + NS_DECLARE_FRAME_PROPERTY_WITHOUT_DTOR(GridFragmentInfo, mozilla::dom::Grid) + mozilla::dom::Grid* GetGridFragmentInfo() { + return GetProperty(GridFragmentInfo()); + } + struct AtomKey { RefPtr mKey;