Bug 344258. Make <use> honour changes to the ID-to-element map. r=longsonr,sr=mats
authorRobert O'Callahan <robert@ocallahan.org>
Thu, 26 Jun 2008 10:41:04 +1200
changeset 15538 b82b4e5254f49391125e18c9588d26f72be7a817
parent 15537 8bb280795d523d04e816d2b499ddb401db200cb1
child 15539 feb994184605472aeb81b09d4c56ea283e455776
push id295
push userrocallahan@mozilla.com
push dateWed, 25 Jun 2008 22:42:17 +0000
treeherdermozilla-central@b82b4e5254f4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslongsonr, mats
bugs344258
milestone1.9.1a1pre
Bug 344258. Make <use> honour changes to the ID-to-element map. r=longsonr,sr=mats
content/svg/content/src/nsSVGUseElement.cpp
content/svg/content/src/nsSVGUseElement.h
layout/base/nsIPresShell.h
layout/base/nsPresShell.cpp
layout/reftests/svg/dynamic-use-01.svg
layout/reftests/svg/reftest.list
--- a/content/svg/content/src/nsSVGUseElement.cpp
+++ b/content/svg/content/src/nsSVGUseElement.cpp
@@ -65,23 +65,23 @@ NS_IMPL_NS_NEW_SVG_ELEMENT(Use)
 // nsISupports methods
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(nsSVGUseElement)
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(nsSVGUseElement,
                                                 nsSVGUseElementBase)
   nsAutoScriptBlocker scriptBlocker;
   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mOriginal)
   tmp->DestroyAnonymousContent();
-  tmp->RemoveListener();
+  tmp->UnlinkSource();
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(nsSVGUseElement,
                                                   nsSVGUseElementBase)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mOriginal)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mClone)
-  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mSourceContent)
+  tmp->mSource.Traverse(&cb);
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_ADDREF_INHERITED(nsSVGUseElement,nsSVGUseElementBase)
 NS_IMPL_RELEASE_INHERITED(nsSVGUseElement,nsSVGUseElementBase)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_INHERITED(nsSVGUseElement)
   NS_INTERFACE_MAP_ENTRY(nsIDOMNode)
   NS_INTERFACE_MAP_ENTRY(nsIDOMElement)
@@ -94,23 +94,23 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_
     foundInterface = reinterpret_cast<nsISupports*>(this);
   else
 NS_INTERFACE_MAP_END_INHERITING(nsSVGUseElementBase)
 
 //----------------------------------------------------------------------
 // Implementation
 
 nsSVGUseElement::nsSVGUseElement(nsINodeInfo *aNodeInfo)
