Bug 1891789 - Allow console.createInstance to work properly in worker…

…s. r=peterv

Also raise an error on the console if 'maxLogLevelPref' is used when it shouldn't be.

Differential Revision: https://phabricator.services.mozilla.com/D208000

dom/console/Console.h

@@ -134,6 +134,7 @@ class Console final : public nsIObserver, public nsSupportsWeakReference {
   MOZ_CAN_RUN_SCRIPT
   static void Clear(const GlobalObject& aGlobal);
 
+  MOZ_CAN_RUN_SCRIPT
   static already_AddRefed<ConsoleInstance> CreateInstance(
       const GlobalObject& aGlobal, const ConsoleInstanceOptions& aOptions);
 

dom/console/ConsoleInstance.cpp

@@ -65,9 +65,24 @@ ConsoleInstance::ConsoleInstance(JSContext* aCx,
 
   if (!aOptions.mMaxLogLevelPref.IsEmpty()) {
     if (!NS_IsMainThread()) {
-      NS_WARNING("Console.maxLogLevelPref is not supported on workers!");
       // Set the log level based on what we have.
       SetLogLevel();
+
+      // Flag an error to the console.
+      JS::Rooted<JS::Value> msg(aCx);
+      if (!ToJSValue(
+              aCx,
+              nsLiteralCString(
+                  "Console.maxLogLevelPref is not supported within workers!"),
+              &msg)) {
+        JS_ClearPendingException(aCx);
+        return;
+      }
+
+      AutoTArray<JS::Value, 1> sequence;
+      SequenceRooter rootedSequence(aCx, &sequence);
+      sequence.AppendElement(std::move(msg));
+      this->Error(aCx, std::move(sequence));
       return;
     }
 
@@ -80,8 +95,9 @@ ConsoleInstance::ConsoleInstance(JSContext* aCx,
 }
 
 ConsoleInstance::~ConsoleInstance() {
-  AssertIsOnMainThread();
-  if (!mMaxLogLevelPref.IsEmpty()) {
+  // We should only ever have set `mMaxLogLevelPref` when on the main thread,
+  // but check it here to be safe.
+  if (!mMaxLogLevelPref.IsEmpty() && NS_IsMainThread()) {
     Preferences::UnregisterCallback(MaxLogLevelPrefChangedCallback,
                                     mMaxLogLevelPref, this);
   }
@@ -128,7 +144,6 @@ void ConsoleInstance::SetLogLevel() {
 // static
 void ConsoleInstance::MaxLogLevelPrefChangedCallback(
     const char* /* aPrefName */, void* aSelf) {
-  AssertIsOnMainThread();
   auto* instance = static_cast<ConsoleInstance*>(aSelf);
   if (MOZ_UNLIKELY(!instance->mConsole)) {
     // We've been unlinked already but not destroyed yet. Bail.

dom/console/ConsoleInstance.h

@@ -16,6 +16,7 @@ class ConsoleInstance final : public nsISupports, public nsWrapperCache {
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_WRAPPERCACHE_CLASS(ConsoleInstance)
 
+  MOZ_CAN_RUN_SCRIPT
   explicit ConsoleInstance(JSContext* aCx,
                            const ConsoleInstanceOptions& aOptions);
 

dom/console/tests/xpcshell/test_worker.js

@@ -0,0 +1,59 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Tests for console.createInstance usage in workers.
+ *
+ * Also tests that the use of `maxLogLevelPref` logs an error, and log levels
+ * fallback to the `maxLogLevel` option.
+ */
+
+"use strict";
+
+let { TestUtils } = ChromeUtils.importESModule(
+  "resource://testing-common/TestUtils.sys.mjs"
+);
+
+add_task(async function () {
+  let endConsoleListening = TestUtils.listenForConsoleMessages();
+  let workerCompleteDeferred = Promise.withResolvers();
+
+  const worker = new ChromeWorker("resource://test/worker.mjs");
+  worker.onmessage = workerCompleteDeferred.resolve;
+  worker.postMessage({});
+
+  await workerCompleteDeferred.promise;
+
+  let messages = await endConsoleListening();
+
+  // Should log that we can't use `maxLogLevelPref`, and the warning message.
+  // The info message should not be logged, as that's lower than the specified
+  // `maxLogLevel` in the worker.
+  Assert.equal(messages.length, 2, "Should have received two messages");
+
+  // First message should be the error that `maxLogLevelPref` cannot be used.
+  Assert.equal(messages[0].level, "error", "Should be an error message");
+  Assert.equal(
+    messages[0].prefix,
+    "testPrefix",
+    "Should have the correct prefix"
+  );
+  Assert.equal(
+    messages[0].arguments[0],
+    "Console.maxLogLevelPref is not supported within workers!",
+    "Should have the correct message text"
+  );
+
+  // Second message should be the warning.
+  Assert.equal(messages[1].level, "warn", "Should be a warning message");
+  Assert.equal(
+    messages[1].prefix,
+    "testPrefix",
+    "Should have the correct prefix"
+  );
+  Assert.equal(
+    messages[1].arguments[0],
+    "Test Warn",
+    "Should have the correct message text"
+  );
+});

dom/console/tests/xpcshell/worker.mjs

@@ -0,0 +1,15 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+onmessage = () => {
+  let logConsole = console.createInstance({
+    maxLogLevelPref: "browser.test.logLevel",
+    maxLogLevel: "Warn",
+    prefix: "testPrefix",
+  });
+
+  logConsole.warn("Test Warn");
+  logConsole.info("Test Info");
+
+  postMessage({});
+};

dom/console/tests/xpcshell/xpcshell.toml

@@ -11,3 +11,8 @@ support-files = ""
 ["test_formatting.js"]
 
 ["test_reportForServiceWorkerScope.js"]
+
+["test_worker.js"]
+support-files = [
+  "worker.mjs",
+]

toolkit/docs/javascript-logging.md

@@ -76,6 +76,9 @@ lazy.logConsole.error("Something bad happened");
 lazy.logConsole.debug("foo", 123)
 ```
 
+**Note:** Workers are not able to access preferences, and therefore must use
+`maxLogLevel` rather than `maxLogLevelPref`.
+
 ### Other Options to console.createInstance
 
 `console.createInstance` may be passed other options. See