Add support for caching font files in the Android SkFontMgr.

src/ports/SkFontMgr_android.cpp

 /*
  * Copyright 2014 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkTypes.h"
 
+#include "SkData.h"
 #include "SkFixed.h"
 #include "SkFontDescriptor.h"
 #include "SkFontHost_FreeType_common.h"
 #include "SkFontMgr.h"
 #include "SkFontMgr_android.h"
 #include "SkFontMgr_android_parser.h"
 #include "SkFontStyle.h"
+#include "SkOSFile.h"
 #include "SkPaint.h"
 #include "SkRefCnt.h"
 #include "SkString.h"
 #include "SkStream.h"
 #include "SkTArray.h"
 #include "SkTDArray.h"
 #include "SkTSearch.h"
 #include "SkTemplates.h"
 #include "SkTypefaceCache.h"
 
@@ skipping 17 matching lines @@
 
     SkString fFamilyName;
 
 private:
     typedef SkTypeface_FreeType INHERITED;
 };
 
 class SkTypeface_AndroidSystem : public SkTypeface_Android {
 public:
     SkTypeface_AndroidSystem(const SkString& pathName,
+                             const bool cacheFontFiles,
                              int index,
                              const SkFixed* axes, int axesCount,
                              const SkFontStyle& style,
                              bool isFixedPitch,
                              const SkString& familyName,
                              const SkLanguage& lang,
                              FontVariant variantStyle)
         : INHERITED(style, isFixedPitch, familyName)
         , fPathName(pathName)
+        , fCacheFontFiles(cacheFontFiles)
         , fIndex(index)
         , fAxes(axes, axesCount)
         , fLang(lang)
-        , fVariantStyle(variantStyle) { }
+        , fVariantStyle(variantStyle) {
+      if (fCacheFontFiles) {
+        fFile = sk_fopen(fPathName.c_str(), kRead_SkFILE_Flag);

[bungeman-skia, 2016/02/08 22:11:56]

This should just be handled in the init list with a fairly simple ternary.

[Khushal, 2016/02/09 00:53:40]

I wanted to add an assert here. It might as well fail on initialization if its
not able to read the font file. Does this look cleaner?

+        SkASSERT(fFile);
+      }
+    }
+
+    ~SkTypeface_AndroidSystem() {
+      if (fCacheFontFiles)
+        sk_fclose(fFile);
+    }
+
+    SkStreamAsset* onOpenStream() const {

[bungeman-skia, 2016/02/08 22:11:56]

The 'on' part of the name indicates that this is an implementation for a
superclass' wrapper to call, which this isn't this should be marked as private
and named something like 'createStream'.

[Khushal, 2016/02/09 00:53:40]

Done.

+      if (fCacheFontFiles) {
+        SkData* data = SkData::NewFromFILE(fFile);
+        return data ? new SkMemoryStream(data) : nullptr;
+      }
+      return SkStream::NewFromFile(fPathName.c_str());
+    }
 
     virtual void onGetFontDescriptor(SkFontDescriptor* desc, bool* serialize) const override {
         SkASSERT(desc);
         SkASSERT(serialize);
         desc->setFamilyName(fFamilyName.c_str());
         *serialize = false;
     }
     SkStreamAsset* onOpenStream(int* ttcIndex) const override {
         *ttcIndex = fIndex;
-        return SkStream::NewFromFile(fPathName.c_str());
+        return this->onOpenStream();
     }
     SkFontData* onCreateFontData() const override {
-        return new SkFontData(SkStream::NewFromFile(fPathName.c_str()), fIndex,
+        return new SkFontData(this->onOpenStream(), fIndex,
                               fAxes.begin(), fAxes.count());
     }
 
     const SkString fPathName;
+    const bool fCacheFontFiles;

[bungeman-skia, 2016/02/08 22:11:56]

This seems redundant with fFile being nullptr or not-nullptr.

[Khushal, 2016/02/09 00:53:40]

Removed.

     int fIndex;
     const SkSTArray<4, SkFixed, true> fAxes;
     const SkLanguage fLang;
     const FontVariant fVariantStyle;
+    FILE* fFile;

[bungeman-skia, 2016/02/08 22:11:56]

This should be SkAutoCallVProc<FILE, sk_fclose> so it automatic and no need for
explicit destructor.

[Khushal, 2016/02/09 00:53:40]

Done.

 
     typedef SkTypeface_Android INHERITED;
 };
 
 class SkTypeface_AndroidStream : public SkTypeface_Android {
 public:
     SkTypeface_AndroidStream(SkFontData* data,
                              const SkFontStyle& style,
                              bool isFixedPitch,
                              const SkString& familyName)
@@ skipping 20 matching lines @@
 
 private:
     const SkAutoTDelete<const SkFontData> fData;
     typedef SkTypeface_Android INHERITED;
 };
 
 class SkFontStyleSet_Android : public SkFontStyleSet {
     typedef SkTypeface_FreeType::Scanner Scanner;
 
 public:
-    explicit SkFontStyleSet_Android(const FontFamily& family, const Scanner& scanner) {
+    explicit SkFontStyleSet_Android(const FontFamily& family, const Scanner& scanner, const bool cacheFontFiles) {

[bungeman-skia, 2016/02/08 22:11:56]

Skia has a 100 character limit (there are a few instances of long line below as
well).

Normally Skia doesn't normally declare a primitive parameter const. I don't
mind, since it states that the value isn't intended to change, but it looks odd
against the other Skia code.

[Khushal, 2016/02/09 00:53:40]

Done.
Yeah. Just wanted to make it explicit in the code that this shouldn't change
since there is some plumbing till it gets where it needs to go.

         const SkString* cannonicalFamilyName = nullptr;
         if (family.fNames.count() > 0) {
             cannonicalFamilyName = &family.fNames[0];
         }
         // TODO? make this lazy
         for (int i = 0; i < family.fFonts.count(); ++i) {
             const FontFileInfo& fontFile = family.fFonts[i];
 
             SkString pathName(family.fBasePath);
             pathName.append(fontFile.fFileName);
@@ skipping 80 matching lines @@
                     }
                     if (!found) {
                         SkDEBUGF(("Requested font axis not found: %s '%c%c%c%c'\n",
                                     familyName.c_str(), (skTag >> 24) & 0xFF,
                                     (skTag >> 16) & 0xFF, (skTag >> 8) & 0xFF, (skTag)&0xFF));
                     }
                 }
             )
 
             fStyles.push_back().reset(new SkTypeface_AndroidSystem(
-                    pathName, ttcIndex, axisValues.get(), axisDefinitions.count(), style,
+                    pathName, cacheFontFiles, ttcIndex, axisValues.get(), axisDefinitions.count(), style,
                     isFixedWidth, familyName, lang, variant));
         }
     }
 
     int count() override {
         return fStyles.count();
     }
     void getStyle(int index, SkFontStyle* style, SkString* name) override {
         if (index < 0 || fStyles.count() <= index) {
             return;
@@ skipping 58 matching lines @@
  *  (non-replicated) set of typefaces.
  *  SkTDict<> doesn't let us do index-based lookup, so we write our own mapping.
  */
 struct NameToFamily {
     SkString name;
     SkFontStyleSet_Android* styleSet;
 };
 
 class SkFontMgr_Android : public SkFontMgr {
 public:
-    SkFontMgr_Android(const SkFontMgr_Android_CustomFonts* custom) {
+    SkFontMgr_Android(const SkFontMgr_Android_CustomFonts* custom)
+        : fCacheFontFiles(custom ? custom->fCacheFontFiles : false) {

[bungeman-skia, 2016/02/08 22:11:56]

The '{' should go on the next line, four spaces in.

[Khushal, 2016/02/09 00:53:40]

Done.

         SkTDArray<FontFamily*> families;
         if (custom && SkFontMgr_Android_CustomFonts::kPreferSystem != custom->fSystemFontUse) {
             SkString base(custom->fBasePath);
             SkFontMgr_Android_Parser::GetCustomFontFamilies(
                 families, base, custom->fFontsXml, custom->fFallbackFontsXml);
         }
         if (!custom ||
             (custom && SkFontMgr_Android_CustomFonts::kOnlyCustom != custom->fSystemFontUse))
         {
             SkFontMgr_Android_Parser::GetSystemFontFamilies(families);
@@ skipping 194 matching lines @@
 
     virtual SkTypeface* onLegacyCreateTypeface(const char familyName[],
                                                unsigned styleBits) const override {
         SkFontStyle style = SkFontStyle(styleBits);
 
         if (familyName) {
             // On Android, we must return nullptr when we can't find the requested
             // named typeface so that the system/app can provide their own recovery
             // mechanism. On other platforms we'd provide a typeface from the
             // default family instead.
-            return this->onMatchFamilyStyle(familyName, style);
+            // We only return the default family when the font manager is caching

[bungeman-skia, 2016/02/08 22:11:56]

Why this change? Does blink on Linux do bad things when getting a nullptr back?
I'd rather not have a change in behavior here.

[Khushal, 2016/02/09 00:53:39]

It just crashes. Because like the comment above says, on other platforms there
is no recovery mechanism.

I tried looking at where this call is coming from, and it seems like it comes
internally from SkTypeFace public API calls, and I wasn't sure if I should be
changing the calls in blink to have an understanding that those calls on
SkTypeface can now return a nullptr.

+            // font families.
+            SkTypeface* typeface = this->onMatchFamilyStyle(familyName, style);
+            if (typeface || !fCacheFontFiles)
+              return typeface;
         }
         return fDefaultFamily->matchStyle(style);
     }
 
 
 private:
 
     SkTypeface_FreeType::Scanner fScanner;
 
     SkTArray<SkAutoTUnref<SkFontStyleSet_Android>, true> fFontStyleSets;
     SkFontStyleSet* fDefaultFamily;
     SkTypeface* fDefaultTypeface;
 
     SkTDArray<NameToFamily> fNameToFamilyMap;
     SkTDArray<NameToFamily> fFallbackNameToFamilyMap;
 
+    const bool fCacheFontFiles;
+
     void buildNameToFamilyMap(SkTDArray<FontFamily*> families) {
         for (int i = 0; i < families.count(); i++) {
             FontFamily& family = *families[i];
 
             SkTDArray<NameToFamily>* nameToFamily = &fNameToFamilyMap;
             if (family.fIsFallbackFont) {
                 nameToFamily = &fFallbackNameToFamilyMap;
 
                 if (0 == family.fNames.count()) {
                     SkString& fallbackName = family.fNames.push_back();
                     fallbackName.printf("%.2x##fallback", i);
                 }
             }
 
-            SkFontStyleSet_Android* newSet = new SkFontStyleSet_Android(family, fScanner);
+            SkFontStyleSet_Android* newSet = new SkFontStyleSet_Android(family, fScanner, fCacheFontFiles);
             if (0 == newSet->count()) {
                 delete newSet;
                 continue;
             }
             fFontStyleSets.push_back().reset(newSet);
 
             for (int j = 0; j < family.fNames.count(); j++) {
                 NameToFamily* nextEntry = nameToFamily->append();
                 new (&nextEntry->name) SkString(family.fNames[j]);
                 nextEntry->styleSet = newSet;
@@ skipping 40 matching lines @@
         SkASSERT(custom->fSystemFontUse < SK_ARRAY_COUNT(gSystemFontUseStrings));
         SkDEBUGF(("SystemFontUse: %s BasePath: %s Fonts: %s FallbackFonts: %s\n",
                   gSystemFontUseStrings[custom->fSystemFontUse],
                   custom->fBasePath,
                   custom->fFontsXml,
                   custom->fFallbackFontsXml));
     }
 
     return new SkFontMgr_Android(custom);
 }