Bug 629982: Attempt to fix crashes due to too deep DOM trees by making GetBaseURI non-recursive. r=jst a=blocker
authorJonas Sicking <jonas@sicking.cc>
Thu, 10 Feb 2011 23:47:00 -0800
changeset 62344 231109adff314e22557f1f6504608ef16777e09c
parent 62343 69e9525dff19aeeb1344eb92b132d9da17fcf296
child 62345 8013e7a30984388b47a3f980088417c3e76ecf75
push id18702
push usersicking@mozilla.com
push dateFri, 11 Feb 2011 07:47:41 +0000
treeherdermozilla-central@8013e7a30984 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjst, blocker
bugs629982
milestone2.0b12pre
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 629982: Attempt to fix crashes due to too deep DOM trees by making GetBaseURI non-recursive. r=jst a=blocker
content/base/public/nsIContent.h
content/base/src/nsGenericDOMDataNode.cpp
content/base/src/nsGenericDOMDataNode.h
content/base/src/nsGenericElement.cpp
content/base/src/nsGenericElement.h
content/base/test/Makefile.in
content/base/test/file_base_xbl.xml
content/base/test/test_base.xhtml
content/svg/content/src/nsSVGElement.cpp
content/svg/content/src/nsSVGElement.h
--- a/content/base/public/nsIContent.h
+++ b/content/base/public/nsIContent.h
@@ -944,16 +944,19 @@ public:
 
   /**
    * If the content is a part of HTML editor, this returns editing
    * host content.  When the content is in designMode, this returns its body
    * element.  Also, when the content isn't editable, this returns null.
    */
   nsIContent* GetEditingHost();
 
+  // Overloaded from nsINode
+  virtual already_AddRefed<nsIURI> GetBaseURI() const;
+
 protected:
   /**
    * Hook for implementing GetID.  This is guaranteed to only be
    * called if the NODE_HAS_ID flag is set.
    */
   virtual nsIAtom* DoGetID() const = 0;
 
 private:
