1673897 - Remote debugging fails on requests returning arrays

Status: RESOLVED FIXED

(DevTools :: Framework, defect, P3)

Description

[Alexandre Poirot [:ochameau]]

The only visible breakage I've been able to reproduce is the blocked request sidebar in the netmonitor.

It triggers the WebConsoleActor.getBlockedUrls() request, which is specified this way:
https://searchfox.org/mozilla-central/source/devtools/shared/specs/webconsole.js#249-252

    getBlockedUrls: {
      request: {},
      response: RetVal("array:string"),
    },

Nothing special on the actor, it just returns an array.

We do go through this code in protocol.js's Actor:
https://searchfox.org/mozilla-central/source/devtools/shared/protocol/Actor.js#194,205

  response.from = this.actorID;
  ...
  conn.send(response);

The different which explains why it works within firefox, but not with remote debugging is in the transport layer:

• Within Firefox, we use the LocalDebuggerTransport:
  https://searchfox.org/mozilla-central/source/devtools/shared/transport/local-transport.js#43,53,68

  send: function(packet) {
    ...
    this._deepFreeze(packet);
    ...
    other.hooks.onPacket(packet);

deepFreeze freeze the object, but doesn't do a JSON serialization.

• For remote debugging, we use DebuggerTransport:
  https://searchfox.org/mozilla-central/source/devtools/shared/transport/transport.js#116-119

  send: function(object) {
    const packet = new JSONPacket(this);
    packet.object = object;

which uses JSONPacket, doing a JSON serialization:
https://searchfox.org/mozilla-central/source/devtools/shared/transport/packets.js#152

 set: function(object) {
    this._object = object;
    const data = JSON.stringify(object);

And this is where we loose the overloaded from attribute.

This sounds tricky to fix without changing the spec to move the RetVal("array:string") into a sub attribute in the response packet. So that we stop messing with the protocol base JSON object.

Comment 1

[Alexandre Poirot [:ochameau]]

Here is the list of requests which probably fail on remote debugging:
https://searchfox.org/mozilla-central/search?q=response%3A+RetVal%28%22array%3A&path=&case=true&regexp=false

Comment 2

[Julian Descottes [:jdescottes]]

We could also fix this in the transport layer. Maybe we should force all requests to wrap their response in an object?

Comment 3

[Julian Descottes [:jdescottes]]

Attachment: Bug 1673897 - [devtools] Wrap array packets in DebuggerTransport

Fixes the request blocking UI in remote debugging.

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:ochameau, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 5

[Julian Descottes [:jdescottes]]

Attachment: Bug 1673897 - [devtools] Stop using arrays as return values for rdp responses

Depends on D96939

Comment 6

[Julian Descottes [:jdescottes]]

Attachment: Bug 1673897 - [devtools] Throw if a spec method defines an array return value

Depends on D96940

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a423cb25ee61
[devtools] Stop using arrays as return values for rdp responses r=ochameau,devtools-backward-compat-reviewers
https://hg.mozilla.org/integration/autoland/rev/acd7500696d6
[devtools] Throw if a spec method defines an array return value r=ochameau

Comment 8

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/a423cb25ee61
https://hg.mozilla.org/mozilla-central/rev/acd7500696d6