Cleanup CFX_CTTGSUBTable.chromium/3521chromium/3520

Do all the parsing inside the constructor. Make GetVerticalGlyph() and
related getters const methods.

Change-Id: I21118cf98048cb6bbfc0999604d2434d4acafef6
Reviewed-on: https://pdfium-review.googlesource.com/39496
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Nicolás Peña Moreno <npm@chromium.org>

3 files changed, 65 insertions, 59 deletions

diff --git a/core/fpdfapi/font/cfx_cttgsubtable.cpp b/core/fpdfapi/font/cfx_cttgsubtable.cpp
index ae868ca5ab..bb889b6c59 100644
--- a/core/fpdfapi/font/cfx_cttgsubtable.cpp
+++ b/core/fpdfapi/font/cfx_cttgsubtable.cpp
@@ -12,9 +12,46 @@
 #include "third_party/base/ptr_util.h"
 #include "third_party/base/stl_util.h"
 
-CFX_CTTGSUBTable::CFX_CTTGSUBTable() : m_bFeautureMapLoad(false) {}
+namespace {
 
-CFX_CTTGSUBTable::~CFX_CTTGSUBTable() {}
+constexpr uint32_t MakeTag(char c1, char c2, char c3, char c4) {
+  return static_cast<uint8_t>(c1) << 24 | static_cast<uint8_t>(c2) << 16 |
+         static_cast<uint8_t>(c3) << 8 | static_cast<uint8_t>(c4);
+}
+
+bool IsVerticalFeatureTag(uint32_t tag) {
+  static constexpr uint32_t kTags[] = {
+      MakeTag('v', 'r', 't', '2'), MakeTag('v', 'e', 'r', 't'),
+  };
+  return tag == kTags[0] || tag == kTags[1];
+}
+
+}  // namespace
+
+CFX_CTTGSUBTable::CFX_CTTGSUBTable(FT_Bytes gsub) {
+  if (!LoadGSUBTable(gsub))
+    return;
+
+  for (const TScriptRecord& script : ScriptList) {
+    for (const auto& record : script.LangSysRecords) {
+      for (uint16_t index : record.FeatureIndices) {
+        if (IsVerticalFeatureTag(FeatureList[index].FeatureTag))
+          m_featureSet.insert(index);
+      }
+    }
+  }
+  if (!m_featureSet.empty())
+    return;
+
+  int i = 0;
+  for (const TFeatureRecord& feature : FeatureList) {
+    if (IsVerticalFeatureTag(feature.FeatureTag))
+      m_featureSet.insert(i);
+    ++i;
+  }
+}
+
+CFX_CTTGSUBTable::~CFX_CTTGSUBTable() = default;
 
 bool CFX_CTTGSUBTable::LoadGSUBTable(FT_Bytes gsub) {
   if ((gsub[0] << 24u | gsub[1] << 16u | gsub[2] << 8u | gsub[3]) != 0x00010000)
@@ -24,61 +61,33 @@ bool CFX_CTTGSUBTable::LoadGSUBTable(FT_Bytes gsub) {
                &gsub[gsub[8] << 8 | gsub[9]]);
 }
 
