From c75051490338e8bb5154ad73bcdc43c804f47711 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 10 May 2017 13:33:08 +0200 Subject: [PATCH 1/2] Simplify `extractText` in `web/pdf_find_controller.js` Currently this method first uses a loop to build a temporary array to hold Promises, which are then resolved from a recursive helper function once the textContent is fetched for each page. To me, this is unncessarily complicated, since we can do everything within one loop by simply chaining the asynchronous calls to retrieve the textContent. (Note that this guarantees that the textContent of the pages is still fetched sequentially.) --- web/pdf_find_controller.js | 44 +++++++++++++++----------------------- 1 file changed, 17 insertions(+), 27 deletions(-) diff --git a/web/pdf_find_controller.js b/web/pdf_find_controller.js index a580abda25b78..e64d899d952fe 100644 --- a/web/pdf_find_controller.js +++ b/web/pdf_find_controller.js @@ -13,6 +13,7 @@ * limitations under the License. */ +import { createPromiseCapability } from './pdfjs'; import { scrollIntoView } from './ui_utils'; var FindStates = { @@ -231,43 +232,32 @@ var PDFFindController = (function PDFFindControllerClosure() { } }, - extractText: function PDFFindController_extractText() { + extractText() { if (this.startedTextExtraction) { return; } this.startedTextExtraction = true; + this.pageContents.length = 0; - this.pageContents = []; - var extractTextPromisesResolves = []; - var numPages = this.pdfViewer.pagesCount; - for (var i = 0; i < numPages; i++) { - this.extractTextPromises.push(new Promise(function (resolve) { - extractTextPromisesResolves.push(resolve); - })); - } + let promise = Promise.resolve(); + for (let i = 0, ii = this.pdfViewer.pagesCount; i < ii; i++) { + let extractTextCapability = createPromiseCapability(); + this.extractTextPromises[i] = extractTextCapability.promise; - var self = this; - function extractPageText(pageIndex) { - self.pdfViewer.getPageTextContent(pageIndex).then( - function textContentResolved(textContent) { - var textItems = textContent.items; - var str = []; + promise = promise.then(() => { + return this.pdfViewer.getPageTextContent(i).then((textContent) => { + let textItems = textContent.items; + let strBuf = []; - for (var i = 0, len = textItems.length; i < len; i++) { - str.push(textItems[i].str); + for (let j = 0, jj = textItems.length; j < jj; j++) { + strBuf.push(textItems[j].str); } - // Store the pageContent as a string. - self.pageContents.push(str.join('')); - - extractTextPromisesResolves[pageIndex](pageIndex); - if ((pageIndex + 1) < self.pdfViewer.pagesCount) { - extractPageText(pageIndex + 1); - } - } - ); + this.pageContents[i] = strBuf.join(''); + extractTextCapability.resolve(i); + }); + }); } - extractPageText(0); }, executeCommand: function PDFFindController_executeCommand(cmd, state) { From 9efd11c5c9aae0540a752638d6ab413397241031 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 10 May 2017 13:51:36 +0200 Subject: [PATCH 2/2] Replace unnecessary `var self = this` statements with arrow functions in `web/pdf_find_controller.js` --- web/pdf_find_controller.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/web/pdf_find_controller.js b/web/pdf_find_controller.js index e64d899d952fe..1304ee9ab3ee8 100644 --- a/web/pdf_find_controller.js +++ b/web/pdf_find_controller.js @@ -312,18 +312,17 @@ var PDFFindController = (function PDFFindControllerClosure() { this.pageMatches = []; this.matchCount = 0; this.pageMatchesLength = null; - var self = this; - for (var i = 0; i < numPages; i++) { + for (let i = 0; i < numPages; i++) { // Wipe out any previous highlighted matches. this.updatePage(i); // As soon as the text is extracted start finding the matches. if (!(i in this.pendingFindMatches)) { this.pendingFindMatches[i] = true; - this.extractTextPromises[i].then(function(pageIdx) { - delete self.pendingFindMatches[pageIdx]; - self.calcFindMatch(pageIdx); + this.extractTextPromises[i].then((pageIdx) => { + delete this.pendingFindMatches[pageIdx]; + this.calcFindMatch(pageIdx); }); } }