Closed Bug 1493714 Opened 2 years ago Closed 2 years ago

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

Categories

(DevTools :: Framework, enhancement, P1)

enhancement

Tracking

(firefox68 fixed)

RESOLVED FIXED
Firefox 68
Tracking Status
firefox68 --- fixed

People

(Reporter: ochameau, Assigned: ystartsev)

References

(Blocks 1 open bug)

Details

(Whiteboard: dt-fission dt-fission-m1)

Attachments

(1 file)

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)
Depends on: 1225492
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
Whiteboard: dt-fission
Blocks: dbg-fission
Assignee: nobody → ystartsev

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

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
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68
Priority: P2 → P1
Whiteboard: dt-fission → dt-fission dt-fission-m1
You need to log in before you can comment on or make changes to this bug.