Bug 1492011 introduce a separate class to hold main-thread data associated with each Console r=baku
authorKarl Tomlinson <karlt+@karlt.net>
Wed, 25 Mar 2020 01:06:16 +0000
changeset 520322 59bebd73934646f0fb7af5b987f81368279e35e0
parent 520321 6c87d6b5244012f70f72bc3f625a6b5aeb92e0de
child 520323 643c68ed31dd24befb68a2695370e4e7dd9c4a43
push id37247
push userbtara@mozilla.com
push dateWed, 25 Mar 2020 09:34:57 +0000
treeherdermozilla-central@2abd352490a4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1492011
milestone76.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 1492011 introduce a separate class to hold main-thread data associated with each Console r=baku This provides that ConsoleRunnable no longer has a reference to Console, which previously needed to be released through a message to the console thread. Differential Revision: https://phabricator.services.mozilla.com/D67999
dom/console/Console.cpp
dom/console/Console.h
--- a/dom/console/Console.cpp
+++ b/dom/console/Console.cpp
@@ -213,16 +213,45 @@ class ConsoleCallData final {
   Maybe<ConsoleStackEntry> mTopStackFrame;
   Maybe<nsTArray<ConsoleStackEntry>> mReifiedStack;
   nsCOMPtr<nsIStackFrame> mStack;
 
  private:
   ~ConsoleCallData() { AssertIsOnOwningThread(); }
 };
 
+// MainThreadConsoleData instances are created on the Console thread and
+// referenced from both main and Console threads in order to provide the same
+// object for any ConsoleRunnables relating to the same Console.  A Console
+// owns a MainThreadConsoleData; MainThreadConsoleData does not keep its
+// Console alive.
+class MainThreadConsoleData final {
+  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(MainThreadConsoleData);
+
+  JSObject* GetOrCreateSandbox(JSContext* aCx, nsIPrincipal* aPrincipal);
+  // This method must receive aCx and aArguments in the same JS::Compartment.
+  void ProcessCallData(JSContext* aCx, ConsoleCallData* aData,
+                       const Sequence<JS::Value>& aArguments);
+
+ private:
+  ~MainThreadConsoleData() {
+    NS_ReleaseOnMainThreadSystemGroup("MainThreadConsoleData::mStorage",
+                                      mStorage.forget());
+    NS_ReleaseOnMainThreadSystemGroup("MainThreadConsoleData::mSandbox",
+                                      mSandbox.forget());
+  }
+
+  // All members, except for mRefCnt, are accessed only on the main thread,
+  // except in MainThreadConsoleData destruction, at which point there are no
+  // other references.
+  nsCOMPtr<nsIConsoleAPIStorage> mStorage;
+  RefPtr<JSObjectHolder> mSandbox;
+  nsTArray<nsString> mGroupStack;
+};
+
 // This base class must be extended for Worker and for Worklet.
 class ConsoleRunnable : public StructuredCloneHolderBase {
  public:
   ~ConsoleRunnable() override {
     // Clear the StructuredCloneHolderBase class.
     Clear();
   }
 
@@ -279,17 +308,17 @@ class ConsoleRunnable : public Structure
     if (NS_WARN_IF(!JS_WriteString(aWriter, jsString))) {
       return false;
     }
 
     return true;
   }
 
   // Helper method for CallData
