Traverse PDF page tree only once in CPDF_Document Try 3

Now, we do not start traversal from where we were at, but from the top.
This makes the code less prone to bugs, as now there is no need to call
methods to recursively fix things. This will save a lot of time when
the trees are rather flat, as in the PDF file in the bug. It can still
be slow, for instance if we have a chain of page nodes, and the last
in the chain contains all of the pages (this is artificial).

Try 2 at https://codereview.chromium.org/2442403002/

Also added test where Try 2 would have failed.

Tested the pdf from the bug on my Mac:
With this CL: load in 21 seconds
Without this CL: did not load in 4 minutes, got tired of waiting

BUG=chromium:638513

Review-Url: https://codereview.chromium.org/2470803003

1 files changed, 25 insertions, 2 deletions

diff --git a/core/fpdfapi/parser/cpdf_document_unittest.cpp b/core/fpdfapi/parser/cpdf_document_unittest.cpp
index c09665b716..71716a649e 100644
--- a/core/fpdfapi/parser/cpdf_document_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_document_unittest.cpp
@@ -95,7 +95,7 @@ TEST_F(cpdf_document_test, GetPages) {
   for (int i = 0; i < 7; i++) {
     CPDF_Dictionary* page = document->GetPage(i);
     ASSERT_TRUE(page);
-    ASSERT_TRUE(page->GetObjectFor("PageNumbering"));
+    ASSERT_TRUE(page->KeyExist("PageNumbering"));
     EXPECT_EQ(i, page->GetIntegerFor("PageNumbering"));
   }
   CPDF_Dictionary* page = document->GetPage(7);
@@ -108,13 +108,36 @@ TEST_F(cpdf_document_test, GetPagesReverseOrder) {
   for (int i = 6; i >= 0; i--) {
     CPDF_Dictionary* page = document->GetPage(i);
     ASSERT_TRUE(page);
-    ASSERT_TRUE(page->GetObjectFor("PageNumbering"));
+    ASSERT_TRUE(page->KeyExist("PageNumbering"));
     EXPECT_EQ(i, page->GetIntegerFor("PageNumbering"));
   }
   CPDF_Dictionary* page = document->GetPage(7);
   EXPECT_FALSE(page);
 }
 
+TEST(cpdf_document, GetPagesInDisorder) {
+  std::unique_ptr<CPDF_TestDocumentForPages> document =
+      pdfium::MakeUnique<CPDF_TestDocumentForPages>();
+
+  CPDF_Dictionary* page = document->GetPage(1);
+  ASSERT_TRUE(page);
+  ASSERT_TRUE(page->KeyExist("PageNumbering"));
+  EXPECT_EQ(1, page->GetIntegerFor("PageNumbering"));
+
+  page = document->GetPage(3);
+  ASSERT_TRUE(page);
+  ASSERT_TRUE(page->KeyExist("PageNumbering"));
+  EXPECT_EQ(3, page->GetIntegerFor("PageNumbering"));
+
+  page = document->GetPage(7);
+  EXPECT_FALSE(page);
+
+  page = document->GetPage(6);
+  ASSERT_TRUE(page);
+  ASSERT_TRUE(page->KeyExist("PageNumbering"));
+  EXPECT_EQ(6, page->GetIntegerFor("PageNumbering"));
+}
+
 TEST_F(cpdf_document_test, UseCachedPageObjNumIfHaveNotPagesDict) {
   // ObjNum can be added in CPDF_DataAvail::IsPageAvail, and PagesDict
   // can be not exists in this case.