Bug 1769763: Part 2 - Add debug names to StructuredCloneHolder objects for about:memory. r=mccr8,devtools-reviewers

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

devtools/shared/transport/child-transport.js

@@ -74,7 +74,11 @@ ChildDebuggerTransport.prototype = {
    */
   _canBeSerialized(object) {
     try {
-      const holder = new StructuredCloneHolder(object);
+      const holder = new StructuredCloneHolder(
+        "ChildDebuggerTransport._canBeSerialized",
+        null,
+        object
+      );
       holder.deserialize(this);
     } catch (e) {
       return false;

docshell/test/unit/test_browsing_context_structured_clone.js

@@ -12,7 +12,9 @@ add_task(async function test_BrowsingContext_structured_clone() {
 
   let { browsingContext } = frame;
 
-  let sch = new StructuredCloneHolder({ browsingContext });
+  let sch = new StructuredCloneHolder("debug name", "<anonymized> debug name", {
+    browsingContext,
+  });
 
   let deserialize = () => sch.deserialize({}, true);
 

dom/base/StructuredCloneBlob.cpp

@@ -22,6 +22,7 @@
 #include "mozilla/dom/BlobImpl.h"
 #include "mozilla/dom/StructuredCloneHolderBinding.h"
 #include "mozilla/dom/StructuredCloneTags.h"
+#include "nsPrintfCString.h"
 #include "xpcpublic.h"
 
 namespace mozilla::dom {
@@ -37,12 +38,16 @@ StructuredCloneBlob::~StructuredCloneBlob() {
 
 /* static */
 already_AddRefed<StructuredCloneBlob> StructuredCloneBlob::Constructor(
-    GlobalObject& aGlobal, JS::Handle<JS::Value> aValue,
+    GlobalObject& aGlobal, const nsACString& aName,
+    const nsACString& aAnonymizedName, JS::Handle<JS::Value> aValue,
     JS::Handle<JSObject*> aTargetGlobal, ErrorResult& aRv) {
   JSContext* cx = aGlobal.Context();
 
   RefPtr<StructuredCloneBlob> holder = StructuredCloneBlob::Create();
 
+  holder->mName = aName;
+  holder->mAnonymizedName = aAnonymizedName.IsVoid() ? aName : aAnonymizedName;
+
   Maybe<JSAutoRealm> ar;
   JS::Rooted<JS::Value> value(cx, aValue);
 
@@ -129,6 +134,14 @@ JSObject* StructuredCloneBlob::ReadStructuredClone(
   {
     RefPtr<StructuredCloneBlob> holder = StructuredCloneBlob::Create();
 
+    if (!StructuredCloneHolder::ReadCString(aReader, holder->mName)) {
+      return nullptr;
+    }
+
+    if (!StructuredCloneHolder::ReadCString(aReader, holder->mAnonymizedName)) {
+      return nullptr;
+    }
+
     if (!holder->mHolder->ReadStructuredCloneInternal(aCx, aReader, aHolder) ||
         !holder->WrapObject(aCx, nullptr, &obj)) {
       return nullptr;
@@ -186,6 +199,13 @@ bool StructuredCloneBlob::WriteStructuredClone(JSContext* aCx,
   if (mHolder.isNothing()) {
     return false;
   }
+
+  if (!JS_WriteUint32Pair(aWriter, SCTAG_DOM_STRUCTURED_CLONE_HOLDER, 0) ||
+      !StructuredCloneHolder::WriteCString(aWriter, mName) ||
+      !StructuredCloneHolder::WriteCString(aWriter, mAnonymizedName)) {
+    return false;
+  }
+
   return mHolder->WriteStructuredClone(aCx, aWriter, aHolder);
 }
 
@@ -193,8 +213,7 @@ bool StructuredCloneBlob::Holder::WriteStructuredClone(
     JSContext* aCx, JSStructuredCloneWriter* aWriter,
     StructuredCloneHolder* aHolder) {
   auto& data = mBuffer->data();
-  if (!JS_WriteUint32Pair(aWriter, SCTAG_DOM_STRUCTURED_CLONE_HOLDER, 0) ||
-      !JS_WriteUint32Pair(aWriter, data.Size(), JS_STRUCTURED_CLONE_VERSION) ||
+  if (!JS_WriteUint32Pair(aWriter, data.Size(), JS_STRUCTURED_CLONE_VERSION) ||
       !JS_WriteUint32Pair(aWriter, aHolder->BlobImpls().Length(),
                           BlobImpls().Length())) {
     return false;
@@ -221,9 +240,12 @@ StructuredCloneBlob::CollectReports(nsIHandleReportCallback* aHandleReport,
     size += mHolder->SizeOfExcludingThis(MallocSizeOf);
   }
 
-  MOZ_COLLECT_REPORT("explicit/dom/structured-clone-holder", KIND_HEAP,
-                     UNITS_BYTES, size,
-                     "Memory used by StructuredCloneHolder DOM objects.");
+  aHandleReport->Callback(
+      ""_ns,
+      nsPrintfCString("explicit/dom/structured-clone-holder/%s",
+                      aAnonymize ? mAnonymizedName.get() : mName.get()),
+      KIND_HEAP, UNITS_BYTES, size,
+      "Memory used by StructuredCloneHolder DOM objects."_ns, aData);
 
   return NS_OK;
 }

dom/base/StructuredCloneBlob.h

@@ -40,7 +40,8 @@ class StructuredCloneBlob final : public nsIMemoryReporter {
                             StructuredCloneHolder* aHolder);
 
   static already_AddRefed<StructuredCloneBlob> Constructor(
-      GlobalObject& aGlobal, JS::Handle<JS::Value> aValue,
+      GlobalObject& aGlobal, const nsACString& aName,
+      const nsACString& aAnonymizedName, JS::Handle<JS::Value> aValue,
       JS::Handle<JSObject*> aTargetGlobal, ErrorResult& aRv);
 
   void Deserialize(JSContext* aCx, JS::Handle<JSObject*> aTargetScope,
@@ -71,6 +72,8 @@ class StructuredCloneBlob final : public nsIMemoryReporter {
                               StructuredCloneHolder* aHolder);
   };
 
+  nsCString mName;
+  nsCString mAnonymizedName;
   Maybe<Holder> mHolder;
 
   static already_AddRefed<StructuredCloneBlob> Create() {

dom/base/test/unit/test_structuredcloneholder.js

@@ -12,7 +12,7 @@ add_task(async function test_structuredCloneHolder() {
 
   const obj = { foo: [{ bar: "baz" }] };
 
-  let holder = new StructuredCloneHolder(obj);
+  let holder = new StructuredCloneHolder("", "", obj);
 
   // Test same-compartment deserialization
 
@@ -39,7 +39,7 @@ add_task(async function test_structuredCloneHolder() {
   // Test non-object-value round-trip.
 
   equal(
-    new StructuredCloneHolder("foo").deserialize(global),
+    new StructuredCloneHolder("", "", "foo").deserialize(global),
     "foo",
     "Round-tripping non-object values works as expected"
   );
@@ -128,7 +128,7 @@ add_task(async function test_structuredCloneHolder_xray() {
   let holder;
   Cu.exportFunction(
     function serialize(val) {
-      holder = new StructuredCloneHolder(val, sandbox1);
+      holder = new StructuredCloneHolder("", "", val, sandbox1);
     },
     sandbox1,
     { defineAs: "serialize" }

dom/chrome-webidl/StructuredCloneHolder.webidl

@@ -16,9 +16,17 @@ interface StructuredCloneHolder {
    *
    * The serialization happens in the compartment of the given global or, if no
    * global is provided, the compartment of the data value.
+   *
+   * The name argument is added to the path of the object in
+   * memory reports, to make it easier to determine the source of leaks. In
+   * anonymized memory reports, the anonymized name is used instead. If
+   * anonymizedName is null, name is used in anonymized reports as well.
+   * Anonymized names should not contain any potentially private information,
+   * such as web URLs or user-provided data.
    */
   [Throws]
-  constructor(any data, optional object? global = null);
+  constructor(UTF8String name, UTF8String? anonymizedName,
+              any data, optional object? global = null);
 
   /**
    * Deserializes the structured clone data in the scope of the given global,

dom/media/PeerConnection.sys.mjs

@@ -1457,7 +1457,11 @@ export class RTCPeerConnection {
       // Exceptions thrown by c++ code do not propagate. In most cases, that's
       // fine because we're using Promises, which can be copied. But this is
       // not promise-based, so we have to do this sketchy stuff.
-      const holder = new StructuredCloneHolder(new ClonedErrorHolder(e));
+      const holder = new StructuredCloneHolder(
+        "",
+        "",
+        new ClonedErrorHolder(e)
+      );
       throw holder.deserialize(this._win);
     }
   }

dom/promise/tests/unit/test_promise_unhandled_rejection.js

@@ -42,7 +42,7 @@ add_task(async function test_unhandled_dom_exception() {
   let messages = await getSandboxMessages(
     sandbox,
     `new Promise(() => {
-      new StructuredCloneHolder(() => {});
+      new StructuredCloneHolder("", "", () => {});
     });`
   );
 

mobile/android/modules/geckoview/GeckoViewWebExtension.sys.mjs

@@ -171,7 +171,11 @@ class EmbedderPort {
 
     switch (aEvent) {
       case "GeckoView:WebExtension:PortMessageFromApp": {
-        const holder = new StructuredCloneHolder(aData.message);
+        const holder = new StructuredCloneHolder(
+          "GeckoView:WebExtension:PortMessageFromApp",
+          null,
+          aData.message
+        );
         this.messenger.sendPortMessage(this.id, holder);
         break;
       }

testing/specialpowers/content/SpecialPowersChild.sys.mjs

@@ -613,9 +613,11 @@ export class SpecialPowersChild extends JSWindowActorChild {
         let name = aMessage.json.name;
         let message = aMessage.json.message;
         if (this.contentWindow) {
-          message = new StructuredCloneHolder(message).deserialize(
-            this.contentWindow
-          );
+          message = new StructuredCloneHolder(
+            `SpecialPowers/receiveMessage/${name}`,
+            null,
+            message
+          ).deserialize(this.contentWindow);
         }
         // Ignore message from other chrome script
         if (messageId != id) {

toolkit/components/extensions/ExtensionChild.jsm

@@ -243,11 +243,11 @@ class MessageEvent extends SimpleEventAPI {
   }
 }
 
-function holdMessage(data, native = null) {
+function holdMessage(name, anonymizedName, data, native = null) {
   if (native && AppConstants.platform !== "android") {
     data = lazy.NativeApp.encodeMessage(native.context, data);
   }
-  return new StructuredCloneHolder(data);
+  return new StructuredCloneHolder(name, anonymizedName, data);
 }
 
 // Implements the runtime.Port extension API object.
@@ -263,7 +263,10 @@ class Port {
     this.context = context;
     this.name = name;
     this.sender = sender;
-    this.holdMessage = native ? data => holdMessage(data, this) : holdMessage;
+    this.holdMessage = native
+      ? (name, anonymizedName, data) =>
+          holdMessage(name, anonymizedName, data, this)
+      : holdMessage;
     this.conduit = context.openConduit(this, {
       portId,
       native,
@@ -307,7 +310,13 @@ class Port {
 
   sendPortMessage(json) {
     if (this.conduit.actor) {
-      return this.conduit.sendPortMessage({ holder: this.holdMessage(json) });
+      return this.conduit.sendPortMessage({
+        holder: this.holdMessage(
+          `Port/${this.context.extension.id}/sendPortMessage/${this.name}`,
+          `Port/${this.context.extension.id}/sendPortMessage/<anonymized>`,
+          json
+        ),
+      });
     }
     throw new this.context.Error("Attempt to postMessage on disconnected port");
   }
@@ -342,14 +351,23 @@ class Messenger {
   }
 
   sendNativeMessage(nativeApp, json) {
-    let holder = holdMessage(json, this);
+    let holder = holdMessage(
+      `Messenger/${this.context.extension.id}/sendNativeMessage/${nativeApp}`,
+      null,
+      json,
+      this
+    );
     return this.conduit.queryNativeMessage({ nativeApp, holder });
   }
 
   sendRuntimeMessage({ extensionId, message, callback, ...args }) {
     let response = this.conduit.queryRuntimeMessage({
       extensionId: extensionId || this.context.extension.id,
-      holder: holdMessage(message),
+      holder: holdMessage(
+        `Messenger/${this.context.extension.id}/sendRuntimeMessage`,
+        null,
+        message
+      ),
       ...args,
     });
     // If |response| is a rejected promise, the value will be sanitized by
@@ -836,7 +854,12 @@ class ChildAPIManager {
           : fire()
       ).then(result => {
         if (result !== undefined) {
-          return new StructuredCloneHolder(result, this.context.cloneScope);
+          return new StructuredCloneHolder(
+            `ChildAPIManager/${this.context.extension.id}/${data.path}`,
+            null,
+            result,
+            this.context.cloneScope
+          );
         }
         return result;
       });

toolkit/components/extensions/ExtensionCommon.jsm

@@ -1507,7 +1507,7 @@ class SchemaAPIManager extends EventEmitter {
 
     this._modulesJSONLoaded = true;
 
-    return new StructuredCloneHolder({
+    return new StructuredCloneHolder("SchemaAPIManager/initModuleJSON", null, {
       modules: this.modules,
       modulePaths: this.modulePaths,
       manifestKeys: this.manifestKeys,

toolkit/components/extensions/ExtensionParent.jsm

@@ -1168,7 +1168,11 @@ ParentAPIManager = {
           result => {
             result = result instanceof SpreadArgs ? [...result] : [result];
 
-            let holder = new StructuredCloneHolder(result);
+            let holder = new StructuredCloneHolder(
+              `ExtensionParent/${context.extension.id}/recvAPICall/${data.path}`,
+              null,
+              result
+            );
 
             reply({ result: holder });
           },
@@ -1213,7 +1217,11 @@ ParentAPIManager = {
         path: data.path,
         urgentSend,
         get args() {
-          return new StructuredCloneHolder(listenerArgs);
+          return new StructuredCloneHolder(
+            `ExtensionParent/${context.extension.id}/recvAddListener/${data.path}`,
+            null,
+            listenerArgs
+          );
         },
       });
       context.trackRunListenerPromise(runListenerPromise);

toolkit/components/extensions/ExtensionStorage.jsm

@@ -80,13 +80,19 @@ class SerializeableMap extends Map {
  * sending a storage value across a message manager, before cloning it
  * into an extension scope.
  *
+ * @param {string} name
+ *        A debugging name for the value, which will appear in the
+ *        StructuredCloneHolder's about:memory path.
+ * @param {string?} anonymizedName
+ *        An anonymized version of `name`, to be used in anonymized memory
+ *        reports. If `null`, then `name` will be used instead.
  * @param {StructuredCloneHolder|*} value
  *        A value to serialize.
  * @returns {*}
  */
-function serialize(value) {
+function serialize(name, anonymizedName, value) {
   if (value && typeof value === "object" && !isStructuredCloneHolder(value)) {
-    return new StructuredCloneHolder(value);
+    return new StructuredCloneHolder(name, anonymizedName, value);
   }
   return value;
 }
@@ -219,8 +225,16 @@ var ExtensionStorage = {
     for (let prop in items) {
       let item = items[prop];
       changes[prop] = {
-        oldValue: serialize(jsonFile.data.get(prop)),
-        newValue: serialize(item),
+        oldValue: serialize(
+          `set/${extensionId}/old/${prop}`,
+          `set/${extensionId}/old/<anonymized>`,
+          jsonFile.data.get(prop)
+        ),
+        newValue: serialize(
+          `set/${extensionId}/new/${prop}`,
+          `set/${extensionId}/new/<anonymized>`,
+          item
+        ),
       };
       jsonFile.data.set(prop, item);
     }
@@ -249,7 +263,13 @@ var ExtensionStorage = {
 
     for (let prop of [].concat(items)) {
       if (jsonFile.data.has(prop)) {
-        changes[prop] = { oldValue: serialize(jsonFile.data.get(prop)) };
+        changes[prop] = {
+          oldValue: serialize(
+            `remove/${extensionId}/${prop}`,
+            `remove/${extensionId}/<anonymized>`,
+            jsonFile.data.get(prop)
+          ),
+        };
         jsonFile.data.delete(prop);
         changed = true;
       }
@@ -282,7 +302,13 @@ var ExtensionStorage = {
 
     for (let [prop, oldValue] of jsonFile.data.entries()) {
       if (shouldNotifyListeners) {
-        changes[prop] = { oldValue: serialize(oldValue) };
+        changes[prop] = {
+          oldValue: serialize(
+            `clear/${extensionId}/${prop}`,
+            `clear/${extensionId}/<anonymized>`,
+            oldValue
+          ),
+        };
       }
 
       jsonFile.data.delete(prop);
@@ -319,17 +345,21 @@ var ExtensionStorage = {
    */
   async get(extensionId, keys) {
     let jsonFile = await this.getFile(extensionId);
-    return this._filterProperties(jsonFile.data, keys);
+    return this._filterProperties(extensionId, jsonFile.data, keys);
   },
 
-  async _filterProperties(data, keys) {
+  async _filterProperties(extensionId, data, keys) {
     let result = {};
     if (keys === null) {
       Object.assign(result, data.toJSON());
     } else if (typeof keys == "object" && !Array.isArray(keys)) {
       for (let prop in keys) {
         if (data.has(prop)) {
-          result[prop] = serialize(data.get(prop));
+          result[prop] = serialize(
+            `filterProperties/${extensionId}/${prop}`,
+            `filterProperties/${extensionId}/<anonymized>`,
+            data.get(prop)
+          );
         } else {
           result[prop] = keys[prop];
         }
@@ -337,7 +367,11 @@ var ExtensionStorage = {
     } else {
       for (let prop of [].concat(keys)) {
         if (data.has(prop)) {
-          result[prop] = serialize(data.get(prop));
+          result[prop] = serialize(
+            `filterProperties/${extensionId}/${prop}`,
+            `filterProperties/${extensionId}/<anonymized>`,
+            data.get(prop)
+          );
         }
       }
     }
@@ -407,7 +441,12 @@ var ExtensionStorage = {
       let result = {};
       for (let [key, value] of Object.entries(items)) {
         try {
-          result[key] = new StructuredCloneHolder(value, context.cloneScope);
+          result[key] = new StructuredCloneHolder(
+            `serializeForContext/${context.extension.id}`,
+            null,
+            value,
+            context.cloneScope
+          );
         } catch (e) {
           throw new ExtensionError(String(e));
         }

toolkit/components/extensions/ExtensionStorageIDB.jsm

@@ -236,6 +236,10 @@ class ExtensionStorageLocalIDB extends IndexedDB {
     );
     const transactionCompleted = transaction.promiseComplete();
 
+    if (!serialize) {
+      serialize = (name, anonymizedName, value) => value;
+    }
+
     for (let key of Object.keys(items)) {
       try {
         let oldValue = await objectStore.get(key);
@@ -243,8 +247,9 @@ class ExtensionStorageLocalIDB extends IndexedDB {
         await objectStore.put(items[key], key);
 
         changes[key] = {
-          oldValue: oldValue && serialize ? serialize(oldValue) : oldValue,
-          newValue: serialize ? serialize(items[key]) : items[key],
+          oldValue:
+            oldValue && serialize(`old/${key}`, `old/<anonymized>`, oldValue),
+          newValue: serialize(`new/${key}`, `new/<anonymized>`, items[key]),
         };
         changed = true;
       } catch (err) {
@@ -701,6 +706,8 @@ ExtensionStorageIDB = {
         // Serialize the nsIPrincipal object into a StructuredCloneHolder related to the privileged
         // js global, ready to be sent to the child processes.
         const serializedPrincipal = new StructuredCloneHolder(
+          "ExtensionStorageIDB/selectBackend/serializedPrincipal",
+          null,
           storagePrincipal,
           this
         );

toolkit/components/extensions/NativeMessaging.jsm

@@ -138,7 +138,14 @@ var NativeApp = class extends EventEmitter {
   onConnect(portId, port) {
     // eslint-disable-next-line
     this.on("message", (_, message) => {
-      port.sendPortMessage(portId, new StructuredCloneHolder(message));
+      port.sendPortMessage(
+        portId,
+        new StructuredCloneHolder(
+          `NativeMessaging/onConnect/${this.name}`,
+          null,
+          message
+        )
+      );
     });
     this.once("disconnect", (_, error) => {
       port.sendPortDisconnect(portId, error && new ClonedErrorHolder(error));

toolkit/components/extensions/Schemas.jsm

@@ -136,7 +136,7 @@ function blobbify(json) {
   // blobbifying.
   json = stripDescriptions(json);
 
-  return new StructuredCloneHolder(json);
+  return new StructuredCloneHolder("Schemas/blobbify", null, json);
 }
 
 async function readJSONAndBlobbify(url) {

toolkit/components/extensions/child/ext-storage.js

@@ -109,13 +109,21 @@ this.storage = class extends ExtensionAPI {
         );
       },
       set(items) {
+        function serialize(name, anonymizedName, value) {
+          return ExtensionStorage.serialize(
+            `set/${context.extension.id}/${name}`,
+            `set/${context.extension.id}/${anonymizedName}`,
+            value
+          );
+        }
+
         return measureOp(
           ExtensionTelemetry.storageLocalSetIDB,
           context.extension,
           async () => {
             const db = await getDB();
             const changes = await db.set(items, {
-              serialize: ExtensionStorage.serialize,
+              serialize,
             });
 
             if (changes) {

toolkit/components/extensions/child/ext-test.js

@@ -259,7 +259,7 @@ this.test = class extends ExtensionAPI {
               // throw if needed.
               v = ChromeUtils.waiveXrays(v);
             }
-            new StructuredCloneHolder(v, globalThis);
+            new StructuredCloneHolder("test.assertEq", null, v, globalThis);
           }
           // When WebIDL bindings are used, the objects are already cloned
           // structurally, so we don't need to check again.

toolkit/components/extensions/parent/ext-storage.js

@@ -337,7 +337,7 @@ this.storage = class extends ExtensionAPIPersistent {
                 message: "Managed storage manifest not found",
               });
             }
-            return ExtensionStorage._filterProperties(data, keys);
+            return ExtensionStorage._filterProperties(extension.id, data, keys);
           },
           // managed storage is currently initialized once.
           onChanged: ignoreEvent(context, "storage.managed.onChanged"),

toolkit/components/extensions/test/xpcshell/test_native_manifests.js

@@ -460,7 +460,7 @@ while True:
   });
 
   let buffer = NativeApp.encodeMessage(mockContext, MSG);
-  app.send(new StructuredCloneHolder(buffer));
+  app.send(new StructuredCloneHolder("", null, buffer));
   await recvPromise;
 
   app._cleanup();