Bug 1041646 - Don't assume mStack is non-null in JSStackFrame, since people sometimes operate on them after unlinking. r=khuey, a=sledru
authorBoris Zbarsky <bzbarsky@mit.edu>
Sat, 26 Jul 2014 01:41:18 -0400
changeset 217295 a51052bf0aab34545c2e8b098d6df949f6274671
parent 217294 e5a79eaa15434029b7ffdcabe34498652f2ffab3
child 217296 feec66ee1e93751d238698795cd4e5397e15e6a6
push id515
push userraliiev@mozilla.com
push dateMon, 06 Oct 2014 12:51:51 +0000
treeherdermozilla-release@267c7a481bef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskhuey, sledru
bugs1041646
milestone33.0a2
Bug 1041646 - Don't assume mStack is non-null in JSStackFrame, since people sometimes operate on them after unlinking. r=khuey, a=sledru
dom/base/DOMException.h
dom/bindings/Exceptions.cpp
dom/bindings/test/mochitest.ini
dom/bindings/test/test_bug1041646.html
--- a/dom/base/DOMException.h
+++ b/dom/base/DOMException.h
@@ -66,17 +66,18 @@ public:
   nsISupports* GetParentObject() const { return nullptr; }
 
   void GetMessageMoz(nsString& retval);
 
   uint32_t Result() const;
 
   void GetName(nsString& retval);
 
-  // The XPCOM GetFilename does the right thing.
+  // The XPCOM GetFilename does the right thing.  It might throw, but we want to
+  // return an empty filename in that case anyway, instead of throwing.
 
   uint32_t LineNumber() const;
 
   uint32_t ColumnNumber() const;
 
   already_AddRefed<nsIStackFrame> GetLocation() const;
 
   already_AddRefed<nsISupports> GetInner() const;
