Bug 1024926 - Invalidate SVGPathData's cached measuring path when script changes the SVGPathList or its segments so that SVGPathElement::getTotalLength() returns up to date values. r=jwatt
authorRobert Longson <longsonr@gmail.com>
Wed, 02 Jul 2014 00:12:16 +0100
changeset 191732 5cfae96d4136ec97d8e1f6871872af21116f3cb3
parent 191731 d4424f171d2ef3b84005df040aa2d40818f39def
child 191733 30fa36882d5767f393046345a0ed78b11e9ae9b0
push id45660
push userjwatt@jwatt.org
push dateTue, 01 Jul 2014 23:13:43 +0000
treeherdermozilla-inbound@5cfae96d4136 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwatt
bugs1024926
milestone33.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1024926 - Invalidate SVGPathData's cached measuring path when script changes the SVGPathList or its segments so that SVGPathElement::getTotalLength() returns up to date values. r=jwatt
content/svg/content/src/DOMSVGPathSeg.cpp
content/svg/content/src/DOMSVGPathSeg.h
content/svg/content/src/DOMSVGPathSegList.cpp
content/svg/content/test/mochitest.ini
content/svg/content/test/test_pathLength.html
--- a/content/svg/content/src/DOMSVGPathSeg.cpp
+++ b/content/svg/content/src/DOMSVGPathSeg.cpp
@@ -175,16 +175,17 @@ DOMSVGPathSeg::IndexIsValid()
       return;                                                                 \
     }                                                                         \
     if (HasOwner()) {                                                         \
       if (InternalItem()[1+index] == float(a##propName)) {                    \
         return;                                                               \
       }                                                                       \
       AutoChangePathSegNotifier notifier(this);                               \
       InternalItem()[1+index] = float(a##propName);                           \
+      InvalidateCachedList();                                                 \
     } else {                                                                  \
       mArgs[index] = float(a##propName);                                      \
     }                                                                         \
   }
 
 // For float, the normal type of arguments
 #define IMPL_FLOAT_PROP(segName, propName, index) \
   IMPL_PROP_WITH_TYPE(segName, propName, index, float)
--- a/content/svg/content/src/DOMSVGPathSeg.h
+++ b/content/svg/content/src/DOMSVGPathSeg.h
@@ -204,16 +204,20 @@ protected:
    *
    * To simplify the code we just have this one method for obtaining both
    * baseVal and animVal internal items. This means that animVal items don't
    * get const protection, but then our setter methods guard against changing
    * animVal items.
    */
   float* InternalItem();
 
+  void InvalidateCachedList() {
+    mList->InternalList().mCachedPath = nullptr;
+  }
+
   virtual float* PtrToMemberArgs() = 0;
 
 #ifdef DEBUG
   bool IndexIsValid();
 #endif
 
   nsRefPtr<DOMSVGPathSegList> mList;
 
--- a/content/svg/content/src/DOMSVGPathSegList.cpp
+++ b/content/svg/content/src/DOMSVGPathSegList.cpp
@@ -378,16 +378,17 @@ DOMSVGPathSegList::InsertItemBefore(DOMS
   AutoChangePathSegListNotifier notifier(this);
   // Now that we know we're inserting, keep animVal list in sync as necessary.
   MaybeInsertNullInAnimValListAt(aIndex, internalIndex, argCount);
 
   float segAsRaw[1 + NS_SVG_PATH_SEG_MAX_ARGS];
   domItem->ToSVGPathSegEncodedData(segAsRaw);
 
   InternalList().mData.InsertElementsAt(internalIndex, segAsRaw, 1 + argCount);
+  InternalList().mCachedPath = nullptr;
   mItems.InsertElementAt(aIndex, ItemProxy(domItem.get(), internalIndex));
 
   // This MUST come after the insertion into InternalList(), or else under the
   // insertion into InternalList() the values read from domItem would be bad
   // data from InternalList() itself!:
   domItem->InsertingIntoList(this, aIndex, IsAnimValList());
 
   UpdateListIndicesFromIndex(aIndex + 1, argCount + 1);
@@ -434,16 +435,17 @@ DOMSVGPathSegList::ReplaceItem(DOMSVGPat
   int32_t newArgCount = SVGPathSegUtils::ArgCountForType(domItem->Type());
 
   float segAsRaw[1 + NS_SVG_PATH_SEG_MAX_ARGS];
   domItem->ToSVGPathSegEncodedData(segAsRaw);
 
   bool ok = !!InternalList().mData.ReplaceElementsAt(
                   internalIndex, 1 + oldArgCount,
                   segAsRaw, 1 + newArgCount);
+  InternalList().mCachedPath = nullptr;
   if (!ok) {
     aError.Throw(NS_ERROR_OUT_OF_MEMORY);
     return nullptr;
   }
   ItemAt(aIndex) = domItem;
 
   // This MUST come after the ToSVGPathSegEncodedData call, otherwise that call
   // would end up reading bad data from InternalList()!
@@ -488,16 +490,17 @@ DOMSVGPathSegList::RemoveItem(uint32_t a
   int32_t argCount = SVGPathSegUtils::ArgCountForType(segType);
 
   // Now that we know we're removing, keep animVal list in sync as necessary.
   // Do this *before* touching InternalList() so the removed item can get its
   // internal value.
   MaybeRemoveItemFromAnimValListAt(aIndex, argCount);
 
   InternalList().mData.RemoveElementsAt(internalIndex, 1 + argCount);
+  InternalList().mCachedPath = nullptr;
   mItems.RemoveElementAt(aIndex);
 
   UpdateListIndicesFromIndex(aIndex, -(argCount + 1));
 
   return result.forget();
 }
 
 already_AddRefed<DOMSVGPathSeg>
--- a/content/svg/content/test/mochitest.ini
+++ b/content/svg/content/test/mochitest.ini
@@ -47,16 +47,17 @@ support-files =
 skip-if = true # disabled-for-intermittent-failures--bug-701060
 [test_length.xhtml]
 skip-if = true
 [test_lengthParsing.html]
 [test_nonAnimStrings.xhtml]
 [test_non-scaling-stroke.html]
 [test_onerror.xhtml]
 [test_pathAnimInterpolation.xhtml]
+[test_pathLength.html]
 [test_pathSeg.xhtml]
 [test_pointAtLength.xhtml]
 [test_pointer-events-1a.xhtml]
 [test_pointer-events-1b.xhtml]
 [test_pointer-events-2.xhtml]
 [test_pointer-events-3.xhtml]
 [test_pointer-events-4.xhtml]
 [test_pointer-events-5.xhtml]
new file mode 100644
--- /dev/null
+++ b/content/svg/content/test/test_pathLength.html
@@ -0,0 +1,40 @@
+<!doctype html>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1024926
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test path length changes when manipulated</title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=946529">Mozilla Bug 1024926</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  <svg width="100%" height="1" id="svg">
+    <path id="path" d="M50,100l0,0l0,-50l100,0l86.3325,122.665z"></path>
+  </svg>
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+  SimpleTest.waitForExplicitFinish();
+
+  var path = document.getElementById("path");
+
+  is(path.getTotalLength(), 500, "Unexpected path length");
+
+  for (var i = 0; i < 2; i++) {
+      path.pathSegList.removeItem(path.pathSegList.numberOfItems - 1);
+  }
+
+  is(path.getTotalLength(), 150, "Unexpected path length");
+
+  SimpleTest.finish();
+
+</script>
+</pre>
+</body>
+</html>