Add support for caching font files in the Android SkFontMgr.

src/ports/SkFontMgr_android.cpp

Index: src/ports/SkFontMgr_android.cpp
diff --git a/src/ports/SkFontMgr_android.cpp b/src/ports/SkFontMgr_android.cpp
index afa78734544f685bfba82be17062e89f6b2f543b..4269b2a82de0db42a1422c922d4b8d0997692dd4 100644
--- a/src/ports/SkFontMgr_android.cpp
+++ b/src/ports/SkFontMgr_android.cpp
@@ -7,6 +7,7 @@
 
 #include "SkTypes.h"
 
+#include "SkData.h"
 #include "SkFixed.h"
 #include "SkFontDescriptor.h"
 #include "SkFontHost_FreeType_common.h"
@@ -14,6 +15,7 @@
 #include "SkFontMgr_android.h"
 #include "SkFontMgr_android_parser.h"
 #include "SkFontStyle.h"
+#include "SkOSFile.h"
 #include "SkPaint.h"
 #include "SkRefCnt.h"
 #include "SkString.h"
@@ -51,6 +53,7 @@ private:
 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,
@@ -63,7 +66,22 @@ public:
         , fIndex(index)
         , fAxes(axes, axesCount)
         , fLang(lang)
-        , fVariantStyle(variantStyle) { }
+        , fVariantStyle(variantStyle)
+        , fFile(

[bungeman-skia, 2016/02/11 20:06:09]

Skia allows up to 100 character lines, can probably pull up the next line.

[Khushal, 2016/02/12 04:54:07]

Done.

+            cacheFontFiles ? sk_fopen(fPathName.c_str(), kRead_SkFILE_Flag)
+                           : nullptr)
+        , closeOnDestroy(fFile) {
+        if(cacheFontFiles)

[bungeman-skia, 2016/02/11 20:06:09]

space between 'if' and '('.

[Khushal, 2016/02/12 04:54:07]

Done.

+          SkASSERT(fFile);

[bungeman-skia, 2016/02/11 20:06:09]

four space indents, here and below

[Khushal, 2016/02/12 04:54:07]

Done.

+    }
+
+    SkStreamAsset* createStream() const {
+      if (fFile) {
+        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);
@@ -73,10 +91,10 @@ public:
     }
     SkStreamAsset* onOpenStream(int* ttcIndex) const override {
         *ttcIndex = fIndex;
-        return SkStream::NewFromFile(fPathName.c_str());
+        return this->createStream();
     }
     SkFontData* onCreateFontData() const override {
-        return new SkFontData(SkStream::NewFromFile(fPathName.c_str()), fIndex,
+        return new SkFontData(this->createStream(), fIndex,
                               fAxes.begin(), fAxes.count());
     }
 
@@ -85,6 +103,8 @@ public:
     const SkSTArray<4, SkFixed, true> fAxes;
     const SkLanguage fLang;
     const FontVariant fVariantStyle;
+    FILE* fFile;
+    SkAutoTCallVProc<FILE, sk_fclose> closeOnDestroy;

[bungeman-skia, 2016/02/11 20:06:09]

This extra field isn't needed, SkAUtoTCallVProc is actually just a wrapper for
unique_ptr with a deleter with calls the given proc. In other words, these two
lines can just be:

SkAutoTCallVProc<FILE, sk_fclose> fFile;

[Khushal, 2016/02/12 04:54:07]

Oh. Done. Thanks!

 
     typedef SkTypeface_Android INHERITED;
 };
@@ -125,7 +145,8 @@ 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) {
         const SkString* cannonicalFamilyName = nullptr;
         if (family.fNames.count() > 0) {
             cannonicalFamilyName = &family.fNames[0];
@@ -226,7 +247,7 @@ public:
             )
 
             fStyles.push_back().reset(new SkTypeface_AndroidSystem(
-                    pathName, ttcIndex, axisValues.get(), axisDefinitions.count(), style,
+                    pathName, cacheFontFiles, ttcIndex, axisValues.get(), axisDefinitions.count(), style,

[bungeman-skia, 2016/02/11 20:06:09]

This line is a bit long (more than 100 characters)

[Khushal, 2016/02/12 04:54:07]

Done.

                     isFixedWidth, familyName, lang, variant));
         }
     }
@@ -305,7 +326,9 @@ struct NameToFamily {
 
 class SkFontMgr_Android : public SkFontMgr {
 public:
-    SkFontMgr_Android(const SkFontMgr_Android_CustomFonts* custom) {
+    SkFontMgr_Android(const SkFontMgr_Android_CustomFonts* custom)
+        : fIsolated(custom ? custom->fIsolated : false)
+    {
         SkTDArray<FontFamily*> families;
         if (custom && SkFontMgr_Android_CustomFonts::kPreferSystem != custom->fSystemFontUse) {
             SkString base(custom->fBasePath);
@@ -520,7 +543,11 @@ protected:
             // 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 running
+            // as isolated since we don't expect the app to have a recovery mechanism.
+            SkTypeface* typeface = this->onMatchFamilyStyle(familyName, style);
+            if (typeface || !fIsolated)

[bungeman-skia, 2016/02/11 20:06:09]

I'm going to have to take quite a look into this. 

[Khushal, 2016/02/12 04:54:07]

I tried reproducing this to trace back the calls. I think I might have made an
error with how I was testing this change with the corresponding change in blink.
I'm not seeing it with the final patch,
https://codereview.chromium.org/1685053002/.
Sorry about the confusion. Removed it.

[bungeman-skia, 2016/02/12 15:13:47]

That is awesome, I was just building Chromium to take a look at this. Saves me a
day of investigating.

+              return typeface;
         }
         return fDefaultFamily->matchStyle(style);
     }
@@ -537,6 +564,8 @@ private:
     SkTDArray<NameToFamily> fNameToFamilyMap;
     SkTDArray<NameToFamily> fFallbackNameToFamilyMap;
 
+    const bool fIsolated;
+
     void buildNameToFamilyMap(SkTDArray<FontFamily*> families) {
         for (int i = 0; i < families.count(); i++) {
             FontFamily& family = *families[i];
@@ -551,7 +580,7 @@ private:
                 }
             }
 
-            SkFontStyleSet_Android* newSet = new SkFontStyleSet_Android(family, fScanner);
+            SkFontStyleSet_Android* newSet = new SkFontStyleSet_Android(family, fScanner, fIsolated);

[bungeman-skia, 2016/02/11 20:06:09]

this line ran a bit long too.

[Khushal, 2016/02/12 04:54:07]

Done.

             if (0 == newSet->count()) {
                 delete newSet;
                 continue;