1696504 - Build Firefox with the macOS 11 SDK in CI

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, task, P3)

Description

[Markus Stange [:mstange]]

We currently use the macOS 11 SDK in CI for arm64 Firefox builds, but we're still using the 10.12 SDK for x86_64 Firefox builds. We should use the macOS 11 SDK for both.

This will have the following benefits:

• It will avoid compatibility paths in macOS system libraries and get us on the "happy path". Apple expects apps to adopt new SDKs somewhat quickly. Building with an older SDK enables runtime behaviors in macOS system libraries that don't receive a lot of testing from Apple. We've seen various amounts of breakage in the compatibility paths when new versions of macOS were released. (The most striking one was a crash after sleep when touch bar macs were originally released, bug 1320048. This only happened on apps built with an older SDK.)
• As an example of the above, updating to the macOS 11 SDK will avoid spammy touch bar related warnings that are currently printed to the console whenever a Firefox window is opened (bug 1687365).
• It is a necessary prerequisite for raising our minimum supported SDK version to the macOS 11 SDK (bug 1696513), which will improve the quality of life for Firefox developers on macOS.

Comment 1

[Mike Hommey [:glandium]]

Attachment: Bug 1696504 - Bump macOS builds to SDK 11.0.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a899281204c7
Bump macOS builds to SDK 11.0. r=firefox-build-system-reviewers,mhentges

Comment 3

[Marian-Vasile Laza]

Backed out changeset a899281204c7 (Bug 1696504) for causing GTest failures.
Backout link
Push with failures
Failure Log

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/572b175efb09
Bump macOS builds to SDK 11.0. r=firefox-build-system-reviewers,mhentges

Comment 5

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/572b175efb09

Comment 6

[Sandor Molnar]

Backed out for causing regressions.

Backout link: https://hg.mozilla.org/mozilla-central/rev/b1571319e4cdb9e3ae3f024bd3a4921a53da7b76

Comment 7

[Haik Aftandilian [:haik]]

The regression bugs have been changed to dependent bugs we will have to fix before re-landing this.

Comment 8

[Andrej Glavic (:aglavic)]

