Remove implicit conversions and some cleanup

core/include/fpdfdoc/fpdf_doc.h

Index: core/include/fpdfdoc/fpdf_doc.h
diff --git a/core/include/fpdfdoc/fpdf_doc.h b/core/include/fpdfdoc/fpdf_doc.h
index 15616be4d5a3178e139b4ade41302fef6e1c366f..8ec88615f8730db496197ba10ab5a4b7c4c61f6d 100644
--- a/core/include/fpdfdoc/fpdf_doc.h
+++ b/core/include/fpdfdoc/fpdf_doc.h
@@ -44,63 +44,51 @@ class CXML_Element;
 
 class CPDF_NameTree {
  public:
-  explicit CPDF_NameTree(CPDF_Dictionary* pRoot) { m_pRoot = pRoot; }
-
+  explicit CPDF_NameTree(CPDF_Dictionary* pRoot) : m_pRoot(pRoot) {}
   CPDF_NameTree(CPDF_Document* pDoc, const CFX_ByteStringC& category);
 
   CPDF_Object* LookupValue(int nIndex, CFX_ByteString& csName) const;
-
   CPDF_Object* LookupValue(const CFX_ByteString& csName) const;
-
   CPDF_Array* LookupNamedDest(CPDF_Document* pDoc,
                               const CFX_ByteStringC& sName);
-
   int GetIndex(const CFX_ByteString& csName) const;
-
   int GetCount() const;
-
   CPDF_Dictionary* GetRoot() const { return m_pRoot; }
 
  protected:
   CPDF_Dictionary* m_pRoot;
 };
+
 class CPDF_BookmarkTree {
  public:
   explicit CPDF_BookmarkTree(CPDF_Document* pDoc) : m_pDocument(pDoc) {}
 
   CPDF_Bookmark GetFirstChild(const CPDF_Bookmark& parent) const;
-
   CPDF_Bookmark GetNextSibling(const CPDF_Bookmark& bookmark) const;
-
   CPDF_Document* GetDocument() const { return m_pDocument; }
 
  protected:
   CPDF_Document* m_pDocument;

[Tom Sepez, 2016/03/01 17:53:35]

Can this be CPDF_Document* const m_pDocument?

[Wei Li, 2016/03/01 21:46:53]

Yes, done.

 };
+
 #define PDFBOOKMARK_ITALIC 1
 #define PDFBOOKMARK_BOLD 2
 class CPDF_Bookmark {
  public:
   CPDF_Bookmark() : m_pDict(NULL) {}
-
   explicit CPDF_Bookmark(CPDF_Dictionary* pDict) : m_pDict(pDict) {}
 
   CPDF_Dictionary* GetDict() const { return m_pDict; }
-
-  operator bool() const { return m_pDict != NULL; }
-
   FX_DWORD GetColorRef() const;
-
   FX_DWORD GetFontStyle() const;
-
   CFX_WideString GetTitle() const;
-
   CPDF_Dest GetDest(CPDF_Document* pDocument) const;
-
   CPDF_Action GetAction() const;
 
+ protected:
   CPDF_Dictionary* m_pDict;

[Tom Sepez, 2016/03/01 17:53:35]

ditto

[Wei Li, 2016/03/01 21:46:53]

This one needs to kept non-const since it is easier to walk the tree in
FindBookmark()

[Tom Sepez, 2016/03/01 23:43:30]

Acknowledged.

 };
+
 #define PDFZOOM_XYZ 1
 #define PDFZOOM_FITPAGE 2
 #define PDFZOOM_FITHORZ 3
