"Better Study Endings" Engineering Plan (Proposed)

[gregglind]

"Better Study Endings" Engineering Plan (Proposed)

revision: 1. (2018 jun 12)

compiled from discussions in #244 #232 #230 #231 #208 #206 #205 #194 #188 #179 #148

Principles:

1. Studies are packaged as addons, but are not addons. Studies have different ending semantics and expectations than usual addon patterns. (No "re-enable", no "undo" see Is this the expected behavior? The "Undo" option is wrongly displayed after the shield study is removed #230).
2. Studies are WebExtensionExperiments, which means they have full privileged power. The shield-studies-addon-utils can interact with Firefox modules "behind the scenes"
3. Studies are written by trusted authors. Study Authors will not intentionally abuse Firefox users. Trust means they can have different tools than usual addon authors.

Design goals of this plan:

Preserve desired ending behaviour for Studies:

1. Allow users to leave studies
2. Reliably detect user intent to remove a Study-implemented feature.
3. For a study, distinguish 'Normandy rollback' from 'user intent to remove'. Ensure telemetry for each is different. Normandy revoke happens to correct bugs, or mis-targeting, and should not reflect badly on a study. "User opt-out" should penalize that variation of a study.
4. For all study endings, give the addon time to do all shutdown / cleanup and final telemetry procedures.

WebExtension Engineering goals

As a study engineer, I want to:

1. Allow addons to centralize all study ending handling in one function in the webExtension space at browser.study.onEndStudy

Firefox Extensions goals

As Firefox engineering Extensions module owner, I want to:

1. Continue to treat general addons (webExtensions) with distrust. Do not block on uninstall, or hang the thread.
2. Support these in a way that doesn't require huge changes in Firefox.

QA Goals:

As a Study QA person, I want to:

1. Have predicable, reliable Study ending behaviour.
2. Be able to trigger endings artificially to check handling

Current State:

1. Both "user intent" and "normandy revoke" use the usuall addon.uninstall(). The addon cannot distinguish these for purposes of surveys and telemetry.
2. addon.uninstall() signals for ADDON_UNINSTALL do not propagate into the 'getAPI' or the webExtension code.

Proposed solution 1: hide from about:addons + Warn all before uninstall

1. Hide Studies from about:addons page.

Reasoning: Users cannot trigger all the 'direct disable' or 'direct uninstall.'

Mechanism ideas:

1. Have a manifest.json flag called "study: true", or "hidden: true' that is only respected for mozilla signed addons.
2. Filter on id regex '@(pioneer|shield).mozilla.org'

(1 is more work but safer from malicious actors).

Code changes:

1. Implement the filtering in about:addons, including the regex, signing check etc.
2. consider a pane / link to about:studies to deal with user unclarity.
3. ensure about:support has a listing for SUMO use.

2. Warn the addon, but dont block. Normandy remove and about:studies warn before uninstall().

Reasoning: The 'warning' lets the addon do these:

a. know which reason (normandy, vs user intent)
b. have (some) time to shutdown.

Mechanism ideas:

1. In Normandy code, and in about:studies, change from addon.uninstall to:

   • warn using nsiObserverService
   • setTimeout for 5 seconds
   • then do AddonManager.getAddonByID(id, addon => addon.uninstall())

Code changes needed:

1. In Normandy's revoke mechanism, implement the 'warn then uninstall'

2. In about:studies revoke mechanism,

   • implement the 'warn then uninstall'.
   • change ui to relfect this in the button state. (ie., an "uninstalling..." state)

3. In shield-studies-addon-utils,

• implement the Listener for the nsiObserverService message. Trigger endStudy so that onEndStudy propogates.
• (good side effect: this also allows QA / devs to trigger endings)

Unclear aspects, with opinions:

1. Should study addons appear in telemetry under addons? I claim for now, yes. This is 'no change'.

2. How to filter out of about:addons. I am not sure if all studies will be mozilla-signed. I think that is a safe constraint.

3. Exact call to nsiObserverService. My naive suggestion is:

   subject:  study-revoke  | about-studies-user-disable 
   topic:  [addonId for the addon]
   

4. How long to wait. I claim even 5 seconds would be enough for most studies. They may need to open a few tabs. The 'longest' scenario is if they have to send a final ping that relies an expensive calculation over history / all tabs. If so, it is up to the study to make that calculation incremental / cheaper.

Proposed solution 2: Super smart Normandy

In addition to the 'hide studies' part, we could make normandy handle much more of the telemetry.

I claim this requires a lot more thought, and reevalution of the what a study addon is. I claim this is too big, and that all the changes in solution 1 are on the path to this plan.

I claim also that this plan has a lot of benefits for QA-ing individual studies being simpler.

I suggest that we punt on this a month or two to think.

Edge cases

• addons are weird. People can still uninstall by removing the addon from a profile while Firefox is not running.
• possible that people will not be able to discover studies, and be annoyed.
• there is no amount of time that guarantees

Comments Wanted

My claim is that proposal 1 is reasonable, and addresses most problems around endings.

@raymak @biancadanforth @pdehaan @aswan @rhelmer

Timeline:

• ASAP because this rides the trains. @aswan?
• changes to `shield-studies-addon-utils would take 3 days to write and qa. Shield team does this.

[ddurst]

Let's not do this via a plethora of issues filed (and filing new ones); it's just going to get more confusing. If this is summarizing everything else, let's close everything else, and discuss this in a a real-time meeting after the Web Extensions team gets a chance to review and plan.

[gregglind]

I am fine with that. I will close them all.

…

On Thu, Jul 12, 2018 at 12:42 PM, David Durst ***@***.***> wrote: Let's not do this via a plethora of issues filed (and filing new ones); it's just going to get more confusing. If this is summarizing everything else, let's close everything else, and discuss this in a a real-time meeting after the Web Extensions team gets a chance to review and plan. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#246 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAKAj_Ua6j9VLSgPQ9OxcU5vc0CwceFZks5uF4qEgaJpZM4VNSBv> .

[ddurst]

(But: thank you for summarizing, because <whew> that was getting complicated.)

[aswan]

Just chatted with @gregglind about this for a while. I think this proposal seems good, my only suggestion is that we make onEndStudy listeners return a Promise, and then uninstall the extension after that Promise resolves, rather than using a fixed timer. I'm happy to help with the implementation of that.
There are a bunch of moving pieces here, it looks like we need changes to at least:

• AddonManager
• Normandy
• shield-studies-utils

The first two need to ride the trains, if we get them done in the next few weeks we'll have them in 63.

[motin]

The first two need to ride the trains, if we get them done in the next few weeks we'll have them in 63.

That would be great! Who is assigned to make this happen in time? :)

[aswan]

We have bug 1365349 for the AddonManager bits, I don't know about the Normandy part.