refactor: small cleanup around realms and handles (#11866)

packages/puppeteer-core/src/bidi/Deserializer.ts

@@ -12,7 +12,57 @@ import {debugError} from '../common/util.js';
  * @internal
  */
 export class BidiDeserializer {
-  static deserializeNumber(value: Bidi.Script.SpecialNumber | number): number {
+  static deserialize(result: Bidi.Script.RemoteValue): any {
+    if (!result) {
+      debugError('Service did not produce a result.');
+      return undefined;
+    }
+
+    switch (result.type) {
+      case 'array':
+        return result.value?.map(value => {
+          return this.deserialize(value);
+        });
+      case 'set':
+        return result.value?.reduce((acc: Set<unknown>, value) => {
+          return acc.add(this.deserialize(value));
+        }, new Set());
+      case 'object':
+        return result.value?.reduce((acc: Record<any, unknown>, tuple) => {
+          const {key, value} = this.#deserializeTuple(tuple);
+          acc[key as any] = value;
+          return acc;
+        }, {});
+      case 'map':
+        return result.value?.reduce((acc: Map<unknown, unknown>, tuple) => {
+          const {key, value} = this.#deserializeTuple(tuple);
+          return acc.set(key, value);
+        }, new Map());
+      case 'promise':
+        return {};
+      case 'regexp':
+        return new RegExp(result.value.pattern, result.value.flags);
+      case 'date':
+        return new Date(result.value);
+      case 'undefined':
+        return undefined;
+      case 'null':
+        return null;
+      case 'number':
+        return this.#deserializeNumber(result.value);
+      case 'bigint':
+        return BigInt(result.value);
+      case 'boolean':
+        return Boolean(result.value);
+      case 'string':
+        return result.value;
+    }
+
+    debugError(`Deserialization of type ${result.type} not supported.`);
+    return undefined;
+  }
+
+  static #deserializeNumber(value: Bidi.Script.SpecialNumber | number): number {
     switch (value) {
       case '-0':
         return -0;
@@ -27,70 +77,16 @@ export class BidiDeserializer {
     }
   }
 
-  static deserializeLocalValue(result: Bidi.Script.RemoteValue): unknown {
+  static #deserializeTuple([serializedKey, serializedValue]: [
-    switch (result.type) {
-      case 'array':
-        return result.value?.map(value => {
-          return BidiDeserializer.deserializeLocalValue(value);
-        });
-      case 'set':
-        return result.value?.reduce((acc: Set<unknown>, value) => {
-          return acc.add(BidiDeserializer.deserializeLocalValue(value));
-        }, new Set());
-      case 'object':
-        return result.value?.reduce((acc: Record<any, unknown>, tuple) => {
-          const {key, value} = BidiDeserializer.deserializeTuple(tuple);
-          acc[key as any] = value;
-          return acc;
-        }, {});
-      case 'map':
-        return result.value?.reduce((acc: Map<unknown, unknown>, tuple) => {
-          const {key, value} = BidiDeserializer.deserializeTuple(tuple);
-          return acc.set(key, value);
-        }, new Map());
-      case 'promise':
-        return {};
-      case 'regexp':
-        return new RegExp(result.value.pattern, result.value.flags);
-      case 'date':
-        return new Date(result.value);
-      case 'undefined':
-        return undefined;
-      case 'null':
-        return null;
-      case 'number':
-        return BidiDeserializer.deserializeNumber(result.value);
-      case 'bigint':
-        return BigInt(result.value);
-      case 'boolean':
-        return Boolean(result.value);
-      case 'string':
-        return result.value;
-    }
-
-    debugError(`Deserialization of type ${result.type} not supported.`);
-    return undefined;
-  }
-
-  static deserializeTuple([serializedKey, serializedValue]: [
     Bidi.Script.RemoteValue | string,
     Bidi.Script.RemoteValue,
   ]): {key: unknown; value: unknown} {
     const key =
       typeof serializedKey === 'string'
         ? serializedKey
-        : BidiDeserializer.deserializeLocalValue(serializedKey);
+        : this.deserialize(serializedKey);
-    const value = BidiDeserializer.deserializeLocalValue(serializedValue);
+    const value = this.deserialize(serializedValue);
 
     return {key, value};
   }
-
-  static deserialize(result: Bidi.Script.RemoteValue): any {
-    if (!result) {
-      debugError('Service did not produce a result.');
-      return undefined;
-    }
-
-    return BidiDeserializer.deserializeLocalValue(result);
-  }
 }

packages/puppeteer-core/src/bidi/ExposedFunction.ts

@@ -144,7 +144,7 @@ export class ExposeableFunction<Args extends unknown[], Ret> {
         }),
         arguments: [
           (await callbacks.resolve.valueOrThrow()) as Bidi.Script.LocalValue,
-          BidiSerializer.serializeRemoteValue(result),
+          BidiSerializer.serialize(result),
         ],
         awaitPromise: false,
         target: {
@@ -172,9 +172,9 @@ export class ExposeableFunction<Args extends unknown[], Ret> {
             ),
             arguments: [
               (await callbacks.reject.valueOrThrow()) as Bidi.Script.LocalValue,
-              BidiSerializer.serializeRemoteValue(error.name),
+              BidiSerializer.serialize(error.name),
-              BidiSerializer.serializeRemoteValue(error.message),
+              BidiSerializer.serialize(error.message),
-              BidiSerializer.serializeRemoteValue(error.stack),
+              BidiSerializer.serialize(error.stack),
             ],
             awaitPromise: false,
             target: {
@@ -193,7 +193,7 @@ export class ExposeableFunction<Args extends unknown[], Ret> {
             ),
             arguments: [
               (await callbacks.reject.valueOrThrow()) as Bidi.Script.LocalValue,
-              BidiSerializer.serializeRemoteValue(error),
+              BidiSerializer.serialize(error),
             ],
             awaitPromise: false,
             target: {

packages/puppeteer-core/src/bidi/JSHandle.ts

@@ -13,7 +13,6 @@ import {UnsupportedOperation} from '../common/Errors.js';
 import {BidiDeserializer} from './Deserializer.js';
 import type {BidiRealm} from './Realm.js';
 import type {Sandbox} from './Sandbox.js';
-import {releaseReference} from './util.js';
 
 /**
  * @internal
@@ -56,12 +55,7 @@ export class BidiJSHandle<T = unknown> extends JSHandle<T> {
       return;
     }
     this.#disposed = true;
-    if ('handle' in this.#remoteValue) {
+    await this.context().destroyHandles([this]);
-      await releaseReference(
-        this.context(),
-        this.#remoteValue as Bidi.Script.RemoteReference
-      );
-    }
   }
 
   get isPrimitiveValue(): boolean {

packages/puppeteer-core/src/bidi/Realm.ts

@@ -9,11 +9,12 @@ import {EventEmitter, type EventType} from '../common/EventEmitter.js';
 import {scriptInjector} from '../common/ScriptInjector.js';
 import type {EvaluateFunc, HandleFor} from '../common/types.js';
 import {
-  PuppeteerURL,
+  debugError,
-  SOURCE_URL_REGEX,
   getSourcePuppeteerURLIfAvailable,
   getSourceUrlComment,
   isString,
+  PuppeteerURL,
+  SOURCE_URL_REGEX,
 } from '../common/util.js';
 import type PuppeteerUtil from '../injected/injected.js';
 import {disposeSymbol} from '../util/disposable.js';
@@ -24,7 +25,6 @@ import {BidiDeserializer} from './Deserializer.js';
 import {BidiElementHandle} from './ElementHandle.js';
 import {BidiJSHandle} from './JSHandle.js';
 import type {Sandbox} from './Sandbox.js';
-import {BidiSerializer} from './Serializer.js';
 import {createEvaluationError} from './util.js';
 
 /**
@@ -184,7 +184,7 @@ export class BidiRealm extends EventEmitter<Record<EventType, any>> {
         arguments: args.length
           ? await Promise.all(
               args.map(arg => {
-                return BidiSerializer.serialize(sandbox, arg);
+                return sandbox.serialize(arg);
               })
             )
           : [],
@@ -207,6 +207,30 @@ export class BidiRealm extends EventEmitter<Record<EventType, any>> {
       : createBidiHandle(sandbox, result.result);
   }
 
+  async destroyHandles(handles: Array<BidiJSHandle<unknown>>): Promise<void> {
+    const handleIds = handles
+      .map(({id}) => {
+        return id;
+      })
+      .filter((id): id is string => {
+        return id !== undefined;
+      });
+    if (handleIds.length === 0) {
+      return;
+    }
+
+    await this.connection
+      .send('script.disown', {
+        target: this.target,
+        handles: handleIds,
+      })
+      .catch(error => {
+        // Exceptions might happen in case of a page been navigated or closed.
+        // Swallow these since they are harmless and we don't leak anything in this case.
+        debugError(error);
+      });
+  }
+
   [disposeSymbol](): void {
     this.connection.off(
       Bidi.ChromiumBidi.Script.EventNames.RealmCreated,

packages/puppeteer-core/src/bidi/Sandbox.ts

@@ -4,8 +4,11 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import type * as Bidi from 'chromium-bidi/lib/cjs/protocol/protocol.js';
+
 import type {JSHandle} from '../api/JSHandle.js';
 import {Realm} from '../api/Realm.js';
+import {LazyArg} from '../common/LazyArg.js';
 import type {TimeoutSettings} from '../common/TimeoutSettings.js';
 import type {EvaluateFunc, HandleFor} from '../common/types.js';
 import {withSourcePuppeteerURLIfNone} from '../common/util.js';
@@ -13,7 +16,9 @@ import {withSourcePuppeteerURLIfNone} from '../common/util.js';
 import type {BrowsingContext} from './BrowsingContext.js';
 import {BidiElementHandle} from './ElementHandle.js';
 import type {BidiFrame} from './Frame.js';
+import {BidiJSHandle} from './JSHandle.js';
 import type {BidiRealm as BidiRealm} from './Realm.js';
+import {BidiSerializer} from './Serializer.js';
 /**
  * A unique key for {@link SandboxChart} to denote the default world.
  * Realms are automatically created in the default sandbox.
@@ -120,4 +125,30 @@ export class Sandbox extends Realm {
       type: 'node',
     });
   }
+
+  async serialize(arg: unknown): Promise<Bidi.Script.LocalValue> {
+    if (arg instanceof LazyArg) {
+      arg = await arg.get(this.realm);
+    }
+    // eslint-disable-next-line rulesdir/use-using -- We want this to continue living.
+    const objectHandle =
+      arg && (arg instanceof BidiJSHandle || arg instanceof BidiElementHandle)
+        ? arg
+        : null;
+    if (objectHandle) {
+      if (
+        objectHandle.realm.environment.context() !== this.environment.context()
+      ) {
+        throw new Error(
+          'JSHandles can be evaluated only in the context they were created!'
+        );
+      }
+      if (objectHandle.disposed) {
+        throw new Error('JSHandle is disposed!');
+      }
+      return objectHandle.remoteValue() as Bidi.Script.RemoteReference;
+    }
+
+    return BidiSerializer.serialize(arg);
+  }
 }

packages/puppeteer-core/src/bidi/Serializer.ts

@@ -6,13 +6,8 @@
 
 import type * as Bidi from 'chromium-bidi/lib/cjs/protocol/protocol.js';
 
-import {LazyArg} from '../common/LazyArg.js';
 import {isDate, isPlainObject, isRegExp} from '../common/util.js';
 
-import {BidiElementHandle} from './ElementHandle.js';
-import {BidiJSHandle} from './JSHandle.js';
-import type {Sandbox} from './Sandbox.js';
-
 /**
  * @internal
  */
@@ -22,7 +17,39 @@ class UnserializableError extends Error {}
  * @internal
  */
 export class BidiSerializer {
-  static serializeNumber(arg: number): Bidi.Script.LocalValue {
+  static serialize(arg: unknown): Bidi.Script.LocalValue {
+    switch (typeof arg) {
+      case 'symbol':
+      case 'function':
+        throw new UnserializableError(`Unable to serializable ${typeof arg}`);
+      case 'object':
+        return this.#serializeObject(arg);
+
+      case 'undefined':
+        return {
+          type: 'undefined',
+        };
+      case 'number':
+        return this.#serializeNumber(arg);
+      case 'bigint':
+        return {
+          type: 'bigint',
+          value: arg.toString(),
+        };
+      case 'string':
+        return {
+          type: 'string',
+          value: arg,
+        };
+      case 'boolean':
+        return {
+          type: 'boolean',
+          value: arg,
+        };
+    }
+  }
+
+  static #serializeNumber(arg: number): Bidi.Script.LocalValue {
     let value: Bidi.Script.SpecialNumber | number;
     if (Object.is(arg, -0)) {
       value = '-0';
@@ -41,14 +68,14 @@ export class BidiSerializer {
     };
   }
 
-  static serializeObject(arg: object | null): Bidi.Script.LocalValue {
+  static #serializeObject(arg: object | null): Bidi.Script.LocalValue {
     if (arg === null) {
       return {
         type: 'null',
       };
     } else if (Array.isArray(arg)) {
       const parsedArray = arg.map(subArg => {
-        return BidiSerializer.serializeRemoteValue(subArg);
+        return this.serialize(subArg);
       });
 
       return {
@@ -70,10 +97,7 @@ export class BidiSerializer {
 
       const parsedObject: Bidi.Script.MappingLocalValue = [];
       for (const key in arg) {
-        parsedObject.push([
+        parsedObject.push([this.serialize(key), this.serialize(arg[key])]);
-          BidiSerializer.serializeRemoteValue(key),
-          BidiSerializer.serializeRemoteValue(arg[key]),
-        ]);
       }
 
       return {
@@ -99,66 +123,4 @@ export class BidiSerializer {
       'Custom object sterilization not possible. Use plain objects instead.'
     );
   }
-
-  static serializeRemoteValue(arg: unknown): Bidi.Script.LocalValue {
-    switch (typeof arg) {
-      case 'symbol':
-      case 'function':
-        throw new UnserializableError(`Unable to serializable ${typeof arg}`);
-      case 'object':
-        return BidiSerializer.serializeObject(arg);
-
-      case 'undefined':
-        return {
-          type: 'undefined',
-        };
-      case 'number':
-        return BidiSerializer.serializeNumber(arg);
-      case 'bigint':
-        return {
-          type: 'bigint',
-          value: arg.toString(),
-        };
-      case 'string':
-        return {
-          type: 'string',
-          value: arg,
-        };
-      case 'boolean':
-        return {
-          type: 'boolean',
-          value: arg,
-        };
-    }
-  }
-
-  static async serialize(
-    sandbox: Sandbox,
-    arg: unknown
-  ): Promise<Bidi.Script.LocalValue> {
-    if (arg instanceof LazyArg) {
-      arg = await arg.get(sandbox.realm);
-    }
-    // eslint-disable-next-line rulesdir/use-using -- We want this to continue living.
-    const objectHandle =
-      arg && (arg instanceof BidiJSHandle || arg instanceof BidiElementHandle)
-        ? arg
-        : null;
-    if (objectHandle) {
-      if (
-        objectHandle.realm.environment.context() !==
-        sandbox.environment.context()
-      ) {
-        throw new Error(
-          'JSHandles can be evaluated only in the context they were created!'
-        );
-      }
-      if (objectHandle.disposed) {
-        throw new Error('JSHandle is disposed!');
-      }
-      return objectHandle.remoteValue() as Bidi.Script.RemoteReference;
-    }
-
-    return BidiSerializer.serializeRemoteValue(arg);
-  }
 }

packages/puppeteer-core/src/bidi/util.ts

@@ -6,32 +6,9 @@
 
 import type * as Bidi from 'chromium-bidi/lib/cjs/protocol/protocol.js';
 
-import {PuppeteerURL, debugError} from '../common/util.js';
+import {PuppeteerURL} from '../common/util.js';
 
 import {BidiDeserializer} from './Deserializer.js';
-import type {BidiRealm} from './Realm.js';
-
-/**
- * @internal
- */
-export async function releaseReference(
-  client: BidiRealm,
-  remoteReference: Bidi.Script.RemoteReference
-): Promise<void> {
-  if (!remoteReference.handle) {
-    return;
-  }
-  await client.connection
-    .send('script.disown', {
-      target: client.target,
-      handles: [remoteReference.handle],
-    })
-    .catch(error => {
-      // Exceptions might happen in case of a page been navigated or closed.
-      // Swallow these since they are harmless and we don't leak anything in this case.
-      debugError(error);
-    });
-}
 
 /**
  * @internal