--- a/content/base/src/nsGenericDOMDataNode.cpp
+++ b/content/base/src/nsGenericDOMDataNode.cpp
@@ -766,30 +766,16 @@ nsGenericDOMDataNode::List(FILE* out, PR
 
 void
 nsGenericDOMDataNode::DumpContent(FILE* out, PRInt32 aIndent,
                                   PRBool aDumpAll) const 
 {
 }
 #endif
 
-already_AddRefed<nsIURI>
-nsGenericDOMDataNode::GetBaseURI() const
-{
-  // DOM Data Node inherits the base from its parent element/document
-  nsIContent *parent = GetParent();
-  if (parent) {
-    return parent->GetBaseURI();
-  }
-
-  nsIDocument *doc = GetOwnerDoc();
-
-  return doc ? doc->GetBaseURI() : nsnull;
-}
-
 PRBool
 nsGenericDOMDataNode::IsLink(nsIURI** aURI) const
 {
   *aURI = nsnull;
   return PR_FALSE;
 }
 
 nsINode::nsSlots*
--- a/content/base/src/nsGenericDOMDataNode.h
+++ b/content/base/src/nsGenericDOMDataNode.h
@@ -242,18 +242,16 @@ public:
 
 #ifdef DEBUG
   virtual void List(FILE* out, PRInt32 aIndent) const;
   virtual void DumpContent(FILE* out, PRInt32 aIndent, PRBool aDumpAll) const;
 #endif
 
   virtual nsIContent *GetBindingParent() const;
   virtual PRBool IsNodeOfType(PRUint32 aFlags) const;
-
-  virtual already_AddRefed<nsIURI> GetBaseURI() const;
   virtual PRBool IsLink(nsIURI** aURI) const;
 
   virtual nsIAtom* DoGetID() const;
   virtual const nsAttrValue* DoGetClasses() const;
   NS_IMETHOD WalkContentStyleRules(nsRuleWalker* aRuleWalker);
   virtual nsICSSStyleRule* GetInlineStyleRule();
   NS_IMETHOD SetInlineStyleRule(nsICSSStyleRule* aStyleRule, PRBool aNotify);
   NS_IMETHOD_(PRBool) IsAttributeMapped(const nsIAtom* aAttribute) const;
--- a/content/base/src/nsGenericElement.cpp
+++ b/content/base/src/nsGenericElement.cpp
@@ -71,16 +71,17 @@
 #include "nsIPrivateDOMEvent.h"
 #include "nsDOMCID.h"
 #include "nsIServiceManager.h"
 #include "nsIDOMCSSStyleDeclaration.h"
 #include "nsDOMCSSAttrDeclaration.h"
 #include "nsINameSpaceManager.h"
 #include "nsContentList.h"
 #include "nsDOMTokenList.h"
+#include "nsXBLPrototypeBinding.h"
 #include "nsDOMError.h"
 #include "nsDOMString.h"
 #include "nsIScriptSecurityManager.h"
 #include "nsIDOMMutationEvent.h"
 #include "nsMutationEvent.h"
 #include "nsNodeUtils.h"
 #include "nsDocument.h"
 #ifdef MOZ_XUL
@@ -911,16 +912,87 @@ nsIContent::LookupNamespaceURI(const nsA
   const nsIContent* content = this;
   do {
     if (content->GetAttr(kNameSpaceID_XMLNS, name, aNamespaceURI))
       return NS_OK;
   } while ((content = content->GetParent()));
   return NS_ERROR_FAILURE;
 }
 
+already_AddRefed<nsIURI>
+nsIContent::GetBaseURI() const
+{
+  nsIDocument* doc = GetOwnerDoc();
+  if (!doc) {
+    // We won't be able to do security checks, etc.  So don't go any
+    // further.  That said, this really shouldn't happen...
+    NS_ERROR("Element without owner document");
+    return nsnull;
+  }
+
+  // Start with document base
+  nsCOMPtr<nsIURI> base = doc->GetDocBaseURI();
+
+  // Collect array of xml:base attribute values up the parent chain. This
+  // is slightly slower for the case when there are xml:base attributes, but
+  // faster for the far more common case of there not being any such
+  // attributes.
+  // Also check for SVG elements which require special handling
+  nsAutoTArray<nsString, 5> baseAttrs;
+  nsString attr;
+  const nsIContent *elem = this;
+  do {
+    // First check for SVG specialness (why is this SVG specific?)
+    if (elem->IsSVG()) {
+      nsIContent* bindingParent = elem->GetBindingParent();
+      if (bindingParent) {
+        nsIDocument* bindingDoc = bindingParent->GetOwnerDoc();
+        if (bindingDoc) {
+          nsXBLBinding* binding =
+            bindingDoc->BindingManager()->GetBinding(bindingParent);
+          if (binding) {
+            // XXX sXBL/XBL2 issue
+            // If this is an anonymous XBL element use the binding
+            // document for the base URI. 
+            // XXX Will fail with xml:base
+            base = binding->PrototypeBinding()->DocURI();
+            break;
+          }
+        }
+      }
+    }
+    
+    // Otherwise check for xml:base attribute
+    elem->GetAttr(kNameSpaceID_XML, nsGkAtoms::base, attr);
+    if (!attr.IsEmpty()) {
+      baseAttrs.AppendElement(attr);
+    }
+    elem = elem->GetParent();
+  } while(elem);
+  
+  // Now resolve against all xml:base attrs
+  for (PRUint32 i = baseAttrs.Length() - 1; i != PRUint32(-1); --i) {
+    nsCOMPtr<nsIURI> newBase;
+    nsresult rv = NS_NewURI(getter_AddRefs(newBase), baseAttrs[i],
+                            doc->GetDocumentCharacterSet().get(), base);
+    // Do a security check, almost the same as nsDocument::SetBaseURL()
+    // Only need to do this on the final uri
+    if (NS_SUCCEEDED(rv) && i == 0) {
+      rv = nsContentUtils::GetSecurityManager()->
+        CheckLoadURIWithPrincipal(NodePrincipal(), newBase,
+                                  nsIScriptSecurityManager::STANDARD);
+    }
+    if (NS_SUCCEEDED(rv)) {
+      base.swap(newBase);
+    }
+  }
+
+  return base.forget();
+}
+
 //----------------------------------------------------------------------
 
 NS_IMPL_ADDREF(nsChildContentList)
 NS_IMPL_RELEASE(nsChildContentList)
 
 NS_INTERFACE_TABLE_HEAD(nsChildContentList)
   NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY
   NS_NODELIST_OFFSET_AND_INTERFACE_TABLE_BEGIN(nsChildContentList)
@@ -3391,61 +3463,16 @@ nsGenericElement::GetExistingAttrNameFro
   }
   else {
     NS_ADDREF(nodeInfo = name->NodeInfo());
   }
 
   return nodeInfo;
 }
 
