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

chrome/browser/resources/pdf/open_pdf_params_parser.js

 // Copyright 2014 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.
 
 'use strict';
 
 /**
  * Creates a new OpenPDFParamsParser. This parses the open pdf parameters
  * passed in the url to set initial viewport settings for opening the pdf.
  * @param {string} url to be parsed.
  */
 function OpenPDFParamsParser(url) {
-  this.url_ = url;
   this.urlParams = {};
-  this.parseOpenPDFParams_();
+  // A dictionary of all the named destinations in the PDF.
+  this.namedDestinations = {};
 }
 
 OpenPDFParamsParser.prototype = {
   /**
    * @private
    * Parse zoom parameter of open PDF parameters. If this
    * parameter is passed while opening PDF then PDF should be opened
    * at the specified zoom level.
    * @param {number} zoom value.
    */
@@ skipping 15 matching lines @@
 
     // Handle #zoom=scale,left,top.
     var position = {x: parseFloat(paramValueSplit[1]),
                     y: parseFloat(paramValueSplit[2])};
     this.urlParams['position'] = position;
     this.urlParams['zoom'] = zoomFactor;
   },
 
   /**
    * @private
-   * Parse open PDF parameters. These parameters are mentioned in the url
+   * Parse PDF url parameters. These parameters are mentioned in the url
    * and specify actions to be performed when opening pdf files.
    * See http://www.adobe.com/content/dam/Adobe/en/devnet/acrobat/
    * pdfs/pdf_open_parameters.pdf for details.
+   * @param {string} inputUrl that need to be parsed.
+   * @return {Object} urlParams data that have the PDF url parameter info.
    */
-  parseOpenPDFParams_: function() {
-    var originalUrl = this.url_;
+  getURLPDFParams: function(inputUrl) {

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

nit: let's change the name because I think it is doing something different.
getViewportFromUrlParams

[Deepak, 2015/01/15 06:30:05]

Done.

+    var originalUrl = inputUrl;
     var paramIndex = originalUrl.search('#');
     if (paramIndex == -1)
       return;
+    var paramTokens = originalUrl.substring(paramIndex + 1).split('&');
 
-    var paramTokens = originalUrl.substring(paramIndex + 1).split('&');
+    if ((paramTokens.length == 1) && (paramTokens[0].search('=') == -1)) {
+      this.urlParams['page'] = this.namedDestinations[paramTokens[0]];
+      return this.urlParams;
+    }

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

I don't think we need this whole if-block

[Deepak, 2015/01/15 06:30:05]

Then how we will handle 
http://XXX.pdf#Chapter5 case. where we need to mode to 'Chapter5' directly as a
part of load.

and even in the current issue we require page number for #US, that is coming
from this.namedDestinations[].

If we can achieve this without above code, please suggest.

+
     var paramsDictionary = {};
     for (var i = 0; i < paramTokens.length; ++i) {
       var keyValueSplit = paramTokens[i].split('=');
       if (keyValueSplit.length != 2)
         continue;
       paramsDictionary[keyValueSplit[0]] = keyValueSplit[1];
     }
 
+    if ('nameddest' in paramsDictionary) {
+      var pageNumber = parseInt(paramsDictionary['nameddest']);
+      if (!isNaN(pageNumber) && pageNumber >= 0)
+        this.urlParams['nameddest'] = pageNumber;

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

Can we just store this in the 'page' param as well?

[Deepak, 2015/01/15 06:30:05]

Done.

+    }
+
     if ('page' in paramsDictionary) {
       // |pageNumber| is 1-based, but goToPage() take a zero-based page number.
       var pageNumber = parseInt(paramsDictionary['page']);
-      if (!isNaN(pageNumber))
+      if (!isNaN(pageNumber) && pageNumber > 0)

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

good catch! thanks :)

[Deepak, 2015/01/15 06:30:05]

Acknowledged.

         this.urlParams['page'] = pageNumber - 1;
     }
 
     if ('zoom' in paramsDictionary)
       this.parseZoomParam_(paramsDictionary['zoom']);
+
+    return this.urlParams;
   }
 };