Bug 1301254 - nsINode::GetBaseURI() should be fallible, r=smaug
authorAndrea Marchesini <amarchesini@mozilla.com>
Tue, 20 Sep 2016 14:02:33 +0200
changeset 314619 31ea105ca2d9b3088d91628ae77603c469d525da
parent 314618 590b6787d6ef0196c75febeeb97bdd0c34a1e3e8
child 314620 aa252c2f85b5919410685eebc6d8c1c34c86e7ea
push id30732
push usercbook@mozilla.com
push dateWed, 21 Sep 2016 10:04:03 +0000
treeherdermozilla-central@560b2c805bf7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1301254
milestone52.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 1301254 - nsINode::GetBaseURI() should be fallible, r=smaug
dom/base/nsINode.cpp
dom/base/nsINode.h
dom/xslt/xpath/txMozillaXPathTreeWalker.cpp
dom/xslt/xpath/txXPathTreeWalker.h
dom/xslt/xslt/txDocumentFunctionCall.cpp
dom/xslt/xslt/txExecutionState.cpp
dom/xslt/xslt/txExecutionState.h
--- a/dom/base/nsINode.cpp
+++ b/dom/base/nsINode.cpp
@@ -682,28 +682,29 @@ nsINode::Normalize()
                  "Should always have a parent unless "
                  "mutation events messed us up");
     if (parent) {
       parent->RemoveChildAt(parent->IndexOf(node), true);
     }
   }
 }
 