-  void ProcessCallData(JSContext* aCx, Console* aConsole,
+  void ProcessCallData(JSContext* aCx, MainThreadConsoleData* aConsoleData,
                        ConsoleCallData* aCallData) {
     AssertIsOnMainThread();
 
     ConsoleCommon::ClearException ce(aCx);
 
     JS::Rooted<JS::Value> argumentsValue(aCx);
     if (!Read(aCx, &argumentsValue)) {
       return;
@@ -316,17 +345,17 @@ class ConsoleRunnable : public Structure
 
       if (!values.AppendElement(value, fallible)) {
         return;
       }
     }
 
     MOZ_ASSERT(values.Length() == length);
 
-    aConsole->ProcessCallData(aCx, aCallData, values);
+    aConsoleData->ProcessCallData(aCx, aCallData, values);
   }
 
   // Generic
   bool WriteArguments(JSContext* aCx, const Sequence<JS::Value>& aArguments) {
     ConsoleCommon::ClearException ce(aCx);
 
     JS::Rooted<JSObject*> arguments(
         aCx, JS::NewArrayObject(aCx, aArguments.Length()));
@@ -409,19 +438,20 @@ class ConsoleRunnable : public Structure
   }
 
   ConsoleStructuredCloneData mClonedData;
 };
 
 class ConsoleWorkletRunnable : public Runnable, public ConsoleRunnable {
  protected:
   explicit ConsoleWorkletRunnable(Console* aConsole)
-      : Runnable("dom::console::ConsoleWorkletRunnable"), mConsole(aConsole) {
+      : Runnable("dom::console::ConsoleWorkletRunnable"),
+        mConsoleData(aConsole->GetOrCreateMainThreadData()) {
     WorkletThread::AssertIsOnWorkletThread();
-    nsCOMPtr<WorkletGlobalScope> global = do_QueryInterface(mConsole->mGlobal);
+    nsCOMPtr<WorkletGlobalScope> global = do_QueryInterface(aConsole->mGlobal);
     MOZ_ASSERT(global);
     mWorkletImpl = global->Impl();
     MOZ_ASSERT(mWorkletImpl);
   }
 
   ~ConsoleWorkletRunnable() override = default;
 
   NS_IMETHOD
@@ -432,29 +462,27 @@ class ConsoleWorkletRunnable : public Ru
       RunOnMainThread();
       RefPtr<ConsoleWorkletRunnable> runnable(this);
       return mWorkletImpl->SendControlMessage(runnable.forget());
     }
 
     WorkletThread::AssertIsOnWorkletThread();
 
     ReleaseData();
-    mConsole = nullptr;
     return NS_OK;
   }
 
  protected:
   // This method is called in the main-thread.
   virtual void RunOnMainThread() = 0;
 
   // This method is called in the owning thread of the Console object.
   virtual void ReleaseData() = 0;
 
