1777906 - Update quitter xpi

Status: RESOLVED FIXED

(Testing :: General, task, P1)

Description

[Tooru Fujisawa [:arai]]

tools/quitter/[e-mail] isn't up to date after bug 1698158 change.
and the const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm"); in parent.js file blocks bug 1667455 patch landing.

Comment 1

[Tooru Fujisawa [:arai]]

according to bug 1601252 comment #16 , quitter extension needs to follow https://github.com/mozilla-extensions/xpi-manifest/blob/master/docs/adding-a-new-xpi.md to be signed

Comment 2

[Tooru Fujisawa [:arai]]

:ahal, can you handle this?

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

I will look into this

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

:arai, do you have more info on what failed? It looks like we use the .xpi that is checked into the tree for pgo builds. I am not finding other cases where we use quitter.

In regards to using the addons-pipeline, that would work, we would need to create a repo within the mozilla-extensions account and setup the building blocks. If we do this, where is the master source? I would say if we are not using the source in-tree, then we shouldn't have it in-tree and we should just proxy the resulting .xpi file in-tree.

Comment 5

[Tooru Fujisawa [:arai]]

Here's the details that happens in bug 1667455 patch:

Bpgo(run) job fails with parts 1-25 applied, but doesn't fail with parts 1-24 applied.
Part 25 removes Services.jsm file.

The job on automation doesn't say the details, but it can be observed with local run.

In local run with debug build (./mach python build/pgo/profileserver.py --binary ${OBJDIR}/dist/NightlyDebug.app/Contents/MacOS/firefox), I get the following error for quitter.xpi's background.js line 11

JavaScript error: moz-extension://9407de24-c97b-4fc5-9bbb-9dddbbd1a56f/background.js, line 11: Error: An unexpected error occurred

The content is same as the in-tree not-packaged background.js

https://searchfox.org/mozilla-central/rev/d8225c5b4874325e3024a6857a65eb87e4d6fee9/tools/quitter/background.js#11

browser.quitter.quit();

and the following is recorded with js::DumpBacktrace(aCx); inside mozJSModuleLoader.cpp failure path, inside the call above:

#0      110239020 i   jar:file:///var/folders/mc/_h6_c03s17746mc49tdqj2340000gn/T/tmppqe52wl3/extensions/[e-mail]!/parent.js:9 (39a81ea70560 @ 22)
#1      16b33bda0 b   resource://gre/modules/ExtensionCommon.jsm:1641 (19a18128c790 @ 36)

the [e-mail]!/parent.js line 9 is the following line:

const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");

The line is removed by https://hg.mozilla.org/integration/autoland/rev/8a98d844e248 in in-tree not-packaged parent.js, but it's still there in xpi file.

Comment 6

[Tooru Fujisawa [:arai]]

about the other case, looks like valgrind job also uses?
https://searchfox.org/mozilla-central/search?q=quitter%40mozilla.org.xpi&case=true&path=

https://searchfox.org/mozilla-central/rev/d8225c5b4874325e3024a6857a65eb87e4d6fee9/build/valgrind/mach_commands.py#44,89-90

