Remove Parser_fetchIfRef since it's obsolete
#6411
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This code was added in PR 1214, but was made obsolete by PRs 1488/1493. Prior to the latter ones, `Dict_get` retured the raw objects. However, afterwards (and currently) `Dict_get` now resolves indirect objects, which makes `Parser_fetchIfRef` redundant. *Potential risks with this patch:* This patch passes all tests locally, but there's a *small* possibility that it could break some weird PDF files. In the current code, wrapping `Dict_get` inside `Parser_fetchIfRef` will potentially mean two back-to-back call of `XRef_fetch`, if a reference points directly to another reference. I'm not sure if this can actually happen in practice, and I'd think that if that were the case we'd already have run into it elsewhere in the code-base, given that `Parser` is the only place where we try to "double" resolve references.
| @@ -416,10 +416,6 @@ var Parser = (function ParserClosure() { | |||
|
|
|||
| return imageStream; | |||
| }, | |||
| fetchIfRef: function Parser_fetchIfRef(obj) { | |||
| // not relying on the xref.fetchIfRef -- xref might not be set | |||
There was a problem hiding this comment.
Also, note how this comment doesn't actually agree entirely with the code. Reading it, I'd expect the next line to read e.g. like this instead:
return (this.xref && isRef(obj) ? this.xref.fetch(obj) : obj);
|
Note that if this PR is accepted, then #6045 becomes obsolete. |
|
/botio test |
From: Bot.io (Linux)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://107.21.233.14:8877/0cc258882004aff/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://107.22.172.223:8877/5d0392485d5a9a5/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/5d0392485d5a9a5/output.txt Total script time: 18.43 mins
|
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/0cc258882004aff/output.txt Total script time: 19.43 mins
|
timvandermeij
added a commit
that referenced
this pull request
Sep 29, 2015
Remove `Parser_fetchIfRef` since it's obsolete
|
Nice find, thank you! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This code was added in PR #1214, but was made obsolete by PRs #1488/#1493. Prior to the latter ones,
Dict_getretured the raw objects. However, afterwards (and currently)Dict_getnow resolves indirect objects, which makesParser_fetchIfRefredundant.Potential risks with this patch:
This patch passes all tests locally, but there's a small possibility that it could break some weird PDF files.
In the current code, wrapping
Dict_getinsideParser_fetchIfRefwill potentially mean two back-to-back call ofXRef_fetch, if a reference points directly to another reference. I'm not sure if this can actually happen in practice, and I'd think that if that were the case we'd already have run into it elsewhere in the code-base, given thatParseris the only place where we try to "double" resolve references.