-already_AddRefed<nsIURI>
-nsGenericElement::GetBaseURI() const
-{
-  nsIDocument* doc = GetOwnerDoc();
-  if (!doc) {
-    // We won't be able to do security checks, etc.  So don't go any
-    // further.  That said, this really shouldn't happen...
-    NS_ERROR("Element without owner document");
-    return nsnull;
-  }
-
-  // Our base URL depends on whether we have an xml:base attribute, as
-  // well as on whether any of our ancestors do.
-  nsCOMPtr<nsIURI> parentBase;
-
-  nsIContent *parent = GetParent();
-  if (parent) {
-    parentBase = parent->GetBaseURI();
-  } else {
-    // No parent, so just use the document (we must be the root or not in the
-    // tree).
-    parentBase = doc->GetBaseURI();
-  }
-  
-  // Now check for an xml:base attr 
-  nsAutoString value;
-  GetAttr(kNameSpaceID_XML, nsGkAtoms::base, value);
-  if (value.IsEmpty()) {
-    // No xml:base, so we just use the parent's base URL
-    return parentBase.forget();
-  }
-
-  nsCOMPtr<nsIURI> ourBase;
-  nsresult rv = NS_NewURI(getter_AddRefs(ourBase), value,
-                          doc->GetDocumentCharacterSet().get(), parentBase);
-  if (NS_SUCCEEDED(rv)) {
-    // do a security check, almost the same as nsDocument::SetBaseURL()
-    rv = nsContentUtils::GetSecurityManager()->
-      CheckLoadURIWithPrincipal(NodePrincipal(), ourBase,
-                                nsIScriptSecurityManager::STANDARD);
-  }
-
-  return NS_SUCCEEDED(rv) ? ourBase.forget() : parentBase.forget();
-}
-
 PRBool
 nsGenericElement::IsLink(nsIURI** aURI) const
 {
   *aURI = nsnull;
   return PR_FALSE;
 }
 
 // static
--- a/content/base/src/nsGenericElement.h
+++ b/content/base/src/nsGenericElement.h
@@ -437,17 +437,16 @@ public:
     return SetText(aStr.BeginReading(), aStr.Length(), aNotify);
   }
   virtual nsresult AppendText(const PRUnichar* aBuffer, PRUint32 aLength,
                               PRBool aNotify);
   virtual PRBool TextIsOnlyWhitespace();
   virtual void AppendTextTo(nsAString& aResult);
   virtual nsIContent *GetBindingParent() const;
   virtual PRBool IsNodeOfType(PRUint32 aFlags) const;
-  virtual already_AddRefed<nsIURI> GetBaseURI() const;
   virtual PRBool IsLink(nsIURI** aURI) const;
 
   virtual PRUint32 GetScriptTypeID() const;
   NS_IMETHOD SetScriptTypeID(PRUint32 aLang);
 
   virtual void DestroyContent();
   virtual void SaveSubtreeState();
 
--- a/content/base/test/Makefile.in
+++ b/content/base/test/Makefile.in
@@ -452,16 +452,18 @@ include $(topsrcdir)/config/rules.mk
 		file_bug604660-5.xml \
 		file_bug604660-6.xsl \
 		test_bug605982.html \
 		test_bug606729.html \
 		test_treewalker_nextsibling.xml \
 		test_bug614058.html \
 		test_bug590771.html \
 		test_bug622117.html \
+		test_base.xhtml \
+		file_base_xbl.xml \
 		test_bug622246.html \
 		test_bug484396.html \
 		test_bug466080.html \
 		bug466080.sjs \
 		test_bug625722.html \
 		test_bug631615.html \
 		$(NULL)
 
