Better refcounting and locking

All major externally accessible objects (especially FontFamily and
FontCollection) are now reference counted. In addition, there is a
global lock intended to make operations thread-safe.

WIP notice: in this version of the patch, not all external API entry
points are protected by the lock. That should be fixed.

Change-Id: I14106196e99eb101e8bf1bcb4b81359759d2086c

engine/src/flutter/include/minikin/FontCollection.h

@@ -19,13 +19,14 @@
 
 #include <vector>
 
+#include <minikin/MinikinRefCounted.h>
 #include <minikin/MinikinFont.h>
 #include <minikin/SparseBitSet.h>
 #include <minikin/FontFamily.h>
 
 namespace android {
 
-class FontCollection {
+class FontCollection : public MinikinRefCounted {
 public:
     explicit FontCollection(const std::vector<FontFamily*>& typefaces);
 
@@ -37,17 +38,17 @@ public:
         // Do copy constructor, assignment, destructor so it can be used in vectors
         Run() : font(NULL) { }
         Run(const Run& other): font(other.font), start(other.start), end(other.end) {
-            if (font) font->Ref();
+            if (font) font->RefLocked();
         }
         Run& operator=(const Run& other) {
-            if (other.font) other.font->Ref();
-            if (font) font->Unref();
+            if (other.font) other.font->RefLocked();
+            if (font) font->UnrefLocked();
             font = other.font;
             start = other.start;
             end = other.end;
             return *this;
         }
-        ~Run() { if (font) font->Unref(); }
+        ~Run() { if (font) font->UnrefLocked(); }
 
         MinikinFont* font;
         int start;

engine/src/flutter/include/minikin/FontFamily.h

@@ -19,6 +19,8 @@
 
 #include <vector>
 
+#include <minikin/MinikinRefCounted.h>
+
 namespace android {
 
 // FontStyle represents all style information needed to select an actual font
@@ -39,8 +41,10 @@ private:
     uint32_t bits;
 };
 
-class FontFamily {
+class FontFamily : public MinikinRefCounted {
 public:
+    ~FontFamily();
+
     // Add font to family, extracting style information from the font
     bool addFont(MinikinFont* typeface);
 

engine/src/flutter/include/minikin/MinikinFont.h

@@ -17,6 +17,8 @@
 #ifndef MINIKIN_FONT_H
 #define MINIKIN_FONT_H
 
+#include <minikin/MinikinRefCounted.h>
+
 // An abstraction for platform fonts, allowing Minikin to be used with
 // multiple actual implementations of fonts.
 
@@ -56,15 +58,8 @@ struct MinikinRect {
 
 class MinikinFontFreeType;
 
-class MinikinFont {
+class MinikinFont : public MinikinRefCounted {
 public:
-    void Ref() { mRefcount_++; }
-    void Unref() { if (--mRefcount_ == 0) { delete this; } }
-
-    MinikinFont() : mRefcount_(1) { }
-
-    virtual ~MinikinFont() { };
-
     virtual bool GetGlyph(uint32_t codepoint, uint32_t *glyph) const = 0;
 
     virtual float GetHorizontalAdvance(uint32_t glyph_id,
@@ -82,14 +77,6 @@ public:
         return ((uint32_t)c1 << 24) | ((uint32_t)c2 << 16) |
             ((uint32_t)c3 << 8) | (uint32_t)c4;
     }
-
-    // This is used to implement a downcast without RTTI
-    virtual MinikinFontFreeType* GetFreeType() {
-        return NULL;
-    }
-
-private:
-    int mRefcount_;
 };
 
 }  // namespace android

engine/src/flutter/include/minikin/MinikinRefCounted.h

@@ -0,0 +1,42 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// Base class for reference counted objects in Minikin
+
+#ifndef MINIKIN_REF_COUNTED_H
+#define MINIKIN_REF_COUNTED_H
+
+namespace android {
+
+class MinikinRefCounted {
+public:
+    void RefLocked() { mRefcount_++; }
+    void UnrefLocked() { if (--mRefcount_ == 0) { delete this; } }
+
+    // These refcount operations take the global lock.
+    void Ref();
+    void Unref();
+
+    MinikinRefCounted() : mRefcount_(1) { }
+
+    virtual ~MinikinRefCounted() { };
+private:
+    int mRefcount_;
+};
+
+}
+
+#endif   // MINIKIN_REF_COUNTED_H

engine/src/flutter/libs/minikin/Android.mk

@@ -24,6 +24,8 @@ LOCAL_SRC_FILES := \
     FontCollection.cpp \
     FontFamily.cpp \
     Layout.cpp \
+    MinikinInternal.cpp \
+    MinikinRefCounted.cpp \
     MinikinFontFreeType.cpp \
     SparseBitSet.cpp
 

engine/src/flutter/libs/minikin/FontCollection.cpp

@@ -41,6 +41,7 @@ FontCollection::FontCollection(const vector<FontFamily*>& typefaces) :
     const FontStyle defaultStyle;
     for (size_t i = 0; i < nTypefaces; i++) {
         FontFamily* family = typefaces[i];
+        family->RefLocked();
         FontInstance dummy;
         mInstances.push_back(dummy);  // emplace_back would be better
         FontInstance* instance = &mInstances.back();
@@ -62,7 +63,6 @@ FontCollection::FontCollection(const vector<FontFamily*>& typefaces) :
 #endif
         mMaxChar = max(mMaxChar, instance->mCoverage->length());
         lastChar.push_back(instance->mCoverage->nextSetBit(0));
-        // TODO: should probably ref typeface here, hmm
     }
     size_t nPages = mMaxChar >> kLogCharsPerPage;
     size_t offset = 0;
@@ -93,7 +93,7 @@ FontCollection::FontCollection(const vector<FontFamily*>& typefaces) :
 FontCollection::~FontCollection() {
     for (size_t i = 0; i < mInstances.size(); i++) {
         delete mInstances[i].mCoverage;
-        // probably unref the typeface here too
+        mInstances[i].mFamily->UnrefLocked();
     }
 }
 
@@ -138,7 +138,7 @@ void FontCollection::itemize(const uint16_t *string, size_t string_size, FontSty
                 run->font = NULL;  // maybe we should do something different here
             } else {
                 run->font = family->getClosestMatch(style);
-                run->font->Ref();
+                run->font->RefLocked();
             }
             lastFamily = family;
             run->start = i;

engine/src/flutter/libs/minikin/FontFamily.cpp

@@ -28,6 +28,12 @@ using std::vector;
 
 namespace android {
 
+FontFamily::~FontFamily() {
+    for (size_t i = 0; i < mFonts.size(); i++) {
+        mFonts[i].typeface->UnrefLocked();
+    }
+}
+
 bool FontFamily::addFont(MinikinFont* typeface) {
     const uint32_t os2Tag = MinikinFont::MakeTag('O', 'S', '/', '2');
     size_t os2Size = 0;
@@ -50,6 +56,7 @@ bool FontFamily::addFont(MinikinFont* typeface) {
 }
 
 void FontFamily::addFont(MinikinFont* typeface, FontStyle style) {
+    typeface->RefLocked();
     mFonts.push_back(Font(typeface, style));
 }
 

engine/src/flutter/libs/minikin/Layout.cpp

@@ -25,8 +25,7 @@
 
 #include <hb-icu.h>
 
-#include <utils/Mutex.h>
-
+#include "MinikinInternal.h"
 #include <minikin/MinikinFontFreeType.h>
 #include <minikin/Layout.h>
 
@@ -38,8 +37,6 @@ namespace android {
 // TODO: globals are not cool, move to a factory-ish object
 hb_buffer_t* buffer = 0;
 
-Mutex gLock;
-
 Bitmap::Bitmap(int width, int height) : width(width), height(height) {
     buf = new uint8_t[width * height]();
 }
@@ -280,7 +277,7 @@ static hb_script_t getScriptRun(const uint16_t *chars, size_t len, ssize_t *iter
 
 // TODO: API should probably take context
 void Layout::doLayout(const uint16_t* buf, size_t nchars) {
-    AutoMutex _l(gLock);
+    AutoMutex _l(gMinikinLock);
     if (buffer == 0) {
         buffer = hb_buffer_create();
     }

engine/src/flutter/libs/minikin/MinikinInternal.cpp

@@ -0,0 +1,25 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// Definitions internal to Minikin
+
+#include "MinikinInternal.h"
+
+namespace android {
+
+Mutex gMinikinLock;
+
+}

engine/src/flutter/libs/minikin/MinikinInternal.h

@@ -0,0 +1,34 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// Definitions internal to Minikin
+
+#ifndef MINIKIN_INTERNAL_H
+#define MINIKIN_INTERNAL_H
+
+#include <utils/Mutex.h>
+
+namespace android {
+
+// All external Minikin interfaces are designed to be thread-safe.
+// Presently, that's implemented by through a global lock, and having
+// all external interfaces take that lock.
+
+extern Mutex gMinikinLock;
+
+}
+
+#endif  // MINIKIN_INTERNAL_H

engine/src/flutter/libs/minikin/MinikinRefCounted.cpp

@@ -0,0 +1,35 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// Base class for reference counted objects in Minikin
+
+#include "MinikinInternal.h"
+
+#include <minikin/MinikinRefCounted.h>
+
+namespace android {
+
+void MinikinRefCounted::Ref() {
+    AutoMutex _l(gMinikinLock);
+    this->RefLocked();
+}
+
+void MinikinRefCounted::Unref() {
+    AutoMutex _l(gMinikinLock);
+    this->UnrefLocked();
+}
+
+}