JS debugger: use addGlobalActor to add chrome debugging actor

RESOLVED FIXED in Firefox 24

Status

defect
P1
normal
RESOLVED FIXED
6 years ago
10 months ago

People

(Reporter: jimb, Assigned: jimb)

Tracking

Trunk
Firefox 24

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

6 years ago
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.
(Assignee)

Updated

6 years ago
Blocks: 797627
Status: NEW → ASSIGNED
Priority: -- → P1
(Assignee)

Updated

6 years ago
Blocks: 870081
(Assignee)

Updated

6 years ago
No longer blocks: 797627
Posted patch Patch v2 (obsolete) — Splinter Review
Rebased.
Attachment #739898 - Attachment is obsolete: true
Tested this and bug 862490 on both Fennec and b2g desktop without any issues.
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 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.
Attachment #753138 - Flags: review+
Posted patch Patch v3Splinter Review
Took care of comment 4 myself. Passing on to b2g and fennec folks for review of their respective bits.
Attachment #753138 - Attachment is obsolete: true
Attachment #753906 - Flags: review?(mark.finkle)
Attachment #753906 - Flags: review?(fabrice)
Attachment #753906 - Flags: review?(mark.finkle) → review+
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.
Attachment #753906 - Flags: review?(fabrice) → review+
Whiteboard: [land-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/978e8629b2af
Assignee: nobody → jimb
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 24

Updated

10 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.