[devtools-rfc] Expose a good API to detect whether an actor is alive
Categories
(DevTools :: General, task)
Tracking
(firefox81 fixed)
| Tracking | Status | |
|---|---|---|
| firefox81 | --- | fixed |
People
(Reporter: julienw, Assigned: jdescottes)
References
(Blocks 1 open bug)
Details
Attachments
(4 files)
Currently, a lot of code check the proprety actorID on a front, to know if an actor is still alive.
This is a bit confusing to a reader who's not used to the devtools code, and makes it hard to change this property.
Instead we should encapsulate this check, either in a property getter (get alive()) or in a plain old method (isAlive()).
I used this crude method to detect where this is used at the moment (I use git cinnabar):
git grep -E 'if ([^)]+.actorID)' devtools/
This gives 53 results, which isn't too bad. I know this isn't a perfect filter, there are some false negatives and false positives but this gives a rough estimation.
actorID it self is used in a lot more locations (783). Ideally somebody who knows better than me would look at these locations to decide if we shouldn't provide a clean API for some of these too, but that's not the scope of this RFC.
Thoughts?
|
||
Comment 1•4 years ago
|
||
I'd love to have that (I prefer method over getters, but that's a detail).
It shouldn't require deep knowledge of the actor/fronts lifecycle to be able to know if it is still alive or not, and relying on the truthiness of what, at first sight, could be seen as unrelated property isn't helping.
|
||
Comment 2•4 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #0)
I used this crude method to detect where this is used at the moment (I use git cinnabar):
git grep -E 'if ([^)]+.actorID)' devtools/
This gives 53 results, which isn't too bad. I know this isn't a perfect filter, there are some false negatives and false positives but this gives a rough estimation.
Here is a searchfox equivalent:
https://searchfox.org/mozilla-central/search?q=if+(%5B%5E)%5D%2B.actorID)&case=false®exp=true&path=devtools
+1 it would be handy to have a more explicit getter/method. I don't have strong opinion between the two.
|
Assignee | |
Updated•4 years ago
|
|
|
Assignee | |
Comment 3•4 years ago
|
||
|
|
Assignee | |
Comment 4•4 years ago
|
||
|
|
Assignee | |
Comment 5•4 years ago
|
||
|
|
Assignee | |
Comment 6•4 years ago
|
||
Depends on D86325
Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3101d2682214 Add isDestroyed helper on DevTools Front base class r=nchevobbe https://hg.mozilla.org/integration/autoland/rev/ae85084ac8b3 Add isDestroyed helper on DevTools Actor base class r=nchevobbe https://hg.mozilla.org/integration/autoland/rev/9e2d306e37b5 Convert accessible isDestroyed getter to Actor::isDestroyed() r=yzen,nchevobbe https://hg.mozilla.org/integration/autoland/rev/6c57e1f4b41e Use Front/Actor::isDestroyed to replace actorID checks r=nchevobbe
|
||
Comment 8•4 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/3101d2682214
https://hg.mozilla.org/mozilla-central/rev/ae85084ac8b3
https://hg.mozilla.org/mozilla-central/rev/9e2d306e37b5
https://hg.mozilla.org/mozilla-central/rev/6c57e1f4b41e
|
Reporter | |
Comment 9•4 years ago
|
||
yay, thanks Julian!
Description
•