Add functions to collect bookmarks from Pdfium

pdf/pdfium/pdfium_engine.cc

Index: pdf/pdfium/pdfium_engine.cc
diff --git a/pdf/pdfium/pdfium_engine.cc b/pdf/pdfium/pdfium_engine.cc
index 213214f5311a35a1064722d5b26b97cb7d3cb06c..0d7c0d452202f8a2d92ac44da8ed1f80e0ca5ec2 100644
--- a/pdf/pdfium/pdfium_engine.cc
+++ b/pdf/pdfium/pdfium_engine.cc
@@ -30,6 +30,7 @@
 #include "ppapi/cpp/trusted/browser_font_trusted.h"
 #include "ppapi/cpp/url_response_info.h"
 #include "ppapi/cpp/var.h"
+#include "ppapi/cpp/var_dictionary.h"
 #include "third_party/pdfium/fpdfsdk/include/fpdf_ext.h"
 #include "third_party/pdfium/fpdfsdk/include/fpdf_flatten.h"
 #include "third_party/pdfium/fpdfsdk/include/fpdf_searchex.h"
@@ -527,6 +528,36 @@ void FormatStringForOS(base::string16* text) {
 #endif
 }
 
+pp::VarDictionary TraverseBookmarks(FPDF_DOCUMENT doc, FPDF_BOOKMARK bookmark) {

[raymes, 2014/12/16 23:35:53]

Add a comment about what this returns (include details about the "root"
bookmark) and what happens when "NULL" is passed in as |bookmark|.

[Alexandre Carlton, 2014/12/17 00:51:13]

Done.

+

[raymes, 2014/12/16 23:35:53]

nit: remove this blank line

[Alexandre Carlton, 2014/12/17 00:51:13]

Done.

+  pp::VarDictionary dict;
+  base::string16 title;
+  size_t buffer_size = FPDFBookmark_GetTitle(bookmark, NULL, 0);
+  if (buffer_size > 1) {

[raymes, 2014/12/16 23:35:54]

buffer_size > 0

[Alexandre Carlton, 2014/12/17 00:51:13]

Done.

+    FPDFBookmark_GetTitle(bookmark,
+            WriteInto(&title, buffer_size + 1), buffer_size);

[raymes, 2014/12/16 23:35:53]

-buffer_size should already include the null characters, so remove the + 1
-use a 4 space indent here

[Alexandre Carlton, 2014/12/17 00:51:13]

Done.

+  }
+  dict.Set(pp::Var("title"), pp::Var(base::UTF16ToUTF8(title)));
+
+

[raymes, 2014/12/16 23:35:54]

nit: remove a blink line here

[Alexandre Carlton, 2014/12/17 00:51:13]

Done.

+  FPDF_DEST dest = FPDFBookmark_GetDest(doc, bookmark);
+  if (dest) {
+    int pageIndex = FPDFDest_GetPageIndex(doc, dest);

[raymes, 2014/12/16 23:35:53]

we don't use camel case in chrome C++ so this should be page_index

[Alexandre Carlton, 2014/12/17 00:51:13]

Done.

+    dict.Set(pp::Var("page"), pp::Var(pageIndex));
+  }

[raymes, 2014/12/16 23:35:53]

if the dest is invalid, should we still set "page" to 0?

[Alexandre Carlton, 2014/12/17 00:51:13]

As discussed, some bookmarks don't have pages at all (they can open urls in a
browser, for example).

+
+  pp::VarArray children;
+
+  FPDF_BOOKMARK childBookmark = FPDFBookmark_GetFirstChild(doc, bookmark);

[raymes, 2014/12/16 23:35:53]

child_bookmark

[Alexandre Carlton, 2014/12/17 00:51:13]

Done.

+  for (int i = 0; childBookmark != NULL; i++) {

[raymes, 2014/12/16 23:35:53]

This is a little unconventional. Maybe just have a while loop.

int child_index = 0;
while (child_bookmark != NULL) {
  ...
  child_index++;
}

[Alexandre Carlton, 2014/12/17 00:51:13]

Done.

+    pp::VarDictionary childDict = TraverseBookmarks(doc, childBookmark);
+    children.Set(i, childDict);
+    childBookmark = FPDFBookmark_GetNextSibling(doc, childBookmark);
+  }
+  dict.Set(pp::Var("children"), children);
+  return dict;
+}
+
 }  // namespace
 
 bool InitializeSDK(void* data) {
@@ -2297,6 +2328,12 @@ int PDFiumEngine::GetNumberOfPages() {
   return pages_.size();
 }
 
+
+pp::VarDictionary PDFiumEngine::GetBookmarks() {
+  pp::VarDictionary dict = TraverseBookmarks(doc_, NULL);

[raymes, 2014/12/16 23:35:53]

as mentioned, let's pull out the array from the root bookmark and return that.

[Alexandre Carlton, 2014/12/17 00:51:13]

Done.

+  return dict;

[raymes, 2014/12/16 23:35:53]

or just:
return TraverseBookmarks(doc_, NULL);

+}
+
 int PDFiumEngine::GetNamedDestinationPage(const std::string& destination) {
   // Look for the destination.
   FPDF_DEST dest = FPDF_GetNamedDestByName(doc_, destination.c_str());