Clean up bookmark related codes.
authorBo Xu <bo_xu@foxitsoftware.com>
Mon, 5 Jan 2015 20:39:36 +0000 (12:39 -0800)
committerBo Xu <bo_xu@foxitsoftware.com>
Mon, 5 Jan 2015 20:39:36 +0000 (12:39 -0800)
Remove CPDF_Dictionary*() operator in CPDF_Bookmark class.
Unify naming conventions and coding styles.
Change some functions to const.

Change the name of function argument to |pDict| for FPDF_xxx type variable.
This makes the code more clear and gives better variable naming

R=tsepez@chromium.org

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

core/include/fpdfdoc/fpdf_doc.h
core/src/fpdfdoc/doc_bookmark.cpp
fpdfsdk/src/fpdfdoc.cpp

index e4cf777..60f41a5 100644 (file)
@@ -76,26 +76,15 @@ protected:
 class CPDF_BookmarkTree : public CFX_Object
 {
 public:
+    CPDF_BookmarkTree(CPDF_Document* pDoc) : m_pDocument(pDoc) {}
 
-    CPDF_BookmarkTree(CPDF_Document* pDoc)
-    {
-        m_pDocument = pDoc;
-    }
-public:
-
-
-
-    CPDF_Bookmark              GetFirstChild(CPDF_Bookmark parent);
+    CPDF_Bookmark              GetFirstChild(const CPDF_Bookmark& parent) const;
 
-    CPDF_Bookmark              GetNextSibling(CPDF_Bookmark bookmark);
+    CPDF_Bookmark              GetNextSibling(const CPDF_Bookmark& bookmark) const;
 
+    CPDF_Document*             GetDocument() const { return m_pDocument; }
 
-    CPDF_Document*             GetDocument() const
-    {
-        return m_pDocument;
-    }
 protected:
-
     CPDF_Document*             m_pDocument;
 };
 #define PDFBOOKMARK_ITALIC                     1
@@ -104,31 +93,23 @@ class CPDF_Bookmark : public CFX_Object
 {
 public:
 
-    CPDF_Bookmark(CPDF_Dictionary* pDict = NULL)
-    {
-        m_pDict = pDict;
-    }
-
-    operator CPDF_Dictionary*() const
-    {
-        return m_pDict;
-    }
-
-
+    CPDF_Bookmark() : m_pDict(NULL) {}
 
-    FX_DWORD                   GetColorRef();
+    explicit CPDF_Bookmark(CPDF_Dictionary* pDict) : m_pDict(pDict) {}
 
-    FX_DWORD                   GetFontStyle();
+    CPDF_Dictionary* GetDict() const { return m_pDict; }
 
-    CFX_WideString             GetTitle();
+    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);
-
-    CPDF_Action                        GetAction();
+    CPDF_Dest                  GetDest(CPDF_Document* pDocument) const;
 
+    CPDF_Action                        GetAction() const;
 
     CPDF_Dictionary*   m_pDict;
 };
@@ -139,7 +120,6 @@ public:
 #define PDFZOOM_FITRECT                                5
 #define PDFZOOM_FITBBOX                                6
 #define PDFZOOM_FITBHORZ                       7
