Bug 978522 - Console must use LinkedList instead a nsTArray, r=smaug
authorAndrea Marchesini <amarchesini@mozilla.com>
Mon, 03 Mar 2014 00:20:34 +0000
changeset 171745 f05e05787c07793b591f8ca26154ac54fd4ad11e
parent 171744 ac3cee816b0be927e56e8630d913ae2435337fbc
child 171746 f57a742a12739158a1493c6b58c624ee729b2002
push id270
push userpvanderbeken@mozilla.com
push dateThu, 06 Mar 2014 09:24:21 +0000
reviewerssmaug
bugs978522
milestone30.0a1
Bug 978522 - Console must use LinkedList instead a nsTArray, r=smaug
dom/base/Console.cpp
dom/base/Console.h
dom/base/test/mochitest.ini
dom/base/test/test_bug978522.html
--- a/dom/base/Console.cpp
+++ b/dom/base/Console.cpp
@@ -127,25 +127,31 @@ ConsoleStructuredCloneCallbacksError(JSC
 }
 
 JSStructuredCloneCallbacks gConsoleCallbacks = {
   ConsoleStructuredCloneCallbacksRead,
   ConsoleStructuredCloneCallbacksWrite,
   ConsoleStructuredCloneCallbacksError
 };
 
-class ConsoleCallData
+class ConsoleCallData MOZ_FINAL : public LinkedListElement<ConsoleCallData>
 {
 public:
   ConsoleCallData()
     : mMethodName(Console::MethodLog)
     , mPrivate(false)
     , mTimeStamp(JS_Now())
     , mMonotonicTimer(0)
   {
+    MOZ_COUNT_CTOR(ConsoleCallData);
+  }
+
+  ~ConsoleCallData()
+  {
+    MOZ_COUNT_DTOR(ConsoleCallData);
   }
 
   void
   Initialize(JSContext* aCx, Console::MethodName aName,
              const nsAString& aString, const Sequence<JS::Value>& aArguments)
   {
     mGlobal = JS::CurrentGlobalOrNull(aCx);
     mMethodName = aName;
@@ -254,49 +260,49 @@ private:
   nsCOMPtr<nsIEventTarget> mSyncLoopTarget;
 };
 
 // This runnable appends a CallData object into the Console queue running on
 // the main-thread.
 class ConsoleCallDataRunnable MOZ_FINAL : public ConsoleRunnable
 {
 public:
-  ConsoleCallDataRunnable(const ConsoleCallData& aCallData)
+  ConsoleCallDataRunnable(ConsoleCallData* aCallData)
     : mCallData(aCallData)
   {
   }
 
 private:
   bool
   PreDispatch(JSContext* aCx) MOZ_OVERRIDE
   {
     ClearException ce(aCx);
-    JSAutoCompartment ac(aCx, mCallData.mGlobal);
+    JSAutoCompartment ac(aCx, mCallData->mGlobal);
 
     JS::Rooted<JSObject*> arguments(aCx,
-      JS_NewArrayObject(aCx, mCallData.mArguments.Length()));
+      JS_NewArrayObject(aCx, mCallData->mArguments.Length()));
     if (!arguments) {
       return false;
     }
 
-    for (uint32_t i = 0; i < mCallData.mArguments.Length(); ++i) {
-      if (!JS_DefineElement(aCx, arguments, i, mCallData.mArguments[i],
+    for (uint32_t i = 0; i < mCallData->mArguments.Length(); ++i) {
+      if (!JS_DefineElement(aCx, arguments, i, mCallData->mArguments[i],
                             nullptr, nullptr, JSPROP_ENUMERATE)) {
         return false;
       }
     }
 
     JS::Rooted<JS::Value> value(aCx, JS::ObjectValue(*arguments));
 
     if (!mArguments.write(aCx, value, &gConsoleCallbacks, &mStrings)) {
       return false;
     }
 
-    mCallData.mArguments.Clear();
-    mCallData.mGlobal = nullptr;
+    mCallData->mArguments.Clear();
+    mCallData->mGlobal = nullptr;
     return true;
   }
 
   void
   RunConsole() MOZ_OVERRIDE
   {
     // Walk up to our containing page
     WorkerPrivate* wp = mWorkerPrivate;
@@ -336,27 +342,27 @@ private:
 
     for (uint32_t i = 0; i < length; ++i) {
       JS::Rooted<JS::Value> value(cx);
 
       if (!JS_GetElement(cx, argumentsObj, i, &value)) {
         return;
       }
 
-      mCallData.mArguments.AppendElement(value);
+      mCallData->mArguments.AppendElement(value);
     }
 
-    MOZ_ASSERT(mCallData.mArguments.Length() == length);
+    MOZ_ASSERT(mCallData->mArguments.Length() == length);
 
-    mCallData.mGlobal = JS::CurrentGlobalOrNull(cx);
-    console->AppendCallData(mCallData);
+    mCallData->mGlobal = JS::CurrentGlobalOrNull(cx);
+    console->AppendCallData(mCallData.forget());
   }
 
 private:
-  ConsoleCallData mCallData;
+  nsAutoPtr<ConsoleCallData> mCallData;
 
   JSAutoStructuredCloneBuffer mArguments;
   nsTArray<nsString> mStrings;
 };
 
 // This runnable calls ProfileMethod() on the console on the main-thread.
 class ConsoleProfileRunnable MOZ_FINAL : public ConsoleRunnable
 {
@@ -475,35 +481,40 @@ private:
 NS_IMPL_CYCLE_COLLECTION_CLASS(Console)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(Console)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mWindow)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mTimer)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mStorage)
   NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER
 
-  tmp->mQueuedCalls.Clear();
+  tmp->ClearConsoleData();
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(Console)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mWindow)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mTimer)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mStorage)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_SCRIPT_OBJECTS
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(Console)
   NS_IMPL_CYCLE_COLLECTION_TRACE_PRESERVED_WRAPPER
 