def valgrind_test(command_context, suppressions):
...
        quitter = os.path.join(
            command_context.topsrcdir, "tools", "quitter", "[e-mail]"

then, there's {OBJDIR}/dist/xpi-stage/[e-mail] in my local build, that doesn't have signature and that contains the up-to-date files.
so the build process is packaging the xpi file (Makefile.in and moz.build), but not using it for PGO run.

Then, to my understanding, the current master source is the files inside https://searchfox.org/mozilla-central/source/tools/quitter
and we should move the files to the github repo, and have only the xpi file in tree.

and the moz.build above needs to be modified to use the in-tree xpi file, instead of packaging sources.

Comment 7

[Tooru Fujisawa [:arai]]

(In reply to Tooru Fujisawa [:arai] from comment #6)

about the other case, looks like valgrind job also uses?
https://searchfox.org/mozilla-central/search?q=quitter%40mozilla.org.xpi&case=true&path=

that seems to be a copy/paste from pgo code, and possibly unused
https://hg.mozilla.org/mozilla-central/rev/32430af5d9d4b8f4c0bb2ba7605b17000e9a0ca8

Comment 8

[Tooru Fujisawa [:arai]]

(In reply to Tooru Fujisawa [:arai] from comment #7)

(In reply to Tooru Fujisawa [:arai] from comment #6)

about the other case, looks like valgrind job also uses?
https://searchfox.org/mozilla-central/search?q=quitter%40mozilla.org.xpi&case=true&path=

that seems to be a copy/paste from pgo code, and possibly unused
https://hg.mozilla.org/mozilla-central/rev/32430af5d9d4b8f4c0bb2ba7605b17000e9a0ca8

maybe not true.
bug 1255567 mentions it was used, and also it says valgrind test needs signature.

Comment 9

[Tooru Fujisawa [:arai]]

the other thing to mention is that, until bug 1255567, the xpi file generated in OBJDIR was used.

https://hg.mozilla.org/mozilla-central/rev/80d84efa1d0c#l1.12

-                             addons=[os.path.join(build.distdir, 'xpi-stage', 'quitter')],
+                             addons=[os.path.join(build.topsrcdir, 'tools', 'quitter', '[e-mail]')],

Given that those 2 consumers now uses the in-tree xpi, it's no longer necessary to package the xpi file during the build.
and having the signed xpi file is sufficient.

Comment 10

[Joel Maher ( :jmaher ) (UTC -8)]

ok, so we need to make a github project in the extensions org, and then run this through addons pipeline, then we can take the resulting artifact and upload it to tools/quitter. Ideally a readme in place in tools/quitter for how to update quitter.

I assume I can make a repo on github and we can then figure out how to get it into the extensions org

Comment 11

[Joel Maher ( :jmaher ) (UTC -8)]

I have a repo that needs to get moved to the mozilla-extensions org in github:
https://github.com/mozilla/quitter

this is the code from m-c, so not the updated code. I think we should verify we can get a signed .xpi file from this and it works then modify the code as needed so we can have confidence we can sign and use it.

:theone, can you help me get this repo transferred ownership?

Comment 12

[Andreas Wagner [:TheOne] [use NI]]

Adding this to the signing pipeline adds a lot of overhead and effort to several teams. Before we do so, what alternatives are there that don't require signing that code as an external add-on?

Comment 13

[Joel Maher ( :jmaher ) (UTC -8)]

I have been trying a variety of things this week:

• marionette to launch and close the browser (results in not clean exit, if I ignore, then no profile files) - lacking env vars
• os.kill(pid) on the browser, again not clean exit, if I ignore no profile files)

I want to try:

• specialpowers addon that we use in CI, I have to build it and make it more static (or download and extract artifact with it), then install it with marionette in the profile warmup phase, then I can theoretically close the browser.

Outside of that, I am running short on ideas. I don't know much about the profile files and their requirements, so there might be some tricks I am not aware of.

Comment 14

[Tooru Fujisawa [:arai]]

If the purpose here is to create a profile directory, I wonder if --screenshot command or equivalent can be used.
With the following command, I see some profile files created inside /tmp/profile/.

.../Nightly.app/Contents/MacOS/firefox --no-remote --profile /tmp/profile/ --screenshot about:blank

Comment 15

[Joel Maher ( :jmaher ) (UTC -8)]

I can get a profile created, it is the .profraw files that we get from Firefox while running the application and accessing webpages is what I am missing. Right now I am just about out of options on how to close the browser.

Comment 16

[Joel Maher ( :jmaher ) (UTC -8)]

I think there is a way to do this with using the quitter addon source and generating a .xpi from it without signing. I need to try a few more things, but ideally this is something that can be done automatically. i.e. we would build quitter and treat it as a build artifact which we download into the run task.

Comment 17

[Joel Maher ( :jmaher ) (UTC -8)]

I have a solution that works and seems to preserve the gains of PGO:
https://treeherder.mozilla.org/jobs?repo=try&tier=1%2C2%2C3&revision=d845117d80dde838126540ae4de8314ede7397a4

I need to figure out how to remove my hack and use the artifact properly- presumably I can figure that out tomorrow and get this up for review.

Comment 18

[Tooru Fujisawa [:arai]]

(In reply to Joel Maher ( :jmaher ) (UTC -0800) from comment #17)

I have a solution that works and seems to preserve the gains of PGO:
https://treeherder.mozilla.org/jobs?repo=try&tier=1%2C2%2C3&revision=d845117d80dde838126540ae4de8314ede7397a4

To my understanding it uploads the quitter xpi packed during the build, and it's not signed.
Using the build result is effectively the same thing as reverting bug 1601252 and bug 1255567 patches,
and it's not compatible with beta/release/esr branches where xpinstall.signatures.required has no effect and signature is always required.

Comment 19

[Joel Maher ( :jmaher ) (UTC -8)]

odd that it would work in m-c. I am not aware of any other tricks, so if what you say is true about beta/release/esr we should look more into going the addons team signing route.

Comment 20

[Joel Maher ( :jmaher ) (UTC -8)]

:theone, could you get the process started for signing the quitter extension?

Comment 21

[Andreas Wagner [:TheOne] [use NI]]

(In reply to Joel Maher ( :jmaher ) (UTC -0800) from comment #20)

:theone, could you get the process started for signing the quitter extension?

Where can I find the current github repository with the code?

Comment 22

[Joel Maher ( :jmaher ) (UTC -8)]

the code lives in mozilla-central, but I created a github project:
https://github.com/mozilla/quitter

Comment 23

[Andreas Wagner [:TheOne] [use NI]]

I added you as a member of mozilla-extensions so you can transfer it to that org. This is necessary for proper shipit/taskcluster integration.

Comment 24

[Shane Caraveo (:mixedpuppy)]

What is the status on this?

Comment 25

[Joel Maher ( :jmaher ) (UTC -8)]

waiting on AMO to sign the binary. :TheOne, is this something that has been done, or is there something blocking this (other than time)?

Comment 26

[Andreas Wagner [:TheOne] [use NI]]

Add-ons Operations is waiting for a review request when the add-on is ready for signing, as mentioned on Slack: https://mozilla.slack.com/archives/CMKP7NPKN/p1658498758174249?thread_ts=1658498141.973329&cid=CMKP7NPKN

Comment 27

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: Bug 1777906 - update quitter xpi to be signed from mozilla-extensions repo. r=gbrown!

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/862901cf0e59
update quitter xpi to be signed from mozilla-extensions repo. r=gbrown

Comment 29

[Cristina Cozmuta (:CrissCozmuta)]

https://hg.mozilla.org/mozilla-central/rev/862901cf0e59