Remove unnecessary domain instance checks after "domains.get()"
Categories
(Remote Protocol :: Agent, enhancement, P3)
Tracking
(Not tracked)
People
(Reporter: impossibus, Unassigned, Mentored)
Details
(Whiteboard: [lang=js])
User Story
Check the following URL to get familiar with the remote agent: https://firefox-source-docs.mozilla.org/remote/index.html
We see a pattern in Domain subclass code like:
const Runtime = this.session.domains.get("Runtime");
if (!Runtime) {
throw new Error("Runtime domain is not instantiated");
}
To avoid repetition, we could throw the error directly in Domains.get
|
||
Comment 1•6 years ago
|
||
Actually while working on the new IO domain I had to debug exactly this get() method yesterday. And at the moment we actually implicitly create a new instance of the domain if none is available yet for the given session. If instantiation fails get() will throw an error itself. As such we won't need this extra check at all. Means we can remove it from everywhere it has been added.
Here the locations that need to be checked:
https://searchfox.org/mozilla-central/search?q=domains.get&case=false®exp=false&path=remote%2F
|
|
||
Comment 3•5 years ago
|
||
Feel free to work on this bug. When you upload a patch the bug will automatically be assigned to you. Check the user story field and the link in how to get started.
I've looked at the user story but I'm still don't know what changes I have to make. Can you guide me?
|
|
||
Comment 5•5 years ago
|
||
Oh, that's interesting. As it looks like we actually fixed that already with some other patch, so that none of the formerly mentioned instances (see my comment 1) have this extra check. Sorry for taking your time here.
If you are interested in a different bug, there is bug 1631570, which is a good starter.
Description
•