Discussion notes: Migrating away from the SDK


(Jared Hirsch) #1

Hi all,

The Test Pilot core devs have been having a conversation in IRC this week about options for moving away from the SDK. I summarized the status in our public meeting etherpad this morning, with some helpful edits. I’m posting the current pro/con breakdown here for feedback.

Note that we haven’t reached a decision yet; the next step is to prototype both approaches, then see which is nicer from the point of view of both experiments and the main test pilot addon.

Since copy-pasting from etherpad loses all author color tracking and all bullet points, I’m just inserting a screenshot here, sorry. When we get further towards a final decision, like, say, when the prototypes are ready for discussion, I’ll bump this thread / this Discourse topic.

The screenshot is available at https://screenshots.firefox.com/KSBV0ZEsP4YL5FMI/public.etherpad-mozilla.org, and it’s inlined below as well. I apologize for the non-accessibility of this bit of the discussion; if you have visual impairments and would like to catch the updates, try the etherpad or ping me in #testpilot IRC.

Comments or questions are welcome, here or in #testpilot.

Cheers,

Jared


(Danny Coates) #2

I implemented a strawman add-on for the webextension API experiment option here: https://github.com/dannycoates/darkwing

One way to address the backward-compatible issue is to add a new namespace for incompatible changes. In my example I added a ‘_v2’ namespace. We’d keep the old API until no experiments used it. New experiments could alias away the version in their code. There’s more ways we could do it too.


(Donovan Preston) #3

After seeing the prototype, I’m more convinced that we might as well just go straight to a webextension api experiment approach.

  • Let’s just version the apis, and keep every old api version around forever, so different Test Pilot Experiments can use different api versions without problems
  • Experiment API update should be automatic through AMO. As long as we keep all the old api versions around we should be ok.
  • Let’s just always design the apis to use promises, so there’s just no question about whether something should be sync or async.
  • I think it will be ok if the extension api experiment is visible in the add-on manager for now; let’s make sure John is ok with it.
  • Let’s install both the api experiment and the test pilot experiment via the test pilot website; that way we don’t have to be blocked something landing in firefox.
    • Later after 1364976 has landed for a while maybe we can switch over to using it.

(Chuck Harmston) #4

Management of this sounds miserable. The JavaScript world already provides means of versioned dependencies, what benefits do we get by inventing our own?


(Jared Hirsch) #5

Management of this sounds miserable. The JavaScript world already provides means of versioned dependencies, what benefits do we get by inventing our own?

We’re in the Firefox world, which is importantly different from the vanilla JS world.

The key issue here is that webextension API experiments are much easier to integrate than an embedding bootstrapped wrapper. The one downside to API experiments is that they are global to a given profile. Since experiment teams may or may not have the time to upgrade their code to the newest version of a backwards-incompatible API, the simplest solution is for the Test Pilot API experiment code to support multiple versions of its own API.

We only have a few experiments running at a time, so I’d imagine we would only need to preserve backwards compatibility one or, at most, two versions back.

By comparison, the embedding webextension approach requires a lot more integration–we’d need to build tooling that integrates into a wide variety of build chains. All that effort would let each experiment manage its own Test Pilot API version, but then we’d have to spend time working on postmessage-like communication, testing bizarre edge cases around bootstrap startup/shutdown ‘aReason’ codes, etc. It’s a lot more boilerplate, and potential sources of bugs, to accomplish the same task.

Most likely the backwards-breaking API changes would be something simple, like a Test Pilot Telemetry API passing slightly different arguments to the native Telemetry code. I think this would be pretty straightforward to manage.

I believe the linked etherpad discusses these same issues in depth, so you can have a look there as well for other pros/cons.


(Jared Hirsch) #6

After seeing the prototype, I’m more convinced that we might as well just go straight to a webextension api experiment approach.

I’m in favor of this approach as well. Hopefully we’ll hear from a few other folks on the Test Pilot team or experiment teams.


(Donovan Preston) #7

@chuck I agree that npm versioning would be a lot cleaner, but the specific details of the API Experiment version are already implemented.

Would you be willing to implement an equivalent bootstrap + npm package version, so that we could have a more concrete discussion about the pros and cons, and be less handwavy? I think it would need:

  • code generator for the bootstrap main file that loads the webextension
  • implementation of the bootstrap side of a sample api
  • implementation of the webextension side of a sample api

Maybe there’s a way to do it without a code generator? I can’t think of exactly how that would work though.


(Jared Hirsch) #8

Would you be willing to implement an equivalent bootstrap + npm package version, so that we could have a more concrete discussion about the pros and cons, and be less handwavy?

I think this is a great idea. We should hash out Telemetry and Survey APIs so that we can see the same functionality exposed in both cases.

I’m really busy with Screenshots stuff at the moment, so I don’t think I have time to help write the code, but I could help with ideas or reviews.

One not-so-documented point to mention right away: the bootstrap code that starts the webextension can copy the approach taken in Screenshots:

function startup(data, reason) { // eslint-disable-line no-unused-vars
  const webExtension = LegacyExtensionsUtils.getEmbeddedExtensionFor({
    id: ADDON_ID,
    resourceURI: data.resourceURI
  });
  webExtension.startup();
}

And something similar for shutdown - probably simplest to hold a module-global reference to the webExtension, then call its shutdown method when needed.

For experiments that need to be embedded webextensions separate from this proposal, it’d be good to provide hooks to let individual experiments call their own bootstrap methods on startup/shutdown, like

  function startup(data, reason) {
      // use callbacks or use a promise, whatever
      runStartupCallbacks(data, reason);
      // ... other startup stuff from above snippet ...
    }

(Donovan Preston) #9

@clouserw brought up a good point: There’s a good chance WebExtensions API Experiments may only work on Nightly or Developer Edition, meaning we may be forced to use the bootstrap approach.

https://webextensions-experiments.readthedocs.io/en/latest/policy.html

We should double check that this is truly the case. If so, we should fire up some more discussion about how the bootstrap version will be architected.


(Clouserw) #10

Andrew said that could be fixed, I think, so stuff signed by our special cert would work everywhere.


(Jared Hirsch) #11

I think we need to add a third API to the list: Preferences.

WebExtensions, by design, do not have access to Prefs. It would be ideal to let experiments transition off of prefs by exposing a Prefs API, with the understanding that we may deprecate or remove it eventually, and it’s just for the 57 migration.


(Clouserw) #12

Follow up discussion for this is on the public calendar for Monday afternoon. Thanks!