Bug 1331683 - Don't attempt to use any Core Text and Core Graphics variation-font APIs on pre-Sierra systems. r=jrmuizel,lsalzman
authorJonathan Kew <jkew@mozilla.com>
Thu, 09 Feb 2017 21:37:24 +0000
changeset 341766 de99933ae32454ca3081aa60a25739f79a9e0cca
parent 341709 db62e3ac853f7c4d8f045629358264ebf822398c
child 341767 6f76cbf70df3634b5967bb509c04433a2e3e65ad
push id31343
push usercbook@mozilla.com
push dateFri, 10 Feb 2017 12:50:15 +0000
treeherdermozilla-central@b9c6246f13ea [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel, lsalzman
bugs1331683
milestone54.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1331683 - Don't attempt to use any Core Text and Core Graphics variation-font APIs on pre-Sierra systems. r=jrmuizel,lsalzman
gfx/2d/NativeFontResourceMac.cpp
gfx/2d/ScaledFontMac.cpp
gfx/cairo/cairo/src/cairo-quartz-font.c
gfx/skia/skia/src/ports/SkFontHost_mac.cpp
gfx/thebes/gfxMacFont.cpp
layout/generic/nsTextFrame.cpp
widget/cocoa/nsCocoaFeatures.h
widget/cocoa/nsCocoaFeatures.mm
--- a/gfx/2d/NativeFontResourceMac.cpp
+++ b/gfx/2d/NativeFontResourceMac.cpp
@@ -8,16 +8,18 @@
 #include "Types.h"
 
 #include "mozilla/RefPtr.h"
 
 #ifdef MOZ_WIDGET_UIKIT
 #include <CoreFoundation/CoreFoundation.h>
 #endif
 
+#include "nsCocoaFeatures.h"
+
 // Simple helper class to automatically release a CFObject when it goes out
 // of scope.
 template<class T>
 class AutoRelease
 {
 public:
   explicit AutoRelease(T aObject)
     : mObject(aObject)
@@ -49,16 +51,22 @@ private:
 
 // This is essentially identical to the similarly-named helper function
 // in gfx/thebes/gfxMacFont.cpp. Maybe we should put it somewhere that
 // can be shared by both Moz2d and Thebes callers?
 static CFDictionaryRef
 CreateVariationDictionaryOrNull(CGFontRef aCGFont, uint32_t aVariationCount,
   const mozilla::gfx::ScaledFont::VariationSetting* aVariations)
 {
+  // Avoid calling potentially buggy variation APIs on pre-Sierra macOS
+  // versions (see bug 1331683)
+  if (!nsCocoaFeatures::OnSierraOrLater()) {
+    return nullptr;
+  }
+
   AutoRelease<CTFontRef>
     ctFont(CTFontCreateWithGraphicsFont(aCGFont, 0, nullptr, nullptr));
   AutoRelease<CFArrayRef> axes(CTFontCopyVariationAxes(ctFont));
   if (!axes) {
     return nullptr;
   }
 
   CFIndex axisCount = CFArrayGetCount(axes);
--- a/gfx/2d/ScaledFontMac.cpp
+++ b/gfx/2d/ScaledFontMac.cpp
@@ -10,16 +10,17 @@
 #include "skia/include/core/SkPath.h"
 #include "skia/include/ports/SkTypeface_mac.h"
 #endif
 #include <vector>
 #include <dlfcn.h>
 #ifdef MOZ_WIDGET_UIKIT
 #include <CoreFoundation/CoreFoundation.h>
 #endif
+#include "nsCocoaFeatures.h"
 
 #ifdef MOZ_WIDGET_COCOA
 // prototype for private API
 extern "C" {
 CGPathRef CGFontGetGlyphPath(CGFontRef fontRef, CGAffineTransform *textTransform, int unknown, CGGlyph glyph);
 };
 #endif
 
@@ -33,16 +34,22 @@ namespace gfx {
 ScaledFontMac::CTFontDrawGlyphsFuncT* ScaledFontMac::CTFontDrawGlyphsPtr = nullptr;
 bool ScaledFontMac::sSymbolLookupDone = false;
 
 // Helper to create a CTFont from a CGFont, copying any variations that were
 // set on the original CGFont.
 static CTFontRef
 CreateCTFontFromCGFontWithVariations(CGFontRef aCGFont, CGFloat aSize)
 {
+    // Avoid calling potentially buggy variation APIs on pre-Sierra macOS
+    // versions (see bug 1331683)
+    if (!nsCocoaFeatures::OnSierraOrLater()) {
+        return CTFontCreateWithGraphicsFont(aCGFont, aSize, nullptr, nullptr);
+    }
+
     CFDictionaryRef vars = CGFontCopyVariations(aCGFont);
     CTFontRef ctFont;
     if (vars) {
         CFDictionaryRef varAttr =
             CFDictionaryCreate(nullptr,
                                (const void**)&kCTFontVariationAttribute,
                                (const void**)&vars, 1,
                                &kCFTypeDictionaryKeyCallBacks,
@@ -270,27 +277,31 @@ ScaledFontMac::GetFontFileData(FontFileD
     memset(&buf.data[checkSumAdjustmentOffset], 0, sizeof(uint32_t));
     uint32_t fontChecksum = CFSwapInt32HostToBig(0xb1b0afba - CalcTableChecksum(reinterpret_cast<const uint32_t*>(buf.data), offset));
     // set checkSumAdjust to the computed checksum
     memcpy(&buf.data[checkSumAdjustmentOffset], &fontChecksum, sizeof(fontChecksum));
 
     // Collect any variation settings that were incorporated into the CTFont.
     uint32_t variationCount = 0;
     VariationSetting* variations = nullptr;
-    if (mCTFont) {
-      CFDictionaryRef dict = CTFontCopyVariation(mCTFont);
-      if (dict) {
-        CFIndex count = CFDictionaryGetCount(dict);
-        if (count > 0) {
-          variations = new VariationSetting[count];
-          VariationSetting* vPtr = variations;
-          CFDictionaryApplyFunction(dict, CollectVariationSetting, &vPtr);
-          variationCount = vPtr - variations;
+    // Avoid calling potentially buggy variation APIs on pre-Sierra macOS
+    // versions (see bug 1331683)
+    if (nsCocoaFeatures::OnSierraOrLater()) {
+      if (mCTFont) {
+        CFDictionaryRef dict = CTFontCopyVariation(mCTFont);
+        if (dict) {
+          CFIndex count = CFDictionaryGetCount(dict);
+          if (count > 0) {
+            variations = new VariationSetting[count];
+            VariationSetting* vPtr = variations;
+            CFDictionaryApplyFunction(dict, CollectVariationSetting, &vPtr);
+            variationCount = vPtr - variations;
+          }
+          CFRelease(dict);
         }
-        CFRelease(dict);
       }
     }
 
     // we always use an index of 0
     aDataCallback(buf.data, buf.offset, 0, mSize, variationCount, variations, aBaton);
     delete[] variations;
 
     return true;
--- a/gfx/cairo/cairo/src/cairo-quartz-font.c
+++ b/gfx/cairo/cairo/src/cairo-quartz-font.c
@@ -335,16 +335,23 @@ const cairo_font_face_backend_t _cairo_q
     _cairo_quartz_font_face_scaled_font_create
 };
 
 // Helper to create a CTFont from a CGFont, copying any variations that were
 // set on the original CGFont.
 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()) {
+        return CTFontCreateWithGraphicsFont(aCGFont, aSize, NULL, NULL);
+    }
     CFDictionaryRef vars = CGFontCopyVariations(aCGFont);
     CTFontRef ctFont;
     if (vars) {
         CFDictionaryRef varAttr =
             CFDictionaryCreate(NULL,
                                (const void**)&kCTFontVariationAttribute,
                                (const void**)&vars, 1,
                                &kCFTypeDictionaryKeyCallBacks,
--- a/gfx/skia/skia/src/ports/SkFontHost_mac.cpp
+++ b/gfx/skia/skia/src/ports/SkFontHost_mac.cpp
@@ -796,32 +796,36 @@ static CTFontRef ctfont_create_exact_cop
     // If non-nullptr then with fonts with variation axes, the copy will fail in
     // CGFontVariationFromDictCallback when it assumes kCGFontVariationAxisName is CFNumberRef
     // which it quite obviously is not.
 
     // Because we cannot setup the CTFont descriptor to match, the same restriction applies here
     // as other uses of CTFontCreateWithGraphicsFont which is that such CTFonts should not escape
     // the scaler context, since they aren't 'normal'.
 
-    // Not AutoCFRelease<> because CGFontCopyVariations can return null!
-    CFDictionaryRef variations = CGFontCopyVariations(baseCGFont);
-    if (variations) {
-        AutoCFRelease<CFDictionaryRef>
-            varAttr(CFDictionaryCreate(nullptr,
-                                       (const void**)&kCTFontVariationAttribute,
-                                       (const void**)&variations,
-                                       1,
-                                       &kCFTypeDictionaryKeyCallBacks,
-                                       &kCFTypeDictionaryValueCallBacks));
-        CFRelease(variations);
+    // Avoid calling potentially buggy variation APIs on pre-Sierra macOS
+    // versions (see bug 1331683)
+    if (darwinVersion() >= 16) {
+        // Not AutoCFRelease<> because CGFontCopyVariations can return null!
+        CFDictionaryRef variations = CGFontCopyVariations(baseCGFont);
+        if (variations) {
+            AutoCFRelease<CFDictionaryRef>
+                varAttr(CFDictionaryCreate(nullptr,
+                                           (const void**)&kCTFontVariationAttribute,
+                                           (const void**)&variations,
+                                           1,
+                                           &kCFTypeDictionaryKeyCallBacks,
+                                           &kCFTypeDictionaryValueCallBacks));
+            CFRelease(variations);
 
-        AutoCFRelease<CTFontDescriptorRef>
-            varDesc(CTFontDescriptorCreateWithAttributes(varAttr));
+            AutoCFRelease<CTFontDescriptorRef>
+                varDesc(CTFontDescriptorCreateWithAttributes(varAttr));
 
-        return CTFontCreateWithGraphicsFont(baseCGFont, textSize, transform, varDesc);
+            return CTFontCreateWithGraphicsFont(baseCGFont, textSize, transform, varDesc);
+        }
     }
 
     return CTFontCreateWithGraphicsFont(baseCGFont, textSize, transform, nullptr);
 }
 
 SkScalerContext_Mac::SkScalerContext_Mac(SkTypeface_Mac* typeface,
                                          const SkScalerContextEffects& effects,
                                          const SkDescriptor* desc)
--- a/gfx/thebes/gfxMacFont.cpp
+++ b/gfx/thebes/gfxMacFont.cpp
@@ -11,16 +11,17 @@
 #include "gfxCoreTextShaper.h"
 #include <algorithm>
 #include "gfxPlatformMac.h"
 #include "gfxContext.h"
 #include "gfxFontUtils.h"
 #include "gfxMacPlatformFontList.h"
 #include "gfxFontConstants.h"
 #include "gfxTextRun.h"
+#include "nsCocoaFeatures.h"
 
 #include "cairo-quartz.h"
 
 using namespace mozilla;
 using namespace mozilla::gfx;
 
 // Simple helper class to automatically release a CFObject when it goes out
 // of scope.
@@ -55,16 +56,22 @@ public:
 private:
     T mObject;
 };
 
 static CFDictionaryRef
 CreateVariationDictionaryOrNull(CGFontRef aCGFont,
                                 const nsTArray<gfxFontVariation>& aVariations)
 {
+    // Avoid calling potentially buggy variation APIs on pre-Sierra macOS
+    // versions (see bug 1331683)
+    if (!nsCocoaFeatures::OnSierraOrLater()) {
+        return nullptr;
+    }
+
     AutoRelease<CTFontRef>
       ctFont(CTFontCreateWithGraphicsFont(aCGFont, 0, nullptr, nullptr));
     AutoRelease<CFArrayRef> axes(CTFontCopyVariationAxes(ctFont));
     if (!axes) {
         return nullptr;
     }
 
     CFIndex axisCount = CFArrayGetCount(axes);
@@ -526,16 +533,22 @@ gfxMacFont::GetCharWidth(CFDataRef aCmap
 }
 
 /* static */
 CTFontRef
 gfxMacFont::CreateCTFontFromCGFontWithVariations(CGFontRef aCGFont,
                                                  CGFloat aSize,
                                                  CTFontDescriptorRef aFontDesc)
 {
+    // Avoid calling potentially buggy variation APIs on pre-Sierra macOS
+    // versions (see bug 1331683)
+    if (!nsCocoaFeatures::OnSierraOrLater()) {
+        return CTFontCreateWithGraphicsFont(aCGFont, aSize, nullptr, aFontDesc);
+    }
+
     CFDictionaryRef variations = ::CGFontCopyVariations(aCGFont);
     CTFontRef ctFont;
     if (variations) {
         CFDictionaryRef varAttr =
             ::CFDictionaryCreate(nullptr,
                                  (const void**)&kCTFontVariationAttribute,
                                  (const void**)&variations, 1,
                                  &kCFTypeDictionaryKeyCallBacks,
--- a/layout/generic/nsTextFrame.cpp
+++ b/layout/generic/nsTextFrame.cpp
@@ -1779,16 +1779,33 @@ GetSpacingFlags(nsIFrame* aFrame, const 
     (eStyleUnit_Coord == ls.GetUnit() && ls.GetCoordValue() != 0) ||
     (eStyleUnit_Coord == ws.GetUnit() && ws.GetCoordValue() != 0) ||
     (eStyleUnit_Percent == ws.GetUnit() && ws.GetPercentValue() != 0) ||
     (eStyleUnit_Calc == ws.GetUnit() && !ws.GetCalcValue()->IsDefinitelyZero());
 
   return nonStandardSpacing ? gfxTextRunFactory::TEXT_ENABLE_SPACING : 0;
 }
 
+static bool
+IsBaselineAligned(const nsStyleCoord& aCoord)
+{
+  switch (aCoord.GetUnit()) {
+    case eStyleUnit_Enumerated:
+      return aCoord.GetIntValue() == NS_STYLE_VERTICAL_ALIGN_BASELINE;
+    case eStyleUnit_Coord:
+      return aCoord.GetCoordValue() == 0;
+    case eStyleUnit_Percent:
+      return aCoord.GetPercentValue() == 0;
+    case eStyleUnit_Calc:
+      return aCoord.GetCalcValue()->IsDefinitelyZero();
+    default:
+      return false;
+  }
+}
+
 bool
 BuildTextRunsScanner::ContinueTextRunAcrossFrames(nsTextFrame* aFrame1, nsTextFrame* aFrame2)
 {
   // We don't need to check font size inflation, since
   // |FindLineContainer| above (via |nsIFrame::CanContinueTextRun|)
   // ensures that text runs never cross block boundaries.  This means
   // that the font size inflation on all text frames in the text run is
   // already guaranteed to be the same as each other (and for the line
@@ -1808,32 +1825,40 @@ BuildTextRunsScanner::ContinueTextRunAcr
   // here. This avoids creating giant textruns for an entire plain text file.
   // Note that we create a single text frame for a preformatted text node,
   // even if it has newlines in it, so typically we won't see trailing newlines
   // until after reflow has broken up the frame into one (or more) frames per
   // line. That's OK though.
   if (textStyle1->NewlineIsSignificant(aFrame1) && HasTerminalNewline(aFrame1))
     return false;
 
+  if (!IsBaselineAligned(sc1->StyleDisplay()->mVerticalAlign)) {
+    return false;
+  }
+
   if (aFrame1->GetContent() == aFrame2->GetContent() &&
       aFrame1->GetNextInFlow() != aFrame2) {
     // aFrame2 must be a non-fluid continuation of aFrame1. This can happen
     // sometimes when the unicode-bidi property is used; the bidi resolver
     // breaks text into different frames even though the text has the same
     // direction. We can't allow these two frames to share the same textrun
     // because that would violate our invariant that two flows in the same
     // textrun have different content elements.
     return false;
   }
 
   nsStyleContext* sc2 = aFrame2->StyleContext();
   const nsStyleText* textStyle2 = sc2->StyleText();
   if (sc1 == sc2)
     return true;
 
+  if (!IsBaselineAligned(sc1->StyleDisplay()->mVerticalAlign)) {
+    return false;
+  }
+
   const nsStyleFont* fontStyle1 = sc1->StyleFont();
   const nsStyleFont* fontStyle2 = sc2->StyleFont();
   nscoord letterSpacing1 = LetterSpacing(aFrame1);
   nscoord letterSpacing2 = LetterSpacing(aFrame2);
   return fontStyle1->mFont == fontStyle2->mFont &&
     fontStyle1->mLanguage == fontStyle2->mLanguage &&
     textStyle1->mTextTransform == textStyle2->mTextTransform &&
     nsLayoutUtils::GetTextRunFlagsForStyle(sc1, fontStyle1, textStyle1, letterSpacing1) ==
--- a/widget/cocoa/nsCocoaFeatures.h
+++ b/widget/cocoa/nsCocoaFeatures.h
@@ -34,9 +34,15 @@ public:
   static int32_t ExtractMinorVersion(int32_t aVersion);
   static int32_t ExtractBugFixVersion(int32_t aVersion);
 
 private:
   static void InitializeVersionNumbers();
 
   static int32_t mOSXVersion;
 };
+
+// C-callable helper for cairo-quartz-font.c
+extern "C" {
+    bool Gecko_OnSierraOrLater();
+}
+
 #endif // nsCocoaFeatures_h_
--- a/widget/cocoa/nsCocoaFeatures.mm
+++ b/widget/cocoa/nsCocoaFeatures.mm
@@ -162,13 +162,20 @@ nsCocoaFeatures::OnElCapitanOrLater()
 }
 
 /* static */ bool
 nsCocoaFeatures::OnSierraOrLater()
 {
     return (OSXVersion() >= MAC_OS_X_VERSION_10_12_HEX);
 }
 
+/* Version of OnSierraOrLater as a global function callable from C (cairo) */
+bool
+Gecko_OnSierraOrLater()
+{
+    return nsCocoaFeatures::OnSierraOrLater();
+}
+
 /* static */ bool
 nsCocoaFeatures::IsAtLeastVersion(int32_t aMajor, int32_t aMinor, int32_t aBugFix)
 {
     return OSXVersion() >= GetVersion(aMajor, aMinor, aBugFix);
 }