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

pdf/out_of_process_instance.cc

Index: pdf/out_of_process_instance.cc
diff --git a/pdf/out_of_process_instance.cc b/pdf/out_of_process_instance.cc
index 59b04394419410bc1eda3518839f2323169bbe94..b5e7e3065b1909e8271b5c91f857f7b62b68b59c 100644
--- a/pdf/out_of_process_instance.cc
+++ b/pdf/out_of_process_instance.cc
@@ -128,6 +128,10 @@ const char kJSRotateCounterclockwiseType[] = "rotateCounterclockwise";
 // Select all text in the document (Page -> Plugin)
 const char kJSSelectAllType[] = "selectAll";
 
+// List of named destinations (Plugin -> Page)
+const char kJSSetNamedDestinations[] = "setNamedDestinations";
+const char kJSNamedDestinations[] = "namedDestinations";
+
 const int kFindResultCooldownMs = 100;
 
 const double kMinZoom = 0.01;
@@ -842,28 +846,13 @@ void OutOfProcessInstance::NavigateTo(const std::string& url,
     // 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 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;
+      // 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;
     }

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

Can we move this code into JS as well? We should have the original URL available
there too (this.streamDetails_.originalUrl)

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

Done.

   }
   pp::VarDictionary message;
@@ -1079,6 +1068,13 @@ void OutOfProcessInstance::DocumentPaintOccurred() {
 }
 
 void OutOfProcessInstance::DocumentLoadComplete(int page_count) {
+  pp::VarDictionary named_message;

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

nit: move this down to right above where it is used. Maybe call it
named_destinations_message

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

Done.

+  pp::VarDictionary named_destinations;
+  engine_->GetAllNameDests(&named_destinations);

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

Can't this function just return a pp::VarDictionary rather than having an out
parameter?

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

Done.

+  named_message.Set(pp::Var(kType), pp::Var(kJSSetNamedDestinations));
+  named_message.Set(pp::Var(kJSNamedDestinations), pp::Var(named_destinations));
+  PostMessage(named_message);

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

nit: let's move the above code down to right above the progress message

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

Done.

+
   // Clear focus state for OSK.
   FormTextFieldFocusChange(false);