From eb2be7fa4413a566212782d8efae5dc225002821 Mon Sep 17 00:00:00 2001 From: bungeman Date: Tue, 10 Feb 2015 07:51:12 -0800 Subject: [PATCH] Additional cleanups to Android config parsing. Properly labels several methods as static. Use XML_GetBuffer to avoid an extra copy. Set the memory allocators to Skia's. Set define in 'defines' instead of cflags. Update debug dumper. Review URL: https://codereview.chromium.org/915443002 --- platform_tools/android/gyp/dependencies.gypi | 4 +- src/ports/SkFontConfigParser_android.cpp | 51 +++++++++++++------- tests/FontConfigParser.cpp | 13 +++-- 3 files changed, 44 insertions(+), 24 deletions(-) diff --git a/platform_tools/android/gyp/dependencies.gypi b/platform_tools/android/gyp/dependencies.gypi index bb0db7184b..6507ee45f7 100644 --- a/platform_tools/android/gyp/dependencies.gypi +++ b/platform_tools/android/gyp/dependencies.gypi @@ -44,7 +44,9 @@ 'cflags': [ '-w', '-fexceptions', - '-DHAVE_EXPAT_CONFIG_H', + ], + 'defines': [ + 'HAVE_EXPAT_CONFIG_H', ], 'direct_dependent_settings': { 'include_dirs': [ diff --git a/src/ports/SkFontConfigParser_android.cpp b/src/ports/SkFontConfigParser_android.cpp index 78d997dccf..20fee40a78 100644 --- a/src/ports/SkFontConfigParser_android.cpp +++ b/src/ports/SkFontConfigParser_android.cpp @@ -115,7 +115,7 @@ static bool memeq(const char* s1, const char* s2, size_t n1, size_t n2) { namespace lmpParser { -void familyElementHandler(FontFamily* family, const char** attributes) { +static void family_element_handler(FontFamily* family, const char** attributes) { // A non-fallback tag must have a canonical name attribute. // A fallback tag has no name, and may have lang and variant // attributes. @@ -142,12 +142,12 @@ void familyElementHandler(FontFamily* family, const char** attributes) { } } -void XMLCALL fontFileNameHandler(void* data, const char* s, int len) { +static void XMLCALL font_file_name_handler(void* data, const char* s, int len) { FamilyData* self = static_cast(data); self->fCurrentFontInfo->fFileName.append(s, len); } -void fontElementHandler(FamilyData* self, FontFileInfo* file, const char** attributes) { +static void font_element_handler(FamilyData* self, FontFileInfo* file, const char** attributes) { // A should have weight (integer) and style (normal, italic) attributes. // NOTE: we ignore the style. // The element should contain a filename. @@ -161,10 +161,10 @@ void fontElementHandler(FamilyData* self, FontFileInfo* file, const char** attri } } } - XML_SetCharacterDataHandler(self->fParser, fontFileNameHandler); + XML_SetCharacterDataHandler(self->fParser, font_file_name_handler); } -FontFamily* findFamily(FamilyData* self, const SkString& familyName) { +static FontFamily* find_family(FamilyData* self, const SkString& familyName) { for (int i = 0; i < self->fFamilies.count(); i++) { FontFamily* candidate = self->fFamilies[i]; for (int j = 0; j < candidate->fNames.count(); j++) { @@ -176,7 +176,7 @@ FontFamily* findFamily(FamilyData* self, const SkString& familyName) { return NULL; } -void aliasElementHandler(FamilyData* self, const char** attributes) { +static void alias_element_handler(FamilyData* self, const char** attributes) { // An must have name and to attributes. // It may have weight (integer). // If it *does not* have a weight, it is a variant name for a . @@ -203,7 +203,7 @@ void aliasElementHandler(FamilyData* self, const char** attributes) { } // Assumes that the named family is already declared - FontFamily* targetFamily = findFamily(self, to); + FontFamily* targetFamily = find_family(self, to); if (!targetFamily) { SkDebugf("---- Font alias target %s (NOT FOUND)", to.c_str()); return; @@ -224,22 +224,22 @@ void aliasElementHandler(FamilyData* self, const char** attributes) { } } -void XMLCALL startElementHandler(void* data, const char* tag, const char** attributes) { +static void XMLCALL start_element_handler(void* data, const char* tag, const char** attributes) { FamilyData* self = static_cast(data); size_t len = strlen(tag); if (MEMEQ("family", tag, len)) { self->fCurrentFamily.reset(new FontFamily(self->fBasePath, self->fIsFallback)); - familyElementHandler(self->fCurrentFamily, attributes); + family_element_handler(self->fCurrentFamily, attributes); } else if (MEMEQ("font", tag, len)) { FontFileInfo* file = &self->fCurrentFamily->fFonts.push_back(); self->fCurrentFontInfo = file; - fontElementHandler(self, file, attributes); + font_element_handler(self, file, attributes); } else if (MEMEQ("alias", tag, len)) { - aliasElementHandler(self, attributes); + alias_element_handler(self, attributes); } } -void XMLCALL endElementHandler(void* data, const char* tag) { +static void XMLCALL end_element_handler(void* data, const char* tag) { FamilyData* self = static_cast(data); size_t len = strlen(tag); if (MEMEQ("family", tag, len)) { @@ -337,8 +337,8 @@ static void XMLCALL start_element_handler(void* data, const char* tag, const cha int version; if (parse_non_negative_integer(valueString, &version) && (version >= 21)) { XML_SetElementHandler(self->fParser, - lmpParser::startElementHandler, - lmpParser::endElementHandler); + lmpParser::start_element_handler, + lmpParser::end_element_handler); self->fVersion = version; } } @@ -405,6 +405,12 @@ static void XMLCALL xml_entity_decl_handler(void *data, XML_StopParser(self->fParser, XML_FALSE); } +static const XML_Memory_Handling_Suite sk_XML_alloc = { + sk_malloc_throw, + sk_realloc_throw, + sk_free +}; + template struct remove_ptr {typedef T type;}; template struct remove_ptr {typedef T type;}; @@ -424,7 +430,8 @@ static int parse_config_file(const char* filename, SkTDArray& famil return -1; } - SkAutoTCallVProc::type, XML_ParserFree> parser(XML_ParserCreate(NULL)); + SkAutoTCallVProc::type, XML_ParserFree> parser( + XML_ParserCreate_MM(NULL, &sk_XML_alloc, NULL)); if (!parser) { SkDebugf("Could not create XML parser.\n"); return -1; @@ -439,12 +446,20 @@ static int parse_config_file(const char* filename, SkTDArray& famil // Start parsing oldschool; switch these in flight if we detect a newer version of the file. XML_SetElementHandler(parser, jbParser::start_element_handler, jbParser::end_element_handler); - char buffer[512]; + // One would assume it would be faster to have a buffer on the stack and call XML_Parse. + // But XML_Parse will call XML_GetBuffer anyway and memmove the passed buffer into it. + // (Unless XML_CONTEXT_BYTES is undefined, but all users define it.) + static const int bufferSize = 512; bool done = false; while (!done) { - size_t len = file.read(buffer, SK_ARRAY_COUNT(buffer)); + void* buffer = XML_GetBuffer(parser, bufferSize); + if (!buffer) { + SkDebugf("Could not buffer enough to continue.\n"); + return -1; + } + size_t len = file.read(buffer, bufferSize); done = file.isAtEnd(); - XML_Status status = XML_Parse(parser, buffer, len, done); + XML_Status status = XML_ParseBuffer(parser, len, done); if (XML_STATUS_ERROR == status) { XML_Error error = XML_GetErrorCode(parser); int line = XML_GetCurrentLineNumber(parser); diff --git a/tests/FontConfigParser.cpp b/tests/FontConfigParser.cpp index 744a41e5f2..dfc8093f2a 100644 --- a/tests/FontConfigParser.cpp +++ b/tests/FontConfigParser.cpp @@ -6,9 +6,12 @@ */ #include "Resources.h" +#include "SkCommandLineFlags.h" #include "SkFontConfigParser_android.h" #include "Test.h" +DECLARE_bool(verboseFontMgr); + int CountFallbacks(SkTDArray fontFamilies) { int countOfFallbackFonts = 0; for (int i = 0; i < fontFamilies.count(); i++) { @@ -29,7 +32,10 @@ void ValidateLoadedFonts(SkTDArray fontFamilies, const char* firstE } void DumpLoadedFonts(SkTDArray fontFamilies) { -#if SK_DEBUG_FONTS + if (!FLAGS_verboseFontMgr) { + return; + } + for (int i = 0; i < fontFamilies.count(); ++i) { SkDebugf("Family %d:\n", i); switch(fontFamilies[i]->fVariant) { @@ -37,9 +43,7 @@ void DumpLoadedFonts(SkTDArray fontFamilies) { case kCompact_FontVariant: SkDebugf(" compact\n"); break; default: break; } - if (fontFamilies[i]->fBasePath) { - SkDebugf(" basePath %s\n", fontFamilies[i]->fBasePath); - } + SkDebugf(" basePath %s\n", fontFamilies[i]->fBasePath.c_str()); if (!fontFamilies[i]->fLanguage.getTag().isEmpty()) { SkDebugf(" language %s\n", fontFamilies[i]->fLanguage.getTag().c_str()); } @@ -51,7 +55,6 @@ void DumpLoadedFonts(SkTDArray fontFamilies) { SkDebugf(" file (%d) %s#%d\n", ffi.fWeight, ffi.fFileName.c_str(), ffi.fIndex); } } -#endif // SK_DEBUG_FONTS } DEF_TEST(FontConfigParserAndroid, reporter) {