-void
+nsresult
 nsINode::GetBaseURI(nsAString &aURI) const
 {
   nsCOMPtr<nsIURI> baseURI = GetBaseURI();
 
   nsAutoCString spec;
   if (baseURI) {
-    // XXX: should handle GetSpec() failure properly. See bug 1301254.
-    Unused << baseURI->GetSpec(spec);
+    nsresult rv = baseURI->GetSpec(spec);
+    NS_ENSURE_SUCCESS(rv, rv);
   }
 
   CopyUTF8toUTF16(spec, aURI);
+  return NS_OK;
 }
 
 void
 nsINode::GetBaseURIFromJS(nsAString& aURI, ErrorResult& aRv) const
 {
   nsCOMPtr<nsIURI> baseURI = GetBaseURI(nsContentUtils::IsCallerChrome());
   nsAutoCString spec;
   if (baseURI) {
--- a/dom/base/nsINode.h
+++ b/dom/base/nsINode.h
@@ -1821,17 +1821,17 @@ public:
   uint32_t Length() const;
 
   void GetNodeName(mozilla::dom::DOMString& aNodeName)
   {
     const nsString& nodeName = NodeName();
     aNodeName.SetStringBuffer(nsStringBuffer::FromString(nodeName),
                               nodeName.Length());
   }
-  void GetBaseURI(nsAString& aBaseURI) const;
+  MOZ_MUST_USE nsresult GetBaseURI(nsAString& aBaseURI) const;
   // Return the base URI for the document.
   // The returned value may differ if the document is loaded via XHR, and
   // when accessed from chrome privileged script and
   // from content privileged script for compatibility.
   void GetBaseURIFromJS(nsAString& aBaseURI, mozilla::ErrorResult& aRv) const;
   bool HasChildNodes() const
   {
     return HasChildren();
@@ -2284,18 +2284,17 @@ ToCanonicalSupports(nsINode* aPointer)
   } \
   NS_IMETHOD UnusedPlaceholder(bool* aResult) __VA_ARGS__ override \
   { \
     *aResult = false; \
     return NS_OK; \
   } \
   NS_IMETHOD GetDOMBaseURI(nsAString& aBaseURI) __VA_ARGS__ override \
   { \
-    nsINode::GetBaseURI(aBaseURI); \
-    return NS_OK; \
+    return nsINode::GetBaseURI(aBaseURI); \
   } \
   NS_IMETHOD CompareDocumentPosition(nsIDOMNode* aOther, uint16_t* aResult) __VA_ARGS__ override \
   { \
     return nsINode::CompareDocumentPosition(aOther, aResult); \
   } \
   NS_IMETHOD GetTextContent(nsAString& aTextContent) __VA_ARGS__ override \
   { \
     mozilla::ErrorResult rv; \
--- a/dom/xslt/xpath/txMozillaXPathTreeWalker.cpp
+++ b/dom/xslt/xpath/txMozillaXPathTreeWalker.cpp
@@ -557,20 +557,20 @@ txXPathNodeUtils::getXSLTId(const txXPat
         CopyASCIItoUTF16(nsPrintfCString(gPrintfFmtAttr,
                                          nodeid, aNode.mIndex), aResult);
     }
 
     return NS_OK;
 }
 
 /* static */
-void
+nsresult
 txXPathNodeUtils::getBaseURI(const txXPathNode& aNode, nsAString& aURI)
 {
-    aNode.mNode->GetBaseURI(aURI);
+    return aNode.mNode->GetBaseURI(aURI);
 }
 
 /* static */
 int
 txXPathNodeUtils::comparePosition(const txXPathNode& aNode,
                                   const txXPathNode& aOtherNode)
 {
     // First check for equal nodes or attribute-nodes on the same element.
--- a/dom/xslt/xpath/txXPathTreeWalker.h
+++ b/dom/xslt/xpath/txXPathTreeWalker.h
@@ -88,17 +88,17 @@ public:
     static uint16_t getNodeType(const txXPathNode& aNode);
     static void appendNodeValue(const txXPathNode& aNode, nsAString& aResult);
     static bool isWhitespace(const txXPathNode& aNode);
     static txXPathNode* getOwnerDocument(const txXPathNode& aNode);
     static int32_t getUniqueIdentifier(const txXPathNode& aNode);
     static nsresult getXSLTId(const txXPathNode& aNode,
                               const txXPathNode& aBase, nsAString& aResult);
     static void release(txXPathNode* aNode);
-    static void getBaseURI(const txXPathNode& aNode, nsAString& aURI);
+    static nsresult getBaseURI(const txXPathNode& aNode, nsAString& aURI);
     static int comparePosition(const txXPathNode& aNode,
                                const txXPathNode& aOtherNode);
     static bool localNameEquals(const txXPathNode& aNode,
                                   nsIAtom* aLocalName);
     static bool isRoot(const txXPathNode& aNode);
     static bool isElement(const txXPathNode& aNode);
     static bool isAttribute(const txXPathNode& aNode);
     static bool isProcessingInstruction(const txXPathNode& aNode);
--- a/dom/xslt/xslt/txDocumentFunctionCall.cpp
+++ b/dom/xslt/xslt/txDocumentFunctionCall.cpp
@@ -99,34 +99,36 @@ DocumentFunctionCall::evaluate(txIEvalCo
         NS_ENSURE_SUCCESS(rv, rv);
 
         // Make this true, even if nodeSet2 is empty. For relative URLs,
         // we'll fail to load the document with an empty base URI, and for
         // absolute URLs, the base URI doesn't matter
         baseURISet = true;
 
         if (!nodeSet2->isEmpty()) {
-            txXPathNodeUtils::getBaseURI(nodeSet2->get(0), baseURI);
+            rv = txXPathNodeUtils::getBaseURI(nodeSet2->get(0), baseURI);
+            NS_ENSURE_SUCCESS(rv, rv);
         }
     }
 
     if (exprResult1->getResultType() == txAExprResult::NODESET) {
         // The first argument is a NodeSet, iterate on its nodes
         txNodeSet* nodeSet1 = static_cast<txNodeSet*>
                                          (static_cast<txAExprResult*>
                                                      (exprResult1));
         int32_t i;
         for (i = 0; i < nodeSet1->size(); ++i) {
             const txXPathNode& node = nodeSet1->get(i);
             nsAutoString uriStr;
             txXPathNodeUtils::appendNodeValue(node, uriStr);
             if (!baseURISet) {
                 // if the second argument wasn't specified, use
                 // the baseUri of node itself
-                txXPathNodeUtils::getBaseURI(node, baseURI);
+                rv = txXPathNodeUtils::getBaseURI(node, baseURI);
+                NS_ENSURE_SUCCESS(rv, rv);
             }
             retrieveNode(es, uriStr, baseURI, nodeSet);
         }
         
         NS_ADDREF(*aResult = nodeSet);
         
         return NS_OK;
     }
--- a/dom/xslt/xslt/txExecutionState.cpp
+++ b/dom/xslt/xslt/txExecutionState.cpp
@@ -11,37 +11,42 @@
 #include "txRtfHandler.h"
 #include "txXSLTProcessor.h"
 #include "txLog.h"
 #include "txURIUtils.h"
 #include "txXMLParser.h"
 
 const int32_t txExecutionState::kMaxRecursionDepth = 20000;
 
-void
+nsresult
 txLoadedDocumentsHash::init(txXPathNode* aSourceDocument)
 {
     mSourceDocument = aSourceDocument;
 
     nsAutoString baseURI;
-    txXPathNodeUtils::getBaseURI(*mSourceDocument, baseURI);
+    nsresult rv = txXPathNodeUtils::getBaseURI(*mSourceDocument, baseURI);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+        return rv;
+    }
 
     PutEntry(baseURI)->mDocument = mSourceDocument;
+    return NS_OK;
 }
 
 txLoadedDocumentsHash::~txLoadedDocumentsHash()
 {
     if (mSourceDocument) {
         nsAutoString baseURI;
-        txXPathNodeUtils::getBaseURI(*mSourceDocument, baseURI);
-
-        txLoadedDocumentEntry* entry = GetEntry(baseURI);
-        if (entry) {
-            delete entry->mDocument.forget();
-        }
+        nsresult rv = txXPathNodeUtils::getBaseURI(*mSourceDocument, baseURI);
+        if (NS_SUCCEEDED(rv)) {
+	    txLoadedDocumentEntry* entry = GetEntry(baseURI);
+	    if (entry) {
+	        delete entry->mDocument.forget();
+	    }
+       }
     }
 }
 
 txExecutionState::txExecutionState(txStylesheet* aStylesheet,
                                    bool aDisableLoads)
     : mOutputHandler(nullptr),
       mResultHandler(nullptr),
       mStylesheet(aStylesheet),
@@ -111,17 +116,18 @@ txExecutionState::init(const txXPathNode
         createHandlerWith(mStylesheet->getOutputFormat(), &handler);
     NS_ENSURE_SUCCESS(rv, rv);
 
     mOutputHandler = handler;
     mResultHandler = handler;
     mOutputHandler->startDocument();
 
     // Set up loaded-documents-hash
-    mLoadedDocuments.init(txXPathNodeUtils::getOwnerDocument(aNode));
+    rv = mLoadedDocuments.init(txXPathNodeUtils::getOwnerDocument(aNode));
+    NS_ENSURE_SUCCESS(rv, rv);
 
     // Init members
     rv = mKeyHash.init();
     NS_ENSURE_SUCCESS(rv, rv);
     
     mRecycler = new txResultRecycler;
     
     // The actual value here doesn't really matter since noone should use this
--- a/dom/xslt/xslt/txExecutionState.h
+++ b/dom/xslt/xslt/txExecutionState.h
@@ -56,17 +56,17 @@ class txLoadedDocumentsHash : public nsT
 {
 public:
     txLoadedDocumentsHash()
         : nsTHashtable<txLoadedDocumentEntry>(4),
           mSourceDocument(nullptr)
     {
     }
     ~txLoadedDocumentsHash();
-    void init(txXPathNode* aSourceDocument);
+    MOZ_MUST_USE nsresult init(txXPathNode* aSourceDocument);
 
 private:
     friend class txExecutionState;
     txXPathNode* mSourceDocument;
 };
 
 
 class txExecutionState : public txIMatchContext