-  for (uint32_t i = 0; i < tmp->mQueuedCalls.Length(); ++i) {
-    NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mQueuedCalls[i].mGlobal);
+  for (ConsoleCallData* data = tmp->mQueuedCalls.getFirst(); data != nullptr;
+       data = data->getNext()) {
+    if (data->mGlobal) {
+      aCallbacks.Trace(&data->mGlobal, "data->mGlobal", aClosure);
+    }
 
-    for (uint32_t j = 0; j < tmp->mQueuedCalls[i].mArguments.Length(); ++j) {
-      NS_IMPL_CYCLE_COLLECTION_TRACE_JSVAL_MEMBER_CALLBACK(mQueuedCalls[i].mArguments[j]);
+    for (uint32_t i = 0; i < data->mArguments.Length(); ++i) {
+      if (JSVAL_IS_TRACEABLE(data->mArguments[i])) {
+        aCallbacks.Trace(&data->mArguments[i], "data->mArguments[i]", aClosure);
+      }
     }
   }
 
 NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(Console)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(Console)
 
@@ -563,17 +574,17 @@ Console::Observe(nsISupports* aSubject, 
 
   if (innerID == mInnerID) {
     nsCOMPtr<nsIObserverService> obs =
       do_GetService("@mozilla.org/observer-service;1");
     if (obs) {
       obs->RemoveObserver(this, "inner-window-destroyed");
     }
 
-    mQueuedCalls.Clear();
+    ClearConsoleData();
     mTimerRegistry.Clear();
 
     if (mTimer) {
       mTimer->Cancel();
       mTimer = nullptr;
     }
   }
 
@@ -730,55 +741,59 @@ void
 Console::Method(JSContext* aCx, MethodName aMethodName,
                 const nsAString& aMethodString,
                 const Sequence<JS::Value>& aData)
 {
   // This RAII class removes the last element of the mQueuedCalls if something
   // goes wrong.
   class RAII {
   public:
-    RAII(nsTArray<ConsoleCallData>& aArray)
-      : mArray(aArray)
+    RAII(LinkedList<ConsoleCallData>& aList)
+      : mList(aList)
       , mUnfinished(true)
     {
     }
 
     ~RAII()
     {
       if (mUnfinished) {
-        mArray.RemoveElementAt(mArray.Length() - 1);
+        ConsoleCallData* data = mList.popLast();
+        MOZ_ASSERT(data);
+        delete data;
       }
     }
 
     void
     Finished()
     {
       mUnfinished = false;
     }
 
   private:
-    nsTArray<ConsoleCallData>& mArray;
+    LinkedList<ConsoleCallData>& mList;
     bool mUnfinished;
   };
 
-  ConsoleCallData& callData = *mQueuedCalls.AppendElement();
-  callData.Initialize(aCx, aMethodName, aMethodString, aData);
+  ConsoleCallData* callData = new ConsoleCallData();
+  mQueuedCalls.insertBack(callData);
+
+  callData->Initialize(aCx, aMethodName, aMethodString, aData);
   RAII raii(mQueuedCalls);
 
   if (mWindow) {
     nsCOMPtr<nsIWebNavigation> webNav = do_GetInterface(mWindow);
     if (!webNav) {
       Throw(aCx, NS_ERROR_FAILURE);
       return;
     }
 
     nsCOMPtr<nsILoadContext> loadContext = do_QueryInterface(webNav);
     MOZ_ASSERT(loadContext);
 
-    loadContext->GetUsePrivateBrowsing(&callData.mPrivate);
+    loadContext->GetUsePrivateBrowsing(&callData->mPrivate);
   }
 
   uint32_t maxDepth = aMethodName == MethodTrace ?
                        DEFAULT_MAX_STACKTRACE_DEPTH : 1;
   nsCOMPtr<nsIStackFrame> stack = CreateStack(aCx, maxDepth);
 
   if (!stack) {
     Throw(aCx, NS_ERROR_FAILURE);
@@ -792,17 +807,17 @@ Console::Method(JSContext* aCx, MethodNa
     nsresult rv = stack->GetLanguage(&language);
     if (NS_FAILED(rv)) {
       Throw(aCx, rv);
       return;
     }
 
     if (language == nsIProgrammingLanguage::JAVASCRIPT ||
         language == nsIProgrammingLanguage::JAVASCRIPT2) {
-      ConsoleStackEntry& data = *callData.mStack.AppendElement();
+      ConsoleStackEntry& data = *callData->mStack.AppendElement();
 
       nsCString string;
       rv = stack->GetFilename(string);
       if (NS_FAILED(rv)) {
         Throw(aCx, rv);
         return;
       }
 
@@ -845,141 +860,149 @@ Console::Method(JSContext* aCx, MethodNa
 
     ErrorResult rv;
     nsRefPtr<nsPerformance> performance = win->GetPerformance(rv);
     if (rv.Failed()) {
       Throw(aCx, rv.ErrorCode());
       return;
     }
 
-    callData.mMonotonicTimer = performance->Now();
+    callData->mMonotonicTimer = performance->Now();
   }
 
+  // The operation is completed. RAII class has to be disabled.
+  raii.Finished();
+
   if (!NS_IsMainThread()) {
-    // Here we are in a worker thread.
+    // Here we are in a worker thread. The ConsoleCallData has to been removed
+    // from the list and it will be deleted by the ConsoleCallDataRunnable or
+    // by the Main-Thread Console object.
+    mQueuedCalls.popLast();
+
     nsRefPtr<ConsoleCallDataRunnable> runnable =
       new ConsoleCallDataRunnable(callData);
     runnable->Dispatch();
     return;
   }
 
-  // The operation is completed. RAII class has to be disabled.
-  raii.Finished();
-
   if (!mTimer) {
     mTimer = do_CreateInstance("@mozilla.org/timer;1");
     mTimer->InitWithCallback(this, CALL_DELAY,
                              nsITimer::TYPE_REPEATING_SLACK);
   }
 }
 
 void
-Console::AppendCallData(const ConsoleCallData& aCallData)
+Console::AppendCallData(ConsoleCallData* aCallData)
 {
-  mQueuedCalls.AppendElement(aCallData);
+  mQueuedCalls.insertBack(aCallData);
 
   if (!mTimer) {
     mTimer = do_CreateInstance("@mozilla.org/timer;1");
     mTimer->InitWithCallback(this, CALL_DELAY,
                              nsITimer::TYPE_REPEATING_SLACK);
   }
 }
 
 // Timer callback used to process each of the queued calls.
 NS_IMETHODIMP
 Console::Notify(nsITimer *timer)
 {
-  MOZ_ASSERT(!mQueuedCalls.IsEmpty());
+  MOZ_ASSERT(!mQueuedCalls.isEmpty());
 
-  uint32_t i = 0;
-  for (; i < MESSAGES_IN_INTERVAL && i < mQueuedCalls.Length();
-       ++i) {
-    ProcessCallData(mQueuedCalls[i]);
+  for (uint32_t i = 0; i < MESSAGES_IN_INTERVAL; ++i) {
+    ConsoleCallData* data = mQueuedCalls.popFirst();
+    if (!data) {
+      break;
+    }
+
+    ProcessCallData(data);
+    delete data;
   }
 
-  mQueuedCalls.RemoveElementsAt(0, i);
-
-  if (mQueuedCalls.IsEmpty()) {
+  if (mQueuedCalls.isEmpty()) {
     mTimer->Cancel();
     mTimer = nullptr;
   }
 
   return NS_OK;
 }
 
 void
-Console::ProcessCallData(ConsoleCallData& aData)
+Console::ProcessCallData(ConsoleCallData* aData)
 {
+  MOZ_ASSERT(aData);
+
   ConsoleStackEntry frame;
-  if (!aData.mStack.IsEmpty()) {
-    frame = aData.mStack[0];
+  if (!aData->mStack.IsEmpty()) {
+    frame = aData->mStack[0];
   }
 
   AutoSafeJSContext cx;
   ClearException ce(cx);
   RootedDictionary<ConsoleEvent> event(cx);
 
-  JSAutoCompartment ac(cx, aData.mGlobal);
+  JSAutoCompartment ac(cx, aData->mGlobal);
 
   event.mID.Construct();
   event.mInnerID.Construct();
   if (mWindow) {
     event.mID.Value().SetAsUnsignedLong() = mOuterID;
     event.mInnerID.Value().SetAsUnsignedLong() = mInnerID;
   } else {
     // If we are in a JSM, the window doesn't exist.
     event.mID.Value().SetAsString() = NS_LITERAL_STRING("jsm");
     event.mInnerID.Value().SetAsString() = frame.mFilename;
   }
 
-  event.mLevel = aData.mMethodString;
+  event.mLevel = aData->mMethodString;
   event.mFilename = frame.mFilename;
   event.mLineNumber = frame.mLineNumber;
   event.mFunctionName = frame.mFunctionName;
-  event.mTimeStamp = aData.mTimeStamp;
-  event.mPrivate = aData.mPrivate;
+  event.mTimeStamp = aData->mTimeStamp;
+  event.mPrivate = aData->mPrivate;
 
-  switch (aData.mMethodName) {
+  switch (aData->mMethodName) {
     case MethodLog:
     case MethodInfo:
     case MethodWarn:
     case MethodError:
     case MethodException:
     case MethodDebug:
     case MethodAssert:
       event.mArguments.Construct();
-      ProcessArguments(cx, aData.mArguments, event.mArguments.Value());
+      ProcessArguments(cx, aData->mArguments, event.mArguments.Value());
       break;
 
     default:
       event.mArguments.Construct();
-      ArgumentsToValueList(aData.mArguments, event.mArguments.Value());
+      ArgumentsToValueList(aData->mArguments, event.mArguments.Value());
   }
 
-  if (aData.mMethodName == MethodTrace) {
+  if (aData->mMethodName == MethodTrace) {
     event.mStacktrace.Construct();
-    event.mStacktrace.Value().SwapElements(aData.mStack);
+    event.mStacktrace.Value().SwapElements(aData->mStack);
   }
 
-  else if (aData.mMethodName == MethodGroup ||
-           aData.mMethodName == MethodGroupCollapsed ||
-           aData.mMethodName == MethodGroupEnd) {
-    ComposeGroupName(cx, aData.mArguments, event.mGroupName);
+  else if (aData->mMethodName == MethodGroup ||
+           aData->mMethodName == MethodGroupCollapsed ||
+           aData->mMethodName == MethodGroupEnd) {
+    ComposeGroupName(cx, aData->mArguments, event.mGroupName);
   }
 
-  else if (aData.mMethodName == MethodTime && !aData.mArguments.IsEmpty()) {
-    event.mTimer = StartTimer(cx, aData.mArguments[0], aData.mMonotonicTimer);
+  else if (aData->mMethodName == MethodTime && !aData->mArguments.IsEmpty()) {
+    event.mTimer = StartTimer(cx, aData->mArguments[0], aData->mMonotonicTimer);
   }
 
-  else if (aData.mMethodName == MethodTimeEnd && !aData.mArguments.IsEmpty()) {
-    event.mTimer = StopTimer(cx, aData.mArguments[0], aData.mMonotonicTimer);
+  else if (aData->mMethodName == MethodTimeEnd && !aData->mArguments.IsEmpty()) {
+    event.mTimer = StopTimer(cx, aData->mArguments[0], aData->mMonotonicTimer);
   }
 
-  else if (aData.mMethodName == MethodCount) {
-    event.mCounter = IncreaseCounter(cx, frame, aData.mArguments);
+  else if (aData->mMethodName == MethodCount) {
+    event.mCounter = IncreaseCounter(cx, frame, aData->mArguments);
   }
 
   JS::Rooted<JS::Value> eventValue(cx);
   if (!event.ToObject(cx, JS::NullPtr(), &eventValue)) {
     Throw(cx, NS_ERROR_FAILURE);
     return;
   }
 
@@ -1390,10 +1413,18 @@ Console::IncreaseCounter(JSContext* aCx,
   JS::Rooted<JS::Value> value(aCx);
   if (!data.ToObject(aCx, JS::NullPtr(), &value)) {
     return JS::UndefinedValue();
   }
 
   return value;
 }
 
+void
+Console::ClearConsoleData()
+{
+  while (ConsoleCallData* data = mQueuedCalls.popFirst()) {
+    delete data;
+  }
+}
+
 } // namespace dom
 } // namespace mozilla
--- a/dom/base/Console.h
+++ b/dom/base/Console.h
@@ -123,20 +123,20 @@ private:
     MethodCount
   };
 
   void
   Method(JSContext* aCx, MethodName aName, const nsAString& aString,
          const Sequence<JS::Value>& aData);
 
   void
-  AppendCallData(const ConsoleCallData& aData);
+  AppendCallData(ConsoleCallData* aData);
 
   void
-  ProcessCallData(ConsoleCallData& aData);
+  ProcessCallData(ConsoleCallData* aData);
 
   // If the first JS::Value of the array is a string, this method uses it to
   // format a string. The supported sequences are:
   //   %s    - string
   //   %d,%i - integer
   //   %f    - double
   //   %o    - a JS object.
   // The output is an array where any object is a separated item, the rest is
@@ -176,21 +176,24 @@ private:
   ProfileMethod(JSContext* aCx, const nsAString& aAction,
                 const Sequence<JS::Value>& aData,
                 ErrorResult& aRv);
 
   JS::Value
   IncreaseCounter(JSContext* aCx, const ConsoleStackEntry& aFrame,
                    const nsTArray<JS::Heap<JS::Value>>& aArguments);
 
+  void
+  ClearConsoleData();
+
   nsCOMPtr<nsPIDOMWindow> mWindow;
   nsCOMPtr<nsITimer> mTimer;
   nsCOMPtr<nsIConsoleAPIStorage> mStorage;
 
-  nsTArray<ConsoleCallData> mQueuedCalls;
+  LinkedList<ConsoleCallData> mQueuedCalls;
   nsDataHashtable<nsStringHashKey, DOMHighResTimeStamp> mTimerRegistry;
   nsDataHashtable<nsStringHashKey, uint32_t> mCounterRegistry;
 
   uint64_t mOuterID;
   uint64_t mInnerID;
 
   friend class ConsoleCallData;
   friend class ConsoleCallDataRunnable;
--- a/dom/base/test/mochitest.ini
+++ b/dom/base/test/mochitest.ini
@@ -4,23 +4,25 @@ support-files =
   iframe_messageChannel_pingpong.html
   iframe_messageChannel_post.html
   file_empty.html
   iframe_postMessage_solidus.html
 
 [test_Image_constructor.html]
 [test_appname_override.html]
 [test_bug913761.html]
+[test_bug978522.html]
 [test_clearTimeoutIntervalNoArg.html]
 [test_consoleEmptyStack.html]
 [test_constructor-assignment.html]
 [test_constructor.html]
 [test_document.all_unqualified.html]
 [test_domcursor.html]
 [test_domrequest.html]
+[test_domwindowutils.html]
 [test_e4x_for_each.html]
 [test_error.html]
 [test_gsp-qualified.html]
 [test_gsp-quirks.html]
 [test_gsp-standards.html]
 [test_history_document_open.html]
 [test_history_state_null.html]
 [test_innersize_scrollport.html]
@@ -47,9 +49,8 @@ support-files =
 [test_urlSearchParams.html]
 [test_urlutils_stringify.html]
 [test_window_constructor.html]
 [test_window_cross_origin_props.html]
 [test_window_enumeration.html]
 [test_window_extensible.html]
 [test_window_indexing.html]
 [test_writable-replaceable.html]
-[test_domwindowutils.html]
new file mode 100644
--- /dev/null
+++ b/dom/base/test/test_bug978522.html
@@ -0,0 +1,32 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=978522
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 978522 - basic support</title>
+  <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=978522">Mozilla Bug 978522</a>
+<script type="application/javascript">
+
+  console.log('%s', {
+    toString: function() {
+      console.log('%s', {
+        toString: function() {
+          ok(true, "Still alive \\o/");
+          SimpleTest.finish();
+          return "hello world";
+        }
+      });
+    }
+  });
+
+  SimpleTest.waitForExplicitFinish();
+
+</script>
+</body>
+</html>