Bug 1301249 - nsIDocument::GetDocumentURI() should be fallible, r=smaug
authorAndrea Marchesini <amarchesini@mozilla.com>
Tue, 20 Sep 2016 14:03:05 +0200
changeset 314620 aa252c2f85b5919410685eebc6d8c1c34c86e7ea
parent 314619 31ea105ca2d9b3088d91628ae77603c469d525da
child 314621 2daffd62a19b2a060e573bddc3fb0536607faf5e
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
bugs1301249
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 1301249 - nsIDocument::GetDocumentURI() should be fallible, r=smaug
dom/base/nsDocument.cpp
dom/base/nsGlobalWindow.cpp
dom/base/nsIDocument.h
dom/events/DOMEventTargetHelper.cpp
layout/base/RestyleTracker.cpp
layout/style/nsStyleContext.cpp
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -7066,52 +7066,56 @@ nsDocument::GetMozSyntheticDocument(bool
   *aSyntheticDocument = mIsSyntheticDocument;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDocument::GetDocumentURI(nsAString& aDocumentURI)
 {
   nsString temp;
-  nsIDocument::GetDocumentURI(temp);
+  nsresult rv = nsIDocument::GetDocumentURI(temp);
   aDocumentURI = temp;
-  return NS_OK;
-}
-
-void
+  return rv;
+}
+
+nsresult
 nsIDocument::GetDocumentURI(nsString& aDocumentURI) const
 {
   if (mDocumentURI) {
     nsAutoCString uri;
-    // XXX: should handle GetSpec() failure properly. See bug 1301249.
-    Unused << mDocumentURI->GetSpec(uri);
+    nsresult rv = mDocumentURI->GetSpec(uri);
+    NS_ENSURE_SUCCESS(rv, rv);
+
     CopyUTF8toUTF16(uri, aDocumentURI);
   } else {
     aDocumentURI.Truncate();
   }
+
+  return NS_OK;
 }
 
 // Alias of above
 NS_IMETHODIMP
 nsDocument::GetURL(nsAString& aURL)
 {
   return GetDocumentURI(aURL);
 }
 