(In reply to Sandor Molnar from comment #6)

Backed out for causing regressions.

Backout link: https://hg.mozilla.org/mozilla-central/rev/b1571319e4cdb9e3ae3f024bd3a4921a53da7b76

== Change summary for alert #32610 (as of Fri, 03 Dec 2021 18:51:30 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
8%	Heap Unclassified	macosx1015-64-shippable-qr	tp6	135,159,397.78 -> 124,175,483.62
7%	Heap Unclassified	macosx1015-64-shippable-qr	tp6	134,191,528.53 -> 124,575,927.96

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=32610

Comment 9

[Acasandrei Beatrice (needinfo me)]

(In reply to Sandor Molnar from comment #6)

Backed out for causing regressions.

Backout link: https://hg.mozilla.org/mozilla-central/rev/b1571319e4cdb9e3ae3f024bd3a4921a53da7b76

== Change summary for alert #32641 (as of Mon, 06 Dec 2021 12:45:59 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
28%	ts_paint_webext	macosx1015-64-shippable-qr	e10s stylo webrender-sw	725.58 -> 521.17
28%	ts_paint	macosx1015-64-shippable-qr	e10s stylo webrender-sw	722.92 -> 520.42
26%	sessionrestore	macosx1015-64-shippable-qr	e10s stylo webrender-sw	755.58 -> 555.92
26%	sessionrestore_many_windows	macosx1015-64-shippable-qr	e10s stylo webrender-sw	741.67 -> 546.50
25%	sessionrestore_no_auto_restore	macosx1015-64-shippable-qr	e10s stylo webrender-sw	779.92 -> 587.33
...	...	...	...	...
17%	startup_about_home_paint_realworld_webextensions	macosx1015-64-shippable-qr	e10s fission stylo webrender	1,157.58 -> 957.58

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=32641

Comment 10

[Alex Finder]

(In reply to Andrej (:andrej) from comment #8)

(In reply to Sandor Molnar from comment #6)

Backed out for causing regressions.

Backout link: https://hg.mozilla.org/mozilla-central/rev/b1571319e4cdb9e3ae3f024bd3a4921a53da7b76

== Change summary for alert #32610 (as of Fri, 03 Dec 2021 18:51:30 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
8%	Heap Unclassified	macosx1015-64-shippable-qr	tp6	135,159,397.78 -> 124,175,483.62
7%	Heap Unclassified	macosx1015-64-shippable-qr	tp6	134,191,528.53 -> 124,575,927.96

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=32610

== Change summary for alert #32613 (as of Fri, 03 Dec 2021 19:19:50 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
11%	bing-search FirstVisualChange	macosx1015-64-shippable-qr	cold webrender	395.00 -> 353.33
10%	bing-search FirstVisualChange	macosx1015-64-shippable-qr	cold fission webrender	405.00 -> 363.33
10%	bing-search ContentfulSpeedIndex	macosx1015-64-shippable-qr	cold fission webrender	443.04 -> 399.67
9%	bing-search ContentfulSpeedIndex	macosx1015-64-shippable-qr	cold webrender	437.62 -> 400.42
7%	amazon ContentfulSpeedIndex	macosx1015-64-shippable-qr	cold fission webrender	540.75 -> 503.25
...	...	...	...	...
6%	amazon ContentfulSpeedIndex	macosx1015-64-shippable-qr	cold webrender	532.83 -> 502.17

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=32613

Comment 11

[Greg Tatum [:gregtatum]]

We're ramping up a bit of work around text recognition for images in the browser using the Vision framework for macOS. I'd like to take some of our prototype code and land it behind a pref so that UX and product can begin experimenting with it live in Nightly.

I could probably land my code behind an ifdef, but then you would need a local build still to run it. I was chatting 1:1 with mstange a bit about what's blocking this patch, and from what I understand it's mostly mechanical work of wontfixing some regressions, and re-landing the code. Would it be possible to prioritize this work so that I can start moving the text recognition work forward?

I'm setting a needinfo for :mstange and :spohl mostly to get your opinions on this, and the path forward here. I'd be happy to lend help if any is needed to move this forward.

Another option would be to bump it up to 10.13 only, since the regressions might not be hit, but it seems like it would be easier to do just one upgrade to 11.

Comment 12

[Stephen A Pohl [:spohl] (OOO until 6/12)]

I support the idea of re-landing this.

Comment 13

[Markus Stange [:mstange]]

Let's wait for bug 1743170 to land and then we can do this.

Comment 14

[Markus Stange [:mstange]]

That bug is now on central, so let's give this another go! I'll press the lando button and lando can figure out if the patch still applies.

Comment 15

[Markus Stange [:mstange]]

It doesn't apply. Glandium, can you update the patch and re-land? Thanks!

Comment 16

[Greg Tatum [:gregtatum]]

I'll take a stab at rebasing and landing since my bug is blocked on this.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/10a2fbadf5d3
Bump macOS builds to SDK 11.0. r=firefox-build-system-reviewers,mhentges

Comment 18

[Cristian Tuns]

Backed out for causing reftest failures on hiragino-kaku-1.html

• Backout link
• Push with failures
• Failure Log
• Failure line: REFTEST TEST-UNEXPECTED-FAIL | layout/reftests/font-matching/hiragino-kaku-1.html != layout/reftests/font-matching/hiragino-kaku-1-notref.html | image comparison, max difference: 0, number of differing pixels: 0

Comment 19

[Markus Stange [:mstange]]

Jonathan, it looks like the new test from bug 1743170 isn't happy. Could you take a look?

Comment 20

[Jonathan Kew [:jfkthame]]

Ugh -- I guess that probably means the build-time test for whether we need to manually add the deprecated names isn't behaving quite as expected. It seemed OK based on my local builds, but who knows the intricacies of mac SDK behavior..... I'll see if I can make it happy on tryserver.

Comment 21

[Jonathan Kew [:jfkthame]]

I think we should just do the deprecated-names thing unconditionally; there are too many unknowns about how the Core Text API will decide to behave across different SDK versions, build-time configurations (DEPLOYMENT_TARGET etc), and runtime environments. In the worst case, this will just mean we get duplicate names in the list, which we then filter out when storing it in SharedFontList, so no harm done.

I propose leaving the #if USE_DEPRECATED_FONT_FAMILY_NAMES guards around the code (just setting it unconditionally true), so that it's easy to see which fragments of code are involved in this hack, and potentially look at removing it sometime in the future.

Comment 22

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1696504 - Force the deprecated macOS font-family names to be handled, for consistent behavior when built with newer SDKs. r=mstange

In my local builds, making this conditional on MAC_OS_X_VERSION_MIN_REQUIRED seemed to work fine,
but tryserver doesn't agree. So I think the simplest/safest way forward is just to unconditionally
include the deprecated names. If Core Text is exposing them in any given build, the duplicates
will just be dropped, so it's harmless enough, and avoids being dependent on quirks of different
SDK behavior.

Comment 23

[Markus Stange [:mstange]]

I've retriggered landing.

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/64af4ef564b1
Force the deprecated macOS font-family names to be handled, for consistent behavior when built with newer SDKs. r=mstange
https://hg.mozilla.org/integration/autoland/rev/c8d9ed133a43
Bump macOS builds to SDK 11.0. r=firefox-build-system-reviewers,mhentges

Comment 25

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/64af4ef564b1
https://hg.mozilla.org/mozilla-central/rev/c8d9ed133a43

Comment 26

[Alexandru Ionescu (needinfo me) [:alexandrui]]

(In reply to Pulsebot from comment #17)

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/10a2fbadf5d3
Bump macOS builds to SDK 11.0. r=firefox-build-system-reviewers,mhentges

== Change summary for alert #34250 (as of Mon, 30 May 2022 15:12:23 GMT) ==

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)
8%	amazon FirstVisualChange	macosx1015-64-shippable-qr	cold fission webrender	408.33 -> 440.00

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=34250