Move attachThread/detachThread code specific to the toolbox in toolbox.js

RESOLVED FIXED in Firefox 68

Status

enhancement
P2
normal
RESOLVED FIXED
9 months ago
27 days ago

People

(Reporter: ochameau, Assigned: yulia)

Tracking

(Blocks 1 bug)

unspecified
Firefox 68
Dependency tree / graph

Firefox Tracking Flags

(firefox68 fixed)

Details

(Whiteboard: dt-fission)

Attachments

(1 attachment)

Reporter

Description

9 months ago
attachThread and detachThread is 90% generic code to instantiate the thread client that has nothing specific to the toolbox.

The only two bits are specific to the toolbox:
https://searchfox.org/mozilla-central/source/devtools/client/framework/attach-thread.js#24-35
This should rather be a listener in toolbox.js, registered from toolbox.open via:
  threadClient.addListener("paused", this.handleThreadState);
  threadClient.addListener("resumed", this.handleThreadState);

And:
https://searchfox.org/mozilla-central/rev/0640ea80fbc8d48f8b197cd363e2535c95a15eb3/devtools/client/framework/attach-thread.js#82-90
This one is harder to refactor as it happens right before instantiating the thread client and doesn't emit an event. But we might be able to do something via an event?
It would be great to understand this error. Is the thread client broken after that? Can we set a flag on thread client so that toolbox.open can later check it?

Then once attachThread and detachThread are no longer having any reference to toolbox, we can inline this code into target.js (bug 1492830)
Reporter

Updated

9 months ago
Depends on: 1225492
Reporter

Comment 1

9 months ago
Note that there is also bug 1225492 for the thread-paused/thread-resumed events being emitted from handleThreadState, but that is yet another thing to refactor.
Severity: normal → enhancement
Priority: -- → P2
Reporter

Updated

8 months ago
Whiteboard: dt-fission
Assignee

Updated

8 months ago
Blocks: 1488204
Assignee

Updated

2 months ago
Assignee: nobody → ystartsev
Assignee

Comment 2

2 months ago

This is the first part of getting rid of framework/attach-thread.js -- here we move the
toolbox related logic back into the toolbox.

Comment 3

Last month
Pushed by ystartsev@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/781152317789
move toolbox specific listeners to threadClient events to the toolbox; r=jdescottes

Comment 4

Last month
bugherder
Status: NEW → RESOLVED
Closed: Last month
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68
You need to log in before you can comment on or make changes to this bug.