Bug 1331683 - Don't attempt to use any Core Text and Core Graphics variation-font APIs on pre-Sierra systems. r=jrmuizel, r=lsalzman, a=gchang
authorJonathan Kew <jkew@mozilla.com>
Sat, 11 Feb 2017 23:49:56 +0000
changeset 376133 8f207d31a7ca908ac716bc7240e141221a997ff0
parent 376132 666413533f5acf0f19b2535f41cff5dcfc78f7cc
child 376134 40ad75d6e9a519ddd7535c0e776c9c6253455b82
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel, lsalzman, gchang
bugs1331683
milestone53.0a2
Bug 1331683 - Don't attempt to use any Core Text and Core Graphics variation-font APIs on pre-Sierra systems. r=jrmuizel, r=lsalzman, a=gchang
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/tests/crashtests/1331683.html
gfx/tests/crashtests/crashtests.list
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)
new file mode 100644
--- /dev/null
+++ b/gfx/tests/crashtests/1331683.html
@@ -0,0 +1,2 @@
+<!-- crashtest for bug 1331683 on Mac OS X 10.9/10.10 -->
+<div style="font-family:Skia">hello world</div>
--- a/gfx/tests/crashtests/crashtests.list
+++ b/gfx/tests/crashtests/crashtests.list
@@ -128,8 +128,9 @@ load 1034403-1.html
 load 1205900.html
 load 1134549-1.svg
 load balinese-letter-spacing.html
 load 1216832-1.html
 load 1225125-1.html
 asserts-if(stylo,2) load 1308394.html # bug 1324669
 skip-if(stylo) load 1317403-1.html # bug 1331533
 load 1325159-1.html
+load 1331683.html
--- 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);
 }