In bug 1271761 CPU instruction set detection on Windows was added to the app update url and we want the same for Linux. This will need to be added to http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/UpdateUtils.jsm After it is added it will automatically be used by application update.

We finally saw a crash from this on linux32. We should deploy the same update block for that OS as well. We have many independent cpuid wrappers in the tree, but the most generic is mozilla::supports_sse2() from mozilla/SSE.h. Seems like calling that would solve the problem on all x86 platforms.

Robert, what should we do to get this deployed? It's a very small number by crash reports, but breaking people's Firefox isn't nice.

Ralph, it would need to be added to the following file https://dxr.mozilla.org/mozilla-central/source/toolkit/modules/UpdateUtils.jsm Would you be interested in adding it?

I don't mind doing it, but I'm unconvinced it's worthwhile. The rust dependency on SSE2 cpus shipped for linux32 in Firefox 48 alongside win32. At best we could uplift the code to beta and block updates starting with 49. What do you think?

(In reply to Ralph Giles (:rillian) needinfo me from comment #4) > I don't mind doing it, but I'm unconvinced it's worthwhile. The rust > dependency on SSE2 cpus shipped for linux32 in Firefox 48 alongside win32. > At best we could uplift the code to beta and block updates starting with 49. > What do you think? I think it would be a good thing. In the Windows case it sends the highest instruction set supported so it is possible to discontinue support of SSE2 if / when that changes.

(In reply to Ralph Giles (:rillian) needinfo me from comment #4) > I don't mind doing it, but I'm unconvinced it's worthwhile. Now that this is marked as blocking bug 1274196, are you planning on writing the patch? If not, how do we get forward motion on this?

If you want me to write a patch I'll see what I can do.

Sorry, I didn't get to this this week, and I'm away all of next week. Feel free to steal this bug in the meantime.

This is the WIP I have so far, very rough. In particular it reports only SSE2 on linux; it would probably be cleaner to replicate the full option check from UpdateUtils.jsm in the c++ implementation and use that for both archs. Robert, how did you test this? I don't know how to trigger the update ping other than leaving it running under pcap and waiting.

It is tested in https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/tests/unit_aus_update/urlConstruction.js

I'll review the changes to the jsm after the supporting code is reviewed.

Comment on attachment 8799062 [details] Bug 1277359 - Add chrome-only navigator.cpuHasSSE2 api. https://reviewboard.mozilla.org/r/84360/#review83186 comments fixed, r+ ::: dom/webidl/Navigator.webidl:446 (Diff revision 1) > readonly attribute unsigned long long hardwareConcurrency; > }; > + > +partial interface Navigator { > + [ChromeOnly] > + readonly attribute boolean cpu_sse2; Just put the attribute to existing partial interface (remember to keep the ChromeOnly annotation) // nsIDOMNavigator partial interface Navigator { using _ in attribute names in unusual. Perhaps sse2CPU ?

Comment on attachment 8799062 [details] Bug 1277359 - Add chrome-only navigator.cpuHasSSE2 api. https://reviewboard.mozilla.org/r/84360/#review86050

Thanks. Carrying-forward r=smaug,rstrong for the api name change. Robert, how can I verify this is sending the correct signal in the update request?

Comment on attachment 8799062 [details] Bug 1277359 - Add chrome-only navigator.cpuHasSSE2 api. https://reviewboard.mozilla.org/r/84360/#review83186 > Just put the attribute to existing partial interface (remember to keep the ChromeOnly annotation) > // nsIDOMNavigator > partial interface Navigator { > > using _ in attribute names in unusual. > Perhaps sse2CPU ? Thanks. I'd like it to start with `cpu` because we might add more later and I'd like them to sort. How about `cpuHasSSE2`?

The url can be tested in (In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #12) > It is tested in > https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/tests/ > unit_aus_update/urlConstruction.js

Ben, heads up regarding Linux adding whether SSE2 support is present to the update url

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #21) > Ben, heads up regarding Linux adding whether SSE2 support is present to the > update url Thanks! No changes needed on the server for this - we added support for all platforms when with our earlier work.

Comment on attachment 8802648 [details] Bug 1277359 - Report SSE2 instruction support on linux update pings. https://reviewboard.mozilla.org/r/84362/#review86764 ::: toolkit/modules/UpdateUtils.jsm:192 (Diff revision 2) > return instructionSet; > } > > + if (AppConstants == "linux") { > + let instructionSet = "unknown"; > + if navigator.cpu_sse2 { forgot to propagate the rename here.

Pushed by rgiles@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9ffa9dc9288e Add chrome-only navigator.cpuHasSSE2 api. r=smaug https://hg.mozilla.org/integration/autoland/rev/87fe724cfc90 Report SSE2 instruction support on linux update pings. r=rstrong

Pushed by gszorc@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4e8884c8a186 Fix syntax error introduced in 87fe724cfc90; r=me

If urlConstruction.js fails please disable it on Linux and I'll fix it separately from this bug.

https://hg.mozilla.org/mozilla-central/rev/9ffa9dc9288e https://hg.mozilla.org/mozilla-central/rev/87fe724cfc90 https://hg.mozilla.org/mozilla-central/rev/4e8884c8a186

Why the empty "partial interface Navigator"?

(In reply to Boris Zbarsky [:bz] (still a bit busy) from comment #30) > Why the empty "partial interface Navigator"? Thanks for pointing that out. Hopefully fixed in bug 1312559.

Is it practical to uplift this to earlier trains?

Oops! Ignore comment #33 and comment #34. I thought this was bug 1311515.

We could uplift to 51 aurora. I think it's too late for 50 beta.

Patches apply cleanly to aurora, but there are some follow-up commits, so here's a squashed version. r=me Approval Request Comment [Feature/regressing bug #]: Bug 1308167 [User impact if declined]: Dropping support for non-SSE2 linux32 machines would have to wait for another release. [Describe test coverage new/current, TreeHerder]: Landed on m-c last week. [Risks and why]: I think the risk is acceptable this late in the Aurora cycle. The change extends update ping code we've already deployed for win32 to linux32, which is a small part of our user base. We should be able to mitigate any incorrect behaviour on the update server. [String/UUID change made/needed]: None.

Comment on attachment 8805698 [details] [diff] [review] aurora 51 backport Support update for linux. Take it in 51 aurora.