1450284 - Finish conversion of ThreadActor to protocol.js

Status: RESOLVED FIXED

(DevTools :: Debugger, task, P3)

Description

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

In bug 1256397, Eddy landed a few parts of converting ThreadActor to protocol.js, but there's still more work that remains:

* `attach` and potentially other request handlers may need special treatment in protocol.js due to the nested event loops used in the debugger
  * https://bugzilla.mozilla.org/attachment.cgi?id=8730334&action=diff from bug 1256397 is an attempt at this that was never finished
* The spec[1] should actually define the methods
  * File should also be renamed to `thread` to match the actor
* The old-style request handlers[2] should be removed and actor methods renamed, just like other conversions of actors to p.js

Additional conversion tips copied from :ochameau:

* `actorPrefix` becomes `typeName` on the spec object,
* `grip()` function becomes `form()` on the actor,
* We can remove `from` attributes in all in returned responses and only keep actual data you want to send to the client,
* remember calling `protocol.Actor.prototype.initialize.call(this, conn);` from your actor's `initialize` method to get events to work as expected and `protocol.Actor.prototype.destroy.call(this);` if you overload `destroy` method, in order to cleans things up correctly.

[1]: https://searchfox.org/mozilla-central/rev/57bbc1ac58816dc054df242948f3ecf75e12df5f/devtools/shared/specs/script.js#11
[2]: https://searchfox.org/mozilla-central/rev/57bbc1ac58816dc054df242948f3ecf75e12df5f/devtools/server/actors/thread.js#1730

Comment 1

[Yulia Startsev [:yulia] OOO until July 2026]

As mentioned in 1473511 -- we should also rename the destroy and exit methods, as they are a bit misleading in what they are doing. Destroy is doing a cleanup of the actor, and is called on detach and onReleaseMany, as well as from exit. The real "destroy" happens in `exit`

Comment 2

[Yulia Startsev [:yulia] OOO until July 2026]

Attachment: Bug 1450284 - Complete method set in the Spec for Thread; r=ochameau,jdescottes

We need a complete specification in order to move forward with the front conversion. I
think this will also impact other parts of the refactoring, such as some of the thread specific code
in the debugger-client.

Comment 3

[Yulia Startsev [:yulia] OOO until July 2026]

Attachment: Bug 1450284 - rename script spec to thread.js; r=jdescottes

Rather straighforward -- as per jryans recommendation, I have renamed the file to reflect
our naming conventions.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dd5b92f89835
Complete method set in the Spec for Thread; r=jdescottes,ochameau
https://hg.mozilla.org/integration/autoland/rev/7e6b874b870a
rename script spec to thread.js; r=jdescottes

Comment 5

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/dd5b92f89835
https://hg.mozilla.org/mozilla-central/rev/7e6b874b870a

Comment 6

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

Does this still need to remain open (and blocking bug 1494796)?

Comment 7

[Yulia Startsev [:yulia] OOO until July 2026]

This needs to remain open. there will be more patches landing for this as part of the patch queue for the conversion.

Comment 8

[BugBot [:suhaib / :marco]]

The leave-open keyword is there and there is no activity for 6 months.
:yulia, maybe it's time to close this bug?

Comment 9

[Yulia Startsev [:yulia] OOO until July 2026]

It looks like the deprecatedThreadActor.js file was removed, which is what this was waiting on. I am not sure when this happened, or in which bug however. This can be closed.