Navigation to relative fragments does not work correctly for OOP pdf.

pdf/pdfium/pdfium_engine.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "pdf/pdfium/pdfium_engine.h"
 
 #include <math.h>
 
 #include "base/json/json_writer.h"
 #include "base/logging.h"
@@ skipping 12 matching lines @@
 #include "ppapi/c/ppb_core.h"
 #include "ppapi/c/private/ppb_pdf.h"
 #include "ppapi/cpp/dev/memory_dev.h"
 #include "ppapi/cpp/input_event.h"
 #include "ppapi/cpp/instance.h"
 #include "ppapi/cpp/module.h"
 #include "ppapi/cpp/private/pdf.h"
 #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"
 #include "third_party/pdfium/fpdfsdk/include/fpdf_sysfontinfo.h"
 #include "third_party/pdfium/fpdfsdk/include/fpdf_transformpage.h"
 #include "third_party/pdfium/fpdfsdk/include/fpdfedit.h"
 #include "third_party/pdfium/fpdfsdk/include/fpdfppo.h"
 #include "third_party/pdfium/fpdfsdk/include/fpdfsave.h"
 #include "third_party/pdfium/fpdfsdk/include/pdfwindow/PDFWindow.h"
 #include "third_party/pdfium/fpdfsdk/include/pdfwindow/PWL_FontMap.h"
@@ skipping 42 matching lines @@
 // do its thing and still fit within the timelimit for 60Hz. For the
 // non-composited case, this doesn't make things worse since we're still
 // painting the scrollbars > 60 Hz.
 #define kMaxInitialProgressivePaintTimeMs 10
 
 // Copied from printing/units.cc because we don't want to depend on printing
 // since it brings in libpng which causes duplicate symbols with PDFium.
 const int kPointsPerInch = 72;
 const int kPixelsPerInch = 96;
 
+// URL reference parameters.
+const char kDelimiters[] = "#&";

[raymes, 2015/01/13 01:59:32]

We can remove this (see below)

[Deepak, 2015/01/13 08:29:08]

Done.

+
 struct ClipBox {
   float left;
   float right;
   float top;
   float bottom;
 };
 
 int ConvertUnit(int value, int old_unit, int new_unit) {
   // With integer arithmetic, to divide a value with correct rounding, you need
   // to add half of the divisor value to the dividend value. You need to do the
@@ skipping 1076 matching lines @@
     need_update = true;
     if (IsPageVisible(i))
       client_->Invalidate(GetPageScreenRect(i));
   }
   if (need_update)
     LoadPageInfo(true);
 
   FinishLoadingDocument();
 }
 
+unsigned long PDFiumEngine::GetNameDestCount() {
+  return FPDF_CountNamedDests(doc_);

[raymes, 2015/01/13 01:59:32]

This is quite simple, so let's inline it :)

[Deepak, 2015/01/13 08:29:07]

Done.

[Deepak, 2015/01/13 08:29:08]

Done.

+}
+
+void PDFiumEngine::GetNameDests(std::vector<std::string>* name_dest) {

[raymes, 2015/01/13 01:59:32]

Given the simplifications I mentioned below, I think we should be able to merge
this function with the one below and probably simplify things a bit

[Deepak, 2015/01/13 08:29:08]

Done.

+  unsigned long count = GetNameDestCount();
+  for (unsigned long i = 0; i < count; i++) {
+    wchar_t* name = NULL;

[raymes, 2015/01/13 01:59:31]

I think we should use a std::string16 and the WriteInto function (look for this
pattern elsewhere in this file, e.g. in line 3800)

[Deepak, 2015/01/13 08:29:08]

done

+    unsigned long len = 0;
+    FPDF_DEST dest = NULL;
+    dest = FPDF_GetNamedDest(doc_, i, name, len);
+    if (dest) {

[raymes, 2015/01/13 01:59:32]

-This is strange - the API documentation says it should return NULL if we pass
in a NULL name. In any case, we shouldn't check the return value here, there is
no need to. We can remove the above dest variable altogether.

[Deepak, 2015/01/13 08:29:07]

agreed

+      name = new wchar_t[len];
+      FPDF_DEST dest = FPDF_GetNamedDest(doc_, i, name, len);
+      if (dest) {
+        name_dest->push_back(
+            base::UTF16ToUTF8(reinterpret_cast<const base::char16*>(name)));
+      }
+      delete[] name;
+    }
+  }
+}
+
+void PDFiumEngine::GetAllNameDests(pp::VarDictionary* named_destinations) {
+  std::vector<std::string> name_dest(GetNameDestCount());
+  GetNameDests(&name_dest);
+  for (size_t i = 0; i < name_dest.size(); ++i) {
+    std::vector<std::string> fragments;
+    Tokenize(name_dest[i], kDelimiters, &fragments);
+    if ((fragments.size() == 1) &&
+        (fragments[0].find('=') == std::string::npos)) {

[raymes, 2015/01/13 01:59:32]

I don't think we need the above 3 lines. I suspect the name of the named
destination will always satisfy these constraints. It would be weird for the API
to return something else.

[Deepak, 2015/01/13 08:29:08]

Done.

+      int page_number = GetNamedDestinationPage(fragments[0]);

[raymes, 2015/01/13 01:59:32]

We can just have GetNamedDestinationPage(name_dest[i])

[Deepak, 2015/01/13 08:29:07]

Done.

+      if (page_number >= 0)
+        named_destinations->Set(fragments[0], pp::Var(page_number));
+    }
+  }
+}
+
 void PDFiumEngine::FinishLoadingDocument() {
   DCHECK(doc_loader_.IsDocumentComplete() && doc_);
   if (called_do_document_action_)
     return;
   called_do_document_action_ = true;
 
   // These can only be called now, as the JS might end up needing a page.
   FORM_DoDocumentJSAction(form_);
   FORM_DoDocumentOpenAction(form_);
   if (most_visible_page_ != -1) {
@@ skipping 2713 matching lines @@
     double* height) {
   FPDF_DOCUMENT doc = FPDF_LoadMemDocument(pdf_buffer, pdf_buffer_size, NULL);
   if (!doc)
     return false;
   bool success = FPDF_GetPageSizeByIndex(doc, page_number, width, height) != 0;
   FPDF_CloseDocument(doc);
   return success;
 }
 
 }  // namespace chrome_pdf