-bool CFX_CTTGSUBTable::GetVerticalGlyph(uint32_t glyphnum,
-                                        uint32_t* vglyphnum) {
-  uint32_t tag[] = {
-      (uint8_t)'v' << 24 | (uint8_t)'r' << 16 | (uint8_t)'t' << 8 |
-          (uint8_t)'2',
-      (uint8_t)'v' << 24 | (uint8_t)'e' << 16 | (uint8_t)'r' << 8 |
-          (uint8_t)'t',
-  };
-  if (!m_bFeautureMapLoad) {
-    for (const TScriptRecord& script : ScriptList) {
-      for (const auto& record : script.LangSysRecords) {
-        for (const auto& index : record.FeatureIndices) {
-          if (FeatureList[index].FeatureTag == tag[0] ||
-              FeatureList[index].FeatureTag == tag[1]) {
-            m_featureSet.insert(index);
-          }
-        }
-      }
-    }
-    if (m_featureSet.empty()) {
-      int i = 0;
-      for (const TFeatureRecord& feature : FeatureList) {
-        if (feature.FeatureTag == tag[0] || feature.FeatureTag == tag[1])
-          m_featureSet.insert(i);
-        ++i;
-      }
-    }
-    m_bFeautureMapLoad = true;
-  }
-  for (const auto& item : m_featureSet) {
-    if (GetVerticalGlyphSub(glyphnum, vglyphnum, &FeatureList[item])) {
-      return true;
-    }
+uint32_t CFX_CTTGSUBTable::GetVerticalGlyph(uint32_t glyphnum) const {
+  uint32_t vglyphnum = 0;
+  for (uint32_t item : m_featureSet) {
+    if (GetVerticalGlyphSub(FeatureList[item], glyphnum, &vglyphnum))
+      break;
   }
-  return false;
+  return vglyphnum;
 }
 
-bool CFX_CTTGSUBTable::GetVerticalGlyphSub(uint32_t glyphnum,
-                                           uint32_t* vglyphnum,
-                                           TFeatureRecord* Feature) {
-  for (int index : Feature->LookupListIndices) {
+bool CFX_CTTGSUBTable::GetVerticalGlyphSub(const TFeatureRecord& feature,
+                                           uint32_t glyphnum,
+                                           uint32_t* vglyphnum) const {
+  for (int index : feature.LookupListIndices) {
     if (!pdfium::IndexInBounds(LookupList, index))
       continue;
     if (LookupList[index].LookupType == 1 &&
-        GetVerticalGlyphSub2(glyphnum, vglyphnum, &LookupList[index])) {
+        GetVerticalGlyphSub2(LookupList[index], glyphnum, vglyphnum)) {
       return true;
     }
   }
   return false;
 }
 
-bool CFX_CTTGSUBTable::GetVerticalGlyphSub2(uint32_t glyphnum,
-                                            uint32_t* vglyphnum,
-                                            TLookup* Lookup) {
-  for (const auto& subTable : Lookup->SubTables) {
+bool CFX_CTTGSUBTable::GetVerticalGlyphSub2(const TLookup& lookup,
+                                            uint32_t glyphnum,
+                                            uint32_t* vglyphnum) const {
+  for (const auto& subTable : lookup.SubTables) {
     switch (subTable->SubstFormat) {
       case 1: {
         auto* tbl1 = static_cast<TSubTable1*>(subTable.get());
diff --git a/core/fpdfapi/font/cfx_cttgsubtable.h b/core/fpdfapi/font/cfx_cttgsubtable.h
index fc1caf5ef2..ff488a5ed7 100644
--- a/core/fpdfapi/font/cfx_cttgsubtable.h
+++ b/core/fpdfapi/font/cfx_cttgsubtable.h
@@ -17,11 +17,10 @@
 
 class CFX_CTTGSUBTable {
  public:
-  CFX_CTTGSUBTable();
+  explicit CFX_CTTGSUBTable(FT_Bytes gsub);
   ~CFX_CTTGSUBTable();
 
-  bool LoadGSUBTable(FT_Bytes gsub);
-  bool GetVerticalGlyph(uint32_t glyphnum, uint32_t* vglyphnum);
+  uint32_t GetVerticalGlyph(uint32_t glyphnum) const;
 
  private:
   struct TLangSysRecord {
@@ -118,6 +117,7 @@ class CFX_CTTGSUBTable {
     std::vector<std::unique_ptr<TSubTableBase>> SubTables;
   };
 
+  bool LoadGSUBTable(FT_Bytes gsub);
   bool Parse(FT_Bytes scriptlist, FT_Bytes featurelist, FT_Bytes lookuplist);
   void ParseScriptList(FT_Bytes raw);
   void ParseScript(FT_Bytes raw, TScriptRecord* rec);
@@ -133,12 +133,12 @@ class CFX_CTTGSUBTable {
   void ParseSingleSubstFormat1(FT_Bytes raw, TSubTable1* rec);
   void ParseSingleSubstFormat2(FT_Bytes raw, TSubTable2* rec);
 
-  bool GetVerticalGlyphSub(uint32_t glyphnum,
-                           uint32_t* vglyphnum,
-                           TFeatureRecord* Feature);
-  bool GetVerticalGlyphSub2(uint32_t glyphnum,
-                            uint32_t* vglyphnum,
-                            TLookup* Lookup);
+  bool GetVerticalGlyphSub(const TFeatureRecord& feature,
+                           uint32_t glyphnum,
+                           uint32_t* vglyphnum) const;
+  bool GetVerticalGlyphSub2(const TLookup& lookup,
+                            uint32_t glyphnum,
+                            uint32_t* vglyphnum) const;
   int GetCoverageIndex(TCoverageFormatBase* Coverage, uint32_t g) const;
 
   uint8_t GetUInt8(FT_Bytes& p) const;
@@ -148,7 +148,6 @@ class CFX_CTTGSUBTable {
   uint32_t GetUInt32(FT_Bytes& p) const;
 
   std::set<uint32_t> m_featureSet;
-  bool m_bFeautureMapLoad;
   std::vector<TScriptRecord> ScriptList;
   std::vector<TFeatureRecord> FeatureList;
   std::vector<TLookup> LookupList;
diff --git a/core/fpdfapi/font/cpdf_cidfont.cpp b/core/fpdfapi/font/cpdf_cidfont.cpp
index d38154331a..e118a91e87 100644
--- a/core/fpdfapi/font/cpdf_cidfont.cpp
+++ b/core/fpdfapi/font/cpdf_cidfont.cpp
@@ -577,14 +577,12 @@ int CPDF_CIDFont::GetGlyphIndex(uint32_t unicode, bool* pVertGlyph) {
   if (error || !m_Font.GetSubData())
     return index;
 
-  m_pTTGSUBTable = pdfium::MakeUnique<CFX_CTTGSUBTable>();
-  m_pTTGSUBTable->LoadGSUBTable((FT_Bytes)m_Font.GetSubData());
+  m_pTTGSUBTable = pdfium::MakeUnique<CFX_CTTGSUBTable>(m_Font.GetSubData());
   return GetVerticalGlyph(index, pVertGlyph);
 }
 
 int CPDF_CIDFont::GetVerticalGlyph(int index, bool* pVertGlyph) {
-  uint32_t vindex = 0;
-  m_pTTGSUBTable->GetVerticalGlyph(index, &vindex);
+  uint32_t vindex = m_pTTGSUBTable->GetVerticalGlyph(index);
   if (!vindex)
     return index;