Merge pull request #10182 from Snuffleupagus/TextLayerBuilder-rm-find…

…Controller-checks Small clean-up of the search related methods in `TextLayerBuilder`
mozilla · Oct 28, 2018 · 04ce2af · 04ce2af
2 parents a77ac5f + d73a71f
commit 04ce2af
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 62 deletions.
diff --git a/web/pdf_find_controller.js b/web/pdf_find_controller.js
@@ -149,7 +149,7 @@ class PDFFindController {
     this._highlightMatches = false;
     this._pdfDocument = null;
     this._pageMatches = [];
-    this._pageMatchesLength = null;
+    this._pageMatchesLength = [];
     this._state = null;
     this._selected = { // Currently selected match.
       pageIdx: -1,
@@ -304,9 +304,6 @@ class PDFFindController {
     }
 
     // Prepare arrays for storing the matches.
-    if (!this._pageMatchesLength) {
-      this._pageMatchesLength = [];
-    }
     this._pageMatchesLength[pageIndex] = [];
     this._pageMatches[pageIndex] = [];
 
@@ -417,7 +414,7 @@ class PDFFindController {
       this._offset.matchIdx = null;
       this._resumePageIdx = null;
       this._pageMatches.length = 0;
-      this._pageMatchesLength = null;
+      this._pageMatchesLength.length = 0;
       this._matchesCountTotal = 0;
 
       for (let i = 0; i < numPages; i++) {

diff --git a/web/text_layer_builder.js b/web/text_layer_builder.js
@@ -108,7 +108,7 @@ class TextLayerBuilder {
     this.textLayerRenderTask.promise.then(() => {
       this.textLayerDiv.appendChild(textLayerFrag);
       this._finishRendering();
-      this.updateMatches();
+      this._updateMatches();
     }, function (reason) {
       // Cancelled or failed to render text layer; skipping errors.
     });
@@ -134,24 +134,25 @@ class TextLayerBuilder {
     this.textContent = textContent;
   }
 
-  convertMatches(matches, matchesLength) {
-    let i = 0;
-    let iIndex = 0;
-    let textContentItemsStr = this.textContentItemsStr;
-    let end = textContentItemsStr.length - 1;
-    let queryLen = (this.findController === null ?
-                    0 : this.findController.state.query.length);
-    let ret = [];
+  _convertMatches(matches, matchesLength) {
+    // Early exit if there is nothing to convert.
     if (!matches) {
-      return ret;
+      return [];
     }
-    for (let m = 0, len = matches.length; m < len; m++) {
+    const { findController, textContentItemsStr, } = this;
+
+    let i = 0, iIndex = 0;
+    const end = textContentItemsStr.length - 1;
+    const queryLen = findController.state.query.length;
+    const result = [];
+
+    for (let m = 0, mm = matches.length; m < mm; m++) {
       // Calculate the start position.
       let matchIdx = matches[m];
 
       // Loop over the divIdxs.
-      while (i !== end && matchIdx >=
-             (iIndex + textContentItemsStr[i].length)) {
+      while (i !== end &&
+             matchIdx >= (iIndex + textContentItemsStr[i].length)) {
         iIndex += textContentItemsStr[i].length;
         i++;
       }
@@ -176,8 +177,8 @@ class TextLayerBuilder {
 
       // Somewhat the same array as above, but use > instead of >= to get
       // the end position right.
-      while (i !== end && matchIdx >
-             (iIndex + textContentItemsStr[i].length)) {
+      while (i !== end &&
+             matchIdx > (iIndex + textContentItemsStr[i].length)) {
         iIndex += textContentItemsStr[i].length;
         i++;
       }
@@ -186,28 +187,22 @@ class TextLayerBuilder {
         divIdx: i,
         offset: matchIdx - iIndex,
       };
-      ret.push(match);
+      result.push(match);
     }
-
-    return ret;
+    return result;
   }
 
-  renderMatches(matches) {
+  _renderMatches(matches) {
     // Early exit if there is nothing to render.
     if (matches.length === 0) {
       return;
     }
+    const { findController, pageIdx, textContentItemsStr, textDivs, } = this;
 
-    let textContentItemsStr = this.textContentItemsStr;
-    let textDivs = this.textDivs;
+    const isSelectedPage = (pageIdx === findController.selected.pageIdx);
+    const selectedMatchIdx = findController.selected.matchIdx;
+    const highlightAll = findController.state.highlightAll;
     let prevEnd = null;
-    let pageIdx = this.pageIdx;
-    let isSelectedPage = (this.findController === null ?
-      false : (pageIdx === this.findController.selected.pageIdx));
-    let selectedMatchIdx = (this.findController === null ?
-                            -1 : this.findController.selected.matchIdx);
-    let highlightAll = (this.findController === null ?
-                        false : this.findController.state.highlightAll);
     let infinity = {
       divIdx: -1,
       offset: undefined,
@@ -250,16 +245,14 @@ class TextLayerBuilder {
       let highlightSuffix = (isSelected ? ' selected' : '');
 
       // Scroll the selected match into view.
-      if (this.findController) {
-        if (this.findController.selected.matchIdx === i &&
-            this.findController.selected.pageIdx === pageIdx) {
-          const spot = {
-            top: MATCH_SCROLL_OFFSET_TOP,
-            left: MATCH_SCROLL_OFFSET_LEFT,
-          };
-          scrollIntoView(textDivs[begin.divIdx], spot,
-                         /* skipOverflowHiddenElements = */ true);
-        }
+      if (findController.selected.matchIdx === i &&
+          findController.selected.pageIdx === pageIdx) {
+        const spot = {
+          top: MATCH_SCROLL_OFFSET_TOP,
+          left: MATCH_SCROLL_OFFSET_LEFT,
+        };
+        scrollIntoView(textDivs[begin.divIdx], spot,
+                       /* skipOverflowHiddenElements = */ true);
       }
 
       // Match inside new div.
@@ -293,20 +286,18 @@ class TextLayerBuilder {
     }
   }
 
-  updateMatches() {
+  _updateMatches() {
     // Only show matches when all rendering is done.
     if (!this.renderingDone) {
       return;
     }
-
-    // Clear all matches.
-    let matches = this.matches;
-    let textDivs = this.textDivs;
-    let textContentItemsStr = this.textContentItemsStr;
+    const {
+      findController, matches, pageIdx, textContentItemsStr, textDivs,
+    } = this;
     let clearedUntilDivIdx = -1;
 
     // Clear all current matches.
-    for (let i = 0, len = matches.length; i < len; i++) {
+    for (let i = 0, ii = matches.length; i < ii; i++) {
       let match = matches[i];
       let begin = Math.max(clearedUntilDivIdx, match.begin.divIdx);
       for (let n = begin, end = match.end.divIdx; n <= end; n++) {
@@ -317,21 +308,16 @@ class TextLayerBuilder {
       clearedUntilDivIdx = match.end.divIdx + 1;
     }
 
-    if (!this.findController || !this.findController.highlightMatches) {
+    if (!findController || !findController.highlightMatches) {
       return;
     }
-
-    // Convert the matches on the page controller into the match format
+    // Convert the matches on the `findController` into the match format
     // used for the textLayer.
-    let pageMatches, pageMatchesLength;
-    if (this.findController !== null) {
-      pageMatches = this.findController.pageMatches[this.pageIdx] || null;
-      pageMatchesLength = (this.findController.pageMatchesLength) ?
-        this.findController.pageMatchesLength[this.pageIdx] || null : null;
-    }
+    const pageMatches = findController.pageMatches[pageIdx] || null;
+    const pageMatchesLength = findController.pageMatchesLength[pageIdx] || null;
 
-    this.matches = this.convertMatches(pageMatches, pageMatchesLength);
-    this.renderMatches(this.matches);
+    this.matches = this._convertMatches(pageMatches, pageMatchesLength);
+    this._renderMatches(this.matches);
   }
 
   /**
@@ -361,7 +347,7 @@ class TextLayerBuilder {
       if (evt.pageIndex !== this.pageIdx && evt.pageIndex !== -1) {
         return;
       }
-      this.updateMatches();
+      this._updateMatches();
     };
 
     eventBus.on('pagecancelled', _boundEvents.pageCancelled);