1679808 - ExecutionContext._toRemoteObject() has to better handle certain objects (Promises, plain objects, window)

Status: RESOLVED WONTFIX

(Remote Protocol :: CDP, defect, P2)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Seen while working on 1553854 to get the Runtime.consoleAPICalled event implemented.

When Runtime.callFunctionOn is called with a plain object it's not getting added as remote object, but returned as value:

 0:23.72 pid:8375 1606751557576	RemoteAgent	TRACE	(connection {5e521551-f815-914d-b7ff-a61a3e16c607})-> {"sessionId":"bedc4580-8b0d-b24a-9b9d-b36cda2d8089","method":"Runtime.callFunctionOn","params":{"functionDeclaration":"() => console.log('hello', 5, { foo: 'bar' })\n//# sourceURL=__puppeteer_evaluation_script__\n","executionContextId":1,"arguments":[],"returnByValue":true,"awaitPromise":true,"userGesture":true},"id":17}
 0:23.73 pid:8375 1606751557582	RemoteAgent	TRACE	<-(connection {5e521551-f815-914d-b7ff-a61a3e16c607}) {"sessionId":"bedc4580-8b0d-b24a-9b9d-b36cda2d8089","method":"Runtime.consoleAPICalled","params":{"args":[{"type":"string","value":"hello"},{"type":"number","value":5},{"type":"object","value":{"foo":"bar"}}],"executionContextId":1,"timestamp":1606751557576,"type":"log"}}

Expected payload as returned (here from Chrome):

 0:24.82 pid:92991 2020-11-27T20:50:48.571Z puppeteer:protocol:RECV ◀ {"method":"Runtime.consoleAPICalled","params":{"type":"log","args":[{"type":"string","value":"hello"},{"type":"number","value":5,"description":"5"}, {"type":"object","className":"Object","description":"Object","objectId":"{\"injectedScriptId\":1,\"id\":1}"}], "executionContextId":1,"timestamp":1.606510248570928e+12,"stackTrace":{"callFrames":[{"functionName":"","scriptId":"5","url":"__puppeteer_evaluation_script__","lineNumber":0,"columnNumber":15}]}},"sessionId":"4DE563AA3B302620283F1775449148DA"}

As such code as executed later cannot reference this object via the following code and would fail:

0:24.29 pid:95792 2020-11-27T21:18:14.747Z puppeteer:protocol:SEND ► {"sessionId":"CA573CEE3C03EDDB9EA9DF8321D8F9F8","method":"Runtime.callFunctionOn","params":{"functionDeclaration":"function() { return this; }","objectId":"{\"injectedScriptId\":1,\"id\":1}","returnByValue":true,"awaitPromise":true},"id":18}
 0:24.29 pid:95792 2020-11-27T21:18:14.748Z puppeteer:protocol:RECV ◀ {"id":18,"result":{"result":{"type":"object","value":{"foo":"bar"}}},"sessionId":"CA573CEE3C03EDDB9EA9DF8321D8F9F8"}

While trying to get this implemented I'm hitting the problem that all the object we store are actually Debugger.Object instances. But how do I get the plain object converted?

Code like the following in _toRemoteObject() fails:

    if (result.type == "object") {
      result.className = "Object";
      result.description = "Object";
      result.objectId = this.setRemoteObject(debuggerObj);
      return result;
    }

Julian, do DevTools have a similar case where we could borrow code?

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

The same has to actually happen for Promises. Chrome returns the following payload:

 0:22.67 pid:14268 2020-11-30T17:14:41.560Z puppeteer:protocol:RECV ◀ {"method":"Runtime.consoleAPICalled","params":{"type":"log","args":[{"type":"object","subtype":"promise","className":"Promise","description":"Promise","objectId":"{\"injectedScriptId\":1,\"id\":1}","preview":{"type":"object","subtype":"promise","description":"Promise","overflow":false,"properties":[{"name":"[[PromiseStatus]]","type":"string","value":"resolved"},{"name":"[[PromiseValue]]","type":"string","value":"should not wait until resolved!"}]}}],"executionContextId":1,"timestamp":1.606756481557949e+12,"stackTrace":{"callFrames":[{"functionName":"","scriptId":"5","url":"__puppeteer_evaluation_script__","lineNumber":8,"columnNumber":24}]}},"sessionId":"3CB9876ADDBC65554D55A2BD3B9EA135"}

Firefox currently returns a plain object:

 0:25.46 pid:14508 1606756617630	RemoteAgent	TRACE	<-(connection {305781c0-95ae-a743-99ff-efc42188322d}) {"sessionId":"73497df2-3187-5f43-a2ae-282bad80212a","method":"Runtime.consoleAPICalled","params":{"args":[{"type":"object","value":{}}],"executionContextId":1,"timestamp":1606756617627,"type":"log"}}