-void
+nsresult
 nsIDocument::GetURL(nsString& aURL) const
 {
   return GetDocumentURI(aURL);
 }
 
 void
 nsIDocument::GetDocumentURIFromJS(nsString& aDocumentURI, ErrorResult& aRv) const
 {
   if (!mChromeXHRDocURI || !nsContentUtils::IsCallerChrome()) {
-    return GetDocumentURI(aDocumentURI);
+    aRv = GetDocumentURI(aDocumentURI);
+    return;
   }
 
   nsAutoCString uri;
   nsresult res = mChromeXHRDocURI->GetSpec(uri);
   if (NS_FAILED(res)) {
     aRv.Throw(res);
     return;
   }
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -8504,17 +8504,19 @@ nsGlobalWindow::CloseOuter(bool aTrusted
     // the window to be closed directly after this event,
     // so the user can see that there was a blocked popup.
     return;
   }
 
   // Don't allow scripts from content to close non-app or non-neterror
   // windows that were not opened by script.
   nsAutoString url;
-  mDoc->GetURL(url);
+  nsresult rv = mDoc->GetURL(url);
+  NS_ENSURE_SUCCESS_VOID(rv);
+
   if (!mDocShell->GetIsApp() &&
       !StringBeginsWith(url, NS_LITERAL_STRING("about:neterror")) &&
       !mHadOriginalOpener && !aTrustedCaller) {
     bool allowClose = mAllowScriptsToClose ||
       Preferences::GetBool("dom.allow_scripts_to_close_windows", true);
     if (!allowClose) {
       // We're blocking the close operation
       // report localized error msg in JS console
@@ -10476,17 +10478,20 @@ nsGlobalWindow::GetSessionStorage(ErrorR
     if (!canAccess) {
       mSessionStorage = nullptr;
     }
   }
 
   if (!mSessionStorage) {
     nsString documentURI;
     if (mDoc) {
-      mDoc->GetDocumentURI(documentURI);
+      aError = mDoc->GetDocumentURI(documentURI);
+      if (NS_WARN_IF(aError.Failed())) {
+        return nullptr;
+      }
     }
 
     // If the document has the sandboxed origin flag set
     // don't allow access to sessionStorage.
     if (!mDoc) {
       aError.Throw(NS_ERROR_FAILURE);
       return nullptr;
     }
@@ -10556,17 +10561,20 @@ nsGlobalWindow::GetLocalStorage(ErrorRes
       do_GetService("@mozilla.org/dom/localStorage-manager;1", &rv);
     if (NS_FAILED(rv)) {
       aError.Throw(rv);
       return nullptr;
     }
 
     nsString documentURI;
     if (mDoc) {
-      mDoc->GetDocumentURI(documentURI);
+      aError = mDoc->GetDocumentURI(documentURI);
+      if (NS_WARN_IF(aError.Failed())) {
+        return nullptr;
+      }
     }
 
     nsCOMPtr<nsIDOMStorage> storage;
     aError = storageManager->CreateStorage(AsInner(), principal, documentURI,
                                            getter_AddRefs(storage));
     if (aError.Failed()) {
       return nullptr;
     }
--- a/dom/base/nsIDocument.h
+++ b/dom/base/nsIDocument.h
@@ -2461,18 +2461,18 @@ public:
   {
     return GetScopeObject();
   }
   static already_AddRefed<nsIDocument>
     Constructor(const GlobalObject& aGlobal,
                 mozilla::ErrorResult& rv);
   virtual mozilla::dom::DOMImplementation*
     GetImplementation(mozilla::ErrorResult& rv) = 0;
-  void GetURL(nsString& retval) const;
-  void GetDocumentURI(nsString& retval) const;
+  MOZ_MUST_USE nsresult GetURL(nsString& retval) const;
+  MOZ_MUST_USE nsresult GetDocumentURI(nsString& retval) const;
   // Return the 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 GetDocumentURIFromJS(nsString& aDocumentURI,
                             mozilla::ErrorResult& aRv) const;
   void GetCompatMode(nsString& retval) const;
   void GetCharacterSet(nsAString& retval) const;
--- a/dom/events/DOMEventTargetHelper.cpp
+++ b/dom/events/DOMEventTargetHelper.cpp
@@ -24,17 +24,17 @@ NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(DOM
   NS_IMPL_CYCLE_COLLECTION_TRACE_PRESERVED_WRAPPER
 NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INTERNAL(DOMEventTargetHelper)
   if (MOZ_UNLIKELY(cb.WantDebugInfo())) {
     char name[512];
     nsAutoString uri;
     if (tmp->mOwnerWindow && tmp->mOwnerWindow->GetExtantDoc()) {
-      tmp->mOwnerWindow->GetExtantDoc()->GetDocumentURI(uri);
+      Unused << tmp->mOwnerWindow->GetExtantDoc()->GetDocumentURI(uri);
     }
 
     nsXPCOMCycleCollectionParticipant* participant = nullptr;
     CallQueryInterface(tmp, &participant);
 
     SprintfLiteral(name, "%s %s",
                    participant->ClassName(),
                    NS_ConvertUTF16toUTF8(uri).get());
--- a/layout/base/RestyleTracker.cpp
+++ b/layout/base/RestyleTracker.cpp
@@ -22,19 +22,22 @@
 
 namespace mozilla {
 
 #ifdef RESTYLE_LOGGING
 static nsCString
 GetDocumentURI(nsIDocument* aDocument)
 {
   nsCString result;
-  nsString url;
-  aDocument->GetDocumentURI(url);
-  result.Append(NS_ConvertUTF16toUTF8(url).get());
+  nsAutoString url;
+  nsresult rv = aDocument->GetDocumentURI(url);
+  if (NS_SUCCEEDED(rv)) {
+    result.Append(NS_ConvertUTF16toUTF8(url).get());
+  }
+
   return result;
 }
 
 static nsCString
 FrameTagToString(dom::Element* aElement)
 {
   nsCString result;
   nsIFrame* frame = aElement->GetPrimaryFrame();
--- a/layout/style/nsStyleContext.cpp
+++ b/layout/style/nsStyleContext.cpp
@@ -246,18 +246,20 @@ nsStyleContext::AssertStructsNotUsedElse
       aDestroyingContext->mCachedInheritedData;
 #define STYLE_STRUCT_INHERITED(name_, checkdata_cb)                            \
     data = destroyingInheritedData.mStyleStructs[eStyleStruct_##name_];        \
     if (data &&                                                                \
         !(aDestroyingContext->mBits & NS_STYLE_INHERIT_BIT(name_)) &&          \
          (mCachedInheritedData.mStyleStructs[eStyleStruct_##name_] == data)) { \
       printf_stderr("style struct %p found on style context %p\n", data, this);\
       nsString url;                                                            \
-      PresContext()->Document()->GetURL(url);                                  \
-      printf_stderr("  in %s\n", NS_ConvertUTF16toUTF8(url).get());            \
+      nsresult rv = PresContext()->Document()->GetURL(url);                    \
+      if (NS_SUCCEEDED(rv)) {                                                  \
+        printf_stderr("  in %s\n", NS_ConvertUTF16toUTF8(url).get());          \
+      }                                                                        \
       MOZ_ASSERT(false, "destroying " #name_ " style struct still present "    \
                         "in style context tree");                              \
     }
 #define STYLE_STRUCT_RESET(name_, checkdata_cb)
 
 #include "nsStyleStructList.h"
 
 #undef STYLE_STRUCT_INHERITED
@@ -271,18 +273,20 @@ nsStyleContext::AssertStructsNotUsedElse
 #define STYLE_STRUCT_RESET(name_, checkdata_cb)                                \
         data = destroyingResetData->mStyleStructs[eStyleStruct_##name_];       \
         if (data &&                                                            \
             !(aDestroyingContext->mBits & NS_STYLE_INHERIT_BIT(name_)) &&      \
             (mCachedResetData->mStyleStructs[eStyleStruct_##name_] == data)) { \
           printf_stderr("style struct %p found on style context %p\n", data,   \
                         this);                                                 \
           nsString url;                                                        \
-          PresContext()->Document()->GetURL(url);                              \
-          printf_stderr("  in %s\n", NS_ConvertUTF16toUTF8(url).get());        \
+          nsresult rv = PresContext()->Document()->GetURL(url);                \
+          if (NS_SUCCEEDED(rv)) {                                              \
+            printf_stderr("  in %s\n", NS_ConvertUTF16toUTF8(url).get());      \
+          }                                                                    \
           MOZ_ASSERT(false, "destroying " #name_ " style struct still present "\
                             "in style context tree");                          \
         }
 
 #include "nsStyleStructList.h"
 
 #undef STYLE_STRUCT_INHERITED
 #undef STYLE_STRUCT_RESET