Add a few missing null-checks. Return error code from GetRuleLine() when it fails. b=462787 r+sr=dbaron
authorMats Palmgren <mats.palmgren@bredband.net>
Tue, 16 Dec 2008 12:26:42 +0100
changeset 22837 cdf705dea843cebbc98d6baf5ace1135260eeb8a
parent 22836 dc44b253cbf7c6884e517bc05434d65b31d23c66
child 22838 2151c732fb5c6220edaee6cf07f80085ab91d77a
push idunknown
push userunknown
push dateunknown
bugs462787
milestone1.9.2a1pre
Add a few missing null-checks. Return error code from GetRuleLine() when it fails. b=462787 r+sr=dbaron
layout/Makefile.in
layout/inspector/src/inDOMUtils.cpp
layout/inspector/tests/Makefile.in
layout/inspector/tests/test_bug462787.html
--- a/layout/Makefile.in
+++ b/layout/Makefile.in
@@ -66,16 +66,19 @@ PARALLEL_DIRS += \
 endif
 
 ifdef MOZ_SVG
 PARALLEL_DIRS += svg/base/src
 endif
 
 ifndef MOZ_NO_INSPECTOR_APIS
 PARALLEL_DIRS += inspector/public inspector/src
+ifdef ENABLE_TESTS
+PARALLEL_DIRS += inspector/tests
+endif
 endif
 
 DIRS           += build
 
 ifdef ENABLE_TESTS
 PARALLEL_DIRS += \
   xul/test \
   xul/base/test \