-  // This must be released on the worker thread.
-  RefPtr<Console> mConsole;
+  RefPtr<MainThreadConsoleData> mConsoleData;
 
   RefPtr<WorkletImpl> mWorkletImpl;
 };
 
 // This runnable appends a CallData object into the Console queue running on
 // the main-thread.
 class ConsoleCallDataWorkletRunnable final : public ConsoleWorkletRunnable {
  public:
@@ -485,43 +513,44 @@ class ConsoleCallDataWorkletRunnable fin
   }
 
   ~ConsoleCallDataWorkletRunnable() override { MOZ_ASSERT(!mCallData); }
 
   void RunOnMainThread() override {
     AutoSafeJSContext cx;
 
     JSObject* sandbox =
-        mConsole->GetOrCreateSandbox(cx, mWorkletImpl->Principal());
+        mConsoleData->GetOrCreateSandbox(cx, mWorkletImpl->Principal());
     JS::Rooted<JSObject*> global(cx, sandbox);
     if (NS_WARN_IF(!global)) {
       return;
     }
 
     // The CreateSandbox call returns a proxy to the actual sandbox object. We
     // don't need a proxy here.
     global = js::UncheckedUnwrap(global);
 
     JSAutoRealm ar(cx, global);
 
     // We don't need to set a parent object in mCallData bacause there are not
     // DOM objects exposed to worklet.
 
-    ProcessCallData(cx, mConsole, mCallData);
+    ProcessCallData(cx, mConsoleData, mCallData);
   }
 
   virtual void ReleaseData() override { mCallData = nullptr; }
 
   RefPtr<ConsoleCallData> mCallData;
 };
 
 class ConsoleWorkerRunnable : public WorkerProxyToMainThreadRunnable,
                               public ConsoleRunnable {
  public:
-  explicit ConsoleWorkerRunnable(Console* aConsole) : mConsole(aConsole) {}
+  explicit ConsoleWorkerRunnable(Console* aConsole)
+      : mConsoleData(aConsole->GetOrCreateMainThreadData()) {}
 
   ~ConsoleWorkerRunnable() override = default;
 
   bool Dispatch(JSContext* aCx, const Sequence<JS::Value>& aArguments) {
     WorkerPrivate* workerPrivate = GetCurrentThreadWorkerPrivate();
     MOZ_ASSERT(workerPrivate);
 
     if (NS_WARN_IF(!WriteArguments(aCx, aArguments))) {
@@ -591,17 +620,17 @@ class ConsoleWorkerRunnable : public Wor
     MOZ_ASSERT(!wp->GetWindow());
 
     AutoJSAPI jsapi;
     jsapi.Init();
 
     JSContext* cx = jsapi.cx();
 
     JS::Rooted<JSObject*> global(
-        cx, mConsole->GetOrCreateSandbox(cx, wp->GetPrincipal()));
+        cx, mConsoleData->GetOrCreateSandbox(cx, wp->GetPrincipal()));
     if (NS_WARN_IF(!global)) {
       return;
     }
 
     // The GetOrCreateSandbox call returns a proxy to the actual sandbox object.
     // We don't need a proxy here.
     global = js::UncheckedUnwrap(global);
 
@@ -614,32 +643,30 @@ class ConsoleWorkerRunnable : public Wor
 
     RunConsole(cx, globalObject, aWorkerPrivate, nullptr, nullptr);
   }
 
   void RunBackOnWorkerThreadForCleanup(WorkerPrivate* aWorkerPrivate) override {
     MOZ_ASSERT(aWorkerPrivate);
     aWorkerPrivate->AssertIsOnWorkerThread();
     ReleaseData();
-    mConsole = nullptr;
   }
 
   // This method is called in the main-thread.
   virtual void RunConsole(JSContext* aCx, nsIGlobalObject* aGlobal,
                           WorkerPrivate* aWorkerPrivate,
                           nsPIDOMWindowOuter* aOuterWindow,
                           nsPIDOMWindowInner* aInnerWindow) = 0;
 
   // This method is called in the owning thread of the Console object.
   virtual void ReleaseData() = 0;
 
   bool ForMessaging() const override { return true; }
 
-  // This must be released on the worker thread.
-  RefPtr<Console> mConsole;
+  RefPtr<MainThreadConsoleData> mConsoleData;
 };
 
 // This runnable appends a CallData object into the Console queue running on
 // the main-thread.
 class ConsoleCallDataWorkerRunnable final : public ConsoleWorkerRunnable {
  public:
   ConsoleCallDataWorkerRunnable(Console* aConsole, ConsoleCallData* aCallData)
       : ConsoleWorkerRunnable(aConsole), mCallData(aCallData) {
@@ -682,17 +709,17 @@ class ConsoleCallDataWorkerRunnable fina
         innerID = NS_LITERAL_STRING("Worker");
       }
 
       mCallData->SetIDs(id, innerID);
     }
 
     mClonedData.mGlobal = aGlobal;
 
-    ProcessCallData(aCx, mConsole, mCallData);
+    ProcessCallData(aCx, mConsoleData, mCallData);
 
     mClonedData.mGlobal = nullptr;
   }
 
   virtual void ReleaseData() override { mCallData = nullptr; }
 
   RefPtr<ConsoleCallData> mCallData;
 };
@@ -723,17 +750,17 @@ class ConsoleProfileWorkletRunnable fina
   }
 
   void RunOnMainThread() override {
     AssertIsOnMainThread();
 
     AutoSafeJSContext cx;
 
     JSObject* sandbox =
-        mConsole->GetOrCreateSandbox(cx, mWorkletImpl->Principal());
+        mConsoleData->GetOrCreateSandbox(cx, mWorkletImpl->Principal());
     JS::Rooted<JSObject*> global(cx, sandbox);
     if (NS_WARN_IF(!global)) {
       return;
     }
 
     // The CreateSandbox call returns a proxy to the actual sandbox object. We
     // don't need a proxy here.
     global = js::UncheckedUnwrap(global);
@@ -778,20 +805,16 @@ class ConsoleProfileWorkerRunnable final
   virtual void ReleaseData() override {}
 
   Console::MethodName mName;
   nsString mAction;
 };
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(Console)
 
