From 3a20fd165f1cdefe16e253c13345924d324de38a Mon Sep 17 00:00:00 2001
From: Jonas Jenwald <jonas.jenwald@gmail.com>
Date: Tue, 13 Jun 2017 10:22:11 +0200
Subject: [PATCH] Refactor `ObjectLoader` to use `Dict`s correctly, rather than
 abusing their internal properties

The `ObjectLoader` currently takes an Object as input, despite actually working with `Dict`s internally. This means that at the (two) existing call-sites, we're passing in the "private" `Dict.map` property directly.

Doing this seems like an anti-pattern, and we could (and even should) simply provide the actual `Dict` when creating an `ObjectLoader` instance.
Accessing properties stored in the `Dict` is now done using the intended methods instead, in particular `getRaw` which (as the name suggests) doesn't do any de-referencing, thus maintaining the current functionality of the code.

The only functional change in this patch is that `ObjectLoader.load` will now ignore empty nodes, such that `ObjectLoader._walk` only needs to deal with nodes that are known to contain data. (This lets us skip, among other checks, meaningless `addChildren` function calls.)
---
 src/core/annotation.js |  5 ++---
 src/core/document.js   |  5 ++---
 src/core/obj.js        | 33 ++++++++++++++++-----------------
 3 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/src/core/annotation.js b/src/core/annotation.js
index 6f81ae6ea..b1320287c 100644
--- a/src/core/annotation.js
+++ b/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;
         });
diff --git a/src/core/document.js b/src/core/document.js
index e3ed0b97b..2aea50098 100644
--- a/src/core/document.js
+++ b/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();
       });
     },
diff --git a/src/core/obj.js b/src/core/obj.js
index e6e114450..5d3b4acfa 100644
--- a/src/core/obj.js
+++ b/src/core/obj.js
@@ -1610,7 +1610,7 @@ 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.
@@ -1626,23 +1626,18 @@ let ObjectLoader = (function() {
   }
 
   function addChildren(node, nodesToVisit) {
-    let value;
     if (isDict(node) || isStream(node)) {
-      let map;
-      if (isDict(node)) {
-        map = node.map;
-      } else {
-        map = node.dict.map;
-      }
-      for (let 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 (let i = 0, ii = node.length; i < ii; i++) {
-        value = node[i];
+        let value = node[i];
         if (mayHaveChildren(value)) {
           nodesToVisit.push(value);
         }
@@ -1650,8 +1645,8 @@ let 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;
@@ -1668,12 +1663,16 @@ let ObjectLoader = (function() {
         return this.capability.promise;
       }
 
-      let keys = this.keys;
+      let { keys, dict, } = this;
       this.refSet = new RefSet();
       // Setup the initial nodes to visit.
       let nodesToVisit = [];
       for (let i = 0, ii = keys.length; i < ii; i++) {
-        nodesToVisit.push(this.obj[keys[i]]);
+        let rawValue = dict.getRaw(keys[i]);
+        // Skip nodes that are guaranteed to be empty.
+        if (rawValue !== undefined) {
+          nodesToVisit.push(rawValue);
+        }
       }
 
       this._walk(nodesToVisit);