Bug 1454094 - Explicitly copy font variation settings from CGFont when creating a CTFont only on macOS Sierra; on HighSierra, rely on Core Text to automatically propagate the required settings. r=lsalzman

gfx/2d/ScaledFontMac.cpp

@@ -78,8 +78,16 @@ static CTFontRef
 CreateCTFontFromCGFontWithVariations(CGFontRef aCGFont, CGFloat aSize)
 {
     // Avoid calling potentially buggy variation APIs on pre-Sierra macOS
-    // versions (see bug 1331683)
-    if (!nsCocoaFeatures::OnSierraOrLater()) {
+    // versions (see bug 1331683).
+    //
+    // And on HighSierra, CTFontCreateWithGraphicsFont properly carries over
+    // variation settings from the CGFont to CTFont, so we don't need to do
+    // the extra work here -- and this seems to avoid Core Text crashiness
+    // seen in bug 1454094.
+    //
+    // So we only need to do this "the hard way" on Sierra; on other releases,
+    // just let the standard CTFont function do its thing.
+    if (!nsCocoaFeatures::OnSierraExactly()) {
         return CTFontCreateWithGraphicsFont(aCGFont, aSize, nullptr, nullptr);
     }
 

gfx/cairo/cairo/src/cairo-quartz-font.c

@@ -340,13 +340,23 @@ const cairo_font_face_backend_t _cairo_quartz_font_face_backend = {
 static CTFontRef
 CreateCTFontFromCGFontWithVariations(CGFontRef aCGFont, CGFloat aSize)
 {
-    // Avoid calling potentially buggy variation APIs on pre-Sierra macOS
-    // versions (see bug 1331683)
     // Declare helper provided by widget/cocoa/nsCocoaFeatures.mm
-    extern bool Gecko_OnSierraOrLater();
-    if (!Gecko_OnSierraOrLater()) {
+    extern bool Gecko_OnSierraExactly();
+
+    // Avoid calling potentially buggy variation APIs on pre-Sierra macOS
+    // versions (see bug 1331683).
+    //
+    // And on HighSierra, CTFontCreateWithGraphicsFont properly carries over
+    // variation settings from the CGFont to CTFont, so we don't need to do
+    // the extra work here -- and this seems to avoid Core Text crashiness
+    // seen in bug 1454094.
+    //
+    // So we only need to do this "the hard way" on Sierra; on other releases,
+    // just let the standard CTFont function do its thing.
+    if (!Gecko_OnSierraExactly()) {
         return CTFontCreateWithGraphicsFont(aCGFont, aSize, NULL, NULL);
     }
+
     CFDictionaryRef vars = CGFontCopyVariations(aCGFont);
     CTFontRef ctFont;
     if (vars) {

gfx/skia/skia/src/ports/SkFontHost_mac.cpp

@@ -756,7 +756,7 @@ private:
 // work with non-system fonts. As a result, create the strike specific CTFonts from the underlying
 // CGFont.
 #ifdef MOZ_SKIA
-extern "C" bool Gecko_OnSierraOrLater();
+extern "C" bool Gecko_OnSierraExactly();
 #endif
 static UniqueCFRef<CTFontRef> ctfont_create_exact_copy(CTFontRef baseFont, CGFloat textSize,
                                                        const CGAffineTransform* transform)
@@ -774,8 +774,16 @@ static UniqueCFRef<CTFontRef> ctfont_create_exact_copy(CTFontRef baseFont, CGFlo
 
 #ifdef MOZ_SKIA
     // Avoid calling potentially buggy variation APIs on pre-Sierra macOS
-    // versions (see bug 1331683)
-    if (Gecko_OnSierraOrLater())
+    // versions (see bug 1331683).
+    //
+    // And on HighSierra, CTFontCreateWithGraphicsFont properly carries over
+    // variation settings from the CGFont to CTFont, so we don't need to do
+    // the extra work here -- and this seems to avoid Core Text crashiness
+    // seen in bug 1454094.
+    //
+    // So we only need to do this "the hard way" on Sierra; on other releases,
+    // just let the standard CTFont function do its thing.
+    if (Gecko_OnSierraExactly())
 #endif
     {
         // Not UniqueCFRef<> because CGFontCopyVariations can return null!

gfx/thebes/gfxMacFont.cpp

@@ -424,8 +424,16 @@ gfxMacFont::CreateCTFontFromCGFontWithVariations(CGFontRef aCGFont,
                                                  CTFontDescriptorRef aFontDesc)
 {
     // Avoid calling potentially buggy variation APIs on pre-Sierra macOS
-    // versions (see bug 1331683)
-    if (!nsCocoaFeatures::OnSierraOrLater()) {
+    // versions (see bug 1331683).
+    //
+    // And on HighSierra, CTFontCreateWithGraphicsFont properly carries over
+    // variation settings from the CGFont to CTFont, so we don't need to do
+    // the extra work here -- and this seems to avoid Core Text crashiness
+    // seen in bug 1454094.
+    //
+    // So we only need to do this "the hard way" on Sierra; on other releases,
+    // just let the standard CTFont function do its thing.
+    if (!nsCocoaFeatures::OnSierraExactly()) {
         return CTFontCreateWithGraphicsFont(aCGFont, aSize, nullptr, aFontDesc);
     }
 

widget/cocoa/nsCocoaFeatures.h

@@ -22,6 +22,7 @@ public:
   static bool OnElCapitanOrLater();
   static bool OnSierraOrLater();
   static bool OnHighSierraOrLater();
+  static bool OnSierraExactly();
 
   static bool IsAtLeastVersion(int32_t aMajor, int32_t aMinor, int32_t aBugFix=0);
 
@@ -41,9 +42,9 @@ private:
   static int32_t mOSXVersion;
 };
 
-// C-callable helper for cairo-quartz-font.c
+// C-callable helper for cairo-quartz-font.c and SkFontHost_mac.cpp
 extern "C" {
-    bool Gecko_OnSierraOrLater();
+    bool Gecko_OnSierraExactly();
 }
 
 #endif // nsCocoaFeatures_h_

widget/cocoa/nsCocoaFeatures.mm

@@ -174,11 +174,18 @@ nsCocoaFeatures::OnHighSierraOrLater()
     return (OSXVersion() >= MAC_OS_X_VERSION_10_13_HEX);
 }
 
-/* Version of OnSierraOrLater as a global function callable from C (cairo) */
-bool
-Gecko_OnSierraOrLater()
+/* static */ bool
+nsCocoaFeatures::OnSierraExactly()
 {
-    return nsCocoaFeatures::OnSierraOrLater();
+    return (OSXVersion() >= MAC_OS_X_VERSION_10_12_HEX) &&
+           (OSXVersion() < MAC_OS_X_VERSION_10_13_HEX);
+}
+
+/* Version of OnSierraExactly as global function callable from cairo & skia */
+bool
+Gecko_OnSierraExactly()
+{
+    return nsCocoaFeatures::OnSierraExactly();
 }
 
 /* static */ bool