863936 - JS debugger: use addGlobalActor to add chrome debugging actor

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P1)

Description

[Jim Blandy :jimb]

Attachment: Use DebuggerServer.addGlobalActor to register ChromeDebuggerActor, instead of writing out code with the same effect.

At present, BrowserRootActor.onListTabs includes code to create the chrome debugging actor and include it on the "listTabs" reply. However, using DebuggerServer.addGlobalActor to register the chrome debugging actor would have almost exactly the same effect, and require no bespoke support in BrowserRootActor.prototype.onListTabs.

The attached patch makes this change.

Comment 1

[Panos Astithas (he/him) [:past] (please ni?)]

Aww yes!

Comment 2

[Panos Astithas (he/him) [:past] (please ni?)]

Attachment: Patch v2

Rebased.

Comment 3

[Panos Astithas (he/him) [:past] (please ni?)]

Tested this and bug 862490 on both Fennec and b2g desktop without any issues.

Comment 4

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 753138 [details] [diff] [review]
Patch v2

Review of attachment 753138 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/devtools/server/actors/webbrowser.js
@@ -162,5 @@
> -      actor = new ChromeDebuggerActor(this);
> -      actor.parentID = this.actorID;
> -      this._chromeDebugger = actor;
> -      actorPool.addActor(actor);
> -    }

This removed block exists in the other two dbg-browser-actor.js files as well.

@@ -207,5 @@
>      let response = {
>        "from": "root",
>        "selected": selected,
> -      "tabs": [actor.grip() for (actor of tabActorList)],
> -      "chromeDebugger": this._chromeDebugger.actorID

This one, too.

Comment 5

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 753138 [details] [diff] [review]
Patch v2

Review of attachment 753138 [details] [diff] [review]:
-----------------------------------------------------------------

This seems fine as long the redundant bits mentioned in comment 4 are removed.

Comment 6

[Panos Astithas (he/him) [:past] (please ni?)]

Attachment: Patch v3

Took care of comment 4 myself. Passing on to b2g and fennec folks for review of their respective bits.

Comment 7

[Panos Astithas (he/him) [:past] (please ni?)]

Fabrice, I tested this patch along with the one from bug 870081 on my Unagi and verified that the chrome debugging actor was not present.

Comment 8

[Panos Astithas (he/him) [:past] (please ni?)]

https://hg.mozilla.org/integration/fx-team/rev/978e8629b2af

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/978e8629b2af