Fix for PDFs with lots of named destinations take a long time to load.

chrome/browser/resources/pdf/open_pdf_params_parser.js

Index: chrome/browser/resources/pdf/open_pdf_params_parser.js
diff --git a/chrome/browser/resources/pdf/open_pdf_params_parser.js b/chrome/browser/resources/pdf/open_pdf_params_parser.js
index c0f4b19d1c8889d979221cc19009aa17f7e8a080..b7e45375f5e4c9a07ef804994fd172d05ff0673e 100644
--- a/chrome/browser/resources/pdf/open_pdf_params_parser.js
+++ b/chrome/browser/resources/pdf/open_pdf_params_parser.js
@@ -7,10 +7,14 @@
 /**
  * Creates a new OpenPDFParamsParser. This parses the open pdf parameters
  * passed in the url to set initial viewport settings for opening the pdf.
+ * @param {Object} getNamedDestination to fectch page number for
+ * namedDestination.
  */
-function OpenPDFParamsParser() {
-  // A dictionary of all the named destinations in the PDF.
+function OpenPDFParamsParser(getNamedDestination) {
+  // A dictionary of all the named destinations in the PDF for testing.
   this.namedDestinations = {};
+  this.outstandingRequests_ = [];
+  this.getNamedDestination_ = getNamedDestination;

[raymes, 2015/02/16 02:02:30]

nit: getNamedDestinationFunction_

[Deepak, 2015/02/16 06:45:38]

Done.

 }
 
 OpenPDFParamsParser.prototype = {
@@ -52,14 +56,15 @@ OpenPDFParamsParser.prototype = {
    * See http://www.adobe.com/content/dam/Adobe/en/devnet/acrobat/
    * pdfs/pdf_open_parameters.pdf for details.
    * @param {string} url that needs to be parsed.
-   * @return {Object} A dictionary containing the viewport which should be
-   * displayed based on the URL.
+   * @param {Object} callback function to be called with viewport info.

[raymes, 2015/02/16 02:02:30]

nit: {Function}

[Deepak, 2015/02/16 06:45:38]

Done.

    */
-  getViewportFromUrlParams: function(url) {
+  getViewportFromUrlParams: function(url, callback) {
     var viewportPosition = {};
     var paramIndex = url.search('#');
-    if (paramIndex == -1)
-      return viewportPosition;
+    if (paramIndex == -1) {
+      callback(viewportPosition);
+      return;
+    }
 
     var paramTokens = url.substring(paramIndex + 1).split('&');
     if ((paramTokens.length == 1) && (paramTokens[0].search('=') == -1)) {
@@ -67,7 +72,17 @@ OpenPDFParamsParser.prototype = {
       // explicitlymentioned except by example in the Adobe
       // "PDF Open Parameters" document.
       viewportPosition['page'] = this.namedDestinations[paramTokens[0]];

[raymes, 2015/02/16 02:02:30]

Let's not bother caching the namedDestinations here, I don't think it will be
particularly useful. So we can remove this line and the 4 below and always just
go to the plugin.

[Deepak, 2015/02/16 06:45:38]

This is done for testcases. As we don't want to load real page.
And we are putting values in namedDestinations in the test case.
so we are not fetching page number for named destination from plugin for test
cases.

[raymes, 2015/02/16 22:44:03]

I still don't think I agree - I think we should change it. For test cases we can
use a fake getNamedDestinations function and call onNamedDestinationReceived
directly.

-      return viewportPosition;
+      if (viewportPosition['page'] != undefined) {
+        callback(viewportPosition);
+        return;
+      }
+
+      this.outstandingRequests_.push({
+        callback: callback,
+        viewportPosition: viewportPosition
+      });
+      this.getNamedDestination_(paramTokens[0]);
+      return;
     }
 
     var paramsDictionary = {};
@@ -78,12 +93,6 @@ OpenPDFParamsParser.prototype = {
       paramsDictionary[keyValueSplit[0]] = keyValueSplit[1];
     }
 
-    if ('nameddest' in paramsDictionary) {
-      var page = this.namedDestinations[paramsDictionary['nameddest']];
-      if (page != undefined)
-        viewportPosition['page'] = page;
-    }
-
     if ('page' in paramsDictionary) {
       // |pageNumber| is 1-based, but goToPage() take a zero-based page number.
       var pageNumber = parseInt(paramsDictionary['page']);
@@ -94,6 +103,35 @@ OpenPDFParamsParser.prototype = {
     if ('zoom' in paramsDictionary)
       this.parseZoomParam_(paramsDictionary['zoom'], viewportPosition);
 
-    return viewportPosition;
-  }
+    if ('nameddest' in paramsDictionary) {
+      viewportPosition['page'] =
+          this.namedDestinations[paramsDictionary['nameddest']];
+      if (viewportPosition['page'] != undefined) {
+        callback(viewportPosition);
+        return;
+      }

[raymes, 2015/02/16 02:02:30]

Same here - let's remove the above few lines and remove this.namedDestinations
altogether

[Deepak, 2015/02/16 06:45:38]

Same reason as above.

+      this.outstandingRequests_.push({
+        callback: callback,
+        viewportPosition: viewportPosition
+      });
+      this.getNamedDestination_(paramsDictionary['nameddest']);
+      return;
+    }
+    callback(viewportPosition);
+    return;
+  },
+
+  /**
+   * It fetch outstanding request if any then update page number for
+   * namedDestination and call callback with viewport info.

[raymes, 2015/02/16 02:02:31]

This is called when a named destination is received and the page number
corresponding to the request for a named destination is passed in. 

[Deepak, 2015/02/16 06:45:38]

Done.

+   * @param {number} pageNumber for viewport.

[raymes, 2015/02/16 02:02:31]

nit: pageNumber The page corresponding to the named destination requested.

[Deepak, 2015/02/16 06:45:38]

Done.

+   */
+  onNamedDestinationReceived: function(pageNumber) {
+    var outstandingRequest = this.outstandingRequests_.shift();
+    if (!outstandingRequest)
+      return;
+    if (!outstandingRequest.viewportPosition.page)
+      outstandingRequest.viewportPosition.page = pageNumber;
+    outstandingRequest.callback(outstandingRequest.viewportPosition);
+  },
 };