commit d524d32f3c7c5618982c443552eaef621a91dec4
author Tom Sepez <tsepez@chromium.org> Tue May 28 21:52:27 2019 +0000
committer Chromium commit bot <commit-bot@chromium.org> Tue May 28 21:52:27 2019 +0000
tree 72c0283c79b58c4bb6870a10736e35e4faf81254
parent 698a4380c2968f2fc742f275955e20c4756a8b70

Avoid circular includes between CPDF_Document and CPDF_LinkList

Introduce a trivial pure virtual interface to resolve the issue.
Ideally, we'd find some other object to own the links, but this
cleans it up in the mean time.

Change-Id: I15e2792da328ac9de16c021dfa2521357b5a886d
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/54991
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>

core/fpdfapi/parser/cpdf_document.h

diff --git a/core/fpdfapi/parser/cpdf_document.h b/core/fpdfapi/parser/cpdf_document.h
index 6684eb8..172e991 100644
--- a/core/fpdfapi/parser/cpdf_document.h
+++ b/core/fpdfapi/parser/cpdf_document.h
@@ -18,7 +18,6 @@
 #include "core/fpdfapi/page/cpdf_page.h"
 #include "core/fpdfapi/parser/cpdf_object.h"
 #include "core/fpdfapi/parser/cpdf_parser.h"
-#include "core/fpdfdoc/cpdf_linklist.h"
 #include "core/fxcrt/observable.h"
 #include "core/fxcrt/retain_ptr.h"
 
@@ -56,6 +55,12 @@
     virtual uint32_t GetUserPermissions() const = 0;
   };
 
+  class LinkListIface {
+   public:
+    // CPDF_Document merely helps manage the lifetime.
+    virtual ~LinkListIface() = default;
+  };
+
   static const int kPageMaxNum = 0xFFFFF;
 
   CPDF_Document();
@@ -85,7 +90,10 @@
   std::unique_ptr<JBig2_DocumentContext>* CodecContext() {
     return &m_pCodecContext;
   }
-  std::unique_ptr<CPDF_LinkList>* LinksContext() { return &m_pLinksContext; }
+  LinkListIface* GetLinksContext() const { return m_pLinksContext.get(); }
+  void SetLinksContext(std::unique_ptr<LinkListIface> pContext) {
+    m_pLinksContext = std::move(pContext);
+  }
 
   CPDF_DocRenderData* GetRenderData() const { return m_pDocRender.get(); }
 
@@ -192,7 +200,7 @@
   std::unique_ptr<CPDF_DocPageData> m_pDocPage;
 
   std::unique_ptr<JBig2_DocumentContext> m_pCodecContext;
-  std::unique_ptr<CPDF_LinkList> m_pLinksContext;
+  std::unique_ptr<LinkListIface> m_pLinksContext;
   std::vector<uint32_t> m_PageList;  // Page number to page's dict objnum.
 
   // Must be second to last.

core/fpdfdoc/BUILD.gn

diff --git a/core/fpdfdoc/BUILD.gn b/core/fpdfdoc/BUILD.gn
index ada4382..82289f3 100644
--- a/core/fpdfdoc/BUILD.gn
+++ b/core/fpdfdoc/BUILD.gn
@@ -95,10 +95,7 @@
     "../fxcrt",
     "../fxge",
   ]
-  allow_circular_includes_from = [
-    "../fpdfapi/parser",
-    "../fpdfapi/render",
-  ]
+  allow_circular_includes_from = [ "../fpdfapi/render" ]
   visibility = [ "../../*" ]
 }
 

core/fpdfdoc/cpdf_linklist.h

diff --git a/core/fpdfdoc/cpdf_linklist.h b/core/fpdfdoc/cpdf_linklist.h
index 129790f..442f66c 100644
--- a/core/fpdfdoc/cpdf_linklist.h
+++ b/core/fpdfdoc/cpdf_linklist.h
@@ -10,16 +10,17 @@
 #include <map>
 #include <vector>
 
+#include "core/fpdfapi/parser/cpdf_document.h"
 #include "core/fpdfdoc/cpdf_link.h"
 #include "core/fxcrt/fx_system.h"
 
 class CPDF_Page;
 class CPDF_Dictionary;
 
-class CPDF_LinkList {
+class CPDF_LinkList : public CPDF_Document::LinkListIface {
  public:
   CPDF_LinkList();
-  ~CPDF_LinkList();
+  ~CPDF_LinkList() override;
 
   CPDF_Link GetLinkAtPoint(CPDF_Page* pPage,
                            const CFX_PointF& point,

fpdfsdk/fpdf_doc.cpp

diff --git a/fpdfsdk/fpdf_doc.cpp b/fpdfsdk/fpdf_doc.cpp
index e523321..58e2b5d 100644
--- a/fpdfsdk/fpdf_doc.cpp
+++ b/fpdfsdk/fpdf_doc.cpp
@@ -8,6 +8,7 @@
 
 #include <memory>
 #include <set>
+#include <utility>
 
 #include "core/fpdfapi/page/cpdf_page.h"
 #include "core/fpdfapi/parser/cpdf_array.h"
@@ -17,6 +18,7 @@
 #include "core/fpdfdoc/cpdf_bookmark.h"
 #include "core/fpdfdoc/cpdf_bookmarktree.h"
 #include "core/fpdfdoc/cpdf_dest.h"
+#include "core/fpdfdoc/cpdf_linklist.h"
 #include "core/fpdfdoc/cpdf_pagelabel.h"
 #include "fpdfsdk/cpdfsdk_helpers.h"
 #include "third_party/base/ptr_util.h"
@@ -53,10 +55,14 @@
 
 CPDF_LinkList* GetLinkList(CPDF_Page* page) {
   CPDF_Document* pDoc = page->GetDocument();
-  std::unique_ptr<CPDF_LinkList>* pHolder = pDoc->LinksContext();
-  if (!pHolder->get())
-    *pHolder = pdfium::MakeUnique<CPDF_LinkList>();
-  return pHolder->get();
+  auto* pList = static_cast<CPDF_LinkList*>(pDoc->GetLinksContext());
+  if (pList)
+    return pList;
+
+  auto pNewList = pdfium::MakeUnique<CPDF_LinkList>();
+  pList = pNewList.get();
+  pDoc->SetLinksContext(std::move(pNewList));
+  return pList;
 }
 
 }  // namespace