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

pdf/out_of_process_instance.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/out_of_process_instance.h"
 
 #include <algorithm>  // for min/max()
 #define _USE_MATH_DEFINES  // for M_PI
 #include <cmath>      // for log() and pow()
 #include <math.h>
@@ skipping 110 matching lines @@
 const char kJSEmailCc[] = "cc";
 const char kJSEmailBcc[] = "bcc";
 const char kJSEmailSubject[] = "subject";
 const char kJSEmailBody[] = "body";
 // Rotation (Page -> Plugin)
 const char kJSRotateClockwiseType[] = "rotateClockwise";
 const char kJSRotateCounterclockwiseType[] = "rotateCounterclockwise";
 // Select all text in the document (Page -> Plugin)
 const char kJSSelectAllType[] = "selectAll";
 
+// List of named destinations (Plugin -> Page)
+const char kJSNamedDestinations[] = "namedDestinations";
+// URL reference parameters.
+const char kDelimiters[] = "#&";
+const char kNamedDest[] = "nameddest";
+
 const int kFindResultCooldownMs = 100;
 
 const double kMinZoom = 0.01;
 
 namespace {
 
 static const char kPPPPdfInterface[] = PPP_PDF_INTERFACE_1;
 
 PP_Var GetLinkAtPosition(PP_Instance instance, PP_Point point) {
   pp::Var var;
@@ skipping 694 matching lines @@
                                       bool open_in_new_tab) {
   std::string url_copy(url);
 
   // Empty |url_copy| is ok, and will effectively be a reload.
   // Skip the code below so an empty URL does not turn into "http://", which
   // will cause GURL to fail a DCHECK.
   if (!url_copy.empty()) {
     // If |url_copy| starts with '#', then it's for the same URL with a
     // different URL fragment.
     if (url_copy[0] == '#') {
-      url_copy = url_ + url_copy;
+      // if '#' is already present in |url_| then remove old fragment and add
+      // new |url_copy| fragment.
+      std::size_t index = url_.find('#');
+      if (index != std::string::npos)
+        url_copy = url_.substr(0, index) + url_copy;
+      else
+        url_copy = url_ + url_copy;
     }
     // If there's no scheme, add http.
     if (url_copy.find("://") == std::string::npos &&
         url_copy.find("mailto:") == std::string::npos) {
       url_copy = std::string("http://") + url_copy;
     }
     // Make sure |url_copy| starts with a valid scheme.
     if (url_copy.find("http://") != 0 &&
         url_copy.find("https://") != 0 &&
         url_copy.find("ftp://") != 0 &&
         url_copy.find("file://") != 0 &&
         url_copy.find("mailto:") != 0) {
       return;
     }
     // Make sure |url_copy| is not only a scheme.
     if (url_copy == "http://" ||
         url_copy == "https://" ||
         url_copy == "ftp://" ||
         url_copy == "file://" ||
         url_copy == "mailto:") {
       return;
     }
   }

[raymes, 2015/01/11 23:08:41]

Can we move all of the above code into a function in the JS side?

[Deepak, 2015/01/12 09:21:43]

Done.

   pp::VarDictionary message;
   message.Set(kType, kJSNavigateType);
   message.Set(kJSNavigateUrl, url_copy);
   message.Set(kJSNavigateNewTab, open_in_new_tab);
   PostMessage(message);
 }
 
 void OutOfProcessInstance::UpdateCursor(PP_CursorType_Dev cursor) {
   if (cursor == cursor_)
     return;
@@ skipping 208 matching lines @@
   UserMetricsRecordAction("PDF.LoadSuccess");
 
   // Note: If we are in print preview mode the scroll location is retained
   // across document loads so we don't want to scroll again and override it.
   if (IsPrintPreview()) {
     AppendBlankPrintPreviewPages();
     OnGeometryChanged(0, 0);
   }
 
   pp::VarDictionary message;
+  pp::VarDictionary named_destinations;
+  GetNamedDestinations(named_destinations);
   message.Set(pp::Var(kType), pp::Var(kJSLoadProgressType));
   message.Set(pp::Var(kJSProgressPercentage), pp::Var(100)) ;
+  message.Set(pp::Var(kJSNamedDestinations), pp::Var(named_destinations));
   PostMessage(message);
 
   if (!full_)
     return;
 
   if (did_call_start_loading_) {
     pp::PDF::DidStopLoading(this);
     did_call_start_loading_ = false;
   }
 
@@ skipping 199 matching lines @@
 
   const PPB_URLLoaderTrusted* trusted_interface =
       reinterpret_cast<const PPB_URLLoaderTrusted*>(
           pp::Module::Get()->GetBrowserInterface(
               PPB_URLLOADERTRUSTED_INTERFACE));
   if (trusted_interface)
     trusted_interface->GrantUniversalAccess(loader.pp_resource());
   return loader;
 }
 
+void OutOfProcessInstance::GetNamedDestinations(

[raymes, 2015/01/11 23:08:41]

Can we move all of this inside pdfium_engine and just have a function there that
returns a pp::VarDictionary?

[Deepak, 2015/01/12 09:21:43]

Done.

+    pp::VarDictionary& named_destinations) {

[raymes, 2015/01/11 23:08:41]

for future reference: we never use references that are out-params

[Deepak, 2015/01/12 09:21:43]

Done.

+  std::vector<std::string> name_dest(engine_->GetNameDestCount());
+  engine_->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)) {
+      int page_number = engine_->GetNamedDestinationPage(fragments[0]);
+      if (page_number >= 0)
+        named_destinations.Set(fragments[0], pp::Var(page_number));
+      continue;
+    }
+
+    std::vector<std::string> key_value;
+    base::SplitString(name_dest[i], '=', &key_value);
+    if (key_value.size() != 2)
+      continue;
+    const std::string& key = key_value[0];
+    const std::string& value = key_value[1];
+    if (base::strcasecmp(kNamedDest, key.c_str()) == 0) {
+      int page_value = engine_->GetNamedDestinationPage(value);
+      if (page_value >= 0)
+        named_destinations.Set(value, page_value);
+    }

[raymes, 2015/01/11 23:08:41]

Is all of this really needed?? Does FPDF_GetNamedDest really ever return
something that looks like nameddest=US? That seems really weird to me. This code
looks like it was used to get the nameddest from the URL but I suspect we don't
need it here.

[Deepak, 2015/01/12 09:21:43]

I agree with you, This code currently no needed,we get the namedests like 
US
UK
AR
PK
and then using these I found out the corresponding page number in the PDF , 

So removing this code.

+  }
+}
+
 void OutOfProcessInstance::SetZoom(double scale) {
   double old_zoom = zoom_;
   zoom_ = scale;
   OnGeometryChanged(old_zoom, device_scale_);
 }
 
 std::string OutOfProcessInstance::GetLocalizedString(PP_ResourceString id) {
   pp::Var rv(pp::PDF::GetLocalizedString(this, id));
   if (!rv.is_string())
     return std::string();
@@ skipping 54 matching lines @@
 pp::FloatPoint OutOfProcessInstance::BoundScrollOffsetToDocument(
     const pp::FloatPoint& scroll_offset) {
   float max_x = document_size_.width() * zoom_ - plugin_dip_size_.width();
   float x = std::max(std::min(scroll_offset.x(), max_x), 0.0f);
   float max_y = document_size_.height() * zoom_ - plugin_dip_size_.height();
   float y = std::max(std::min(scroll_offset.y(), max_y), 0.0f);
   return pp::FloatPoint(x, y);
 }
 
 }  // namespace chrome_pdf