-
 #define PDFZOOM_FITBVERT                       8
 class CPDF_Dest : public CFX_Object
 {
index 0ae649e..e7b383e 100644 (file)
@@ -5,32 +5,32 @@
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
 #include "../../include/fpdfdoc/fpdf_doc.h"
-CPDF_Bookmark CPDF_BookmarkTree::GetFirstChild(CPDF_Bookmark Parent)
+CPDF_Bookmark CPDF_BookmarkTree::GetFirstChild(const CPDF_Bookmark& parent) const
 {
-    if (Parent.m_pDict == NULL) {
+    if (!parent.m_pDict) {
         CPDF_Dictionary* pRoot = m_pDocument->GetRoot()->GetDict("Outlines");
-        if (pRoot == NULL) {
-            return NULL;
+        if (!pRoot) {
+            return CPDF_Bookmark();
         }
-        return pRoot->GetDict("First");
+        return CPDF_Bookmark(pRoot->GetDict("First"));
     }
-    return Parent.m_pDict->GetDict("First");
+    return CPDF_Bookmark(parent.m_pDict->GetDict("First"));
 }
-CPDF_Bookmark CPDF_BookmarkTree::GetNextSibling(CPDF_Bookmark This)
+CPDF_Bookmark CPDF_BookmarkTree::GetNextSibling(const CPDF_Bookmark& bookmark) const
 {
-    if (This.m_pDict == NULL) {
-        return NULL;
+    if (!bookmark.m_pDict) {
+        return CPDF_Bookmark();
     }
-    CPDF_Dictionary *pNext = This.m_pDict->GetDict("Next");
-    return pNext == This.m_pDict ? NULL : pNext;
+    CPDF_Dictionary *pNext = bookmark.m_pDict->GetDict("Next");
+    return pNext == bookmark.m_pDict ? CPDF_Bookmark() : CPDF_Bookmark(pNext);
 }
-FX_DWORD CPDF_Bookmark::GetColorRef()
+FX_DWORD CPDF_Bookmark::GetColorRef() const
 {
     if (!m_pDict) {
         return 0;
     }
     CPDF_Array* pColor = m_pDict->GetArray("C");
-    if (pColor == NULL) {
+    if (!pColor) {
         return FXSYS_RGB(0, 0, 0);
     }
     int r = FXSYS_round(pColor->GetNumber(0) * 255);
@@ -38,39 +38,40 @@ FX_DWORD CPDF_Bookmark::GetColorRef()
     int b = FXSYS_round(pColor->GetNumber(2) * 255);
     return FXSYS_RGB(r, g, b);
 }
-FX_DWORD CPDF_Bookmark::GetFontStyle()
+FX_DWORD CPDF_Bookmark::GetFontStyle() const
 {
     if (!m_pDict) {
         return 0;
     }
     return m_pDict->GetInteger("F");
 }
-CFX_WideString CPDF_Bookmark::GetTitle()
+CFX_WideString CPDF_Bookmark::GetTitle() const
 {
     if (!m_pDict) {
         return CFX_WideString();
     }
     CPDF_String* pString = (CPDF_String*)m_pDict->GetElementValue("Title");
-    if (pString == NULL || pString->GetType() != PDFOBJ_STRING) {
+    if (!pString || pString->GetType() != PDFOBJ_STRING) {
         return CFX_WideString();
     }
     CFX_WideString title = pString->GetUnicodeText();
     FX_LPWSTR buf = title.LockBuffer();
-    int len = title.GetLength(), i;
-    for (i = 0; i < len; i ++)
+    int len = title.GetLength();
+    for (int i = 0; i < len; i++) {
         if (buf[i] < 0x20) {
             buf[i] = 0x20;
         }
+    }
     title.ReleaseBuffer(len);
     return title;
 }
-CPDF_Dest CPDF_Bookmark::GetDest(CPDF_Document* pDocument)
+CPDF_Dest CPDF_Bookmark::GetDest(CPDF_Document* pDocument) const
 {
     if (!m_pDict) {
         return NULL;
     }
     CPDF_Object* pDest = m_pDict->GetElementValue("Dest");
-    if (pDest == NULL) {
+    if (!pDest) {
         return NULL;
     }
     if (pDest->GetType() == PDFOBJ_STRING || pDest->GetType() == PDFOBJ_NAME) {
@@ -82,7 +83,7 @@ CPDF_Dest CPDF_Bookmark::GetDest(CPDF_Document* pDocument)
     }
     return NULL;
 }
-CPDF_Action CPDF_Bookmark::GetAction()
+CPDF_Action CPDF_Bookmark::GetAction() const
 {
     if (!m_pDict) {
         return NULL;
index 19ca06c..ab2351f 100644 (file)
 #include "../include/fsdk_define.h"
 #include "../include/fpdfdoc.h"
 
-static int this_module = 0;
+static int THISMODULE = 0;
 
-static CPDF_Bookmark FindBookmark(CPDF_BookmarkTree& tree, CPDF_Bookmark This, const CFX_WideString& title)
+static CPDF_Bookmark FindBookmark(const CPDF_BookmarkTree& tree, CPDF_Bookmark bookmark, const CFX_WideString& title)
 {
-       if (This != NULL) {
+       if (bookmark && bookmark.GetTitle().CompareNoCase(title) == 0) {
                // First check this item
-               CFX_WideString this_title = This.GetTitle();
-               if (this_title.CompareNoCase(title) == 0)
-                       return This;
+               return bookmark;
        }
        // go into children items
-       CPDF_Bookmark Child = tree.GetFirstChild(This);
-       while (Child != NULL) {
+       CPDF_Bookmark child = tree.GetFirstChild(bookmark);
+       while (child) {
                // check if this item
-               CPDF_Bookmark Found = FindBookmark(tree, Child, title);
-               if (Found) return Found;
-               Child = tree.GetNextSibling(Child);
+               CPDF_Bookmark found = FindBookmark(tree, child, title);
+               if (found)
+                       return found;
+               child = tree.GetNextSibling(child);
        }
-       return NULL;
+       return CPDF_Bookmark();
 }
 
 DLLEXPORT FPDF_BOOKMARK STDCALL FPDFBookmark_Find(FPDF_DOCUMENT document, FPDF_WIDESTRING title)
 {
-       if (document == NULL) return NULL;
-       if (title == NULL || title[0] == 0) return NULL;
-
+       if (!document)
+               return NULL;
+       if (!title || title[0] == 0)
+               return NULL;
        CPDF_Document* pDoc = (CPDF_Document*)document;
        CPDF_BookmarkTree tree(pDoc);
-
        FX_STRSIZE len = CFX_WideString::WStringLength(title);
-       CFX_WideString wstr = CFX_WideString::FromUTF16LE(title, len);
-       return FindBookmark(tree, NULL, wstr);
+       CFX_WideString encodedTitle = CFX_WideString::FromUTF16LE(title, len);
+       return FindBookmark(tree, CPDF_Bookmark(), encodedTitle).GetDict();
 }
 
-DLLEXPORT FPDF_DEST STDCALL FPDFBookmark_GetDest(FPDF_DOCUMENT document, FPDF_BOOKMARK bookmark)
+DLLEXPORT FPDF_DEST STDCALL FPDFBookmark_GetDest(FPDF_DOCUMENT document, FPDF_BOOKMARK pDict)
 {
-       if (document == NULL) return NULL;
-       if (bookmark == NULL) return NULL;
-
-       CPDF_Bookmark Bookmark = (CPDF_Dictionary*)bookmark;
+       if (!document)
+               return NULL;
+       if (!pDict)
+               return NULL;
+       CPDF_Bookmark bookmark((CPDF_Dictionary*)pDict);
        CPDF_Document* pDoc = (CPDF_Document*)document;
-       CPDF_Dest dest = Bookmark.GetDest(pDoc);
-       if (dest != NULL) return dest;
-
+       CPDF_Dest dest = bookmark.GetDest(pDoc);
+       if (dest)
+               return dest;
        // If this bookmark is not directly associated with a dest, we try to get action
-       CPDF_Action Action = Bookmark.GetAction();
-       if (Action == NULL) return NULL;
-       return Action.GetDest(pDoc);
+       CPDF_Action action = bookmark.GetAction();
+       if (!action)
+               return NULL;
+       return action.GetDest(pDoc);
 }
 
-DLLEXPORT FPDF_ACTION STDCALL FPDFBookmark_GetAction(FPDF_BOOKMARK bookmark)
+DLLEXPORT FPDF_ACTION STDCALL FPDFBookmark_GetAction(FPDF_BOOKMARK pDict)
 {
-       if (bookmark == NULL) return NULL;
-
-       CPDF_Bookmark Bookmark = (CPDF_Dictionary*)bookmark;
-       return Bookmark.GetAction();
+       if (!pDict)
+               return NULL;
+       CPDF_Bookmark bookmark((CPDF_Dictionary*)pDict);
+       return bookmark.GetAction();
 }
 
-DLLEXPORT unsigned long STDCALL FPDFAction_GetType(FPDF_ACTION action)
+DLLEXPORT unsigned long STDCALL FPDFAction_GetType(FPDF_ACTION pDict)
 {
-       if (action == NULL) return 0;
-
-       CPDF_Action Action = (CPDF_Dictionary*)action;
-       CPDF_Action::ActionType type = Action.GetType();
+       if (!pDict)
+               return 0;
+       CPDF_Action action = (CPDF_Dictionary*)pDict;
+       CPDF_Action::ActionType type = action.GetType();
        switch (type) {
-       case CPDF_Action::GoTo:
-               return PDFACTION_GOTO;
-       case CPDF_Action::GoToR:
-               return PDFACTION_REMOTEGOTO;
-       case CPDF_Action::URI:
-               return PDFACTION_URI;
-       case CPDF_Action::Launch:
-               return PDFACTION_LAUNCH;
-       default:
-               return PDFACTION_UNSUPPORTED;
+               case CPDF_Action::GoTo:
+                       return PDFACTION_GOTO;
+               case CPDF_Action::GoToR:
+                       return PDFACTION_REMOTEGOTO;
+               case CPDF_Action::URI:
+                       return PDFACTION_URI;
+               case CPDF_Action::Launch:
+                       return PDFACTION_LAUNCH;
+               default:
+                       return PDFACTION_UNSUPPORTED;
        }
        return PDFACTION_UNSUPPORTED;
 }
 
-DLLEXPORT FPDF_DEST STDCALL FPDFAction_GetDest(FPDF_DOCUMENT document, FPDF_ACTION action)
+DLLEXPORT FPDF_DEST STDCALL FPDFAction_GetDest(FPDF_DOCUMENT document, FPDF_ACTION pDict)
 {
-       if (document == NULL) return NULL;
-       if (action == NULL) return NULL;
+       if (!document)
+               return NULL;
+       if (!pDict)
+               return NULL;
        CPDF_Document* pDoc = (CPDF_Document*)document;
-       CPDF_Action Action = (CPDF_Dictionary*)action;
-
-       return Action.GetDest(pDoc);
+       CPDF_Action action = (CPDF_Dictionary*)pDict;
+       return action.GetDest(pDoc);
 }
 
-DLLEXPORT unsigned long STDCALL FPDFAction_GetURIPath(FPDF_DOCUMENT document, FPDF_ACTION action, 
+DLLEXPORT unsigned long STDCALL FPDFAction_GetURIPath(FPDF_DOCUMENT document, FPDF_ACTION pDict,
                                                                                          void* buffer, unsigned long buflen)
 {
-       if (document == NULL) return 0;
-       if (action == NULL) return 0;
+       if (!document)
+               return 0;
+       if (!pDict)
+               return 0;
        CPDF_Document* pDoc = (CPDF_Document*)document;
-       CPDF_Action Action = (CPDF_Dictionary*)action;
-
-       CFX_ByteString path = Action.GetURI(pDoc);
+       CPDF_Action action = (CPDF_Dictionary*)pDict;
+       CFX_ByteString path = action.GetURI(pDoc);
        unsigned long len = path.GetLength() + 1;
        if (buffer != NULL && buflen >= len)
                FXSYS_memcpy(buffer, path.c_str(), len);
        return len;
 }
 
-DLLEXPORT unsigned long STDCALL FPDFDest_GetPageIndex(FPDF_DOCUMENT document, FPDF_DEST dest)
+DLLEXPORT unsigned long STDCALL FPDFDest_GetPageIndex(FPDF_DOCUMENT document, FPDF_DEST pDict)
 {
-       if (document == NULL) return 0;
-       if (dest == NULL) return 0;
+       if (!document)
+               return 0;
+       if (!pDict)
+               return 0;
        CPDF_Document* pDoc = (CPDF_Document*)document;
-       CPDF_Dest Dest = (CPDF_Array*)dest;
-
-       return Dest.GetPageIndex(pDoc);
+       CPDF_Dest dest = (CPDF_Array*)pDict;
+       return dest.GetPageIndex(pDoc);
 }
 
 static void ReleaseLinkList(FX_LPVOID data)
@@ -128,42 +132,44 @@ static void ReleaseLinkList(FX_LPVOID data)
 
 DLLEXPORT FPDF_LINK STDCALL FPDFLink_GetLinkAtPoint(FPDF_PAGE page, double x, double y)
 {
-       if (page == NULL) return NULL;
+       if (!page)
+               return NULL;
        CPDF_Page* pPage = (CPDF_Page*)page;
-
        // Link list is stored with the document
        CPDF_Document* pDoc = pPage->m_pDocument;
-       CPDF_LinkList* pLinkList = (CPDF_LinkList*)pDoc->GetPrivateData(&this_module);
-       if (pLinkList == NULL) {
+       CPDF_LinkList* pLinkList = (CPDF_LinkList*)pDoc->GetPrivateData(&THISMODULE);
+       if (!pLinkList) {
                pLinkList = FX_NEW CPDF_LinkList(pDoc);
-               pDoc->SetPrivateData(&this_module, pLinkList, ReleaseLinkList);
+               pDoc->SetPrivateData(&THISMODULE, pLinkList, ReleaseLinkList);
        }
-
        return pLinkList->GetLinkAtPoint(pPage, (FX_FLOAT)x, (FX_FLOAT)y);
 }
 
-DLLEXPORT FPDF_DEST STDCALL FPDFLink_GetDest(FPDF_DOCUMENT document, FPDF_LINK link)
+DLLEXPORT FPDF_DEST STDCALL FPDFLink_GetDest(FPDF_DOCUMENT document, FPDF_LINK pDict)
 {
-       if (document == NULL) return NULL;
+       if (!document)
+               return NULL;
        CPDF_Document* pDoc = (CPDF_Document*)document;
-       if (link == NULL) return NULL;
-       CPDF_Link Link = (CPDF_Dictionary*)link;
-
-       FPDF_DEST dest = Link.GetDest(pDoc);
-       if (dest) return dest;
+       if (!pDict)
+               return NULL;
+       CPDF_Link link = (CPDF_Dictionary*)pDict;
 
+       FPDF_DEST dest = link.GetDest(pDoc);
+       if (dest)
+               return dest;
        // If this link is not directly associated with a dest, we try to get action
-       CPDF_Action Action = Link.GetAction();
-       if (Action == NULL) return NULL;
-       return Action.GetDest(pDoc);
+       CPDF_Action action = link.GetAction();
+       if (!action)
+               return NULL;
+       return action.GetDest(pDoc);
 }
 
-DLLEXPORT FPDF_ACTION STDCALL FPDFLink_GetAction(FPDF_LINK link)
+DLLEXPORT FPDF_ACTION STDCALL FPDFLink_GetAction(FPDF_LINK pDict)
 {
-       if (link == NULL) return NULL;
-       CPDF_Link Link = (CPDF_Dictionary*)link;
-
-       return Link.GetAction();
+       if (!pDict)
+               return NULL;
+       CPDF_Link link = (CPDF_Dictionary*)pDict;
+       return link.GetAction();
 }
 
 DLLEXPORT FPDF_BOOL STDCALL FPDFLink_Enumerate(FPDF_PAGE page, int* startPos, FPDF_LINK* linkAnnot)
@@ -171,15 +177,17 @@ DLLEXPORT FPDF_BOOL STDCALL FPDFLink_Enumerate(FPDF_PAGE page, int* startPos, FP
        if(!page || !startPos || !linkAnnot)
                return FALSE;
        CPDF_Page* pPage = (CPDF_Page*)page;
-       if(!pPage->m_pFormDict) return FALSE;
+       if(!pPage->m_pFormDict)
+               return FALSE;
        CPDF_Array* pAnnots = pPage->m_pFormDict->GetArray("Annots");
-       if(!pAnnots) return FALSE;
-       for (int i = *startPos; i < (int)pAnnots->GetCount(); i ++) {
+       if(!pAnnots)
+               return FALSE;
+       for (int i = *startPos; i < (int)pAnnots->GetCount(); i++) {
                CPDF_Dictionary* pDict = (CPDF_Dictionary*)pAnnots->GetElementValue(i);
-               if (pDict == NULL || pDict->GetType() != PDFOBJ_DICTIONARY) continue;
-               if(pDict->GetString(FX_BSTRC("Subtype")).Equal(FX_BSTRC("Link")))
-               {
-                       *startPos = i+1;
+               if (!pDict || pDict->GetType() != PDFOBJ_DICTIONARY)
+                       continue;
+               if(pDict->GetString(FX_BSTRC("Subtype")).Equal(FX_BSTRC("Link"))) {
+                       *startPos = i + 1;
                        *linkAnnot = (FPDF_LINK)pDict; 
                        return TRUE;
                }
@@ -206,7 +214,7 @@ DLLEXPORT int STDCALL FPDFLink_CountQuadPoints(FPDF_LINK linkAnnot)
                return 0;
        CPDF_Dictionary* pAnnotDict = (CPDF_Dictionary*)linkAnnot;
        CPDF_Array* pArray = pAnnotDict->GetArray(FX_BSTRC("QuadPoints"));
-       if (pArray == NULL)
+       if (!pArray)
                return 0;
        else
                return pArray->GetCount() / 8;
@@ -219,8 +227,8 @@ DLLEXPORT FPDF_BOOL STDCALL FPDFLink_GetQuadPoints(FPDF_LINK linkAnnot, int quad
        CPDF_Dictionary* pAnnotDict = (CPDF_Dictionary*)linkAnnot;
        CPDF_Array* pArray = pAnnotDict->GetArray(FX_BSTRC("QuadPoints"));
        if (pArray) {
-               if (0 > quadIndex || quadIndex >= (int)pArray->GetCount()/8 ||
-                       ((quadIndex*8+7) >= (int)pArray->GetCount())) return FALSE;
+               if (quadIndex < 0 || quadIndex >= (int)pArray->GetCount()/8 || ((quadIndex*8+7) >= (int)pArray->GetCount()))
+                       return FALSE;
                quadPoints->x1 = pArray->GetNumber(quadIndex*8);
                quadPoints->y1 = pArray->GetNumber(quadIndex*8+1);
                quadPoints->x2 = pArray->GetNumber(quadIndex*8+2);
@@ -234,27 +242,25 @@ DLLEXPORT FPDF_BOOL STDCALL FPDFLink_GetQuadPoints(FPDF_LINK linkAnnot, int quad
        return FALSE;
 }
 
-
 DLLEXPORT unsigned long STDCALL FPDF_GetMetaText(FPDF_DOCUMENT doc, FPDF_BYTESTRING tag,
                                                                                                 void* buffer, unsigned long buflen)
 {
-       if (doc == NULL || tag == NULL) return 0;
-
+       if (!doc || !tag)
+               return 0;
        CPDF_Document* pDoc = (CPDF_Document*)doc;
        // Get info dictionary
        CPDF_Dictionary* pInfo = pDoc->GetInfo();
-       if (pInfo == NULL) return 0;
-
+       if (!pInfo)
+               return 0;
        CFX_WideString text = pInfo->GetUnicodeText(tag);
-
        // Use UTF-16LE encoding
-       CFX_ByteString bstr = text.UTF16LE_Encode();
-       unsigned long len = bstr.GetLength();
-       if (buffer != NULL && buflen >= len+2) {
-               FXSYS_memcpy(buffer, bstr.c_str(), len);
+       CFX_ByteString encodedText = text.UTF16LE_Encode();
+       unsigned long len = encodedText.GetLength();
+       if (buffer && buflen >= len + 2) {
+               FXSYS_memcpy(buffer, encodedText.c_str(), len);
                // use double zero as trailer
-               ((FX_BYTE*)buffer)[len] = ((FX_BYTE*)buffer)[len+1] = 0;
+               ((FX_BYTE*)buffer)[len] = 0;
+               ((FX_BYTE*)buffer)[len + 1] = 0;
        }
        return len+2;
 }
-