@@ -114,9 +102,7 @@ class CPDF_Dest {
   CPDF_Dest() : m_pObj(nullptr) {}
   explicit CPDF_Dest(CPDF_Object* pObj) : m_pObj(pObj) {}
 
-  operator bool() const { return m_pObj != NULL; }
   CPDF_Object* GetObject() const { return m_pObj; }
-
   CFX_ByteString GetRemoteName();
   int GetPageIndex(CPDF_Document* pDoc);
   FX_DWORD GetPageObjNum();
@@ -126,6 +112,7 @@ class CPDF_Dest {
  protected:
   CPDF_Object* m_pObj;
 };
+
 class CPDF_OCContext : public IPDF_OCContext {
  public:
   enum UsageType { View = 0, Design, Print, Export };
@@ -165,21 +152,14 @@ class CPDF_OCContext : public IPDF_OCContext {
 
 class CPDF_ActionFields {
  public:
-  // TODO(thestig): Examine why this cannot be explicit.
-  CPDF_ActionFields(const CPDF_Action* pAction) {
-    m_pAction = (CPDF_Action*)pAction;
-  }
-
-  // TODO(thestig): Replace this.
-  operator CPDF_Action*() const { return m_pAction; }
+  explicit CPDF_ActionFields(const CPDF_Action* pAction) : m_pAction(pAction) {}
 
   FX_DWORD GetFieldsCount() const;
-
   std::vector<CPDF_Object*> GetAllFields() const;
-
   CPDF_Object* GetField(FX_DWORD iIndex) const;
 
-  CPDF_Action* m_pAction;
+ protected:
+  const CPDF_Action* m_pAction;

[Tom Sepez, 2016/03/01 17:53:35]

ditto

[Wei Li, 2016/03/01 21:46:53]

Done.

 };
 
 #define PDFNAMED_NEXTPAGE 1
@@ -232,8 +212,6 @@ class CPDF_Action {
 
   FX_BOOL GetMouseMap() const { return m_pDict->GetBooleanBy("IsMap"); }
 
-  CPDF_ActionFields GetWidgets() const { return this; }
-
   FX_BOOL GetHideStatus() const { return m_pDict->GetBooleanBy("H", TRUE); }
 
   CFX_ByteString GetNamedAction() const { return m_pDict->GetStringBy("N"); }
@@ -263,10 +241,10 @@ class CPDF_Action {
  protected:
   CPDF_Dictionary* m_pDict;
 };
+
 class CPDF_AAction {
  public:
-  // TODO(thestig): Examine why this cannot be explicit.
-  CPDF_AAction(CPDF_Dictionary* pDict = NULL) { m_pDict = pDict; }
+  explicit CPDF_AAction(CPDF_Dictionary* pDict) : m_pDict(pDict) {}
 
   operator CPDF_Dictionary*() const { return m_pDict; }
 
@@ -471,8 +449,7 @@ class CPDF_AnnotList {
 #define COLORTYPE_CMYK 3
 class CPDF_DefaultAppearance {
  public:
-  // TODO(thestig): Examine why this cannot be explicit.
-  CPDF_DefaultAppearance(const CFX_ByteString& csDA = "") { m_csDA = csDA; }
+  explicit CPDF_DefaultAppearance(const CFX_ByteString& csDA) : m_csDA(csDA) {}
 
   CPDF_DefaultAppearance(const CPDF_DefaultAppearance& cDA) {
     m_csDA = (CFX_ByteString)(CPDF_DefaultAppearance&)cDA;
@@ -868,8 +845,7 @@ CPDF_Object* FPDF_GetFieldAttr(CPDF_Dictionary* pFieldDict,
                                int nLevel = 0);
 class CPDF_IconFit {
  public:
-  // TODO(thestig): Examine why this cannot be explicit.
-  CPDF_IconFit(CPDF_Dictionary* pDict = NULL) { m_pDict = pDict; }
+  CPDF_IconFit(CPDF_Dictionary* pDict) : m_pDict(pDict) {}

[Tom Sepez, 2016/03/01 17:53:35]

Did you mean to add "explicit" here?

[Wei Li, 2016/03/01 21:46:53]

yes, thank you for spotting this :)

 
   operator CPDF_Dictionary*() const { return m_pDict; }

[Tom Sepez, 2016/03/01 17:53:35]

You might want to try to kill this operator as a follow-up, instead adding a
GetDictionary() method.

[Wei Li, 2016/03/01 21:46:53]

Done.