--- a/layout/inspector/src/inDOMUtils.cpp
+++ b/layout/inspector/src/inDOMUtils.cpp
@@ -68,19 +68,20 @@ NS_IMPL_ISUPPORTS1(inDOMUtils, inIDOMUti
 
 ///////////////////////////////////////////////////////////////////////////////
 // inIDOMUtils
 
 NS_IMETHODIMP
 inDOMUtils::IsIgnorableWhitespace(nsIDOMCharacterData *aDataNode,
                                   PRBool *aReturn)
 {
-  NS_PRECONDITION(aDataNode, "Must have a character data node");
   NS_PRECONDITION(aReturn, "Must have an out parameter");
 
+  NS_ENSURE_ARG_POINTER(aDataNode);
+
   *aReturn = PR_FALSE;
 
   nsCOMPtr<nsIContent> content = do_QueryInterface(aDataNode);
   NS_ASSERTION(content, "Does not implement nsIContent!");
 
   if (!content->TextIsOnlyWhitespace()) {
     return NS_OK;
   }
@@ -114,17 +115,19 @@ inDOMUtils::IsIgnorableWhitespace(nsIDOM
   return NS_OK;
 }
 
 NS_IMETHODIMP
 inDOMUtils::GetParentForNode(nsIDOMNode* aNode,
                              PRBool aShowingAnonymousContent,
                              nsIDOMNode** aParent)
 {
-    // First do the special cases -- document nodes and anonymous content
+  NS_ENSURE_ARG_POINTER(aNode);
+
+  // First do the special cases -- document nodes and anonymous content
   nsCOMPtr<nsIDOMDocument> doc(do_QueryInterface(aNode));
   nsCOMPtr<nsIDOMNode> parent;
 
   if (doc) {
     parent = inLayoutUtils::GetContainerFor(doc);
   } else if (aShowingAnonymousContent) {
     nsCOMPtr<nsIContent> content = do_QueryInterface(aNode);
     if (content) {
@@ -146,17 +149,17 @@ inDOMUtils::GetParentForNode(nsIDOMNode*
   NS_IF_ADDREF(*aParent = parent);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 inDOMUtils::GetCSSStyleRules(nsIDOMElement *aElement,
                              nsISupportsArray **_retval)
 {
-  if (!aElement) return NS_ERROR_NULL_POINTER;
+  NS_ENSURE_ARG_POINTER(aElement);
 
   *_retval = nsnull;
 
   nsRuleNode* ruleNode = nsnull;
   nsCOMPtr<nsIContent> content = do_QueryInterface(aElement);
   mCSSUtils->GetRuleNodeForContent(content, &ruleNode);
   if (!ruleNode) {
     // This can fail for content nodes that are not in the document or
@@ -189,37 +192,39 @@ inDOMUtils::GetCSSStyleRules(nsIDOMEleme
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 inDOMUtils::GetRuleLine(nsIDOMCSSStyleRule *aRule, PRUint32 *_retval)
 {
   *_retval = 0;
-  if (!aRule)
-    return NS_OK;
+
+  NS_ENSURE_ARG_POINTER(aRule);
+
   nsCOMPtr<nsICSSStyleRuleDOMWrapper> rule = do_QueryInterface(aRule);
   nsCOMPtr<nsICSSStyleRule> cssrule;
-  rule->GetCSSStyleRule(getter_AddRefs(cssrule));
-  if (cssrule)
-    *_retval = cssrule->GetLineNumber();
+  nsresult rv = rule->GetCSSStyleRule(getter_AddRefs(cssrule));
+  NS_ENSURE_SUCCESS(rv, rv);
+  NS_ENSURE_TRUE(cssrule != nsnull, NS_ERROR_FAILURE);
+  *_retval = cssrule->GetLineNumber();
   return NS_OK;
 }
 
 NS_IMETHODIMP 
 inDOMUtils::GetBindingURLs(nsIDOMElement *aElement, nsIArray **_retval)
 {
+  NS_ENSURE_ARG_POINTER(aElement);
   return mCSSUtils->GetBindingURLs(aElement, _retval);
 }
 
 NS_IMETHODIMP
 inDOMUtils::SetContentState(nsIDOMElement *aElement, PRInt32 aState)
 {
-  if (!aElement)
-    return NS_ERROR_NULL_POINTER;
+  NS_ENSURE_ARG_POINTER(aElement);
   
   nsCOMPtr<nsIEventStateManager> esm = inLayoutUtils::GetEventStateManagerFor(aElement);
   if (esm) {
     nsCOMPtr<nsIContent> content;
     content = do_QueryInterface(aElement);
   
     return esm->SetContentState(content, aState);
   }
@@ -227,18 +232,17 @@ inDOMUtils::SetContentState(nsIDOMElemen
   return NS_ERROR_FAILURE;
 }
 
 NS_IMETHODIMP
 inDOMUtils::GetContentState(nsIDOMElement *aElement, PRInt32* aState)
 {
   *aState = 0;
 
-  if (!aElement)
-    return NS_ERROR_NULL_POINTER;
+  NS_ENSURE_ARG_POINTER(aElement);
 
   nsCOMPtr<nsIEventStateManager> esm = inLayoutUtils::GetEventStateManagerFor(aElement);
   if (esm) {
     nsCOMPtr<nsIContent> content;
     content = do_QueryInterface(aElement);
   
     return esm->GetContentState(content, *aState);
   }
new file mode 100644
--- /dev/null
+++ b/layout/inspector/tests/Makefile.in
@@ -0,0 +1,52 @@
+#
+# ***** BEGIN LICENSE BLOCK *****
+# Version: MPL 1.1/GPL 2.0/LGPL 2.1
+#
+# The contents of this file are subject to the Mozilla Public License Version
+# 1.1 (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+# http://www.mozilla.org/MPL/
+#
+# Software distributed under the License is distributed on an "AS IS" basis,
+# WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+# for the specific language governing rights and limitations under the
+# License.
+#
+# The Original Code is mozilla.org code.
+#
+# The Initial Developer of the Original Code is
+# Mozilla Foundation.
+# Portions created by the Initial Developer are Copyright (C) 2008
+# the Initial Developer. All Rights Reserved.
+#
+# Contributor(s):
+#
+# Alternatively, the contents of this file may be used under the terms of
+# either of the GNU General Public License Version 2 or later (the "GPL"),
+# or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+# in which case the provisions of the GPL or the LGPL are applicable instead
+# of those above. If you wish to allow use of your version of this file only
+# under the terms of either the GPL or the LGPL, and not to allow others to
+# use your version of this file under the terms of the MPL, indicate your
+# decision by deleting the provisions above and replace them with the notice
+# and other provisions required by the GPL or the LGPL. If you do not delete
+# the provisions above, a recipient may use your version of this file under
+# the terms of any one of the MPL, the GPL or the LGPL.
+#
+# ***** END LICENSE BLOCK *****
+
+DEPTH		= ../../..
+topsrcdir	= @top_srcdir@
+srcdir		= @srcdir@
+VPATH		= @srcdir@
+relativesrcdir	= layout/inspector/tests
+
+include $(DEPTH)/config/autoconf.mk
+include $(topsrcdir)/config/rules.mk
+
+_TEST_FILES =\
+		test_bug462787.html \
+		$(NULL)
+
+libs:: $(_TEST_FILES)
+	$(INSTALL) $^ $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
new file mode 100644
--- /dev/null
+++ b/layout/inspector/tests/test_bug462787.html
@@ -0,0 +1,81 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=462787
+-->
+<head>
+  <title>Test for Bug 462787</title>
+  <script type="application/javascript" src="/MochiKit/MochiKit.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=462787">Mozilla Bug 462787</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 462787 **/
+
+function do_test() {
+  const INVALID_POINTER = 0x80004003;
+  
+  netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
+  var utils = Components.classes["@mozilla.org/inspector/dom-utils;1"]
+                             .getService(Components.interfaces.inIDOMUtils);
+  try {
+    utils.getCSSStyleRules(null); 
+    ok(false, "expected an exception"); 
+  }
+  catch(e) { is(e.result, INVALID_POINTER, "got the expected exception"); }
+
+  try {
+    utils.getRuleLine(null); 
+    ok(false, "expected an exception"); 
+  }
+  catch(e) { is(e.result, INVALID_POINTER, "got the expected exception"); }
+
+  try {
+    utils.isIgnorableWhitespace(null);
+    ok(false, "expected an exception");
+  }
+  catch(e) { is(e.result, INVALID_POINTER, "got the expected exception"); }
+
+  try {
+    utils.getParentForNode(null, true); 
+    ok(false, "expected an exception");
+  }
+  catch(e) { is(e.result, INVALID_POINTER, "got the expected exception"); }
+
+  try {
+    utils.getBindingURLs(null); 
+    ok(false, "expected an exception"); 
+  }
+  catch(e) { is(e.result, INVALID_POINTER, "got the expected exception"); }
+
+  try {
+    utils.getContentState(null); 
+    ok(false, "expected an exception"); 
+  }
+  catch(e) { is(e.result, INVALID_POINTER, "got the expected exception"); }
+
+  try {
+    utils.setContentState(null, false); 
+    ok(false, "expected an exception"); 
+  }
+  catch(e) { is(e.result, INVALID_POINTER, "got the expected exception"); }
+
+  SimpleTest.finish();
+}
+
+SimpleTest.waitForExplicitFinish();
+addLoadEvent(do_test);
+
+
+</script>
+</pre>
+</body>
+</html>