-  : nsSVGUseElementBase(aNodeInfo)
+  : nsSVGUseElementBase(aNodeInfo), mSource(this)
 {
 }
 
 nsSVGUseElement::~nsSVGUseElement()
 {
-  RemoveListener();
+  UnlinkSource();
 }
 
 //----------------------------------------------------------------------
 // nsIDOMNode methods
 
 nsresult
 nsSVGUseElement::Clone(nsINodeInfo *aNodeInfo, nsINode **aResult) const
 {
@@ -227,42 +227,40 @@ nsSVGUseElement::ContentRemoved(nsIDocum
   if (nsContentUtils::IsInSameAnonymousTree(this, aChild)) {
     TriggerReclone();
   }
 }
 
 void
 nsSVGUseElement::NodeWillBeDestroyed(const nsINode *aNode)
 {
-  RemoveListener();
+  UnlinkSource();
 }
 
 //----------------------------------------------------------------------
 
 nsIContent*
 nsSVGUseElement::CreateAnonymousContent()
 {
 #ifdef DEBUG_tor
   const nsString &href = mStringAttributes[HREF].GetAnimValue();
   fprintf(stderr, "<svg:use> reclone of \"%s\"\n", ToNewCString(href));
 #endif
 
   mClone = nsnull;
 
-  nsCOMPtr<nsIContent> targetContent = LookupHref();
+  if (mSource.get()) {
+    mSource.get()->RemoveMutationObserver(this);
+  }
 
+  LookupHref();
+  nsIContent* targetContent = mSource.get();
   if (!targetContent)
     return nsnull;
 
-  PRBool needAddObserver = PR_FALSE;
-  if (mSourceContent != targetContent) {
-    RemoveListener();
-    needAddObserver = PR_TRUE;
-  }
-
   // make sure target is valid type for <use>
   // QIable nsSVGGraphicsElement would eliminate enumerating all elements
   nsIAtom *tag = targetContent->Tag();
   if (tag != nsGkAtoms::svg &&
       tag != nsGkAtoms::symbol &&
       tag != nsGkAtoms::g &&
       tag != nsGkAtoms::path &&
       tag != nsGkAtoms::text &&
@@ -372,20 +370,17 @@ nsSVGUseElement::CreateAnonymousContent(
 
     if (HasAttr(kNameSpaceID_None, nsGkAtoms::height)) {
       nsAutoString height;
       GetAttr(kNameSpaceID_None, nsGkAtoms::height, height);
       newcontent->SetAttr(kNameSpaceID_None, nsGkAtoms::height, height, PR_FALSE);
     }
   }
 
-  if (needAddObserver) {
-    targetContent->AddMutationObserver(this);
-  }
-  mSourceContent = targetContent;
+  targetContent->AddMutationObserver(this);
   mClone = newcontent;
   return mClone;
 }
 
 void
 nsSVGUseElement::DestroyAnonymousContent()
 {
   nsContentUtils::DestroyAnonymousContent(&mClone);
@@ -410,47 +405,47 @@ nsSVGUseElement::SyncWidthHeight(PRUint8
         nsAutoString height;
         GetAttr(kNameSpaceID_None, nsGkAtoms::height, height);
         mClone->SetAttr(kNameSpaceID_None, nsGkAtoms::height, height, PR_FALSE);
       }
     }
   }
 }
 
-nsIContent *
+void
 nsSVGUseElement::LookupHref()
 {
   const nsString &href = mStringAttributes[HREF].GetAnimValue();
   if (href.IsEmpty())
-    return nsnull;
+    return;
 
   nsCOMPtr<nsIURI> targetURI, baseURI = GetBaseURI();
   nsContentUtils::NewURIWithDocumentCharset(getter_AddRefs(targetURI), href,
                                             GetCurrentDoc(), baseURI);
 
-  return nsContentUtils::GetReferencedElement(targetURI, this);
+  mSource.Reset(this, targetURI);
 }
 
 void
 nsSVGUseElement::TriggerReclone()
 {
   nsIDocument *doc = GetCurrentDoc();
   if (!doc) return;
   nsIPresShell *presShell = doc->GetPrimaryShell();
   if (!presShell) return;
-  presShell->RecreateFramesFor(this);
+  presShell->PostRecreateFramesFor(this);
 }
 
 void
