Closed Bug 1478595 Opened 5 years ago Closed 4 years ago

Use XPCOMUtils.defineLazyPreferenceGetter instead of Services.prefs.addObserver

Categories

(Remote Protocol :: Marionette, enhancement, P3)

Version 3
enhancement

Tracking

(firefox72 fixed)

RESOLVED FIXED
mozilla72
Tracking Status
firefox72 --- fixed

People

(Reporter: whimboo, Assigned: rgpt)

Details

Attachments

(1 file)

For lazy addition and probably better performance in the near future we should flip to make use of `XPCOMUtils.defineLazyPreferenceGetter` in marionette.js:

https://dxr.mozilla.org/mozilla-central/rev/085cdfb90903d4985f0de1dc7786522d9fb45596/testing/marionette/components/marionette.js#282

Here the definition of the lazy helper:

https://dxr.mozilla.org/mozilla-central/rev/085cdfb90903d4985f0de1dc7786522d9fb45596/js/xpconnect/loader/XPCOMUtils.jsm#411

>   * Defines a getter on a specified object for preference value. The
>   * preference is read the first time that the property is accessed,
>   * and is thereafter kept up-to-date using a preference observer.

Rishi, is that something you might want to have a look at? I know it's Javascript and not Python but it would give you actually a chance to work on the Marionette server component, which is in Firefox.

Flags: needinfo?(rishigpt2009)

@whimboo, i got the part that for performance we are going to use lazy addition and XPCOM expose such functions but i am little unaware on working of Services and XPCOMutil which is a part of xpconnect (which is a bridge between XPCOM and JS).
Also what should be the default value while using XPCOMUtils.defineLazyPreferenceGetter for this preference and after making change i can verify this by simple running one of marionette-test cases right as marionette server would run in that case or do i have to check preference in some other way.

Flags: needinfo?(rishigpt2009) → needinfo?(hskupin)

The required work for this bug is all JS and all part of the above mentioned marionette.js file. There is nothing in Services, XPCOMUtils, or other files where changes are necessary.

In regards of the default value, you can find it a couple lines above:
https://dxr.mozilla.org/mozilla-central/rev/085cdfb90903d4985f0de1dc7786522d9fb45596/testing/marionette/components/marionette.js#279

The new API smoothly combines both cases into a single routine. So no need to explicitly read the pref during start-up, and observing it as long as Firefox is running.

Let me know if you have questions, and we can discuss those on IRC. Please note that I will not be around next week on Monday and Tuesday.

Flags: needinfo?(hskupin)
Assignee: nobody → rishigpt2009
Status: NEW → ASSIGNED
Attachment #9099503 - Attachment description: Bug 1478595 - using lazy pref getter through XPCOMUtils instead of services.pref.addObserver r=whimboo → Bug 1478595 - [marionette] Use defineLazyPreferenceGetter for observing "marionette.enabled" preference. r=whimboo
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d0220841f74b
[marionette] Use defineLazyPreferenceGetter for observing "marionette.enabled" preference. r=whimboo,ato
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.