Reland of Support for OpenType Font Variations on Windows (patchset c…

…hromium#1 id:1 of https://codereview.chromium.org/2786263002/ )

Reason for revert:
The revert didn't help.

https://luci-milo.appspot.com/buildbot/chromium.linux/Linux%20Builder%20(dbg)(32)/65398

Original issue's description:
> Revert of Support for OpenType Font Variations on Windows (patchset chromium#8 id:140001 of https://codereview.chromium.org/2780133002/ )
>
> Reason for revert:
> Compile failure on linux bot: https://uberchromegw.corp.google.com/i/chromium.linux/builders/Linux%20Builder%20%28dbg%29%2832%29/builds/65396
>
> [6410/39636] CXX obj/third_party/pdfium/formfiller/cffl_interactiveformfiller.o
> FAILED: obj/third_party/pdfium/formfiller/cffl_interactiveformfiller.o
> /b/c/goma_client/gomacc ../../third_party/llvm-build/Release+Asserts/bin/clang++ -MMD -MF obj/third_party/pdfium/formfiller/cffl_interactiveformfiller.o.d -DV8_DEPRECATION_WARNINGS -DUSE_UDEV -DUSE_AURA=1 -DUSE_PANGO=1 -DUSE_CAIRO=1 -DUSE_GLIB=1 -DUSE_NSS_CERTS=1 -DUSE_X11=1 -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL -DCHROMIUM_BUILD -DENABLE_MEDIA_ROUTER=1 -DFIELDTRIAL_TESTING_ENABLED -DCR_CLANG_REVISION=\"298539-1\" -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -DCOMPONENT_BUILD -D_DEBUG -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DWTF_USE_DYNAMIC_ANNOTATIONS=1 -DV8_DEPRECATION_WARNINGS -D_FX_CPU_=_FX_X86_ -DOPJ_STATIC -DPNG_PREFIX -DPNG_USE_READ_MACROS -DPDF_ENABLE_V8 -DPDFIUM_PRINT_TEXT_WITH_GDI -I../.. -Igen -I../../third_party/pdfium -I../../build/linux/debian_jessie_i386-sysroot/usr/include/freetype2 -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= -funwind-tables -fPIC -pipe -B../../third_party/binutils/Linux_x64/Release/bin -fcolor-diagnostics -m32 -msse2 -mfpmath=sse -mmmx -momit-leaf-frame-pointer -pthread -mstack-alignment=16 -mstackrealign -Wall -Werror -Wextra -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default -Wno-unneeded-internal-declaration -Wno-inconsistent-missing-override -Wno-shift-negative-value -Wno-undefined-var-template -Wno-nonportable-include-path -Wno-address-of-packed-member -Wno-unused-lambda-capture -Wno-user-defined-warnings -O0 -fno-omit-frame-pointer -g2 -gsplit-dwarf --sysroot=../../build/linux/debian_jessie_i386-sysroot -fvisibility=hidden -Xclang -load -Xclang ../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.so -Xclang -add-plugin -Xclang find-bad-constructs -Xclang -plugin-arg-find-bad-constructs -Xclang check-auto-raw-pointer -Xclang -plugin-arg-find-bad-constructs -Xclang check-ipc -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -fvisibility-inlines-hidden -Wno-undefined-bool-conversion -Wno-tautological-undefined-compare -std=gnu++11 -fno-rtti -fno-exceptions -c ../../third_party/pdfium/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp -o obj/third_party/pdfium/formfiller/cffl_interactiveformfiller.o
> In file included from ../../third_party/pdfium/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp:12:
> In file included from ../../third_party/pdfium/core/fxge/cfx_pathdata.h:14:
> In file included from ../../third_party/pdfium/core/fxge/cfx_renderdevice.h:12:
> In file included from ../../third_party/pdfium/core/fxge/cfx_gemodule.h:12:
> In file included from ../../third_party/pdfium/core/fxge/cfx_fontmgr.h:13:
> In file included from ../../third_party/pdfium/core/fxge/fx_font.h:17:
> In file included from ../../third_party/pdfium/core/fxge/fx_freetype.h:11:
> In file included from ../../build/linux/debian_jessie_i386-sysroot/usr/include/freetype2/freetype.h:33:
> ../../build/linux/debian_jessie_i386-sysroot/usr/include/freetype2/config/ftconfig.h:453:5: error: 'register' storage class specifier is deprecated and incompatible with C++1z [-Werror,-Wdeprecated-register]
>     register FT_Int32  result;
>     ^~~~~~~~~
> 1 error generated.
>
> Original issue's description:
> > Support for OpenType Font Variations on Windows
> >
> > Enable support for variable fonts on Windows through using
> > SkFontMgr_Custom_Empty, a FreeType backed font-blob only SkTypeface
> > factory (no access to system fonts). We will use a hybrid DirectWrite
> > and FreeType font stack on Windows for at least as long as most of
> > Windows versions we support with Chrome still do not have native support
> > for font variations.
> >
> > Thanks to Ben Wagner for the help with enabling and prototyping this.
> >
> > BUG=700926
> >
> > Review-Url: https://codereview.chromium.org/2780133002
> > Cr-Commit-Position: refs/heads/master@{#460895}
> > Committed: https://chromium.googlesource.com/chromium/src/+/5265e65e5ea1ae3987c27f76914c9dd9a41506cd
>
> TBR=bungeman@chromium.org,bungeman@google.com,eae@chromium.org,kjellander@chromium.org,machenbach@chromium.org,drott@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=700926
>
> Review-Url: https://codereview.chromium.org/2786263002
> Cr-Commit-Position: refs/heads/master@{#460909}
> Committed: https://chromium.googlesource.com/chromium/src/+/b0e1e4e71a556604c95d25562f9447c175e541be

TBR=bungeman@chromium.org,bungeman@google.com,eae@chromium.org,kjellander@chromium.org,machenbach@chromium.org,drott@chromium.org,ortuno@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=700926

Review-Url: https://codereview.chromium.org/2792453002
Cr-Commit-Position: refs/heads/master@{#460926}

DEPS

@@ -315,6 +315,9 @@ deps = {
 
 deps_os = {
   'win': {
+    'src/third_party/freetype/src':
+      Var('chromium_git') + '/chromium/src/third_party/freetype2.git' + '@' + Var('freetype_revision'),
+
     'src/third_party/cygwin':
       Var('chromium_git') + '/chromium/deps/cygwin.git' + '@' + 'c89e446b273697fadf3a10ff1007a97c0b7de6df',
 

build_overrides/pdfium.gni

@@ -8,8 +8,9 @@ pdf_enable_v8_override = true
 # Disable XFA forms in Chromium builds.
 pdf_enable_xfa_override = false
 
-# Bundle FreeType on all platforms expect Linux. Use system FreeType on Linux.
-pdf_bundle_freetype_override = !is_linux
+# Build PDFium either using the bundled FreeType, or using a third-party
+# FreeType, configured by the embedder in //build/config/freetype.
+pdf_bundle_freetype_override = !is_linux && !is_win
 
 # Disable use of Skia backend.
 pdf_use_skia_override = false

skia/BUILD.gn

@@ -43,6 +43,10 @@ config("skia_config") {
 
   defines = skia_for_chromium_defines
 
+  if (is_win) {
+    defines += [ "SK_FREETYPE_MINIMUM_RUNTIME_VERSION=(((FREETYPE_MAJOR) << 24) | ((FREETYPE_MINOR) << 16) | ((FREETYPE_PATCH) << 8))" ]
+  }
+
   if (is_component_build) {
     defines += [
       "SKIA_DLL",
@@ -358,9 +362,6 @@ component("skia") {
 
   if (is_linux || is_android) {
     sources += [
-      "//third_party/skia/src/ports/SkFontHost_FreeType.cpp",
-      "//third_party/skia/src/ports/SkFontHost_FreeType_common.cpp",
-
       # Retain the files for the SkFontMgr_Android on linux to emulate android
       # fonts. See content/zygote/zygote_main_linux.cc
       # Note that this requires expat.
@@ -369,9 +370,22 @@ component("skia") {
     ]
   }
 
-  # Select the right BitmapPlatformDevice.
+  if (is_linux || is_android || is_win) {
+    sources += [
+      "//third_party/skia/src/ports/SkFontHost_FreeType.cpp",
+      "//third_party/skia/src/ports/SkFontHost_FreeType_common.cpp",
+    ]
+  }
+
   if (is_win) {
-    sources += [ "ext/raster_handle_allocator_win.cc" ]
+    sources += [
+      # Add the custom FreeType font manager to instantiate variable fonts on Windows.
+      "//third_party/skia/src/ports/SkFontMgr_custom.cpp",
+      "//third_party/skia/src/ports/SkFontMgr_custom_empty.cpp",
+
+      # Select the right BitmapPlatformDevice.
+      "ext/raster_handle_allocator_win.cc",
+    ]
   }
 
   if (is_clang && !is_nacl) {
@@ -400,8 +414,8 @@ component("skia") {
       configs += [ "//build/config/linux/pangocairo" ]
     }
     deps += [
-      "//build/linux:fontconfig",
       "//build/config/freetype",
+      "//build/linux:fontconfig",
       "//third_party/expat",
       "//third_party/icu:icuuc",
     ]
@@ -411,10 +425,13 @@ component("skia") {
     deps += [
       "//third_party/android_tools:cpu_features",
       "//third_party/expat",
-      "//build/config/freetype",
     ]
   }
 
+  if (is_win || is_android) {
+    deps += [ "//build/config/freetype" ]
+  }
+
   if (skia_support_pdf) {
     deps += [
       "//third_party/sfntly",

third_party/WebKit/LayoutTests/NeverFixTests

@@ -175,11 +175,9 @@ crbug.com/551843 [ Linux Win ] fast/text/emoji-font-weight-mac.html [ WontFix ]
 
 # Variable system font only supported on Mac atm.
 crbug.com/670246 [ Linux Android Win Mac10.9 Mac10.10 ] fast/text/variable-fonts/variable-mac-system-font.html [ WontFix ]
-# OpenType Variations not supported on Windows at the moment
-# Linux disabled until we can ship a statically linked newer version of FreeType
-crbug.com/669453 [ Win Mac10.9 Mac10.10 ] fast/text/variable-fonts/variable-box-font.html [ WontFix ]
-crbug.com/669453 [ Win Mac10.9 Mac10.10 ] http/tests/webfont/variable-box-font-arraybuffer.html [ WontFix ]
-crbug.com/669453 [ Win Mac10.9 Mac10.10 ] virtual/mojo-loading/http/tests/webfont/variable-box-font-arraybuffer.html [ WontFix ]
+crbug.com/669453 [ Mac10.9 Mac10.10 ] fast/text/variable-fonts/variable-box-font.html [ WontFix ]
+crbug.com/669453 [ Mac10.9 Mac10.10 ] http/tests/webfont/variable-box-font-arraybuffer.html [ WontFix ]
+crbug.com/669453 [ Mac10.9 Mac10.10 ] virtual/mojo-loading/http/tests/webfont/variable-box-font-arraybuffer.html [ WontFix ]
 
 # prefer_compositing_to_lcd_text causes things to get composited regardless of their opaqueness, causing the test to fail
 crbug.com/381840 virtual/prefer_compositing_to_lcd_text/compositing/overflow/overflow-scroll-background-opaque-to-transparent.html [ WontFix ]

third_party/WebKit/LayoutTests/fast/text/variable-fonts/variable-box-font-expected.html

@@ -10,5 +10,11 @@
     font-family: variabletest_box, sans-serif;
     font-size: 200px;
     }
+
+    /* Set a dummy value to trigger going
+     * through the font varitions code path. */
+    .ref_up {
+    font-variation-settings: "none" 0;
+    }
 </style>
-▄ ▀
+▄ <span class="ref_up">▀</span>

third_party/WebKit/Source/platform/fonts/FontCustomPlatformData.cpp

@@ -40,13 +40,16 @@
 #include "platform/fonts/opentype/FontSettings.h"
 #include "third_party/skia/include/core/SkStream.h"
 #include "third_party/skia/include/core/SkTypeface.h"
+#if OS(WIN)
+#include "third_party/skia/include/ports/SkFontMgr_empty.h"
+#endif
 #include "wtf/PtrUtil.h"
 
 namespace blink {
 
 FontCustomPlatformData::FontCustomPlatformData(sk_sp<SkTypeface> typeface,
                                                size_t dataSize)
-    : m_baseTypeface(typeface), m_dataSize(dataSize) {}
+    : m_baseTypeface(std::move(typeface)), m_dataSize(dataSize) {}
 
 FontCustomPlatformData::~FontCustomPlatformData() {}
 
@@ -67,7 +70,11 @@ FontPlatformData FontCustomPlatformData::fontPlatformData(
   // reasonable upper limit and leaving the deduplication for TODO(drott),
   // crbug.com/674878 second duplicate value should supersede first..
   if (variationSettings && variationSettings->size() < UINT16_MAX) {
+#if OS(WIN)
+    sk_sp<SkFontMgr> fm(SkFontMgr_New_Custom_Empty());
+#else
     sk_sp<SkFontMgr> fm(SkFontMgr::RefDefault());
+#endif
     Vector<SkFontMgr::FontParameters::Axis, 0> axes;
     axes.reserveCapacity(variationSettings->size());
     for (size_t i = 0; i < variationSettings->size(); ++i) {

third_party/freetype/BUILD.gn

@@ -50,13 +50,16 @@ component("freetype") {
     "src/src/base/ftsystem.c",
     "src/src/base/fttype1.c",
     "src/src/cff/cff.c",
+    "src/src/cid/type1cid.c",
     "src/src/gzip/ftgzip.c",
+    "src/src/psaux/psaux.c",
     "src/src/pshinter/pshinter.c",
     "src/src/psnames/psnames.c",
     "src/src/raster/raster.c",
     "src/src/sfnt/sfnt.c",
     "src/src/smooth/smooth.c",
     "src/src/truetype/truetype.c",
+    "src/src/type1/type1.c",
   ]
 
   if (is_linux || is_chromecast) {

third_party/freetype/include/freetype-custom-config/ftmodule.h

@@ -16,8 +16,11 @@
 /***************************************************************************/
 
 FT_USE_MODULE( FT_Module_Class, autofit_module_class )
+FT_USE_MODULE( FT_Driver_ClassRec, t1_driver_class )
+FT_USE_MODULE( FT_Driver_ClassRec, t1cid_driver_class )
 FT_USE_MODULE( FT_Driver_ClassRec, tt_driver_class )
 FT_USE_MODULE( FT_Driver_ClassRec, cff_driver_class )
+FT_USE_MODULE( FT_Module_Class, psaux_module_class )
 FT_USE_MODULE( FT_Module_Class, psnames_module_class )
 FT_USE_MODULE( FT_Module_Class, pshinter_module_class )
 FT_USE_MODULE( FT_Renderer_Class, ft_raster1_renderer_class )
@@ -27,12 +30,9 @@ FT_USE_MODULE( FT_Renderer_Class, ft_smooth_lcd_renderer_class )
 FT_USE_MODULE( FT_Renderer_Class, ft_smooth_lcdv_renderer_class )
 
 /*
-FT_USE_MODULE( FT_Driver_ClassRec, t1_driver_class )
-FT_USE_MODULE( FT_Driver_ClassRec, t1cid_driver_class )
 FT_USE_MODULE( FT_Driver_ClassRec, pfr_driver_class )
 FT_USE_MODULE( FT_Driver_ClassRec, t42_driver_class )
 FT_USE_MODULE( FT_Driver_ClassRec, winfnt_driver_class )
 FT_USE_MODULE( FT_Driver_ClassRec, pcf_driver_class )
 FT_USE_MODULE( FT_Driver_ClassRec, bdf_driver_class )
-FT_USE_MODULE( FT_Module_Class, psaux_module_class )
 */

third_party/freetype/include/freetype-custom-config/ftoption.h

@@ -102,7 +102,7 @@ FT_BEGIN_HEADER
   /*                       cff:no-stem-darkening=1 \                       */
   /*                       autofitter:warping=1                            */
   /*                                                                       */
-/*#define FT_CONFIG_OPTION_ENVIRONMENT_PROPERTIES*/
+#define FT_CONFIG_OPTION_ENVIRONMENT_PROPERTIES
 
 
   /*************************************************************************/
@@ -122,7 +122,7 @@ FT_BEGIN_HEADER
   /* This is done to allow FreeType clients to run unmodified, forcing     */
   /* them to display normal gray-level anti-aliased glyphs.                */
   /*                                                                       */
-/* #define FT_CONFIG_OPTION_SUBPIXEL_RENDERING */
+#define FT_CONFIG_OPTION_SUBPIXEL_RENDERING
 
 
   /*************************************************************************/
@@ -414,7 +414,7 @@ FT_BEGIN_HEADER
   /* supply font data incrementally as the document is parsed, such        */
   /* as the Ghostscript interpreter for the PostScript language.           */
   /*                                                                       */
-/* #define FT_CONFIG_OPTION_INCREMENTAL */
+#define FT_CONFIG_OPTION_INCREMENTAL
 
 
   /*************************************************************************/
@@ -790,7 +790,7 @@ FT_BEGIN_HEADER
   /*                                                                       */
   /* The Chinese font MingTiEG-Medium (CNS 11643 character set) needs 256. */
   /*                                                                       */
-#define T1_MAX_CHARSTRINGS_OPERANDS  256
+#define T1_MAX_CHARSTRINGS_OPERANDS  512
 
 
   /*************************************************************************/