-nsSVGUseElement::RemoveListener()
+nsSVGUseElement::UnlinkSource()
 {
-  if (mSourceContent) {
-    mSourceContent->RemoveMutationObserver(this);
-    mSourceContent = nsnull;
+  if (mSource.get()) {
+    mSource.get()->RemoveMutationObserver(this);
   }
+  mSource.Unlink();
 }
 
 //----------------------------------------------------------------------
 // nsSVGElement methods
 
 void
 nsSVGUseElement::DidChangeLength(PRUint8 aAttrEnum, PRBool aDoSetAttr)
 {
--- a/content/svg/content/src/nsSVGUseElement.h
+++ b/content/svg/content/src/nsSVGUseElement.h
@@ -40,22 +40,24 @@
 #include "nsIDOMSVGAnimatedString.h"
 #include "nsIDOMSVGURIReference.h"
 #include "nsIDOMSVGUseElement.h"
 #include "nsStubMutationObserver.h"
 #include "nsSVGGraphicElement.h"
 #include "nsSVGLength2.h"
 #include "nsSVGString.h"
 #include "nsTArray.h"
+#include "nsReferencedElement.h"
 
 class nsIContent;
 class nsINodeInfo;
 
 #define NS_SVG_USE_ELEMENT_IMPL_CID \
-{ 0xa95c13d3, 0xc193, 0x465f, {0x81, 0xf0, 0x02, 0x6d, 0x67, 0x05, 0x54, 0x58 } }
+{ 0x55fb86fe, 0xd81f, 0x4ae4, \
+  { 0x80, 0x3f, 0xeb, 0x90, 0xfe, 0xe0, 0x7a, 0xe9 } }
 
 nsresult
 NS_NewSVGSVGElement(nsIContent **aResult, nsINodeInfo *aNodeInfo);
 
 typedef nsSVGGraphicElement nsSVGUseElementBase;
 
 class nsSVGUseElement : public nsSVGUseElementBase,
                         public nsIDOMSVGURIReference,
@@ -99,33 +101,44 @@ public:
   virtual void DidChangeLength(PRUint8 aAttrEnum, PRBool aDoSetAttr);
   virtual void DidChangeString(PRUint8 aAttrEnum, PRBool aDoSetAttr);
 
   // nsIContent interface
   virtual nsresult Clone(nsINodeInfo *aNodeInfo, nsINode **aResult) const;
   NS_IMETHOD_(PRBool) IsAttributeMapped(const nsIAtom* aAttribute) const;
 
 protected:
+  class SourceReference : public nsReferencedElement {
+  public:
+    SourceReference(nsSVGUseElement* aContainer) : mContainer(aContainer) {}
+  protected:
+    virtual void ContentChanged(nsIContent* aFrom, nsIContent* aTo) {
+      nsReferencedElement::ContentChanged(aFrom, aTo);
+      mContainer->TriggerReclone();
+    }
+  private:
+    nsSVGUseElement* mContainer;
+  };
 
   virtual LengthAttributesInfo GetLengthInfo();
   virtual StringAttributesInfo GetStringInfo();
 
   void SyncWidthHeight(PRUint8 aAttrEnum);
-  nsIContent *LookupHref();
+  void LookupHref();
   void TriggerReclone();
-  void RemoveListener();
+  void UnlinkSource();
 
   enum { X, Y, WIDTH, HEIGHT };
   nsSVGLength2 mLengthAttributes[4];
   static LengthInfo sLengthInfo[4];
 
   enum { HREF };
   nsSVGString mStringAttributes[1];
   static StringInfo sStringInfo[1];
 
   nsCOMPtr<nsIContent> mOriginal; // if we've been cloned, our "real" copy
   nsCOMPtr<nsIContent> mClone;    // cloned tree
-  nsCOMPtr<nsIContent> mSourceContent;  // observed element
+  SourceReference      mSource;   // observed element
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsSVGUseElement, NS_SVG_USE_ELEMENT_IMPL_CID)
 
 #endif
--- a/layout/base/nsIPresShell.h
+++ b/layout/base/nsIPresShell.h
@@ -97,20 +97,21 @@ template<class E> class nsCOMArray;
 class nsWeakFrame;
 class nsIScrollableFrame;
 class gfxASurface;
 class gfxContext;
 
 typedef short SelectionType;
 typedef PRUint32 nsFrameState;
 
-// 228a7d67-811b-4d75-85c0-1ee22c0d2af0
+
+// 23e048f6-49bb-4ac4-b900-c63865363ad3
 #define NS_IPRESSHELL_IID \
-{ 0x228a7d67, 0x811b, 0x4d75, \
-  { 0x85, 0xc0, 0x1e, 0xe2, 0x2c, 0x0d, 0x2a, 0xf0 } }
+{ 0x23e048f6, 0x49bb, 0x4ac4, \
+  { 0xb9, 0x00, 0xc6, 0x38, 0x65, 0x36, 0x3a, 0xd3 } }
 
 // Constants for ScrollContentIntoView() function
 #define NS_PRESSHELL_SCROLL_TOP      0
 #define NS_PRESSHELL_SCROLL_BOTTOM   100
 #define NS_PRESSHELL_SCROLL_LEFT     0
 #define NS_PRESSHELL_SCROLL_RIGHT    100
 #define NS_PRESSHELL_SCROLL_CENTER   50
 #define NS_PRESSHELL_SCROLL_ANYWHERE -1
@@ -380,16 +381,18 @@ public:
 
   NS_IMETHOD CancelAllPendingReflows() = 0;
 
   /**
    * Recreates the frames for a node
    */
   NS_IMETHOD RecreateFramesFor(nsIContent* aContent) = 0;
 
+  void PostRecreateFramesFor(nsIContent* aContent);
+  
   /**
    * Determine if it is safe to flush all pending notifications
    * @param aIsSafeToFlush PR_TRUE if it is safe, PR_FALSE otherwise.
    * 
    */
   NS_IMETHOD IsSafeToFlush(PRBool& aIsSafeToFlush) = 0;
 
   /**
--- a/layout/base/nsPresShell.cpp
+++ b/layout/base/nsPresShell.cpp
@@ -3412,16 +3412,23 @@ PresShell::RecreateFramesFor(nsIContent*
   
   batch.EndUpdateViewBatch(NS_VMREFRESH_NO_SYNC);
 #ifdef ACCESSIBILITY
   InvalidateAccessibleSubtree(aContent);
 #endif
   return rv;
 }
 
+void
+nsIPresShell::PostRecreateFramesFor(nsIContent* aContent)
+{
+  FrameConstructor()->PostRestyleEvent(aContent, eReStyle_Self,
+          nsChangeHint_ReconstructFrame);
+}
+
 NS_IMETHODIMP
 PresShell::ClearFrameRefs(nsIFrame* aFrame)
 {
   mPresContext->EventStateManager()->ClearFrameRefs(aFrame);
   
   if (aFrame == mCurrentEventFrame) {
     mCurrentEventContent = aFrame->GetContent();
     mCurrentEventFrame = nsnull;
new file mode 100644
--- /dev/null
+++ b/layout/reftests/svg/dynamic-use-01.svg
@@ -0,0 +1,91 @@
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/licenses/publicdomain/
+-->
+<svg xmlns="http://www.w3.org/2000/svg" version="1.1"
+     xmlns:xlink="http://www.w3.org/1999/xlink">
+  <title>Testing that dynamic changes to the element for a given ID are reflected in 'use'</title>
+
+  <use id="u1" x="10%" xlink:href="#r1"/>
+  <script>
+    // force frame construction; test that parsing "r1" after frame construction
+    // is still bound to "u1" eventually
+    var rect = document.getElementById("u1").getBoundingClientRect();
+  </script>
+  <rect width="11%" height="100%" fill="lime" id="r1"/>
+
+  <rect width="11%" height="100%" fill="lime" id="x"/>
+  <use id="u2" x="20%" xlink:href="#r2"/>
+  <script>
+    var rect = document.getElementById("u2").getBoundingClientRect();
+    // check that changing an id to "r2" lets u2 find it
+    var r2 = document.getElementById("x");
+    r2.setAttribute("id", "r2");
+  </script>
+
+  <rect width="10%" height="100%" fill="red" id="r3"/>
+  <rect width="11%" height="100%" fill="lime" id="r3"/>
+  <use id="u3" x="30%" xlink:href="#r3"/>
+  <script>
+    var rect = document.getElementById("u3").getBoundingClientRect();
+    // check that removing the bad r3 lets u3 find the good one
+    var r3 = document.getElementById("r3");
+    r3.parentNode.removeChild(r3);
+  </script>
+
+  <rect width="10%" height="100%" fill="red" id="r4"/>
+  <rect width="11%" height="100%" fill="lime" id="r4"/>
+  <use id="u4" x="40%" xlink:href="#r4"/>
+  <script>
+    var rect = document.getElementById("u4").getBoundingClientRect();
+    // check that renaming the bad r4 lets u4 find the good one
+    var r4 = document.getElementById("r4");
+    r4.removeAttribute("id");
+  </script>
+
+  <rect width="10%" height="100%" fill="red" id="r5"/>
+  <use id="u5" x="50%" xlink:href="#r5"/>
+  <script>
+    var rect = document.getElementById("u5").getBoundingClientRect();
+    // check that changing u5's reference works
+    var u5 = document.getElementById("u5");
+    u5.setAttributeNS("http://www.w3.org/1999/xlink", "href", "#r1");
+  </script>
+
+  <rect width="10%" height="100%" fill="red" id="r6"/>
+  <rect width="11%" height="100%" fill="lime" id="r6-2"/>
+  <use id="u6" x="60%" xlink:href="#r6"/>
+  <script>
+    var rect = document.getElementById("u6").getBoundingClientRect();
+    // check that inserting a good element before the bad r6 works
+    var r6 = document.getElementById("r6-2");
+    r6.parentNode.removeChild(r6);
+    r6.setAttribute("id", "r6");
+    document.documentElement.insertBefore(r6, document.documentElement.firstChild);
+  </script>
+
+  <rect width="11%" height="100%" fill="lime" id="r7"/>
+  <rect width="10%" height="100%" fill="red" id="r7-2"/>
+  <use id="u7" x="70%" xlink:href="#r7"/>
+  <script>
+    var rect = document.getElementById("u7").getBoundingClientRect();
+    // check that inserting a bad element after a good one doesn't break anything
+    var r7 = document.getElementById("r7-2");
+    r7.parentNode.removeChild(r7);
+    r7.setAttribute("id", "r7");
+    document.documentElement.appendChild(r7);
+  </script>
+
+  <rect width="11%" height="100%" fill="lime" id="r8-2"/>
+  <rect width="10%" height="100%" fill="red" id="r8"/>
+  <use id="u8" x="80%" xlink:href="#r8"/>
+  <script>
+    var rect = document.getElementById("u8").getBoundingClientRect();
+    // check that renaming a good element to r8 works
+    var r8 = document.getElementById("r8-2");
+    r8.setAttribute("id", "r8");
+  </script>
+
+  <rect width="11%" height="100%" fill="lime"/>
+  <rect x="90%" width="11%" height="100%" fill="lime"/>
+</svg>
--- a/layout/reftests/svg/reftest.list
+++ b/layout/reftests/svg/reftest.list
@@ -19,16 +19,17 @@ include moz-only/reftest.list
 == dynamic-rect-01.svg dynamic-rect-01-ref.svg
 == dynamic-rect-02.svg dynamic-rect-02-ref.svg
 == dynamic-rect-03.svg dynamic-rect-03-ref.svg
 == dynamic-rect-04.xhtml pass.svg
 == dynamic-reflow-01.svg dynamic-reflow-01-ref.svg
 == dynamic-text-01.svg dynamic-text-01-ref.svg
 == dynamic-text-02.svg dynamic-text-02-ref.svg
 == dynamic-text-03.svg dynamic-text-03-ref.svg
+== dynamic-use-01.svg pass.svg
 == fallback-color-01a.svg pass.svg
 == fallback-color-01b.svg pass.svg
 == fallback-color-02a.svg fallback-color-02-ref.svg
 == fallback-color-02b.svg fallback-color-02-ref.svg
 == fallback-color-03.svg pass.svg
 == filter-basic-01.svg pass.svg
 == filter-basic-02.svg pass.svg
 == filter-basic-03.svg pass.svg