From 160cfc408440aad5f45875cadb1af6995b7ed802 Mon Sep 17 00:00:00 2001
From: Jonas Jenwald <jonas.jenwald@gmail.com>
Date: Fri, 31 Jan 2020 11:39:48 +0100
Subject: [PATCH] Slightly simplify the lookup of data in `Dict.{get, getAsync,
 has}`

Note that `Dict.set` will only be called with values returned through `Parser.getObj`, and thus indirectly via `Lexer.getObj`. Since neither of those methods will ever return `undefined`, we can simply assert that that's the case when inserting data into the `Dict` and thus get rid of `in` checks when doing the data lookups.
In this case, since `Dict.set` is fairly hot, the patch utilizes an *inline check* and when necessary a direct call to `unreachable` to not affect performance of `gulp server/test` too much (rather than always just calling `assert`).

For very large and complex PDF files this will help performance *slightly*, since `Dict.{get, getAsync, has}` is called *a lot* during parsing in the worker.

This patch was tested using the PDF file from issue 2618, i.e. http://bugzilla-attachments.gnome.org/attachment.cgi?id=226471, with the following manifest file:
```
[
    {  "id": "issue2618",
       "file": "../web/pdfs/issue2618.pdf",
       "md5": "",
       "rounds": 250,
       "type": "eq"
    }
]
```

which gave the following results when comparing this patch against the `master` branch:
```
-- Grouped By browser, stat --
browser | stat         | Count | Baseline(ms) | Current(ms) | +/- |    %  | Result(P<.05)
------- | ------------ | ----- | ------------ | ----------- | --- | ----- | -------------
Firefox | Overall      |   250 |         2838 |        2820 | -18 | -0.65 |        faster
Firefox | Page Request |   250 |            1 |           2 |   0 | 11.92 |        slower
Firefox | Rendering    |   250 |         2837 |        2818 | -19 | -0.65 |        faster
```
---
 src/core/evaluator.js        |  2 +-
 src/core/primitives.js       | 19 +++++++++++++------
 test/unit/primitives_spec.js | 10 ++++++----
 3 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/src/core/evaluator.js b/src/core/evaluator.js
index 1fff07543..17a73f569 100644
--- a/src/core/evaluator.js
+++ b/src/core/evaluator.js
@@ -3017,7 +3017,7 @@ var PartialEvaluator = (function PartialEvaluatorClosure() {
           // is a tagged pdf. Create a barbebones one to get by.
           descriptor = new Dict(null);
           descriptor.set("FontName", Name.get(type));
-          descriptor.set("FontBBox", dict.getArray("FontBBox"));
+          descriptor.set("FontBBox", dict.getArray("FontBBox") || [0, 0, 0, 0]);
         } else {
           // Before PDF 1.5 if the font was one of the base 14 fonts, having a
           // FontDescriptor was not required.
diff --git a/src/core/primitives.js b/src/core/primitives.js
index 6ec9e4eec..b3c597124 100644
--- a/src/core/primitives.js
+++ b/src/core/primitives.js
@@ -14,7 +14,7 @@
  */
 /* uses XRef */
 
-import { assert } from "../shared/util.js";
+import { assert, unreachable } from "../shared/util.js";
 
 var EOF = {};
 
@@ -85,9 +85,9 @@ var Dict = (function DictClosure() {
     // automatically dereferences Ref objects
     get(key1, key2, key3) {
       let value = this._map[key1];
-      if (value === undefined && !(key1 in this._map) && key2 !== undefined) {
+      if (value === undefined && key2 !== undefined) {
         value = this._map[key2];
-        if (value === undefined && !(key2 in this._map) && key3 !== undefined) {
+        if (value === undefined && key3 !== undefined) {
           value = this._map[key3];
         }
       }
@@ -100,9 +100,9 @@ var Dict = (function DictClosure() {
     // Same as get(), but returns a promise and uses fetchIfRefAsync().
     async getAsync(key1, key2, key3) {
       let value = this._map[key1];
-      if (value === undefined && !(key1 in this._map) && key2 !== undefined) {
+      if (value === undefined && key2 !== undefined) {
         value = this._map[key2];
-        if (value === undefined && !(key2 in this._map) && key3 !== undefined) {
+        if (value === undefined && key3 !== undefined) {
           value = this._map[key3];
         }
       }
@@ -138,11 +138,18 @@ var Dict = (function DictClosure() {
     },
 
     set: function Dict_set(key, value) {
+      if (
+        (typeof PDFJSDev === "undefined" ||
+          PDFJSDev.test("!PRODUCTION || TESTING")) &&
+        value === undefined
+      ) {
+        unreachable('Dict.set: The "value" cannot be undefined.');
+      }
       this._map[key] = value;
     },
 
     has: function Dict_has(key) {
-      return key in this._map;
+      return this._map[key] !== undefined;
     },
 
     forEach: function Dict_forEach(callback) {
diff --git a/test/unit/primitives_spec.js b/test/unit/primitives_spec.js
index bb54c2e7c..219a81129 100644
--- a/test/unit/primitives_spec.js
+++ b/test/unit/primitives_spec.js
@@ -121,11 +121,13 @@ describe("primitives", function() {
       checkInvalidKeyValues(dictWithSizeKey);
     });
 
-    it("should return correct value for stored Size key with undefined value", function() {
-      var dict = new Dict();
-      dict.set("Size");
+    it("should not accept to set a key with an undefined value", function() {
+      const dict = new Dict();
+      expect(function() {
+        dict.set("Size");
+      }).toThrow(new Error('Dict.set: The "value" cannot be undefined.'));
 
-      expect(dict.has("Size")).toBeTruthy();
+      expect(dict.has("Size")).toBeFalsy();
 
       checkInvalidKeyValues(dict);
     });