Verified Commit bec689db authored by Daniel Holbert's avatar Daniel Holbert Committed by ma1
Browse files

Bug 1742738 part 3: Tighten up tearoff-table removal for DOMSVGPoint.... 

Bug 1742738 part 3: Tighten up tearoff-table removal for DOMSVGPoint. r=firefox-svg-reviewers,longsonr

I'm doing this one in its own patch since it's slightly more subtle than the
others, due to the existence of multiple instance-creation codepaths, some of
which generate instances that never end up in the tearoff table.

Differential Revision: https://phabricator.services.mozilla.com/D246065
parent 52f4b900

dom/svg/DOMSVGPoint.cpp

+12 −5
Original line number Diff line number Diff line
@@ -167,6 +167,7 @@ already_AddRefed<DOMSVGPoint> DOMSVGPoint::GetTranslateTearOff( 
  if (!domPoint) {

    domPoint = new DOMSVGPoint(aVal, aSVGSVGElement);

    sSVGTranslateTearOffTable.AddTearoff(aVal, domPoint);

    domPoint->mIsInTearoffTable = true;

  }



  return domPoint.forget();
@@ -203,12 +204,18 @@ void DOMSVGPoint::CleanupWeakRefs() { 
    pointList->mItems[mListIndex] = nullptr;

  }



  if (mVal) {

    if (mIsTranslatePoint) {

  if (mIsInTearoffTable) {

    // Similarly, we must update the tearoff table to remove its (non-owning)

    // pointer to mVal.

    MOZ_ASSERT(mVal && mIsTranslatePoint,

               "Tearoff table should only be used for translate-point objects "

               "with non-null mVal (see GetTranslateTearOff and its callers)");

    sSVGTranslateTearOffTable.RemoveTearoff(mVal);

    } else {

    mIsInTearoffTable = false;

  }



  if (mVal) {

    if (!mIsTranslatePoint) {

      // In this case we own mVal

      delete mVal;

    }

dom/svg/DOMSVGPoint.h

+15 −4
Original line number Diff line number Diff line
@@ -17,7 +17,7 @@ 
#include "mozilla/dom/SVGSVGElement.h"

#include "mozilla/gfx/2D.h"



#define MOZ_SVG_LIST_INDEX_BIT_COUNT 30

#define MOZ_SVG_LIST_INDEX_BIT_COUNT 29



namespace mozilla::dom {

struct DOMMatrix2DInit;
@@ -51,7 +51,8 @@ class DOMSVGPoint final : public nsWrapperCache { 
        mOwner(aList),

        mListIndex(aListIndex),

        mIsAnimValItem(aIsAnimValItem),

        mIsTranslatePoint(false) {

        mIsTranslatePoint(false),

        mIsInTearoffTable(false) {

    // These shifts are in sync with the members.

    MOZ_ASSERT(aList && aListIndex <= MaxListIndex(), "bad arg");
@@ -60,7 +61,10 @@ class DOMSVGPoint final : public nsWrapperCache { 


  // Constructor for unowned points and SVGSVGElement.createSVGPoint

  explicit DOMSVGPoint(const Point& aPt)

      : mListIndex(0), mIsAnimValItem(false), mIsTranslatePoint(false) {

      : mListIndex(0),

        mIsAnimValItem(false),

        mIsTranslatePoint(false),

        mIsInTearoffTable(false) {

    // In this case we own mVal

    mVal = new SVGPoint(aPt.x, aPt.y);

  }
@@ -72,7 +76,8 @@ class DOMSVGPoint final : public nsWrapperCache { 
        mOwner(ToSupports(aSVGSVGElement)),

        mListIndex(0),

        mIsAnimValItem(false),

        mIsTranslatePoint(true) {}

        mIsTranslatePoint(true),

        mIsInTearoffTable(false) {}



  virtual ~DOMSVGPoint() { CleanupWeakRefs(); }
@@ -178,6 +183,12 @@ class DOMSVGPoint final : public nsWrapperCache { 
  uint32_t mListIndex : MOZ_SVG_LIST_INDEX_BIT_COUNT;

  uint32_t mIsAnimValItem : 1;     // True if We're the animated value of a list

  uint32_t mIsTranslatePoint : 1;  // true iff our owner is a SVGSVGElement



  // Tracks whether we're in the tearoff table. Initialized to false in the

  // ctor, but then immediately set to true if/when we're added to the table

  // (not all instances are).  Updated to false when we're removed from the

  // table (at which point we're being destructed or soon-to-be destructed).

  uint32_t mIsInTearoffTable : 1;

};



}  // namespace mozilla::dom