commit aaf077e92594466bf926f36617fff80ca6ef2553
author Lei Zhang <thestig@chromium.org> Sat May 09 03:02:05 2020 +0000
committer Chromium commit bot <commit-bot@chromium.org> Sat May 09 03:02:05 2020 +0000
tree 11a7e5f4c5c8e9651b07c62a9beaec89f7bfc7c1
parent fd7ac2dd2f23315d26bd8fc51ab0333de941e297

Make CPDF_NameTree::LookupNamedDest() work for old style destinations.

The existing CPDF_NameTree::LookupNamedDestImpl() used to work for old
style named destinations, but that regressed in
https://pdfium-review.googlesource.com/68251, because now one can no
longer instantiate a CPDF_NameTree unless there is a name tree for the
new style destinations.

To make it work again, split CPDF_NameTree::LookupNamedDestImpl() into
LookupOldStyleNamedDest() and CPDF_NameTree::LookupNewStyleNamedDest().
Then hook up the functions to CPDF_NameTree::LookupNamedDest().

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

core/fpdfdoc/cpdf_nametree.cpp

diff --git a/core/fpdfdoc/cpdf_nametree.cpp b/core/fpdfdoc/cpdf_nametree.cpp
index e5319ee..07e172b 100644
--- a/core/fpdfdoc/cpdf_nametree.cpp
+++ b/core/fpdfdoc/cpdf_nametree.cpp
@@ -308,6 +308,14 @@
   return nullptr;
 }
 
+CPDF_Array* LookupOldStyleNamedDest(CPDF_Document* pDoc,
+                                    const ByteString& name) {
+  CPDF_Dictionary* pDests = pDoc->GetRoot()->GetDictFor("Dests");
+  if (!pDests)
+    return nullptr;
+  return GetNamedDestFromObject(pDests->GetDirectObjectFor(name));
+}
+
 }  // namespace
 
 CPDF_NameTree::CPDF_NameTree(CPDF_Dictionary* pRoot) : m_pRoot(pRoot) {
@@ -370,10 +378,13 @@
 // static
 CPDF_Array* CPDF_NameTree::LookupNamedDest(CPDF_Document* pDoc,
                                            const ByteString& name) {
+  CPDF_Array* dest_array = nullptr;
   std::unique_ptr<CPDF_NameTree> name_tree = Create(pDoc, "Dests");
-  if (!name_tree)
-    return nullptr;
-  return name_tree->LookupNamedDestImpl(pDoc, name);
+  if (name_tree)
+    dest_array = name_tree->LookupNewStyleNamedDest(name);
+  if (!dest_array)
+    dest_array = LookupOldStyleNamedDest(pDoc, name);
+  return dest_array;
 }
 
 size_t CPDF_NameTree::GetCount() const {
@@ -471,14 +482,6 @@
                               nullptr);
 }
 
-CPDF_Array* CPDF_NameTree::LookupNamedDestImpl(CPDF_Document* pDoc,
-                                               const ByteString& name) {
-  CPDF_Object* pValue = LookupValue(PDF_DecodeText(name.raw_span()));
-  if (!pValue) {
-    CPDF_Dictionary* pDests = pDoc->GetRoot()->GetDictFor("Dests");
-    if (!pDests)
-      return nullptr;
-    pValue = pDests->GetDirectObjectFor(name);
-  }
-  return GetNamedDestFromObject(pValue);
+CPDF_Array* CPDF_NameTree::LookupNewStyleNamedDest(const ByteString& sName) {
+  return GetNamedDestFromObject(LookupValue(PDF_DecodeText(sName.raw_span())));
 }

core/fpdfdoc/cpdf_nametree.h

diff --git a/core/fpdfdoc/cpdf_nametree.h b/core/fpdfdoc/cpdf_nametree.h
index 325867b..b443b7b 100644
--- a/core/fpdfdoc/cpdf_nametree.h
+++ b/core/fpdfdoc/cpdf_nametree.h
@@ -51,7 +51,7 @@
  private:
   explicit CPDF_NameTree(CPDF_Dictionary* pRoot);
 
-  CPDF_Array* LookupNamedDestImpl(CPDF_Document* pDoc, const ByteString& name);
+  CPDF_Array* LookupNewStyleNamedDest(const ByteString& name);
 
   const RetainPtr<CPDF_Dictionary> m_pRoot;
 };

fpdfsdk/fpdf_view_embeddertest.cpp

diff --git a/fpdfsdk/fpdf_view_embeddertest.cpp b/fpdfsdk/fpdf_view_embeddertest.cpp
index 4ab89a0..f5f9b33 100644
--- a/fpdfsdk/fpdf_view_embeddertest.cpp
+++ b/fpdfsdk/fpdf_view_embeddertest.cpp
@@ -696,9 +696,9 @@
   EXPECT_FALSE(FPDF_GetNamedDestByName(document(), ""));
   EXPECT_FALSE(FPDF_GetNamedDestByName(document(), "NoSuchName"));
 
-  // TODO(crbug.com/1080663): These should return a valid destination.
-  EXPECT_FALSE(FPDF_GetNamedDestByName(document(), "FirstAlternate"));
-  EXPECT_FALSE(FPDF_GetNamedDestByName(document(), "LastAlternate"));
+  // These should return a valid destination.
+  EXPECT_TRUE(FPDF_GetNamedDestByName(document(), "FirstAlternate"));
+  EXPECT_TRUE(FPDF_GetNamedDestByName(document(), "LastAlternate"));
 }
 
 // The following tests pass if the document opens without crashing.