1393162 - PGO and Valgrind builds are going to be broken when Gecko 57 merges to Beta on 2017-09-20

Status: VERIFIED FIXED

(Firefox Build System :: General, defect)

Description

[Ryan VanderMeulen [:RyanVM]]

[Tracking Requested - why for this release]: Broken builds on the next merge day.

This affects both Linux and Windows, where PGO is the default for builds on Beta. Looking at the logs, it appears that quitter.xpi is being blocked and as a result, the browser can't shut down after the profiling run until the run eventually times out.

https://public-artifacts.taskcluster.net/WnfamXPZRreQpUCn0Dpkvg/0/public/logs/live_backing.log

INFO -  1503504724460	addons.xpi	WARN	disabling legacy extension [e-mail]

Comment 1

[Ryan VanderMeulen [:RyanVM]]

Affects Valgrind too (same problem).
https://public-artifacts.taskcluster.net/fwKO3-MoQheDihStzqmZuA/0/public/logs/live_backing.log

Comment 2

[Andrew Swan [:aswan]]

Andy, the expedient fix is to just sign the quitter extension with "Mozilla Extensions" and that looks to be safe (the extension just adds a content-accessible api for quitting the browser).  I don't know what the actual process is for getting it signed, can you get it signed?  (actually there's an xpi checked into the tree but people have been updating the sources without updating the xpi so updating to the current sources would be a nice extra step)

Comment 3

[Andy McKay]

Having a content accessible API to quit Firefox doesn't sound great, since anyone could install this and get that API in their Firefox and we haven't really got a good way to prevent its distribution. However it seems kinda pointless, I can't spot many attack vectors here.

If you can attach an .xpi to this bug, we can get Jason to sign it. If there's more to do we might want to consider setting up someone like Ryan up with API access to do this on a regular basis. Details on that can be found in the mana page from https://wiki.mozilla.org/Add-ons/InternalSigning

We should probably file a bug to remove quitter from the tree and find a work around for this.

Comment 4

[Mike Hommey [:glandium]]

I don't know why there's an xpi in the source tree, but it's not what's used, the source is. And we can't sign the xpi that was just built from there, because that won't work for downstreams.

Comment 5

[Andrew Swan [:aswan]]

(In reply to Mike Hommey [:glandium] from comment #4)
> I don't know why there's an xpi in the source tree, but it's not what's
> used, the source is. And we can't sign the xpi that was just built from
> there, because that won't work for downstreams.

I assumed the checked-in xpi is what is used since it is signed.  If we build another xpi from the sources, how does it get loaded on beta/release during PGO builds?

(In reply to Andy McKay [:andym] from comment #3)
> We should probably file a bug to remove quitter from the tree and find a
> work around for this.

I didn't think too hard about this but I don't see an obvious work around, and its not like we have a lot of time to do new work.  I also don't think quitter is a big problem, if an "attacker" tricks somebody into installing it and then tricks them into visiting a page that uses the API, that would be annoying but not harmful.  I would worry more about some exploitable bug in quitter, I'm not very well-trained at looking for these but its code is pretty simple, it looks safe to me.

Comment 6

[Mike Hommey [:glandium]]

I looked more closely, and we do use the xpi for pgo profile and valgrind, which makes me cringe. The addon being signed is not going to make it less a legacy addon, too...

Comment 7

[Andrew Swan [:aswan]]

(In reply to Mike Hommey [:glandium] from comment #6)
> The addon being signed is not going to make it less a
> legacy addon, too...

Well being signed with "Mozilla Extensions" will.

Comment 8

[Andrew Swan [:aswan]]

Attachment: [e-mail]

Jason, can you please have the xpi from this attachment signed with "Mozilla Extensions"

Comment 9

[(not currently active) Ted Mielczarek]

For the short-term fix to unbreak tests for beta it'd be good to just get a signed xpi. For a longer-term fix we should just change these tests to use marionette to start and shutdown the browser.

Comment 10

[(not currently active) Ted Mielczarek]

Filed bug 1393449 on that.

Comment 11

[Jason Thomas [:jason]]

Attachment: [e-mail] signed

Please see attached.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8900724 [details]
[e-mail] signed

I've confirmed on Try that this gets PGO and Valgrind builds working again.

Comment 13

[Andrew Swan [:aswan]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4d4296e650423faa07ef7ed55cbac7ab5a1c6f05
Bug 1393162 Re-sign quitter.xpi as privileged r=ryanvm

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4d4296e65042