Comment 2

[Julian Descottes [:jdescottes]]

In devtools we create debugger object using the following helper: https://searchfox.org/mozilla-central/rev/168c45a7acc44e9904cfd4eebcb9eb080e05699c/devtools/server/actors/object/utils.js#403-418

function makeDebuggeeValue(targetActor, value) {
  if (isObject(value)) {
    try {
      const global = Cu.getGlobalForObject(value);
      const dbgGlobal = targetActor.dbg.makeGlobalObjectReference(global);
      return dbgGlobal.makeDebuggeeValue(value);
    } catch (ex) {
      // The above can throw an exception if value is not an actual object
      // or 'Object in compartment marked as invisible to Debugger'
    }
  }
  const dbgGlobal = targetActor.dbg.makeGlobalObjectReference(
    targetActor.window || targetActor.workerGlobal
  );
  return dbgGlobal.makeDebuggeeValue(value);
}

The targetActor.dbg is a "Debugger" instance created via the make-debugger.js helper (https://searchfox.org/mozilla-central/source/devtools/server/actors/utils/make-debugger.js). The "Debugger" created by require("Debugger") is actually mapped to:

defineLazyGetter(exports.modules, "Debugger", () => {
  const global = Cu.getGlobalForObject(this);
  // Debugger may already have been added.
  if (global.Debugger) {
    return global.Debugger;
  }
  const { addDebuggerToGlobal } = ChromeUtils.import(
    "resource://gre/modules/jsdebugger.jsm"
  );
  addDebuggerToGlobal(global);
  return global.Debugger;
});

https://searchfox.org/mozilla-central/rev/168c45a7acc44e9904cfd4eebcb9eb080e05699c/devtools/shared/builtin-modules.js#217-228

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

This actually also applies to window. When it gets returned, Chrome sends the following:

{"type":"object","className":"Window","description":"Window","objectId":"{\"injectedScriptId\":1,\"id\":1}","preview":{"type":"object","description":"Window","overflow":true,"properties":[{"name":"parent","type":"object","value":"Window"}

Whereby in our case we completely fail to send the event from the content session to the TabSession. See bug 1680443.

Comment 5

[xmo]

Copying over from my duplicate report (1734587): given

class Foo {
    constructor() {
        // show recursive behaviour for rich objects
        this.bar = new TypeError("an error");
    }
    async thing() {
        await 1;
    }
}
console.log(new Foo());

Firefox sends this payload over Runtime.consoleAPICalled:

{'args': [{'type': 'object', 'value': {'bar': {}}}],
 'executionContextId': 2,
 'stackTrace': {'callFrames': []},
 'timestamp': 1633605521787,
 'type': 'log'}

The stacktrace is empty (which is probably related to 1548480), and more problematically the RemoteObject is missing every non-required property which leaves the client with very little to work with.

The missing properties which I think are pretty critical here are:

• objectId to be able to inspect the object (though ideally it would not be necessary) and is documented as [present] "for non-primitive values".
• className, documented as "Specified for object type values only", especially as this is available through the normal console.
• subtype, not visible in that example but if new Foo() is replaced by Promise.resolve(1) then Chrome provides it which, alongside the objectId, allows calling Runtime.awaitPromise.

For reference this is what Chrome returns:

{'args': [{'className': 'Foo',
           'description': 'Foo',
           'objectId': '402199672542313785.4.1',
           'preview': {'description': 'Foo',
                       'overflow': False,
                       'properties': [{'name': 'bar',
                                       'subtype': 'error',
                                       'type': 'object',
                                       'value': 'TypeError: an error\n'
                                                '    at new Foo '
                                                '(<anonymous>:5:20)\n'
                                                '    at <anonymous>:11:13'}],
                       'type': 'object'},
           'type': 'object'}],
 'executionContextId': 4,
 'stackTrace': {'callFrames': [{'columnNumber': 8,
                                'functionName': '',
                                'lineNumber': 10,
                                'scriptId': '7',
                                'url': ''}]},
 'timestamp': 1633605689964.1672,
 'type': 'log'}

however

• It doesn't have value and has preview, preview is much richer but is considered EXPERIMENTAL by the CDTP documentation so seems fair enough not to support it (even if annoying for non-primitive properties).
• It always captures a stacktrace.
• Per documentation, it also captures async strack traces if the console call type is one of assert, error, trace, warning.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

The serialization and deserialization of remote objects work fine with WebDriver BiDi. We are not going to improve CDP at this point.