chore: follow up to address evaluation review comments (#2380)

src/chromium/crAccessibility.ts

@@ -95,7 +95,7 @@ class CRAXNode implements accessibility.AXNode {
   }
 
   async _findElement(element: dom.ElementHandle): Promise<CRAXNode | null> {
-    const objectId = element._objectId!;
+    const objectId = element._objectId;
     const {node: {backendNodeId}} = await this._client.send('DOM.describeNode', { objectId });
     const needle = this.find(node => node._payload.backendDOMNodeId === backendNodeId);
     return needle || null;

src/chromium/crExecutionContext.ts

@@ -21,7 +21,7 @@ import { getExceptionMessage, releaseObject } from './crProtocolHelper';
 import { Protocol } from './protocol';
 import * as js from '../javascript';
 import * as debugSupport from '../debug/debugSupport';
-import { RemoteObject, parseEvaluationResultValue } from '../remoteObject';
+import { parseEvaluationResultValue } from '../utilityScriptSerializers';
 
 export class CRExecutionContext implements js.ExecutionContextDelegate {
   _client: CRSession;
@@ -32,14 +32,14 @@ export class CRExecutionContext implements js.ExecutionContextDelegate {
     this._contextId = contextPayload.id;
   }
 
-  async rawEvaluate(expression: string): Promise<RemoteObject> {
+  async rawEvaluate(expression: string): Promise<string> {
     const { exceptionDetails, result: remoteObject } = await this._client.send('Runtime.evaluate', {
       expression: debugSupport.ensureSourceUrl(expression),
       contextId: this._contextId,
     }).catch(rewriteError);
     if (exceptionDetails)
       throw new Error('Evaluation failed: ' + getExceptionMessage(exceptionDetails));
-    return remoteObject;
+    return remoteObject.objectId!;
   }
 
   async evaluate(context: js.ExecutionContext, returnByValue: boolean, pageFunction: Function | string, ...args: any[]): Promise<any> {
@@ -100,6 +100,10 @@ export class CRExecutionContext implements js.ExecutionContextDelegate {
     return result;
   }
 
+  createHandle(context: js.ExecutionContext, remoteObject: Protocol.Runtime.RemoteObject): js.JSHandle {
+    return new js.JSHandle(context, remoteObject.subtype || remoteObject.type, remoteObject.objectId, potentiallyUnserializableValue(remoteObject));
+  }
+
   async releaseHandle(handle: js.JSHandle): Promise<void> {
     if (!handle._objectId)
       return;
@@ -129,3 +133,9 @@ function rewriteError(error: Error): Protocol.Runtime.evaluateReturnValue {
     error.message += ' Are you passing a nested JSHandle?';
   throw error;
 }
+
+function potentiallyUnserializableValue(remoteObject: Protocol.Runtime.RemoteObject): any {
+  const value = remoteObject.value;
+  const unserializableValue = remoteObject.unserializableValue;
+  return unserializableValue ? js.parseUnserializableValue(unserializableValue) : value;
+}

src/dom.ts

@@ -65,9 +65,9 @@ export class FrameExecutionContext extends js.ExecutionContext {
     }, Number.MAX_SAFE_INTEGER, waitForNavigations ? undefined : { noWaitAfter: true });
   }
 
-  createHandle(remoteObject: any): js.JSHandle {
+  createHandle(remoteObject: js.RemoteObject): js.JSHandle {
     if (this.frame._page._delegate.isElementHandle(remoteObject))
-      return new ElementHandle(this, remoteObject);
+      return new ElementHandle(this, remoteObject.objectId!);
     return super.createHandle(remoteObject);
   }
 
@@ -81,7 +81,7 @@ export class FrameExecutionContext extends js.ExecutionContext {
           ${custom.join(',\n')}
         ])
       `;
-      this._injectedPromise = this._delegate.rawEvaluate(source).then(object => this.createHandle(object));
+      this._injectedPromise = this._delegate.rawEvaluate(source).then(objectId => new js.JSHandle(this, 'object', objectId));
     }
     return this._injectedPromise;
   }
@@ -90,9 +90,11 @@ export class FrameExecutionContext extends js.ExecutionContext {
 export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
   readonly _context: FrameExecutionContext;
   readonly _page: Page;
+  readonly _objectId: string;
 
-  constructor(context: FrameExecutionContext, remoteObject: any) {
-    super(context, remoteObject);
+  constructor(context: FrameExecutionContext, objectId: string) {
+    super(context, 'node', objectId);
+    this._objectId = objectId;
     this._context = context;
     this._page = context.frame._page;
   }

src/firefox/ffExecutionContext.ts

@@ -20,7 +20,7 @@ import * as js from '../javascript';
 import { FFSession } from './ffConnection';
 import { Protocol } from './protocol';
 import * as debugSupport from '../debug/debugSupport';
-import { RemoteObject, parseEvaluationResultValue } from '../remoteObject';
+import { parseEvaluationResultValue } from '../utilityScriptSerializers';
 
 export class FFExecutionContext implements js.ExecutionContextDelegate {
   _session: FFSession;
@@ -31,14 +31,14 @@ export class FFExecutionContext implements js.ExecutionContextDelegate {
     this._executionContextId = executionContextId;
   }
 
-  async rawEvaluate(expression: string): Promise<RemoteObject> {
+  async rawEvaluate(expression: string): Promise<string> {
     const payload = await this._session.send('Runtime.evaluate', {
       expression: debugSupport.ensureSourceUrl(expression),
       returnByValue: false,
       executionContextId: this._executionContextId,
     }).catch(rewriteError);
     checkException(payload.exceptionDetails);
-    return payload.result!;
+    return payload.result!.objectId!;
   }
 
   async evaluate(context: js.ExecutionContext, returnByValue: boolean, pageFunction: Function | string, ...args: any[]): Promise<any> {
@@ -97,6 +97,10 @@ export class FFExecutionContext implements js.ExecutionContextDelegate {
     return result;
   }
 
+  createHandle(context: js.ExecutionContext, remoteObject: Protocol.Runtime.RemoteObject): js.JSHandle {
+    return new js.JSHandle(context, remoteObject.subtype || remoteObject.type || '', remoteObject.objectId, potentiallyUnserializableValue(remoteObject));
+  }
+
   async releaseHandle(handle: js.JSHandle): Promise<void> {
     if (!handle._objectId)
       return;
@@ -135,3 +139,9 @@ function rewriteError(error: Error): (Protocol.Runtime.evaluateReturnValue | Pro
     error.message += ' Are you passing a nested JSHandle?';
   throw error;
 }
+
+function potentiallyUnserializableValue(remoteObject: Protocol.Runtime.RemoteObject): any {
+  const value = remoteObject.value;
+  const unserializableValue = remoteObject.unserializableValue;
+  return unserializableValue ? js.parseUnserializableValue(unserializableValue) : value;
+}

src/firefox/ffPage.ts

@@ -373,7 +373,7 @@ export class FFPage implements PageDelegate {
   async getContentFrame(handle: dom.ElementHandle): Promise<frames.Frame | null> {
     const { contentFrameId } = await this._session.send('Page.describeNode', {
       frameId: handle._context.frame._id,
-      objectId: handle._objectId!,
+      objectId: handle._objectId,
     });
     if (!contentFrameId)
       return null;
@@ -383,7 +383,7 @@ export class FFPage implements PageDelegate {
   async getOwnerFrame(handle: dom.ElementHandle): Promise<string | null> {
     const { ownerFrameId } = await this._session.send('Page.describeNode', {
       frameId: handle._context.frame._id,
-      objectId: handle._objectId!,
+      objectId: handle._objectId
     });
     return ownerFrameId || null;
   }
@@ -414,7 +414,7 @@ export class FFPage implements PageDelegate {
   async scrollRectIntoViewIfNeeded(handle: dom.ElementHandle, rect?: types.Rect): Promise<'success' | 'invisible'> {
     return await this._session.send('Page.scrollIntoViewIfNeeded', {
       frameId: handle._context.frame._id,
-      objectId: handle._objectId!,
+      objectId: handle._objectId,
       rect,
     }).then(() => 'success' as const).catch(e => {
       if (e instanceof Error && e.message.includes('Node is detached from document'))
@@ -433,7 +433,7 @@ export class FFPage implements PageDelegate {
   async getContentQuads(handle: dom.ElementHandle): Promise<types.Quad[] | null> {
     const result = await this._session.send('Page.getContentQuads', {
       frameId: handle._context.frame._id,
-      objectId: handle._objectId!,
+      objectId: handle._objectId,
     }).catch(logError(this._page));
     if (!result)
       return null;
@@ -452,7 +452,7 @@ export class FFPage implements PageDelegate {
   async adoptElementHandle<T extends Node>(handle: dom.ElementHandle<T>, to: dom.FrameExecutionContext): Promise<dom.ElementHandle<T>> {
     const result = await this._session.send('Page.adoptNode', {
       frameId: handle._context.frame._id,
-      objectId: handle._objectId!,
+      objectId: handle._objectId,
       executionContextId: (to._delegate as FFExecutionContext)._executionContextId
     });
     if (!result.remoteObject)

src/injected/utilityScript.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { serializeAsCallArgument, parseEvaluationResultValue } from '../remoteObject';
+import { serializeAsCallArgument, parseEvaluationResultValue } from '../utilityScriptSerializers';
 
 export default class UtilityScript {
   evaluate(returnByValue: boolean, expression: string) {

src/javascript.ts

@@ -19,12 +19,18 @@ import * as dom from './dom';
 import * as utilityScriptSource from './generated/utilityScriptSource';
 import { InnerLogger } from './logger';
 import * as debugSupport from './debug/debugSupport';
-import { RemoteObject, serializeAsCallArgument } from './remoteObject';
+import { serializeAsCallArgument } from './utilityScriptSerializers';
+
+export type RemoteObject = {
+  objectId?: string,
+  value?: any
+};
 
 export interface ExecutionContextDelegate {
   evaluate(context: ExecutionContext, returnByValue: boolean, pageFunction: string | Function, ...args: any[]): Promise<any>;
-  rawEvaluate(pageFunction: string): Promise<RemoteObject>;
+  rawEvaluate(pageFunction: string): Promise<string>;
   getProperties(handle: JSHandle): Promise<Map<string, JSHandle>>;
+  createHandle(context: ExecutionContext, remoteObject: RemoteObject): JSHandle;
   releaseHandle(handle: JSHandle): Promise<void>;
   handleJSONValue<T>(handle: JSHandle<T>): Promise<T>;
 }
@@ -62,13 +68,13 @@ export class ExecutionContext {
   utilityScript(): Promise<JSHandle> {
     if (!this._utilityScriptPromise) {
       const source = `new (${utilityScriptSource.source})()`;
-      this._utilityScriptPromise = this._delegate.rawEvaluate(source).then(object => this.createHandle(object));
+      this._utilityScriptPromise = this._delegate.rawEvaluate(source).then(objectId => new JSHandle(this, 'object', objectId));
     }
     return this._utilityScriptPromise;
   }
 
   createHandle(remoteObject: RemoteObject): JSHandle {
-    return new JSHandle(this, remoteObject);
+    return this._delegate.createHandle(this, remoteObject);
   }
 }
 
@@ -79,14 +85,11 @@ export class JSHandle<T = any> {
   readonly _value: any;
   private _type: string;
 
-  constructor(context: ExecutionContext, remoteObject: RemoteObject) {
+  constructor(context: ExecutionContext, type: string, objectId?: string, value?: any) {
     this._context = context;
-    this._objectId = remoteObject.objectId;
-    // Remote objects for primitive (or unserializable) objects carry value.
-    this._value = potentiallyUnserializableValue(remoteObject);
-    // WebKit does not have a 'promise' type.
-    const isPromise = remoteObject.className === 'Promise';
-    this._type = isPromise ? 'promise' : remoteObject.subtype || remoteObject.type || 'object';
+    this._objectId = objectId;
+    this._value = value;
+    this._type = type;
   }
 
   async evaluate<R, Arg>(pageFunction: types.FuncOn<T, Arg, R>, arg: Arg): Promise<R>;
@@ -207,13 +210,7 @@ export async function prepareFunctionCall(
   return { functionText, values: [ args.length, ...args ], handles: resultHandles, dispose };
 }
 
-function potentiallyUnserializableValue(remoteObject: RemoteObject): any {
-  const value = remoteObject.value;
-  let unserializableValue = remoteObject.unserializableValue;
-  if (remoteObject.type === 'number' && value === null)
-    unserializableValue = remoteObject.description;
-  if (!unserializableValue)
-    return value;
+export function parseUnserializableValue(unserializableValue: string): any {
   if (unserializableValue === 'NaN')
     return NaN;
   if (unserializableValue === 'Infinity')
@@ -222,5 +219,4 @@ function potentiallyUnserializableValue(remoteObject: RemoteObject): any {
     return -Infinity;
   if (unserializableValue === '-0')
     return -0;
-  return undefined;
 }

src/utilityScriptSerializers.ts

@@ -16,16 +16,6 @@
 
 // This file can't have dependencies, it is a part of the utility script.
 
-export type RemoteObject = {
-  type?: string,
-  subtype?: string,
-  className?: string,
-  objectId?: string,
-  value?: any,
-  unserializableValue?: string
-  description?: string
-};
-
 export function parseEvaluationResultValue(value: any, handles: any[] = []): any {
   // { type: 'undefined' } does not even have value.
   if (value === 'undefined')

src/webkit/wkExecutionContext.ts

@@ -20,7 +20,7 @@ import { helper } from '../helper';
 import { Protocol } from './protocol';
 import * as js from '../javascript';
 import * as debugSupport from '../debug/debugSupport';
-import { RemoteObject, parseEvaluationResultValue } from '../remoteObject';
+import { parseEvaluationResultValue } from '../utilityScriptSerializers';
 
 export class WKExecutionContext implements js.ExecutionContextDelegate {
   private readonly _session: WKSession;
@@ -40,7 +40,7 @@ export class WKExecutionContext implements js.ExecutionContextDelegate {
     this._contextDestroyedCallback();
   }
 
-  async rawEvaluate(expression: string): Promise<RemoteObject> {
+  async rawEvaluate(expression: string): Promise<string> {
     const contextId = this._contextId;
     const response = await this._session.send('Runtime.evaluate', {
       expression: debugSupport.ensureSourceUrl(expression),
@@ -49,7 +49,7 @@ export class WKExecutionContext implements js.ExecutionContextDelegate {
     });
     if (response.wasThrown)
       throw new Error('Evaluation failed: ' + response.result.description);
-    return response.result;
+    return response.result.objectId!;
   }
 
   async evaluate(context: js.ExecutionContext, returnByValue: boolean, pageFunction: Function | string, ...args: any[]): Promise<any> {
@@ -154,6 +154,11 @@ export class WKExecutionContext implements js.ExecutionContextDelegate {
     return result;
   }
 
+  createHandle(context: js.ExecutionContext, remoteObject: Protocol.Runtime.RemoteObject): js.JSHandle {
+    const isPromise = remoteObject.className === 'Promise';
+    return new js.JSHandle(context, isPromise ? 'promise' : remoteObject.subtype || remoteObject.type, remoteObject.objectId, potentiallyUnserializableValue(remoteObject));
+  }
+
   async releaseHandle(handle: js.JSHandle): Promise<void> {
     if (!handle._objectId)
       return;
@@ -183,3 +188,9 @@ const contextDestroyedResult = {
     description: 'Protocol error: Execution context was destroyed, most likely because of a navigation.'
   } as Protocol.Runtime.RemoteObject
 };
+
+function potentiallyUnserializableValue(remoteObject: Protocol.Runtime.RemoteObject): any {
+  const value = remoteObject.value;
+  const unserializableValue = remoteObject.type === 'number' && value === null ? remoteObject.description : undefined;
+  return unserializableValue ? js.parseUnserializableValue(unserializableValue) : value;
+}

src/webkit/wkPage.ts

@@ -710,7 +710,7 @@ export class WKPage implements PageDelegate {
 
   async getContentFrame(handle: dom.ElementHandle): Promise<frames.Frame | null> {
     const nodeInfo = await this._session.send('DOM.describeNode', {
-      objectId: handle._objectId!
+      objectId: handle._objectId
     });
     if (!nodeInfo.contentFrameId)
       return null;
@@ -751,7 +751,7 @@ export class WKPage implements PageDelegate {
 
   async scrollRectIntoViewIfNeeded(handle: dom.ElementHandle, rect?: types.Rect): Promise<'success' | 'invisible'> {
     return await this._session.send('DOM.scrollIntoViewIfNeeded', {
-      objectId: handle._objectId!,
+      objectId: handle._objectId,
       rect,
     }).then(() => 'success' as const).catch(e => {
       if (e instanceof Error && e.message.includes('Node does not have a layout object'))
@@ -771,7 +771,7 @@ export class WKPage implements PageDelegate {
 
   async getContentQuads(handle: dom.ElementHandle): Promise<types.Quad[] | null> {
     const result = await this._session.send('DOM.getContentQuads', {
-      objectId: handle._objectId!
+      objectId: handle._objectId
     }).catch(logError(this._page));
     if (!result)
       return null;
@@ -788,13 +788,13 @@ export class WKPage implements PageDelegate {
   }
 
   async setInputFiles(handle: dom.ElementHandle<HTMLInputElement>, files: types.FilePayload[]): Promise<void> {
-    const objectId = handle._objectId!;
+    const objectId = handle._objectId;
     await this._session.send('DOM.setInputFiles', { objectId, files: dom.toFileTransferPayload(files) });
   }
 
   async adoptElementHandle<T extends Node>(handle: dom.ElementHandle<T>, to: dom.FrameExecutionContext): Promise<dom.ElementHandle<T>> {
     const result = await this._session.send('DOM.resolveNode', {
-      objectId: handle._objectId!,
+      objectId: handle._objectId,
       executionContextId: (to._delegate as WKExecutionContext)._contextId
     }).catch(logError(this._page));
     if (!result || result.object.subtype === 'null')