--- a/dom/bindings/Exceptions.cpp
+++ b/dom/bindings/Exceptions.cpp
@@ -354,17 +354,19 @@ NS_IMETHODIMP JSStackFrame::GetLanguageN
 {
   aLanguageName.AssignLiteral("JavaScript");
   return NS_OK;
 }
 
 /* readonly attribute AString filename; */
 NS_IMETHODIMP JSStackFrame::GetFilename(nsAString& aFilename)
 {
-  if (!mFilenameInitialized) {
+  // We can get called after unlink; in that case we can't do much
+  // about producing a useful value.
+  if (!mFilenameInitialized && mStack) {
     ThreadsafeAutoJSContext cx;
     JS::Rooted<JSObject*> stack(cx, mStack);
     JS::ExposeObjectToActiveJS(mStack);
     JSAutoCompartment ac(cx, stack);
     JS::Rooted<JS::Value> filenameVal(cx);
     if (!JS_GetProperty(cx, stack, "source", &filenameVal) ||
         !filenameVal.isString()) {
       return NS_ERROR_UNEXPECTED;
@@ -390,17 +392,19 @@ NS_IMETHODIMP StackFrame::GetFilename(ns
   }
 
   return NS_OK;
 }
 
 /* readonly attribute AString name; */
 NS_IMETHODIMP JSStackFrame::GetName(nsAString& aFunction)
 {
-  if (!mFunnameInitialized) {
+  // We can get called after unlink; in that case we can't do much
+  // about producing a useful value.
+  if (!mFunnameInitialized && mStack) {
     ThreadsafeAutoJSContext cx;
     JS::Rooted<JSObject*> stack(cx, mStack);
     JS::ExposeObjectToActiveJS(mStack);
     JSAutoCompartment ac(cx, stack);
     JS::Rooted<JS::Value> nameVal(cx);
     // functionDisplayName can be null
     if (!JS_GetProperty(cx, stack, "functionDisplayName", &nameVal) ||
         (!nameVal.isString() && !nameVal.isNull())) {
@@ -430,17 +434,19 @@ NS_IMETHODIMP StackFrame::GetName(nsAStr
 
   return NS_OK;
 }
 
 // virtual
 nsresult
 JSStackFrame::GetLineno(int32_t* aLineNo)
 {
-  if (!mLinenoInitialized) {
+  // We can get called after unlink; in that case we can't do much
+  // about producing a useful value.
+  if (!mLinenoInitialized && mStack) {
     ThreadsafeAutoJSContext cx;
     JS::Rooted<JSObject*> stack(cx, mStack);
     JS::ExposeObjectToActiveJS(mStack);
     JSAutoCompartment ac(cx, stack);
     JS::Rooted<JS::Value> lineVal(cx);
     if (!JS_GetProperty(cx, stack, "line", &lineVal) ||
         !lineVal.isNumber()) {
       return NS_ERROR_UNEXPECTED;
@@ -463,17 +469,19 @@ NS_IMETHODIMP StackFrame::GetSourceLine(
 {
   aSourceLine.Truncate();
   return NS_OK;
 }
 
 /* readonly attribute nsIStackFrame caller; */
 NS_IMETHODIMP JSStackFrame::GetCaller(nsIStackFrame** aCaller)
 {
-  if (!mCallerInitialized) {
+  // We can get called after unlink; in that case we can't do much
+  // about producing a useful value.
+  if (!mCallerInitialized && mStack) {
     ThreadsafeAutoJSContext cx;
     JS::Rooted<JSObject*> stack(cx, mStack);
     JS::ExposeObjectToActiveJS(mStack);
     JSAutoCompartment ac(cx, stack);
     JS::Rooted<JS::Value> callerVal(cx);
     if (!JS_GetProperty(cx, stack, "parent", &callerVal) ||
         !callerVal.isObjectOrNull()) {
       return NS_ERROR_UNEXPECTED;
@@ -496,17 +504,19 @@ NS_IMETHODIMP JSStackFrame::GetCaller(ns
 NS_IMETHODIMP StackFrame::GetCaller(nsIStackFrame** aCaller)
 {
   NS_IF_ADDREF(*aCaller = mCaller);
   return NS_OK;
 }
 
 NS_IMETHODIMP JSStackFrame::GetFormattedStack(nsAString& aStack)
 {
-  if (!mFormattedStackInitialized) {
+  // We can get called after unlink; in that case we can't do much
+  // about producing a useful value.
+  if (!mFormattedStackInitialized && mStack) {
     ThreadsafeAutoJSContext cx;
     JS::Rooted<JS::Value> stack(cx, JS::ObjectValue(*mStack));
     JS::ExposeObjectToActiveJS(mStack);
     JSAutoCompartment ac(cx, mStack);
     JS::Rooted<JSString*> formattedStack(cx, JS::ToString(cx, stack));
     if (!formattedStack) {
       return NS_ERROR_UNEXPECTED;
     }
--- a/dom/bindings/test/mochitest.ini
+++ b/dom/bindings/test/mochitest.ini
@@ -15,16 +15,17 @@ support-files =
 [test_bug773326.html]
 [test_bug788369.html]
 [test_bug852846.html]
 [test_bug862092.html]
 # When bug 923904 lands, this test can be turned on, but only for debug builds
 # where we have our test component. So this should become skip-if = debug == false.
 [test_bug923904.html]
 skip-if = true
+[test_bug1041646.html]
 [test_barewordGetsWindow.html]
 [test_callback_default_thisval.html]
 [test_cloneAndImportNode.html]
 [test_defineProperty.html]
 [test_enums.html]
 [test_exceptionThrowing.html]
 [test_exception_messages.html]
 [test_forOf.html]
new file mode 100644
--- /dev/null
+++ b/dom/bindings/test/test_bug1041646.html
@@ -0,0 +1,49 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1041646
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 1041646</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+  <script type="application/javascript">
+
+  /** Test for Bug 1041646 **/
+  // We need to reject the promise with a DOMException, so make sure we have
+  // something that produces one.
+  function throwException() {
+    document.createTextNode("").appendChild(document);
+  }
+  try {
+    throwException();
+  } catch (e) {
+    ok(e instanceof DOMException, "This test won't test what it should be testing");
+  }
+
+  SimpleTest.waitForExplicitFinish();
+
+  // We want a new DOMException each time here.
+  for (var i = 0; i < 100; ++i) {
+    new Promise(throwException);
+  }
+
+  // Now make sure we wait for all those promises above to reject themselves
+  Promise.resolve(1).then(function() {
+    SpecialPowers.gc(); // This should not assert or crash
+    SimpleTest.finish();
+  });
+
+  </script>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1041646">Mozilla Bug 1041646</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+
+</div>
+<pre id="test">
+</pre>
+</body>
+</html>