Bug 1040575 - Make a copy of SVGSVGElement.currentTranslate if it is inserted into an SVGPointList. r=longsonr

dom/svg/DOMSVGPoint.h

@@ -97,7 +97,7 @@ public:
     return mList;
   }
 
-  nsISVGPoint* Clone() {
+  virtual DOMSVGPoint* Copy() MOZ_OVERRIDE {
     return new DOMSVGPoint(this);
   }
 

dom/svg/DOMSVGPointList.cpp

@@ -247,8 +247,9 @@ DOMSVGPointList::Initialize(nsISVGPoint& aNewItem, ErrorResult& aError)
   // from happening we have to do the clone here, if necessary.
 
   nsCOMPtr<nsISVGPoint> domItem = &aNewItem;
-  if (domItem->HasOwner() || domItem->IsReadonly()) {
-    domItem = domItem->Clone(); // must do this before changing anything!
+  if (domItem->HasOwner() || domItem->IsReadonly() ||
+      domItem->IsTranslatePoint()) {
+    domItem = domItem->Copy(); // must do this before changing anything!
   }
 
   ErrorResult rv;
@@ -298,8 +299,9 @@ DOMSVGPointList::InsertItemBefore(nsISVGPoint& aNewItem, uint32_t aIndex,
   }
 
   nsCOMPtr<nsISVGPoint> domItem = &aNewItem;
-  if (domItem->HasOwner() || domItem->IsReadonly()) {
-    domItem = domItem->Clone(); // must do this before changing anything!
+  if (domItem->HasOwner() || domItem->IsReadonly() ||
+      domItem->IsTranslatePoint()) {
+    domItem = domItem->Copy(); // must do this before changing anything!
   }
 
   // Ensure we have enough memory so we can avoid complex error handling below:
@@ -341,8 +343,9 @@ DOMSVGPointList::ReplaceItem(nsISVGPoint& aNewItem, uint32_t aIndex,
   }
 
   nsCOMPtr<nsISVGPoint> domItem = &aNewItem;
-  if (domItem->HasOwner() || domItem->IsReadonly()) {
-    domItem = domItem->Clone(); // must do this before changing anything!
+  if (domItem->HasOwner() || domItem->IsReadonly() ||
+      domItem->IsTranslatePoint()) {
+    domItem = domItem->Copy(); // must do this before changing anything!
   }
 
   AutoChangePointListNotifier notifier(this);

dom/svg/SVGSVGElement.cpp

@@ -74,10 +74,10 @@ SVGSVGElement::~SVGSVGElement()
 {
 }
 
-nsISVGPoint*
-DOMSVGTranslatePoint::Clone()
+DOMSVGPoint*
+DOMSVGTranslatePoint::Copy()
 {
-  return new DOMSVGTranslatePoint(this);
+  return new DOMSVGPoint(mPt.GetX(), mPt.GetY());
 }
 
 nsISupports*

dom/svg/SVGSVGElement.h

@@ -47,15 +47,15 @@ class SVGSVGElement;
 class DOMSVGTranslatePoint MOZ_FINAL : public nsISVGPoint {
 public:
   DOMSVGTranslatePoint(SVGPoint* aPt, SVGSVGElement *aElement)
-    : nsISVGPoint(aPt), mElement(aElement) {}
+    : nsISVGPoint(aPt, true), mElement(aElement) {}
 
   DOMSVGTranslatePoint(DOMSVGTranslatePoint* aPt)
-    : nsISVGPoint(&aPt->mPt), mElement(aPt->mElement) {}
+    : nsISVGPoint(&aPt->mPt, true), mElement(aPt->mElement) {}
 
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(DOMSVGTranslatePoint, nsISVGPoint)
 
-  virtual nsISVGPoint* Clone() MOZ_OVERRIDE;
+  virtual DOMSVGPoint* Copy() MOZ_OVERRIDE;
 
   // WebIDL
   virtual float X() MOZ_OVERRIDE { return mPt.GetX(); }

dom/svg/nsISVGPoint.h

@@ -18,7 +18,7 @@ class nsSVGElement;
   { 0xd6b6c440, 0xaf8d, 0x40ee, \
     { 0x85, 0x6b, 0x02, 0xa3, 0x17, 0xca, 0xb2, 0x75 } }
 
-#define MOZ_SVG_LIST_INDEX_BIT_COUNT 30
+#define MOZ_SVG_LIST_INDEX_BIT_COUNT 29
 
 namespace mozilla {
 
@@ -48,15 +48,17 @@ public:
     , mListIndex(0)
     , mIsReadonly(false)
     , mIsAnimValItem(false)
+    , mIsTranslatePoint(false)
   {
     SetIsDOMBinding();
   }
 
-  explicit nsISVGPoint(SVGPoint* aPt)
+  explicit nsISVGPoint(SVGPoint* aPt, bool aIsTranslatePoint)
     : mList(nullptr)
     , mListIndex(0)
     , mIsReadonly(false)
     , mIsAnimValItem(false)
+    , mIsTranslatePoint(aIsTranslatePoint)
   {
     SetIsDOMBinding();
     mPt.mX = aPt->GetX();
@@ -76,10 +78,9 @@ protected:
 
 public:
   /**
-   * Create an unowned copy of this object. The caller is responsible for the
-   * first AddRef()!
+   * Creates an unowned copy of this object's point as a DOMSVGPoint.
    */
-  virtual nsISVGPoint* Clone() = 0;
+  virtual DOMSVGPoint* Copy() = 0;
 
   SVGPoint ToSVGPoint() const {
     return HasOwner() ? const_cast<nsISVGPoint*>(this)->InternalItem() : mPt;
@@ -99,6 +100,10 @@ public:
     return !!mList;
   }
 
+  bool IsTranslatePoint() const {
+    return mIsTranslatePoint;
+  }
+
   /**
    * This method is called to notify this DOM object that it is being inserted
    * into a list, and give it the information it needs as a result.
@@ -158,8 +163,9 @@ protected:
   // that if you change the capacity of any of the following.
 
   uint32_t mListIndex:MOZ_SVG_LIST_INDEX_BIT_COUNT;
-  uint32_t mIsReadonly:1;    // uint32_t because MSVC won't pack otherwise
-  uint32_t mIsAnimValItem:1; // uint32_t because MSVC won't pack otherwise
+  uint32_t mIsReadonly:1;       // These flags are uint32_t because MSVC won't
+  uint32_t mIsAnimValItem:1;    // pack otherwise.
+  uint32_t mIsTranslatePoint:1;
 
   /**
    * Get a reference to the internal SVGPoint list item that this DOM wrapper

dom/svg/test/test_SVGPointList.xhtml

@@ -14,6 +14,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=629200
 <div id="content" style="display:none;">
 <svg id="svg" xmlns="http://www.w3.org/2000/svg" width="100" height="100">
   <polyline id="polyline" points="50,375 150,380"/>
+  <polyline id="polyline2" points="10,20"/>
 </svg>
 </div>
 <pre id="test">
@@ -63,6 +64,60 @@ function run_tests()
   polyline.removeAttributeNS(null, "points");
   eventChecker.finish();
 
+  // Test that the addition of an owned SVGPoint to an SVGPointList creates a
+  // copy of the SVGPoint
+  var polyline2 = document.getElementById("polyline2");
+  var subtests = [
+    function initialize(aItem) {
+      polyline.removeAttribute("points");
+      return points.initialize(aItem);
+    },
+    function insertItemBefore(aItem) {
+      polyline.removeAttribute("points");
+      return points.insertItemBefore(aItem, 0);
+    },
+    function replaceItem(aItem) {
+      polyline.setAttribute("points", "10,20");
+      return points.replaceItem(aItem, 0);
+    },
+    function appendItem(aItem) {
+      polyline.removeAttribute("points");
+      return points.appendItem(aItem);
+    }
+  ];
+  subtests.forEach(function(aFunction) {
+    // -- Adding SVGSVGElement.currentTranslate, which is the only instance
+    //    of an owned, single SVGPoint
+    var svg = document.getElementById("svg");
+    var name = aFunction.name;
+    var existingItem = svg.currentTranslate;
+    var newItem = aFunction(existingItem);
+    is(newItem, points.getItem(0), name + " return value is correct when passed currentTranslate");
+    isnot(newItem, existingItem, name + " made a copy when passed currentTranslate");
+    is(newItem.value, existingItem.value, name + " made a copy with the right values when passed currentTranslate");
+    todo(svg.currentTranslate == existingItem, name + " left the original object alone when passed currentTranslate");
+  });
+  subtests.forEach(function(aFunction) {
+    // -- Adding an SVGPoint that is in a baseVal list
+    var name = aFunction.name;
+    var existingItem = polyline2.points.getItem(0);
+    var newItem = aFunction(existingItem);
+    is(newItem, points.getItem(0), name + " return value is correct when passed a baseVal list item");
+    isnot(newItem, existingItem, name + " made a copy when passed a baseVal list item");
+    is(newItem.value, existingItem.value, name + " made a copy with the right values when passed a baseVal list item");
+    is(polyline2.points.getItem(0), existingItem, name + " left the original object alone when passed a baseVal list item");
+  });
+  subtests.forEach(function(aFunction) {
+    // -- Adding an SVGPoint that is in a animVal list
+    var name = aFunction.name;
+    var existingItem = polyline2.animatedPoints.getItem(0);
+    var newItem = aFunction(existingItem);
+    is(newItem, points.getItem(0), name + " return value is correct when passed a animVal list item");
+    isnot(newItem, existingItem, name + " made a copy when passed a animVal list item");
+    is(newItem.value, existingItem.value, name + " made a copy with the right values when passed a animVal list item");
+    is(polyline2.animatedPoints.getItem(0), existingItem, name + " left the original object alone when passed a animVal list item");
+  });
+
   SimpleTest.finish();
 }