-// We don't need to traverse/unlink mStorage and mSandbox because they are not
-// CCed objects and they are only used on the main thread, even when this
-// Console object is used on workers.
-
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(Console)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mGlobal)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mConsoleEventNotifier)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mDumpFunction)
   NS_IMPL_CYCLE_COLLECTION_UNLINK_WEAK_REFERENCE
   tmp->Shutdown();
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
@@ -928,19 +951,16 @@ void Console::Shutdown() {
   if (NS_IsMainThread()) {
     nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
     if (obs) {
       obs->RemoveObserver(this, "inner-window-destroyed");
       obs->RemoveObserver(this, "memory-pressure");
     }
   }
 
-  NS_ReleaseOnMainThreadSystemGroup("Console::mStorage", mStorage.forget());
-  NS_ReleaseOnMainThreadSystemGroup("Console::mSandbox", mSandbox.forget());
-
   mTimerRegistry.Clear();
   mCounterRegistry.Clear();
 
   ClearStorage();
   mCallDataStorage.Clear();
 
   mStatus = eShuttingDown;
 }
@@ -1432,17 +1452,17 @@ void Console::MethodInternal(JSContext* 
       nsAutoString filename;
       if (callData->mTopStackFrame.isSome()) {
         filename = callData->mTopStackFrame->mFilename;
       }
 
       callData->SetIDs(NS_LITERAL_STRING("jsm"), filename);
     }
 
-    ProcessCallData(aCx, callData, aData);
+    GetOrCreateMainThreadData()->ProcessCallData(aCx, callData, aData);
 
     // Just because we don't want to expose
     // retrieveConsoleEvents/setConsoleEventHandler to main-thread, we can
     // cleanup the mCallDataStorage:
     UnstoreCallData(callData);
     return;
   }
 
@@ -1462,16 +1482,26 @@ void Console::MethodInternal(JSContext* 
 
   if (StaticPrefs::dom_worker_console_dispatch_events_to_main_thread()) {
     RefPtr<ConsoleCallDataWorkerRunnable> runnable =
         new ConsoleCallDataWorkerRunnable(this, callData);
     Unused << NS_WARN_IF(!runnable->Dispatch(aCx, aData));
   }
 }
 
