Remove implicit conversions and some cleanup

core/include/fpdfdoc/fpdf_doc.h

 // Copyright 2014 PDFium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
 #ifndef CORE_INCLUDE_FPDFDOC_FPDF_DOC_H_
 #define CORE_INCLUDE_FPDFDOC_FPDF_DOC_H_
 
 #include <map>
@@ skipping 26 matching lines @@
 class CPDF_NumberTree;
 class CPDF_OCContext;
 class CPDF_Page;
 class CPDF_RenderOptions;
 class CPDF_TextObject;
 class CPDF_ViewerPreferences;
 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
 #define PDFZOOM_FITVERT 4
 #define PDFZOOM_FITRECT 5
 #define PDFZOOM_FITBBOX 6
 #define PDFZOOM_FITBHORZ 7
 #define PDFZOOM_FITBVERT 8
 class CPDF_Dest {
  public:
   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();
   int GetZoomMode();
   FX_FLOAT GetParam(int index);
 
  protected:
   CPDF_Object* m_pObj;
 };
+
 class CPDF_OCContext : public IPDF_OCContext {
  public:
   enum UsageType { View = 0, Design, Print, Export };
 
   explicit CPDF_OCContext(CPDF_Document* pDoc, UsageType eUsageType = View);
   ~CPDF_OCContext() override;
 
   CPDF_Document* GetDocument() const { return m_pDocument; }
 
   UsageType GetUsageType() const { return m_eUsageType; }
@@ skipping 19 matching lines @@
 
   CPDF_Document* m_pDocument;
 
   UsageType m_eUsageType;
 
   std::map<const CPDF_Dictionary*, FX_BOOL> m_OCGStates;
 };
 
 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
 #define PDFNAMED_PREVPAGE 2
 #define PDFNAMED_FIRSTPAGE 3
 #define PDFNAMED_LASTPAGE 4
 #define PDFJS_MAXLENGTH 64
 class CPDF_Action {
  public:
   enum ActionType {
@@ skipping 32 matching lines @@
   CPDF_Dest GetDest(CPDF_Document* pDoc) const;
 
   CFX_WideString GetFilePath() const;
 
   FX_BOOL GetNewWindow() const { return m_pDict->GetBooleanBy("NewWindow"); }
 
   CFX_ByteString GetURI(CPDF_Document* pDoc) const;
 
   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"); }
 
   FX_DWORD GetFlags() const { return m_pDict->GetIntegerBy("Flags"); }
 
   CFX_WideString GetJavaScript() const;
 
   CPDF_Dictionary* GetAnnot() const;
 
   int32_t GetOperationType() const;
 
   CPDF_Stream* GetSoundStream() const { return m_pDict->GetStreamBy("Sound"); }
 
   FX_FLOAT GetVolume() const { return m_pDict->GetNumberBy("Volume"); }
 
   FX_BOOL IsSynchronous() const { return m_pDict->GetBooleanBy("Synchronous"); }
 
   FX_BOOL IsRepeat() const { return m_pDict->GetBooleanBy("Repeat"); }
 
   FX_BOOL IsMixPlay() const { return m_pDict->GetBooleanBy("Mix"); }
 
   FX_DWORD GetSubActionsCount() const;
 
   CPDF_Action GetSubAction(FX_DWORD iIndex) const;
 
  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; }
 
   enum AActionType {
     CursorEnter = 0,
     CursorExit,
     ButtonDown,
     ButtonUp,
     GetFocus,
     LoseFocus,
@@ skipping 184 matching lines @@
   CPDF_Document* const m_pDocument;
   std::vector<CPDF_Annot*> m_AnnotList;
 };
 
 #define COLORTYPE_TRANSPARENT 0
 #define COLORTYPE_GRAY 1
 #define COLORTYPE_RGB 2
 #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;
   }
 
   operator CFX_ByteString() const { return m_csDA; }
 
   const CPDF_DefaultAppearance& operator=(const CFX_ByteString& csDA) {
     m_csDA = csDA;
     return *this;
@@ skipping 375 matching lines @@
   CPDF_Font* m_pFont;
 
   friend class CPDF_InterForm;
   friend class CPDF_FormControl;
 };
 CPDF_Object* FPDF_GetFieldAttr(CPDF_Dictionary* pFieldDict,
                                const FX_CHAR* name,
                                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.

 
   enum ScaleMethod { Always = 0, Bigger, Smaller, Never };

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

nit: Maybe put this as the first thing after public while we're at it.

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

Done.

 
   ScaleMethod GetScaleMethod();
 
   FX_BOOL IsProportionalScale();
 
   void GetIconPosition(FX_FLOAT& fLeft, FX_FLOAT& fBottom);
 
   FX_BOOL GetFittingBounds();
 
   CPDF_Dictionary* m_pDict;
@@ skipping 242 matching lines @@
   CFX_WideString GetCaption(const CFX_ByteStringC& csEntry) const;
 
   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_