1787997 - Extract a method to create a UUID to a shared helper

Status: RESOLVED FIXED

(Remote Protocol :: Agent, task, P3)

Description

[Julian Descottes [:jdescottes]]

In the remote codebase, we generate a lot of UUIDs, usually duplicating the same code on the fly:

https://searchfox.org/mozilla-central/search?q=generateUUID%28%29&path=remote%2F&case=false&regexp=false

  return Services.uuid
    .generateUUID()
    .toString()
    .slice(1, -1);

We already have three helpers wrapping this code in various parts of the codebase. A uuid() in cdp/, a getUUID() in webdriver-bidi/, a generateUUID in marionette/ ....

Ideally, we should have it in a shared helper under remote/shared. I am not sure if we already have a file which would be a good match here, but otherwise we can have a remote/shared/UUID.jsm.

Can probably be a mentored/good-first-bug, let's first discuss it in triage.

From https://phabricator.services.mozilla.com/D155445?id=619093#inline-857827

Comment 1

[michael s]

hi there, is it okay if i take a crack at this? thanks!

Comment 2

[michael s]

Got the codebase installed and digging into the code. wow! codebase is much more complicated than I thought. I have several questions about the codebase now! A lot of basic things are hard to figure out because VSCode isn't doing the linking for me.

• How do modules get attached to Services? I couldn't find an exact line that says 'this uuid object/module is assigned to Services'
• Are Services a binding to native code?
• Are there nuances to calling exported functions in .jsm (i.e. the new UUID.jsm file) from .mjs files?

Thanks!

Comment 3

[Julian Descottes [:jdescottes]]

(In reply to michael s from comment #1)

hi there, is it okay if i take a crack at this? thanks!

Thanks! Will assign it to you.

(In reply to michael s from comment #2)

Got the codebase installed and digging into the code. wow! codebase is much more complicated than I thought. I have several questions about the codebase now! A lot of basic things are hard to figure out because VSCode isn't doing the linking for me.

• How do modules get attached to Services? I couldn't find an exact line that says 'this uuid object/module is assigned to Services'

You can assume that Services.uuid is always available.

• Are Services a binding to native code?

Services.uuid relies on https://searchfox.org/mozilla-central/source/xpcom/base/nsIUUIDGenerator.idl and https://searchfox.org/mozilla-central/source/xpcom/base/nsUUIDGenerator.cpp

• Are there nuances to calling exported functions in .jsm (i.e. the new UUID.jsm file) from .mjs files?

The bug description is a bit old, since then we migrated all the .jsm files under remote/ to sys.mjs. The new shared module should also be a module and use a sys.mjs extension. Otherwise no there are no real differences when it comes to importing and using functions from jsm or sys.mjs files.

Let us know if you have any question!

Comment 4

[michael s]

Thanks Julian, have another question now -
I've come across different ways of importing functions - what are their differences and when should i use either or?

1

const lazy = {};

ChromeUtils.defineESModuleGetters(lazy, {
  UnsupportedError: "chrome://remote/content/cdp/Error.sys.mjs",
  generateUUID: "chrome://remote/content/shared/UUID.sys.mjs"
});
//...
const uuid = lazy.generateUUID();

2

import { ContentProcessDomain } from "chrome://remote/content/cdp/domains/ContentProcessDomain.sys.mjs";
//... soon to be
import { generateUUID } from "chrome://remote/content/shared/UUID.sys.mjs";

Thank you!

Comment 5

[michael s]

PS - Let me know any tips for how I can learn about the codebase myself :)

Also, when I use VS Code, I cannot see where this is implemented (or referenced if I'm at the source):
import { XPCOMUtils } from "resource://gre/modules/XPCOMUtils.sys.mjs";
or

ChromeUtils.defineESModuleGetters(lazy, {
  Addon: "chrome://remote/content/marionette/addon.sys.mjs",

Is this expected or have I not fully set-up VS Code? Thanks!

Comment 6

[Julian Descottes [:jdescottes]]

(In reply to michael s from comment #4)

Thanks Julian, have another question now -
I've come across different ways of importing functions - what are their differences and when should i use either or?

1

const lazy = {};

ChromeUtils.defineESModuleGetters(lazy, {
  UnsupportedError: "chrome://remote/content/cdp/Error.sys.mjs",
  generateUUID: "chrome://remote/content/shared/UUID.sys.mjs"
});
//...
const uuid = lazy.generateUUID();

2

import { ContentProcessDomain } from "chrome://remote/content/cdp/domains/ContentProcessDomain.sys.mjs";
//... soon to be
import { generateUUID } from "chrome://remote/content/shared/UUID.sys.mjs";

Thank you!

The first option is a uses "lazy loading" to make sure we only load the module when we need to access it. This is the preferred way of importing modules. The only reason you would use option 2 is if module B is used immediately when module A is loaded. And in any case the linter should tell you about this, so always use option 1. And if the linter complains because the module is used immediately switch to option 2.

I think in most cases here option 1 (lazy loading) should be enough.

(In reply to michael s from comment #5)

PS - Let me know any tips for how I can learn about the codebase myself :)

Also, when I use VS Code, I cannot see where this is implemented (or referenced if I'm at the source):
import { XPCOMUtils } from "resource://gre/modules/XPCOMUtils.sys.mjs";
or

ChromeUtils.defineESModuleGetters(lazy, {
  Addon: "chrome://remote/content/marionette/addon.sys.mjs",

Is this expected or have I not fully set-up VS Code? Thanks!

I'm not using VS code so I'm not sure, but the protocols and paths used when importing modules in Firefox use special rules & directives to map to the filesystem, so it's very likely that VS code is not able to resolve those? There's a page about setting up VS code on our documentation https://firefox-source-docs.mozilla.org/contributing/editors/vscode.html, but I'm guessing it mostly helps with C++ development?

You can learn more about the build system at https://firefox-source-docs.mozilla.org/build/buildsystem/index.html

Comment 7

[michael s]

Okay I've got it working! was running through some testing issues but I figured out it was because I didn't add the file to jar.mn
Now i've hit a roadblock with submitting the changes to Phabricator. I wrote a message here:

https://matrix.to/#/!ykhjAGdElNmYcYkwvB:mozilla.org/$BVzW-aFJsBsXR0cTh2GBOlkK8nMJpQhMnbybMGT-W8I?via=mozilla.org&via=matrix.org&via=kde.org

Please let me know what I can do differently. Thank you!

Comment 8

[michael s]

Attachment: Bug 1787997 - Refactor remote UUID Usage to a Shared UUID Module r=jdescottes

Comment 9

[michael s]

Got moz-phab to work! I committed my changes to a branch, rebased it to a ref that moz-phab said (Commit 2f6c9949625d is not a child of 18aa3eff184c, unable to continue), and then the submit worked.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d80adca627e1
Refactor remote UUID Usage to a Shared UUID Module r=jdescottes,webdriver-reviewers

Comment 11

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/d80adca627e1