new file mode 100644
--- /dev/null
+++ b/content/base/test/file_base_xbl.xml
@@ -0,0 +1,9 @@
+<bindings xmlns="http://www.mozilla.org/xbl"
+          xmlns:xhtml="http://www.w3.org/1999/xhtml"
+          xmlns:svg="http://www.w3.org/2000/svg">
+<binding id="test">
+  <content>
+    <svg:g><svg:circle><xhtml:div xml:base="#shesellsseashellsbytheseashore"/></svg:circle><children/></svg:g>
+  </content>
+ </binding>
+</bindings>
new file mode 100644
--- /dev/null
+++ b/content/base/test/test_base.xhtml
@@ -0,0 +1,56 @@
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+  <title>Test for base URIs</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+  <base href="/tests/content/base/" />
+  <style>
+    #bound { -moz-binding: url("test/file_base_xbl.xml#test"); }
+  </style>
+</head>
+<body>
+<div id="1" xml:base="supercalifragilisticexpialidocious"><p><p xml:base="hello/"><p xml:base="world"><span xml:base="#iamtheverymodelofamodernmajorgeneral">text</span></p></p></p></div>
+<div id="bound"/>
+<pre id="test">
+<script type="application/javascript">
+SimpleTest.waitForExplicitFinish();
+addLoadEvent(function() {
+  is(document.baseURI, "http://mochi.test:8888/tests/content/base/",
+     "document base");
+  is(document.body.baseURI, "http://mochi.test:8888/tests/content/base/",
+     "body base");
+
+  var expected =
+    ["http://mochi.test:8888/tests/content/base/supercalifragilisticexpialidocious",
+     "http://mochi.test:8888/tests/content/base/supercalifragilisticexpialidocious",
+     "http://mochi.test:8888/tests/content/base/hello/",
+     "http://mochi.test:8888/tests/content/base/hello/world",
+     "http://mochi.test:8888/tests/content/base/hello/world#iamtheverymodelofamodernmajorgeneral",
+     "http://mochi.test:8888/tests/content/base/hello/world#iamtheverymodelofamodernmajorgeneral",
+     ];
+  var node = document.getElementById("1");
+  while(node) {
+    is(node.baseURI, expected.shift(), "node base");
+    node = node.firstChild;
+  }
+  is(expected.length, 0, "found all expected nodes");
+
+  var svgExpected =
+    ["http://mochi.test:8888/tests/content/base/test/file_base_xbl.xml",
+     "http://mochi.test:8888/tests/content/base/test/file_base_xbl.xml",
+     "http://mochi.test:8888/tests/content/base/test/file_base_xbl.xml#shesellsseashellsbytheseashore",
+     ];
+  node = document.getAnonymousNodes(document.getElementById("bound"))[0];
+  while(node) {
+    is(node.baseURI, svgExpected.shift(), "node base");
+    node = node.firstChild;
+  }
+  is(svgExpected.length, 0, "found all expected nodes");
+
+  SimpleTest.finish();
+});
+</script>
+</pre>
+</body>
+</html>
--- a/content/svg/content/src/nsSVGElement.cpp
+++ b/content/svg/content/src/nsSVGElement.cpp
@@ -830,38 +830,16 @@ nsSVGElement::GetAttributeChangeHint(con
 }
 
 PRBool
 nsSVGElement::IsNodeOfType(PRUint32 aFlags) const
 {
   return !(aFlags & ~(eCONTENT | eSVG));
 }
 
-already_AddRefed<nsIURI>
-nsSVGElement::GetBaseURI() const
-{
-  nsCOMPtr<nsIURI> baseURI = nsSVGElementBase::GetBaseURI();
-
-  nsIContent* bindingParent = GetBindingParent();
-  if (bindingParent) {
-    nsIDocument* doc = bindingParent->GetOwnerDoc();
-    if (doc) {
-      nsXBLBinding* binding = doc->BindingManager()->GetBinding(bindingParent);
-      if (binding) {
-        // XXX sXBL/XBL2 issue
-        // If this is an anonymous XBL element use the binding
-        // document for the base URI. 
-        // XXX Will fail with xml:base
-        baseURI = binding->PrototypeBinding()->DocURI();
-      }
-    }
-  }
-  return baseURI.forget();
-}
-
 NS_IMETHODIMP
 nsSVGElement::WalkContentStyleRules(nsRuleWalker* aRuleWalker)
 {
 #ifdef DEBUG
 //  printf("nsSVGElement(%p)::WalkContentStyleRules()\n", this);
 #endif
   if (!mContentStyleRule)
     UpdateContentStyleRule();
--- a/content/svg/content/src/nsSVGElement.h
+++ b/content/svg/content/src/nsSVGElement.h
@@ -111,18 +111,16 @@ public:
   virtual nsresult UnsetAttr(PRInt32 aNameSpaceID, nsIAtom* aAttribute,
                              PRBool aNotify);
 
   virtual nsChangeHint GetAttributeChangeHint(const nsIAtom* aAttribute,
                                               PRInt32 aModType) const;
 
   virtual PRBool IsNodeOfType(PRUint32 aFlags) const;
 
-  virtual already_AddRefed<nsIURI> GetBaseURI() const;
-
   NS_IMETHOD WalkContentStyleRules(nsRuleWalker* aRuleWalker);
 
   static const MappedAttributeEntry sFillStrokeMap[];
   static const MappedAttributeEntry sGraphicsMap[];
   static const MappedAttributeEntry sTextContentElementsMap[];
   static const MappedAttributeEntry sFontSpecificationMap[];
   static const MappedAttributeEntry sGradientStopMap[];
   static const MappedAttributeEntry sViewportsMap[];