+MainThreadConsoleData* Console::GetOrCreateMainThreadData() {
+  AssertIsOnOwningThread();
+
+  if (!mMainThreadData) {
+    mMainThreadData = new MainThreadConsoleData();
+  }
+
+  return mMainThreadData;
+}
+
 // We store information to lazily compute the stack in the reserved slots of
 // LazyStackGetter.  The first slot always stores a JS object: it's either the
 // JS wrapper of the nsIStackFrame or the actual reified stack representation.
 // The second slot is a PrivateValue() holding an nsIStackFrame* when we haven't
 // reified the stack yet, or an UndefinedValue() otherwise.
 enum { SLOT_STACKOBJ, SLOT_RAW_STACK };
 
 bool LazyStackGetter(JSContext* aCx, unsigned aArgc, JS::Value* aVp) {
@@ -1498,18 +1528,19 @@ bool LazyStackGetter(JSContext* aCx, uns
 
   js::SetFunctionNativeReserved(callee, SLOT_STACKOBJ, stackVal);
   js::SetFunctionNativeReserved(callee, SLOT_RAW_STACK, JS::UndefinedValue());
 
   args.rval().set(stackVal);
   return true;
 }
 
-void Console::ProcessCallData(JSContext* aCx, ConsoleCallData* aData,
-                              const Sequence<JS::Value>& aArguments) {
+void MainThreadConsoleData::ProcessCallData(
+    JSContext* aCx, ConsoleCallData* aData,
+    const Sequence<JS::Value>& aArguments) {
   AssertIsOnMainThread();
   MOZ_ASSERT(aData);
 
   JS::Rooted<JS::Value> eventValue(aCx);
 
   // We want to create a console event object and pass it to our
   // nsIConsoleAPIStorage implementation.  We want to define some accessor
   // properties on this object, and those will need to keep an nsIStackFrame
@@ -1518,17 +1549,17 @@ void Console::ProcessCallData(JSContext*
   // Object Xrays.  So we want to wrap in a system-principal scope here.  But
   // which one?  We could cheat and try to get the underlying JSObject* of
   // mStorage, but that's a bit fragile.  Instead, we just use the junk scope,
   // with explicit permission from the XPConnect module owner.  If you're
   // tempted to do that anywhere else, talk to said module owner first.
 
   // aCx and aArguments are in the same compartment.
   JS::Rooted<JSObject*> targetScope(aCx, xpc::PrivilegedJunkScope());
-  if (NS_WARN_IF(!PopulateConsoleNotificationInTheTargetScope(
+  if (NS_WARN_IF(!Console::PopulateConsoleNotificationInTheTargetScope(
           aCx, aArguments, targetScope, &eventValue, aData, &mGroupStack))) {
     return;
   }
 
   if (!mStorage) {
     mStorage = do_GetService("@mozilla.org/consoleAPI-storage;1");
   }
 
@@ -1544,17 +1575,17 @@ void Console::ProcessCallData(JSContext*
     outerID = aData->mOuterIDString;
     innerID = aData->mInnerIDString;
   } else {
     MOZ_ASSERT(aData->mIDType == ConsoleCallData::eNumber);
     outerID.AppendInt(aData->mOuterIDNumber);
     innerID.AppendInt(aData->mInnerIDNumber);
   }
 
-  if (aData->mMethodName == MethodClear) {
+  if (aData->mMethodName == Console::MethodClear) {
     DebugOnly<nsresult> rv = mStorage->ClearEvents(innerID);
     NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "ClearEvents failed");
   }
 
   if (NS_FAILED(mStorage->RecordEvent(innerID, outerID, eventValue))) {
     NS_WARNING("Failed to record a console event.");
   }
 }
@@ -2355,18 +2386,18 @@ bool Console::ShouldIncludeStackTrace(Me
     case MethodAssert:
     case MethodTrace:
       return true;
     default:
       return false;
   }
 }
 
-JSObject* Console::GetOrCreateSandbox(JSContext* aCx,
-                                      nsIPrincipal* aPrincipal) {
+JSObject* MainThreadConsoleData::GetOrCreateSandbox(JSContext* aCx,
+                                                    nsIPrincipal* aPrincipal) {
   AssertIsOnMainThread();
 
   if (!mSandbox) {
     nsIXPConnect* xpc = nsContentUtils::XPConnect();
     MOZ_ASSERT(xpc, "This should never be null!");
 
     JS::Rooted<JSObject*> sandbox(aCx);
     nsresult rv = xpc->CreateSandbox(aCx, aPrincipal, sandbox.address());
--- a/dom/console/Console.h
+++ b/dom/console/Console.h
@@ -27,16 +27,17 @@ namespace dom {
 
 class AnyCallback;
 class ConsoleCallData;
 class ConsoleInstance;
 class ConsoleInstanceDumpCallback;
 class ConsoleRunnable;
 class ConsoleCallDataRunnable;
 class ConsoleProfileRunnable;
+class MainThreadConsoleData;
 
 class Console final : public nsIObserver, public nsSupportsWeakReference {
  public:
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS_AMBIGUOUS(Console, nsIObserver)
   NS_DECL_NSIOBSERVER
 
   static already_AddRefed<Console> Create(JSContext* aCx,
@@ -217,19 +218,17 @@ class Console final : public nsIObserver
                            const nsAString& aMethodString);
 
   MOZ_CAN_RUN_SCRIPT
   void StringMethodInternal(JSContext* aCx, const nsAString& aLabel,
                             const Sequence<JS::Value>& aData,
                             MethodName aMethodName,
                             const nsAString& aMethodString);
 
-  // This method must receive aCx and aArguments in the same JS::Compartment.
-  void ProcessCallData(JSContext* aCx, ConsoleCallData* aData,
-                       const Sequence<JS::Value>& aArguments);
+  MainThreadConsoleData* GetOrCreateMainThreadData();
 
   // Returns true on success; otherwise false.
   bool StoreCallData(JSContext* aCx, ConsoleCallData* aCallData,
                      const Sequence<JS::Value>& aArguments);
 
   void UnstoreCallData(ConsoleCallData* aData);
 
   // aCx and aArguments must be in the same JS compartment.
@@ -345,18 +344,16 @@ class Console final : public nsIObserver
   // * aCx - the JSContext rooting aData.
   // * aData - the arguments received by the console.count() method.
   // * aCountLabel - the label that will be populated by this method.
   uint32_t ResetCounter(JSContext* aCx, const Sequence<JS::Value>& aData,
                         nsAString& aCountLabel);
 
   static bool ShouldIncludeStackTrace(MethodName aMethodName);
 
-  JSObject* GetOrCreateSandbox(JSContext* aCx, nsIPrincipal* aPrincipal);
-
   void AssertIsOnOwningThread() const;
 
   bool IsShuttingDown() const;
 
   bool MonotonicTimer(JSContext* aCx, MethodName aMethodName,
                       const Sequence<JS::Value>& aData,
                       DOMHighResTimeStamp* aTimeStamp);
 
@@ -396,35 +393,34 @@ class Console final : public nsIObserver
 
     NS_DECL_OWNINGTHREAD;
     JS::Heap<JSObject*> mGlobal;
     nsTArray<JS::Heap<JS::Value>> mArguments;
   };
 
   // Owning/CC thread only
   nsCOMPtr<nsIGlobalObject> mGlobal;
-  // These nsCOMPtr are touched on main thread only.
-  nsCOMPtr<nsIConsoleAPIStorage> mStorage;
-  RefPtr<JSObjectHolder> mSandbox;
 
   // Touched on the owner thread.
   nsDataHashtable<nsStringHashKey, DOMHighResTimeStamp> mTimerRegistry;
   nsDataHashtable<nsStringHashKey, uint32_t> mCounterRegistry;
 
   nsTArray<RefPtr<ConsoleCallData>> mCallDataStorage;
   // These are references to the arguments we received in each call
   // from the DOM bindings.
   // Vector<T> supports non-memmovable types such as ArgumentData
   // (without any need to jump through hoops like
   // MOZ_DECLARE_RELOCATE_USING_MOVE_CONSTRUCTOR_FOR_TEMPLATE for nsTArray).
   Vector<ArgumentData> mArgumentStorage;
 
   RefPtr<AnyCallback> mConsoleEventNotifier;
 
-  // This is the stack for groupping.
+  RefPtr<MainThreadConsoleData> mMainThreadData;
+  // This is the stack for grouping relating to Console-thread events, when
+  // the Console thread is not the main thread.
   nsTArray<nsString> mGroupStack;
 
   uint64_t mOuterID;
   uint64_t mInnerID;
 
   // Set only by ConsoleInstance:
   nsString mConsoleID;
   nsString mPassedInnerID;
@@ -443,14 +439,15 @@ class Console final : public nsIObserver
   friend class ConsoleCallData;
   friend class ConsoleCallDataWorkletRunnable;
   friend class ConsoleInstance;
   friend class ConsoleProfileWorkerRunnable;
   friend class ConsoleProfileWorkletRunnable;
   friend class ConsoleRunnable;
   friend class ConsoleWorkerRunnable;
   friend class ConsoleWorkletRunnable;
+  friend class MainThreadConsoleData;
 };
 
 }  // namespace dom
 }  // namespace mozilla
 
 #endif /* mozilla_dom_Console_h */