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

chrome/browser/resources/pdf/pdf.js

Index: chrome/browser/resources/pdf/pdf.js
diff --git a/chrome/browser/resources/pdf/pdf.js b/chrome/browser/resources/pdf/pdf.js
index 735c508fdd197c0924cf7b8127f5f9d6c0eb9ee0..9b8b86ebdc06bfbc27cdb601382262c4f7aee35d 100644
--- a/chrome/browser/resources/pdf/pdf.js
+++ b/chrome/browser/resources/pdf/pdf.js
@@ -136,8 +136,7 @@ function PDFViewer(streamDetails) {
   }
 
   // Parse open pdf parameters.
-  var paramsParser = new OpenPDFParamsParser(this.streamDetails_.originalUrl);
-  this.urlParams_ = paramsParser.urlParams;
+  this.paramsParser = new OpenPDFParamsParser();
 }
 
 PDFViewer.prototype = {
@@ -284,17 +283,21 @@ PDFViewer.prototype = {
    * important as later actions can override the effects of previous actions.
    */
   handleURLParams_: function() {
-    if (this.urlParams_.page)
-      this.viewport_.goToPage(this.urlParams_.page);
-    if (this.urlParams_.position) {
+    var pageNumber =
+        this.paramsParser.getInitalPage(this.streamDetails_.originalUrl);
+    if (pageNumber >= 0)
+      this.viewport_.goToPage(pageNumber);
+    var urlParams =
+        this.paramsParser.getURLPDFParams(this.streamDetails_.originalUrl);
+    if (urlParams.position) {
       // Make sure we don't cancel effect of page parameter.
       this.viewport_.position = {
-        x: this.viewport_.position.x + this.urlParams_.position.x,
-        y: this.viewport_.position.y + this.urlParams_.position.y
+        x: this.viewport_.position.x + urlParams.position.x,
+        y: this.viewport_.position.y + urlParams.position.y
       };
     }
-    if (this.urlParams_.zoom)
-      this.viewport_.setZoom(this.urlParams_.zoom);
+    if (urlParams.zoom)
+      this.viewport_.setZoom(urlParams.zoom);
   },
 
   /**
@@ -341,6 +344,62 @@ PDFViewer.prototype = {
 
   /**
    * @private
+   * Helper function to process url from message and to navigate within the
+   * PDF page or opening new url in the same tab.

[raymes, 2015/01/14 00:19:00]

nit: Helper function to navigate to the given URL. This might involve navgating
within the PDF page or opening a new url (in the same tab or a new tab)

[Deepak, 2015/01/14 06:29:01]

Done.

+   * @param {Object} message data that have the url and newTab info.

[raymes, 2015/01/14 00:19:00]

Please update this with the new params :)

[Deepak, 2015/01/14 06:29:01]

Done.

+   */
+  navigate_: function(urlFragment, isNewTab) {
+  if (urlFragment.length != 0) {

[raymes, 2015/01/14 00:19:00]

nit: everything from here below should be indented one more level

[Deepak, 2015/01/14 06:29:01]

Done.

+    // If |urlFragment| starts with '#', then it's for the same URL with a
+    // different URL fragment.
+    if (urlFragment.charAt(0) == '#') {
+      // if '#' is already present in |originalUrl| then remove old fragment and
+      // add new |url_copy| fragment.
+      var hashIndex = this.streamDetails_.originalUrl.search('#');

[raymes, 2015/01/14 00:19:00]

nit: let's put the original url in a local variable above to simplify the code a
bit

var originalUrl = this.streamDetails_.originalUrl;

[Deepak, 2015/01/14 06:29:01]

Done.

+      if (hashIndex != -1) {
+        urlFragment = this.streamDetails_.originalUrl.substring(0, hashIndex) +
+            urlFragment;
+      }
+      else {
+        urlFragment = this.streamDetails_.originalUrl + urlFragment;
+      }
+    }

[raymes, 2015/01/14 00:19:00]

the logic from here down differs from Instance::NavigateTo. Can we keep it
similar? In navigateTo it does the internal scrolling at this point

[Deepak, 2015/01/14 06:29:01]

If I scroll here like Instance::NavigateTo() then we are getting jurky effect in
the page, and it will not be smooth scrolling in the page.and I am getting
effect like page is getting loaded again.
As thestig mentioned that we want smooth scrolling when navigation with in the
page.

[raymes, 2015/01/15 04:51:10]

What I mean is to move the following lines below up to here: 

if (namedFragment)
  pageNumber = this.paramsParser.namedDestinations[namedFragment];

Does moving those up into the same place as inside NavigateTo cause something to
become janky? 

+  }
+  var inputURL = urlFragment;
+    // If there's no scheme, add http.
+    if (inputURL.indexOf('://') == -1 && inputURL.indexOf('mailto:') == -1) {
+        inputURL = 'http://' + inputURL;
+    }
+    // Make sure |url_copy| starts with a valid scheme.
+    if (inputURL.indexOf('http://') != 0 &&
+        inputURL.indexOf('https://') != 0 &&
+        inputURL.indexOf('ftp://') != 0 &&
+        inputURL.indexOf('file://') != 0 &&
+        inputURL.indexOf('mailto:') != 0) {
+      return;
+    }
+    // Make sure |url_copy| is not only a scheme.
+    if (inputURL == 'http://' ||
+        inputURL == 'https://' ||
+        inputURL == 'ftp://' ||
+        inputURL == 'file://' ||
+        inputURL == 'mailto:') {
+      return;
+    }
+
+    if (isNewTab) {
+      chrome.tabs.create({ url: inputURL });

[raymes, 2015/01/14 00:19:00]

As I mentioned earlier - we don't need to use the chrome.tabs API. window.open
is good enough.

[Deepak, 2015/01/14 06:29:01]

I remember, you told me to use window.open(url);
But this call is crashing.
I found that window.open does not work properly issue already logged.

I tried window.open(inputURl,'_self') or window.open(inputURl,'_parent') but
these open the link in the same tab.

+    } else {
+      var pageNumber = this.paramsParser.getInitalPage(inputURL);
+      if (pageNumber >= 0)
+          this.viewport_.goToPage(pageNumber);
+      else
+          window.location.href = inputURL;
+    }
+  },
+
+  /**
+   * @private
    * An event handler for handling message events received from the plugin.
    * @param {MessageObject} message a message event.
    */
@@ -386,10 +445,10 @@ PDFViewer.prototype = {
         this.updateProgress_(message.data.progress);
         break;
       case 'navigate':
-        if (message.data.newTab)
-          window.open(message.data.url);
-        else
-          window.location.href = message.data.url;
+        this.navigate_(message.data.url, message.data.newTab);
+        break;
+      case 'setNamedDestinations':
+        this.paramsParser.namedDestinations = message.data.namedDestinations;
         break;
       case 'setScrollPosition':
         var position = this.viewport_.position;