Merge pull request #8536 An error occurred from Snuffleupagus/refactor-ObjectLoader

Refactor `ObjectLoader` to use `Dict`s correctly, rather than abusing their internal properties

src/core/annotation.js

@@ -390,9 +390,8 @@ var Annotation = (function AnnotationClosure() {
         if (!resources) {
           return;
         }
-        var objectLoader = new ObjectLoader(resources.map,
-                                            keys,
-                                            resources.xref);
+        let objectLoader = new ObjectLoader(resources, keys, resources.xref);
+
         return objectLoader.load().then(function() {
           return resources;
         });

src/core/document.js

@@ -186,9 +186,8 @@ var Page = (function PageClosure() {
         this.resourcesPromise = this.pdfManager.ensure(this, 'resources');
       }
       return this.resourcesPromise.then(() => {
-        var objectLoader = new ObjectLoader(this.resources.map,
-                                            keys,
-                                            this.xref);
+        let objectLoader = new ObjectLoader(this.resources, keys, this.xref);
+
         return objectLoader.load();
       });
     },

src/core/obj.js

@@ -1610,39 +1610,34 @@ var FileSpec = (function FileSpecClosure() {
 })();
 
 /**
- * A helper for loading missing data in object graphs. It traverses the graph
+ * A helper for loading missing data in `Dict` graphs. It traverses the graph
  * depth first and queues up any objects that have missing data. Once it has
  * has traversed as many objects that are available it attempts to bundle the
  * missing data requests and then resume from the nodes that weren't ready.
  *
  * NOTE: It provides protection from circular references by keeping track of
- * of loaded references. However, you must be careful not to load any graphs
+ * loaded references. However, you must be careful not to load any graphs
  * that have references to the catalog or other pages since that will cause the
  * entire PDF document object graph to be traversed.
  */
-var ObjectLoader = (function() {
+let ObjectLoader = (function() {
   function mayHaveChildren(value) {
     return isRef(value) || isDict(value) || isArray(value) || isStream(value);
   }
 
   function addChildren(node, nodesToVisit) {
-    var value;
     if (isDict(node) || isStream(node)) {
-      var map;
-      if (isDict(node)) {
-        map = node.map;
-      } else {
-        map = node.dict.map;
-      }
-      for (var key in map) {
-        value = map[key];
-        if (mayHaveChildren(value)) {
-          nodesToVisit.push(value);
+      let dict = isDict(node) ? node : node.dict;
+      let dictKeys = dict.getKeys();
+      for (let i = 0, ii = dictKeys.length; i < ii; i++) {
+        let rawValue = dict.getRaw(dictKeys[i]);
+        if (mayHaveChildren(rawValue)) {
+          nodesToVisit.push(rawValue);
         }
       }
     } else if (isArray(node)) {
-      for (var i = 0, ii = node.length; i < ii; i++) {
-        value = node[i];
+      for (let i = 0, ii = node.length; i < ii; i++) {
+        let value = node[i];
         if (mayHaveChildren(value)) {
           nodesToVisit.push(value);
         }
@@ -1650,8 +1645,8 @@ var ObjectLoader = (function() {
     }
   }
 
-  function ObjectLoader(obj, keys, xref) {
-    this.obj = obj;
+  function ObjectLoader(dict, keys, xref) {
+    this.dict = dict;
     this.keys = keys;
     this.xref = xref;
     this.refSet = null;
@@ -1659,8 +1654,7 @@ var ObjectLoader = (function() {
   }
 
   ObjectLoader.prototype = {
-    load: function ObjectLoader_load() {
-      var keys = this.keys;
+    load() {
       this.capability = createPromiseCapability();
       // Don't walk the graph if all the data is already loaded.
       if (!(this.xref.stream instanceof ChunkedStream) ||
@@ -1669,23 +1663,28 @@ var ObjectLoader = (function() {
         return this.capability.promise;
       }
 
+      let { keys, dict, } = this;
       this.refSet = new RefSet();
       // Setup the initial nodes to visit.
-      var nodesToVisit = [];
-      for (var i = 0; i < keys.length; i++) {
-        nodesToVisit.push(this.obj[keys[i]]);
+      let nodesToVisit = [];
+      for (let i = 0, ii = keys.length; i < ii; i++) {
+        let rawValue = dict.getRaw(keys[i]);
+        // Skip nodes that are guaranteed to be empty.
+        if (rawValue !== undefined) {
+          nodesToVisit.push(rawValue);
+        }
       }
 
       this._walk(nodesToVisit);
       return this.capability.promise;
     },
 
-    _walk: function ObjectLoader_walk(nodesToVisit) {
-      var nodesToRevisit = [];
-      var pendingRequests = [];
+    _walk(nodesToVisit) {
+      let nodesToRevisit = [];
+      let pendingRequests = [];
       // DFS walk of the object graph.
       while (nodesToVisit.length) {
-        var currentNode = nodesToVisit.pop();
+        let currentNode = nodesToVisit.pop();
 
         // Only references or chunked streams can cause missing data exceptions.
         if (isRef(currentNode)) {
@@ -1694,28 +1693,24 @@ var ObjectLoader = (function() {
             continue;
           }
           try {
-            var ref = currentNode;
-            this.refSet.put(ref);
+            this.refSet.put(currentNode);
             currentNode = this.xref.fetch(currentNode);
-          } catch (e) {
-            if (!(e instanceof MissingDataException)) {
-              throw e;
+          } catch (ex) {
+            if (!(ex instanceof MissingDataException)) {
+              throw ex;
             }
             nodesToRevisit.push(currentNode);
-            pendingRequests.push({ begin: e.begin, end: e.end, });
+            pendingRequests.push({ begin: ex.begin, end: ex.end, });
           }
         }
         if (currentNode && currentNode.getBaseStreams) {
-          var baseStreams = currentNode.getBaseStreams();
-          var foundMissingData = false;
-          for (var i = 0; i < baseStreams.length; i++) {
-            var stream = baseStreams[i];
+          let baseStreams = currentNode.getBaseStreams();
+          let foundMissingData = false;
+          for (let i = 0, ii = baseStreams.length; i < ii; i++) {
+            let stream = baseStreams[i];
             if (stream.getMissingChunks && stream.getMissingChunks().length) {
               foundMissingData = true;
-              pendingRequests.push({
-                begin: stream.start,
-                end: stream.end,
-              });
+              pendingRequests.push({ begin: stream.start, end: stream.end, });
             }
           }
           if (foundMissingData) {
@@ -1728,16 +1723,15 @@ var ObjectLoader = (function() {
 
       if (pendingRequests.length) {
         this.xref.stream.manager.requestRanges(pendingRequests).then(() => {
-          nodesToVisit = nodesToRevisit;
-          for (var i = 0; i < nodesToRevisit.length; i++) {
-            var node = nodesToRevisit[i];
-            // Remove any reference nodes from the currrent refset so they
+          for (let i = 0, ii = nodesToRevisit.length; i < ii; i++) {
+            let node = nodesToRevisit[i];
+            // Remove any reference nodes from the current `RefSet` so they
             // aren't skipped when we revist them.
             if (isRef(node)) {
               this.refSet.remove(node);
             }
           }
-          this._walk(nodesToVisit);
+          this._walk(nodesToRevisit);
         }, this.capability.reject);
         return;
       }

src/core/primitives.js

@@ -60,7 +60,7 @@ var Dict = (function DictClosure() {
   // xref is optional
   function Dict(xref) {
     // Map should only be used internally, use functions below to access.
-    this.map = Object.create(null);
+    this._map = Object.create(null);
     this.xref = xref;
     this.objId = null;
     this.suppressEncryption = false;
@@ -76,15 +76,15 @@ var Dict = (function DictClosure() {
     get: function Dict_get(key1, key2, key3) {
       var value;
       var xref = this.xref, suppressEncryption = this.suppressEncryption;
-      if (typeof (value = this.map[key1]) !== 'undefined' || key1 in this.map ||
-          typeof key2 === 'undefined') {
+      if (typeof (value = this._map[key1]) !== 'undefined' ||
+          key1 in this._map || typeof key2 === 'undefined') {
         return xref ? xref.fetchIfRef(value, suppressEncryption) : value;
       }
-      if (typeof (value = this.map[key2]) !== 'undefined' || key2 in this.map ||
-          typeof key3 === 'undefined') {
+      if (typeof (value = this._map[key2]) !== 'undefined' ||
+          key2 in this._map || typeof key3 === 'undefined') {
         return xref ? xref.fetchIfRef(value, suppressEncryption) : value;
       }
-      value = this.map[key3] || null;
+      value = this._map[key3] || null;
       return xref ? xref.fetchIfRef(value, suppressEncryption) : value;
     },
 
@@ -92,21 +92,21 @@ var Dict = (function DictClosure() {
     getAsync: function Dict_getAsync(key1, key2, key3) {
       var value;
       var xref = this.xref, suppressEncryption = this.suppressEncryption;
-      if (typeof (value = this.map[key1]) !== 'undefined' || key1 in this.map ||
-          typeof key2 === 'undefined') {
+      if (typeof (value = this._map[key1]) !== 'undefined' ||
+          key1 in this._map || typeof key2 === 'undefined') {
         if (xref) {
           return xref.fetchIfRefAsync(value, suppressEncryption);
         }
         return Promise.resolve(value);
       }
-      if (typeof (value = this.map[key2]) !== 'undefined' || key2 in this.map ||
-          typeof key3 === 'undefined') {
+      if (typeof (value = this._map[key2]) !== 'undefined' ||
+          key2 in this._map || typeof key3 === 'undefined') {
         if (xref) {
           return xref.fetchIfRefAsync(value, suppressEncryption);
         }
         return Promise.resolve(value);
       }
-      value = this.map[key3] || null;
+      value = this._map[key3] || null;
       if (xref) {
         return xref.fetchIfRefAsync(value, suppressEncryption);
       }
@@ -132,23 +132,23 @@ var Dict = (function DictClosure() {
 
     // no dereferencing
     getRaw: function Dict_getRaw(key) {
-      return this.map[key];
+      return this._map[key];
     },
 
     getKeys: function Dict_getKeys() {
-      return Object.keys(this.map);
+      return Object.keys(this._map);
     },
 
     set: function Dict_set(key, value) {
-      this.map[key] = value;
+      this._map[key] = value;
     },
 
     has: function Dict_has(key) {
-      return key in this.map;
+      return key in this._map;
     },
 
     forEach: function Dict_forEach(callback) {
-      for (var key in this.map) {
+      for (var key in this._map) {
         callback(key, this.get(key));
       }
     },
@@ -156,19 +156,19 @@ var Dict = (function DictClosure() {
 
   Dict.empty = new Dict(null);
 
-  Dict.merge = function Dict_merge(xref, dictArray) {
-    var mergedDict = new Dict(xref);
+  Dict.merge = function(xref, dictArray) {
+    let mergedDict = new Dict(xref);
 
-    for (var i = 0, ii = dictArray.length; i < ii; i++) {
-      var dict = dictArray[i];
+    for (let i = 0, ii = dictArray.length; i < ii; i++) {
+      let dict = dictArray[i];
       if (!isDict(dict)) {
         continue;
       }
-      for (var keyName in dict.map) {
-        if (mergedDict.map[keyName]) {
+      for (let keyName in dict._map) {
+        if (mergedDict._map[keyName] !== undefined) {
           continue;
         }
-        mergedDict.map[keyName] = dict.map[keyName];
+        mergedDict._map[keyName] = dict._map[keyName];
       }
     }
     return mergedDict;

src/shared/fonts_utils.js

@@ -361,9 +361,9 @@ var Type2Parser = function type2Parser(aFilePath) {
     dump('privateData:' + privateDict);
     parseAsToken(privateDict, CFFDictPrivateDataMap);
 
-    for (var p in font.map) {
-      dump(p + '::' + font.get(p));
-    }
+    font.forEach(function(key, value) {
+      dump(key + '::' + value);
+    });
 
     // Read CharStrings Index
     var charStringsOffset = font.get('CharStrings');