Bug 842495 - Cleanup contentDocument/contentWindow getters; r=mounir

content/html/content/src/HTMLFrameElement.cpp

@@ -116,33 +116,6 @@ HTMLFrameElement::GetAttributeMappingFunction() const
   return &MapAttributesIntoRule;
 }
 
-already_AddRefed<nsIDocument>
-HTMLFrameElement::GetContentDocument(ErrorResult& aRv)
-{
-  nsCOMPtr<nsIDOMDocument> doc;
-  nsresult rv = nsGenericHTMLFrameElement::GetContentDocument(getter_AddRefs(doc));
-  if (NS_FAILED(rv)) {
-    aRv.Throw(rv);
-    return nullptr;
-  }
-
-  nsCOMPtr<nsIDocument> ret = do_QueryInterface(doc);
-  return ret.forget();
-}
-
-already_AddRefed<nsIDOMWindow>
-HTMLFrameElement::GetContentWindow(ErrorResult& aRv)
-{
-  nsCOMPtr<nsIDOMWindow> win;
-  nsresult rv = nsGenericHTMLFrameElement::GetContentWindow(getter_AddRefs(win));
-  if (NS_FAILED(rv)) {
-    aRv.Throw(rv);
-    return nullptr;
-  }
-
-  return win.forget();
-}
-
 JSObject*
 HTMLFrameElement::WrapNode(JSContext* aCx, JSObject* aScope,
                            bool* aTriedToWrap)

content/html/content/src/HTMLFrameElement.h

@@ -102,9 +102,8 @@ public:
     SetAttrHelper(nsGkAtoms::src, aSrc);
   }
 
-  already_AddRefed<nsIDocument> GetContentDocument(ErrorResult& aRv);
+  using nsGenericHTMLFrameElement::GetContentDocument;
-
+  using nsGenericHTMLFrameElement::GetContentWindow;
-  already_AddRefed<nsIDOMWindow> GetContentWindow(ErrorResult& aRv);
 
 protected:
   virtual JSObject* WrapNode(JSContext* aCx, JSObject* aScope,

content/html/content/src/nsGenericHTMLFrameElement.cpp

@@ -51,69 +51,71 @@ nsresult
 nsGenericHTMLFrameElement::GetContentDocument(nsIDOMDocument** aContentDocument)
 {
   NS_PRECONDITION(aContentDocument, "Null out param");
-  *aContentDocument = nullptr;
+  nsIDocument* document = GetContentDocument();
+  nsIDOMDocument* domDocument =
+    static_cast<nsIDOMDocument*>(document->AsDOMNode());
+  NS_ADDREF(*aContentDocument = domDocument);
+  return NS_OK;
+}
 
-  nsCOMPtr<nsIDOMWindow> win;
+nsIDocument*
-  GetContentWindow(getter_AddRefs(win));
+nsGenericHTMLFrameElement::GetContentDocument()
-
+{
-  if (!win) {
+  nsCOMPtr<nsPIDOMWindow> win = GetContentWindow();
-    return NS_OK;
+  return win ? win->GetDoc() : nullptr;
-  }
-
-  return win->GetDocument(aContentDocument);
 }
 
 nsresult
 nsGenericHTMLFrameElement::GetContentWindow(nsIDOMWindow** aContentWindow)
 {
   NS_PRECONDITION(aContentWindow, "Null out param");
-  *aContentWindow = nullptr;
+  nsCOMPtr<nsPIDOMWindow> window = GetContentWindow();
+  window.forget(aContentWindow);
+  return NS_OK;
+}
 
-  nsresult rv = EnsureFrameLoader();
+already_AddRefed<nsPIDOMWindow>
-  NS_ENSURE_SUCCESS(rv, rv);
+nsGenericHTMLFrameElement::GetContentWindow()
+{
+  EnsureFrameLoader();
 
   if (!mFrameLoader) {
-    return NS_OK;
+    return nullptr;
   }
 
   bool depthTooGreat = false;
   mFrameLoader->GetDepthTooGreat(&depthTooGreat);
   if (depthTooGreat) {
     // Claim to have no contentWindow
-    return NS_OK;
+    return nullptr;
   }
 
   nsCOMPtr<nsIDocShell> doc_shell;
   mFrameLoader->GetDocShell(getter_AddRefs(doc_shell));
 
-  nsCOMPtr<nsPIDOMWindow> win(do_GetInterface(doc_shell));
+  nsCOMPtr<nsPIDOMWindow> win = do_GetInterface(doc_shell);
 
   if (!win) {
-    return NS_OK;
+    return nullptr;
   }
 
   NS_ASSERTION(win->IsOuterWindow(),
                "Uh, this window should always be an outer window!");
 
-  return CallQueryInterface(win, aContentWindow);
+  return win.forget();
 }
 
-nsresult
+void
 nsGenericHTMLFrameElement::EnsureFrameLoader()
 {
   if (!GetParent() || !IsInDoc() || mFrameLoader || mFrameLoaderCreationDisallowed) {
     // If frame loader is there, we just keep it around, cached
-    return NS_OK;
+    return;
   }
 
+  // Strangely enough, this method doesn't actually ensure that the
+  // frameloader exists.  It's more of a best-effort kind of thing.
   mFrameLoader = nsFrameLoader::Create(this, mNetworkCreated);
-  if (!mFrameLoader) {
-    // Strangely enough, this method doesn't actually ensure that the
-    // frameloader exists.  It's more of a best-effort kind of thing.
-    return NS_OK;
-  }
-
-  return NS_OK;
 }
 
 nsresult
@@ -150,14 +152,13 @@ nsGenericHTMLFrameElement::SwapFrameLoaders(nsIFrameLoaderOwner* aOtherOwner)
 nsresult
 nsGenericHTMLFrameElement::LoadSrc()
 {
-  nsresult rv = EnsureFrameLoader();
+  EnsureFrameLoader();
-  NS_ENSURE_SUCCESS(rv, rv);
 
   if (!mFrameLoader) {
     return NS_OK;
   }
 
-  rv = mFrameLoader->LoadFrame();
+  nsresult rv = mFrameLoader->LoadFrame();
 #ifdef DEBUG
   if (NS_FAILED(rv)) {
     NS_WARNING("failed to load URL");

content/html/content/src/nsGenericHTMLFrameElement.h

@@ -90,9 +90,11 @@ protected:
 
   // This doesn't really ensure a frame loade in all cases, only when
   // it makes sense.
-  nsresult EnsureFrameLoader();
+  void EnsureFrameLoader();
   nsresult LoadSrc();
+  nsIDocument* GetContentDocument();
   nsresult GetContentDocument(nsIDOMDocument** aContentDocument);
+  already_AddRefed<nsPIDOMWindow> GetContentWindow();
   nsresult GetContentWindow(nsIDOMWindow** aContentWindow);
 
   nsRefPtr<nsFrameLoader> mFrameLoader;

dom/webidl/HTMLFrameElement.webidl

@@ -24,9 +24,7 @@ interface HTMLFrameElement : HTMLElement {
            attribute DOMString longDesc;
            [SetterThrows]
            attribute boolean noResize;
-  [GetterThrows]
   readonly attribute Document? contentDocument;
-  [GetterThrows]
   readonly attribute WindowProxy? contentWindow;
 
   [TreatNullAs=EmptyString, SetterThrows] attribute DOMString marginHeight;