Cleanup CPDF_ApSettings and CJS_Parameters.

R=tsepez@chromium.org

Review URL: https://codereview.chromium.org/1432863004 .

5 files changed, 109 insertions, 139 deletions

diff --git a/core/include/fpdfdoc/fpdf_doc.h b/core/include/fpdfdoc/fpdf_doc.h
index ac7b0cf774..76264c93f9 100644
--- a/core/include/fpdfdoc/fpdf_doc.h
+++ b/core/include/fpdfdoc/fpdf_doc.h
@@ -943,7 +943,7 @@ class CPDF_FormControl {
 
   HighlightingMode GetHighlightingMode();
 
-  FX_BOOL HasMKEntry(CFX_ByteString csEntry);
+  bool HasMKEntry(CFX_ByteString csEntry) const;
 
   int GetRotation();
 
@@ -951,37 +951,37 @@ class CPDF_FormControl {
     return GetColor(iColorType, "BC");
   }
 
-  inline FX_FLOAT GetOriginalBorderColor(int index) {
+  FX_FLOAT GetOriginalBorderColor(int index) {
     return GetOriginalColor(index, "BC");
   }
 
-  inline void GetOriginalBorderColor(int& iColorType, FX_FLOAT fc[4]) {
+  void GetOriginalBorderColor(int& iColorType, FX_FLOAT fc[4]) {
     GetOriginalColor(iColorType, fc, "BC");
   }
 
-  inline FX_ARGB GetBackgroundColor(int& iColorType) {
+  FX_ARGB GetBackgroundColor(int& iColorType) {
     return GetColor(iColorType, "BG");
   }
 
-  inline FX_FLOAT GetOriginalBackgroundColor(int index) {
+  FX_FLOAT GetOriginalBackgroundColor(int index) {
     return GetOriginalColor(index, "BG");
   }
 
-  inline void GetOriginalBackgroundColor(int& iColorType, FX_FLOAT fc[4]) {
+  void GetOriginalBackgroundColor(int& iColorType, FX_FLOAT fc[4]) {
     GetOriginalColor(iColorType, fc, "BG");
   }
 
-  inline CFX_WideString GetNormalCaption() { return GetCaption("CA"); }
+  CFX_WideString GetNormalCaption() { return GetCaption("CA"); }
 
-  inline CFX_WideString GetRolloverCaption() { return GetCaption("RC"); }
+  CFX_WideString GetRolloverCaption() { return GetCaption("RC"); }
 
-  inline CFX_WideString GetDownCaption() { return GetCaption("AC"); }
+  CFX_WideString GetDownCaption() { return GetCaption("AC"); }
 
-  inline CPDF_Stream* GetNormalIcon() { return GetIcon("I"); }
+  CPDF_Stream* GetNormalIcon() { return GetIcon("I"); }
 
-  inline CPDF_Stream* GetRolloverIcon() { return GetIcon("RI"); }
+  CPDF_Stream* GetRolloverIcon() { return GetIcon("RI"); }
 
-  inline CPDF_Stream* GetDownIcon() { return GetIcon("IX"); }
+  CPDF_Stream* GetDownIcon() { return GetIcon("IX"); }
 
   CPDF_IconFit GetIconFit();
 
@@ -1018,7 +1018,7 @@ class CPDF_FormControl {
 
   CPDF_Stream* GetIcon(CFX_ByteString csEntry);
 
-  CPDF_ApSettings GetMK(FX_BOOL bCreate);
+  CPDF_ApSettings GetMK() const;
 
   CPDF_InterForm* m_pForm;
 
@@ -1107,74 +1107,71 @@ class CPDF_ViewerPreferences {
 
 class CPDF_ApSettings {
  public:
-  // TODO(thestig): Examine why this cannot be explicit.
-  CPDF_ApSettings(CPDF_Dictionary* pDict = NULL) { m_pDict = pDict; }
-
-  operator CPDF_Dictionary*() const { return m_pDict; }
-
-  FX_BOOL HasMKEntry(const CFX_ByteStringC& csEntry);
+  explicit CPDF_ApSettings(CPDF_Dictionary* pDict);
 
-  int GetRotation();
+  bool HasMKEntry(const CFX_ByteStringC& csEntry) const;
+  int GetRotation() const;
 
-  inline FX_ARGB GetBorderColor(int& iColorType) {
+  FX_ARGB GetBorderColor(int& iColorType) const {
     return GetColor(iColorType, FX_BSTRC("BC"));
   }
 
-  inline FX_FLOAT GetOriginalBorderColor(int index) {
+  FX_FLOAT GetOriginalBorderColor(int index) const {
     return GetOriginalColor(index, FX_BSTRC("BC"));
   }
 
-  inline void GetOriginalBorderColor(int& iColorType, FX_FLOAT fc[4]) {
+  void GetOriginalBorderColor(int& iColorType, FX_FLOAT fc[4]) const {
     GetOriginalColor(iColorType, fc, FX_BSTRC("BC"));
   }
 
-  inline FX_ARGB GetBackgroundColor(int& iColorType) {
+  FX_ARGB GetBackgroundColor(int& iColorType) const {
     return GetColor(iColorType, FX_BSTRC("BG"));
   }
 
-  inline FX_FLOAT GetOriginalBackgroundColor(int index) {
+  FX_FLOAT GetOriginalBackgroundColor(int index) const {
     return GetOriginalColor(index, FX_BSTRC("BG"));
   }
 
-  inline void GetOriginalBackgroundColor(int& iColorType, FX_FLOAT fc[4]) {
+  void GetOriginalBackgroundColor(int& iColorType, FX_FLOAT fc[4]) const {
     GetOriginalColor(iColorType, fc, FX_BSTRC("BG"));
   }
 
-  inline CFX_WideString GetNormalCaption() {
-    return GetCaption(FX_BSTRC("CA"));
-  }
+  CFX_WideString GetNormalCaption() const { return GetCaption(FX_BSTRC("CA")); }
 
-  inline CFX_WideString GetRolloverCaption() {
+  CFX_WideString GetRolloverCaption() const {
     return GetCaption(FX_BSTRC("RC"));
   }
 
-  inline CFX_WideString GetDownCaption() { return GetCaption(FX_BSTRC("AC")); }
-
-  inline CPDF_Stream* GetNormalIcon() { return GetIcon(FX_BSTRC("I")); }
+  CFX_WideString GetDownCaption() const { return GetCaption(FX_BSTRC("AC")); }
 
-  inline CPDF_Stream* GetRolloverIcon() { return GetIcon(FX_BSTRC("RI")); }
+  CPDF_Stream* GetNormalIcon() const { return GetIcon(FX_BSTRC("I")); }
 
-  inline CPDF_Stream* GetDownIcon() { return GetIcon(FX_BSTRC("IX")); }
+  CPDF_Stream* GetRolloverIcon() const { return GetIcon(FX_BSTRC("RI")); }
 
-  CPDF_IconFit GetIconFit();
+  CPDF_Stream* GetDownIcon() const { return GetIcon(FX_BSTRC("IX")); }
 
-  int GetTextPosition();
+  CPDF_IconFit GetIconFit() const;
 
-  CPDF_Dictionary* m_pDict;
+  int GetTextPosition() const;
 
  protected:
-  FX_ARGB GetColor(int& iColorType, const CFX_ByteStringC& csEntry);
+  friend class CPDF_FormControl;
 
-  FX_FLOAT GetOriginalColor(int index, const CFX_ByteStringC& csEntry);
+  FX_ARGB GetColor(int& iColorType, const CFX_ByteStringC& csEntry) const;
+
+  FX_FLOAT GetOriginalColor(int index, const CFX_ByteStringC& csEntry) const;
 
   void GetOriginalColor(int& iColorType,
                         FX_FLOAT fc[4],
-                        const CFX_ByteStringC& csEntry);
+                        const CFX_ByteStringC& csEntry) const;
 
-  CFX_WideString GetCaption(const CFX_ByteStringC& csEntry);
+  CFX_WideString GetCaption(const CFX_ByteStringC& csEntry) const;
 
-  CPDF_Stream* GetIcon(const CFX_ByteStringC& csEntry);
-  friend class CPDF_FormControl;
+  CPDF_Stream* GetIcon(const CFX_ByteStringC& csEntry) const;
+
+  CPDF_Dictionary* const m_pDict;
 };
 
+CFX_WideString FILESPEC_EncodeFileName(const CFX_WideStringC& filepath);
+
 #endif  // CORE_INCLUDE_FPDFDOC_FPDF_DOC_H_
diff --git a/core/src/fpdfdoc/doc_form.cpp b/core/src/fpdfdoc/doc_form.cpp
index 776f19fdca..857bf5acea 100644
--- a/core/src/fpdfdoc/doc_form.cpp
+++ b/core/src/fpdfdoc/doc_form.cpp
@@ -1145,9 +1145,6 @@ CFDF_Document* CPDF_InterForm::ExportToFDF(const CFX_WideStringC& pdf_path,
   return ExportToFDF(pdf_path, fields, true, bSimpleFileSpec);
 }
 
-// TODO(thestig): Fix this.
-CFX_WideString FILESPEC_EncodeFileName(const CFX_WideStringC& filepath);
-
 CFDF_Document* CPDF_InterForm::ExportToFDF(
     const CFX_WideStringC& pdf_path,
     const std::vector<CPDF_FormField*>& fields,
diff --git a/core/src/fpdfdoc/doc_formcontrol.cpp b/core/src/fpdfdoc/doc_formcontrol.cpp
index 0796b6b434..88b68bc7f7 100644
--- a/core/src/fpdfdoc/doc_formcontrol.cpp
+++ b/core/src/fpdfdoc/doc_formcontrol.cpp
@@ -189,58 +189,49 @@ CPDF_FormControl::HighlightingMode CPDF_FormControl::GetHighlightingMode() {
   }
   return Invert;
 }
-CPDF_ApSettings CPDF_FormControl::GetMK(FX_BOOL bCreate) {
-  if (!m_pWidgetDict) {
-    return NULL;
-  }
-  CPDF_ApSettings mk = m_pWidgetDict->GetDict(FX_BSTRC("MK"));
-  if (!mk && bCreate) {
-    mk = CPDF_Dictionary::Create();
-    if (mk == NULL) {
-      return NULL;
-    }
-    m_pWidgetDict->SetAt(FX_BSTRC("MK"), mk);
-  }
-  return mk;
+
+CPDF_ApSettings CPDF_FormControl::GetMK() const {
+  return CPDF_ApSettings(m_pWidgetDict ? m_pWidgetDict->GetDict(FX_BSTRC("MK"))
+                                       : nullptr);
 }
-FX_BOOL CPDF_FormControl::HasMKEntry(CFX_ByteString csEntry) {
-  CPDF_ApSettings mk = GetMK(FALSE);
-  return mk.HasMKEntry(csEntry);
+
+bool CPDF_FormControl::HasMKEntry(CFX_ByteString csEntry) const {
+  return GetMK().HasMKEntry(csEntry);
 }
+
 int CPDF_FormControl::GetRotation() {
-  CPDF_ApSettings mk = GetMK(FALSE);
-  return mk.GetRotation();
+  return GetMK().GetRotation();
 }
+
 FX_ARGB CPDF_FormControl::GetColor(int& iColorType, CFX_ByteString csEntry) {
-  CPDF_ApSettings mk = GetMK(FALSE);
-  return mk.GetColor(iColorType, csEntry);
+  return GetMK().GetColor(iColorType, csEntry);
 }
+
 FX_FLOAT CPDF_FormControl::GetOriginalColor(int index, CFX_ByteString csEntry) {
-  CPDF_ApSettings mk = GetMK(FALSE);
-  return mk.GetOriginalColor(index, csEntry);
+  return GetMK().GetOriginalColor(index, csEntry);
 }
+
 void CPDF_FormControl::GetOriginalColor(int& iColorType,
                                         FX_FLOAT fc[4],
                                         CFX_ByteString csEntry) {
-  CPDF_ApSettings mk = GetMK(FALSE);
-  mk.GetOriginalColor(iColorType, fc, csEntry);
+  GetMK().GetOriginalColor(iColorType, fc, csEntry);
 }
 CFX_WideString CPDF_FormControl::GetCaption(CFX_ByteString csEntry) {
-  CPDF_ApSettings mk = GetMK(FALSE);
-  return mk.GetCaption(csEntry);
+  return GetMK().GetCaption(csEntry);
 }
+
 CPDF_Stream* CPDF_FormControl::GetIcon(CFX_ByteString csEntry) {
-  CPDF_ApSettings mk = GetMK(FALSE);
-  return mk.GetIcon(csEntry);
+  return GetMK().GetIcon(csEntry);
 }
+
 CPDF_IconFit CPDF_FormControl::GetIconFit() {
-  CPDF_ApSettings mk = GetMK(FALSE);
-  return mk.GetIconFit();
+  return GetMK().GetIconFit();
 }
+
 int CPDF_FormControl::GetTextPosition() {
-  CPDF_ApSettings mk = GetMK(FALSE);
-  return mk.GetTextPosition();
+  return GetMK().GetTextPosition();
 }
+
 CPDF_Action CPDF_FormControl::GetAction() {
   if (!m_pWidgetDict) {
     return CPDF_Action();
@@ -331,29 +322,28 @@ int CPDF_FormControl::GetControlAlignment() {
   }
   return pObj->GetInteger();
 }
-FX_BOOL CPDF_ApSettings::HasMKEntry(const CFX_ByteStringC& csEntry) {
-  if (m_pDict == NULL) {
-    return FALSE;
-  }
-  return m_pDict->KeyExist(csEntry);
+
+CPDF_ApSettings::CPDF_ApSettings(CPDF_Dictionary* pDict) : m_pDict(pDict) {}
+
+bool CPDF_ApSettings::HasMKEntry(const CFX_ByteStringC& csEntry) const {
+  return m_pDict && m_pDict->KeyExist(csEntry);
 }
-int CPDF_ApSettings::GetRotation() {
-  if (m_pDict == NULL) {
-    return 0;
-  }
-  return m_pDict->GetInteger(FX_BSTRC("R"));
+
+int CPDF_ApSettings::GetRotation() const {
+  return m_pDict ? m_pDict->GetInteger(FX_BSTRC("R")) : 0;
 }
+
 FX_ARGB CPDF_ApSettings::GetColor(int& iColorType,
-                                  const CFX_ByteStringC& csEntry) {
+                                  const CFX_ByteStringC& csEntry) const {
   iColorType = COLORTYPE_TRANSPARENT;
-  if (m_pDict == NULL) {
+  if (!m_pDict)
     return 0;
-  }
-  FX_ARGB color = 0;
+
   CPDF_Array* pEntry = m_pDict->GetArray(csEntry);
-  if (pEntry == NULL) {
-    return color;
-  }
+  if (!pEntry)
+    return 0;
+
+  FX_ARGB color = 0;
   FX_DWORD dwCount = pEntry->GetCount();
   if (dwCount == 1) {
     iColorType = COLORTYPE_GRAY;
@@ -378,20 +368,20 @@ FX_ARGB CPDF_ApSettings::GetColor(int& iColorType,
   }
   return color;
 }
-FX_FLOAT CPDF_ApSettings::GetOriginalColor(int index,
-                                           const CFX_ByteStringC& csEntry) {
-  if (m_pDict == NULL) {
+
+FX_FLOAT CPDF_ApSettings::GetOriginalColor(
+    int index,
+    const CFX_ByteStringC& csEntry) const {
+  if (!m_pDict)
     return 0;
-  }
+
   CPDF_Array* pEntry = m_pDict->GetArray(csEntry);
-  if (pEntry != NULL) {
-    return pEntry->GetNumber(index);
-  }
-  return 0;
+  return pEntry ? pEntry->GetNumber(index) : 0;
 }
+
 void CPDF_ApSettings::GetOriginalColor(int& iColorType,
                                        FX_FLOAT fc[4],
-                                       const CFX_ByteStringC& csEntry) {
+                                       const CFX_ByteStringC& csEntry) const {
   iColorType = COLORTYPE_TRANSPARENT;
   for (int i = 0; i < 4; i++) {
     fc[i] = 0;
@@ -420,28 +410,21 @@ void CPDF_ApSettings::GetOriginalColor(int& iColorType,
     fc[3] = pEntry->GetNumber(3);
   }
 }
-CFX_WideString CPDF_ApSettings::GetCaption(const CFX_ByteStringC& csEntry) {
-  CFX_WideString csCaption;
-  if (m_pDict == NULL) {
-    return csCaption;
-  }
-  return m_pDict->GetUnicodeText(csEntry);
+
+CFX_WideString CPDF_ApSettings::GetCaption(
+    const CFX_ByteStringC& csEntry) const {
+  return m_pDict ? m_pDict->GetUnicodeText(csEntry) : CFX_WideString();
 }
-CPDF_Stream* CPDF_ApSettings::GetIcon(const CFX_ByteStringC& csEntry) {
-  if (m_pDict == NULL) {
-    return NULL;
-  }
-  return m_pDict->GetStream(csEntry);
+
+CPDF_Stream* CPDF_ApSettings::GetIcon(const CFX_ByteStringC& csEntry) const {
+  return m_pDict ? m_pDict->GetStream(csEntry) : nullptr;
 }
-CPDF_IconFit CPDF_ApSettings::GetIconFit() {
-  if (m_pDict == NULL) {
-    return NULL;
-  }
-  return m_pDict->GetDict(FX_BSTRC("IF"));
+
+CPDF_IconFit CPDF_ApSettings::GetIconFit() const {
+  return m_pDict ? m_pDict->GetDict(FX_BSTRC("IF")) : nullptr;
 }
-int CPDF_ApSettings::GetTextPosition() {
-  if (m_pDict == NULL) {
-    return TEXTPOS_CAPTION;
-  }
-  return m_pDict->GetInteger(FX_BSTRC("TP"), TEXTPOS_CAPTION);
+
+int CPDF_ApSettings::GetTextPosition() const {
+  return m_pDict ? m_pDict->GetInteger(FX_BSTRC("TP"), TEXTPOS_CAPTION)
+                 : TEXTPOS_CAPTION;
 }
diff --git a/fpdfsdk/src/javascript/Document.cpp b/fpdfsdk/src/javascript/Document.cpp
index bae068a57f..2bd800af82 100644
--- a/fpdfsdk/src/javascript/Document.cpp
+++ b/fpdfsdk/src/javascript/Document.cpp
@@ -567,8 +567,7 @@ FX_BOOL Document::resetForm(IJS_Context* cc,
   CJS_Runtime* pRuntime = CJS_Runtime::FromContext(cc);
   CJS_Array aName(pRuntime);
 
-  // TODO(thestig) Convert CJS_Parameters to a std::vector and use .empty().
-  if (params.size() <= 0) {
+  if (params.empty()) {
     pPDFForm->ResetForm(TRUE);
     m_pDocument->SetChangeMark();
     return TRUE;
@@ -1509,9 +1508,9 @@ FX_BOOL Document::getPageNthWord(IJS_Context* cc,
   if (!m_pDocument->GetPermissions(FPDFPERM_EXTRACT_ACCESS))
     return FALSE;
 
-  int nPageNo = params.GetSize() > 0 ? params[0].ToInt() : 0;
-  int nWordNo = params.GetSize() > 1 ? params[1].ToInt() : 0;
-  bool bStrip = params.GetSize() > 2 ? params[2].ToBool() : true;
+  int nPageNo = params.size() > 0 ? params[0].ToInt() : 0;
+  int nWordNo = params.size() > 1 ? params[1].ToInt() : 0;
+  bool bStrip = params.size() > 2 ? params[2].ToBool() : true;
 
   CPDF_Document* pDocument = m_pDocument->GetPDFDocument();
   if (!pDocument)
@@ -1583,7 +1582,7 @@ FX_BOOL Document::getPageNumWords(IJS_Context* cc,
   if (!m_pDocument->GetPermissions(FPDFPERM_EXTRACT_ACCESS))
     return FALSE;
 
-  int nPageNo = params.GetSize() > 0 ? params[0].ToInt() : 0;
+  int nPageNo = params.size() > 0 ? params[0].ToInt() : 0;
 
   CPDF_Document* pDocument = m_pDocument->GetPDFDocument();
   ASSERT(pDocument != NULL);
diff --git a/fpdfsdk/src/javascript/JS_Value.h b/fpdfsdk/src/javascript/JS_Value.h
index b8c5563f70..68e643e055 100644
--- a/fpdfsdk/src/javascript/JS_Value.h
+++ b/fpdfsdk/src/javascript/JS_Value.h
@@ -88,13 +88,7 @@ class CJS_Value {
   CJS_Runtime* m_pJSRuntime;
 };
 
-class CJS_Parameters : public CFX_ArrayTemplate<CJS_Value> {
- public:
-  void push_back(const CJS_Value& newElement) {
-    CFX_ArrayTemplate<CJS_Value>::Add(newElement);
-  }
-  int size() const { return CFX_ArrayTemplate<CJS_Value>::GetSize(); }
-};
+using CJS_Parameters = std::vector<CJS_Value>;
 
 class CJS_PropValue : public CJS_Value {
  public: