995431 - Update pdf.js to version 1.0.21

Status: RESOLVED FIXED

(Firefox :: PDF Viewer, task)

Description

[Brendan Dahl [:bdahl]]

+++ This bug was initially created as a clone of Bug #990852 +++

Changes since last update:
#4535 Avoid drawing text when the font size is zero (issue 4484)
#4512 Clean up duplicate string conversion functions
#4539 Workaround for missing 'Rect' entry in annotation dictionaries (issue 4537)
#4554 Better test process killing for Windows
#4546 Prevent the Document Properties from being empty when the dialog is opened before the file has started loading
#4524 Use full screen width in presentation mode
#4545 Use new Chromium APIs for FTP and redirects
#4482 Rewrite 'Preferences' to make it async
#4516 Added CSS rules to improve scrolling on iOS devices
#4455 Fix download button waiting if document has started but not completed downloading
#4559 Refactors history and how the database is stored
#4563 Test with opera
#4565 Read color info from JPX stream + fix color problem #4540
#4286 WebGL and misc memory optimizations
#4374 Doesn't traverse cyclic references in Dict.getAll; reduces empty-Dict GC
#4578 Workaround for cases where FontName in the FontDescriptor differs from BaseFont in the Font (bug 847420)
#4569 Handle 'space' character correctly in MacRomanEncoding (bug 878026)
#4423 Treat fonts with the same font descriptor and encoding as aliases
#4574 Handle 'space' character correctly in WinAnsiEncoding (bug 850854)
#4568 Jpx optimization
#4570 Build the text layer geometry on the worker.
#4587 Fixes text-selection example
#4584 Create the WORKER_SRC_FILES and EXT_SRC_FILES lists in make 
#4585 Redo and add more documentation to gh-pages.
#4593 Fix version numbers.
#4590 Relaxes murmurhash array requirement.
#4589 Remove leading and trailing spaces from info.Producer and info.Creator when printing debug info in the console (bug 872827)
#4556 Use Opaque Canvas
#4577 Removes custom test/reporter.js from the lint process
#4550 Heuristics to recognize the unknown glyphs for toUnicode
#4588 Adds text-only example
#4597 Print WebGL status in the console
#4595 Append whitespace divs so matches still line up.
#4602 Introduces .jshintignore
#4601 Fixes documentation for PDFPage.render
#4599 Adds then method to the RenderTask
#4596 Fix seac regression.
#4598 Work-around for filesystem:-URL bug
#4604 Fixes broken link in the documentation
#4605 1.0 release.

Comment 1

[Brendan Dahl [:bdahl]]

Attachment: bug-995431-fix.patch

PdfStreamConverter.js changes were reviewed by mossop in https://github.com/mozilla/pdf.js/pull/4559

Comment 2

[Yury Delendik (:yury)]

Comment on attachment 8405611 [details] [diff] [review]
bug-995431-fix.patch

Review of attachment 8405611 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/extensions/pdfjs/content/PdfStreamConverter.jsm
@@ -306,5 @@
> -  getDatabase: function() {
> -    if (this.isInPrivateBrowsing())
> -      return '{}';
> -    return getStringPref(PREF_PREFIX + '.database', '{}');
> -  },

Reviewed by :Mossop at https://github.com/mozilla/pdf.js/pull/4559#issuecomment-39488022

@@ +502,5 @@
> +    if (sendResponse) {
> +      sendResponse(JSON.stringify(currentPrefs));
> +    } else {
> +      return JSON.stringify(currentPrefs);
> +    }

Not sure if this needs additional review -- looks trivial. See https://github.com/mozilla/pdf.js/pull/4482

Comment 3

[Yury Delendik (:yury)]

https://tbpl.mozilla.org/?tree=Try&rev=300dd207f081

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/8ea9fae20fa5

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Backed out for mochitest-bc crashes.
https://hg.mozilla.org/integration/fx-team/rev/15503d9a400a

https://tbpl.mozilla.org/php/getParsedLog.php?id=37704533&tree=Fx-Team

Note that thanks to bug 963075, browser_pdfjs_views.js was skipped on Windows on the next fx-team push, so the test will need to be re-enabled manually to verify that the crash is fixed.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

"Thankfully" it was crashing on Linux as well.

https://tbpl.mozilla.org/php/getParsedLog.php?id=37705058&tree=Fx-Team
https://tbpl.mozilla.org/php/getParsedLog.php?id=37705211&tree=Fx-Team

Comment 7

[Yury Delendik (:yury)]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #6)
> "Thankfully" it was crashing on Linux as well.
> 
> https://tbpl.mozilla.org/php/getParsedLog.php?id=37705058&tree=Fx-Team
> https://tbpl.mozilla.org/php/getParsedLog.php?id=37705211&tree=Fx-Team

Ryan, it is crashing has nothing to do with bug 963075 (memory leaks), but with crashing at js::CallJSNative . Also, please notice https://tbpl.mozilla.org/?tree=Try&rev=300dd207f081 was run with browser_pdfjs_views.js enabled (see full logs). Is this JavaScript engine crash appeared only after this patch?

Comment 8

[Ryan VanderMeulen [:RyanVM]]

(In reply to Yury Delendik (:yury) from comment #7)
> Ryan, it is crashing has nothing to do with bug 963075 (memory leaks), but
> with crashing at js::CallJSNative .

My point was that if the test had already been disabled (due to the leaks) when this got pushed, we wouldn't have seen these crashes on Windows. So the "thankfully" there was that we still would have seen them on Linux at least.

> Is this JavaScript engine crash appeared only after this patch?

That seems very likely to me. That said, surely you agree that we can't have a test perma-crashing on TBPL no matter the cause? :) I'm not sure what you would have had me do differently under the circumstances.

Comment 9

[Yury Delendik (:yury)]

Attachment: Waiting for entire PDF data during testing

Let's try add this, see it will help to bypass the JavaScript engine crash. The only intermittent behavior can be caused by incremental data loading, I think. I'm okay with changing PDF.js behavior to wait to entire data before calling "documentload".

Ryan, can you check if this patch helps with bug 963075?

https://tbpl.mozilla.org/?tree=Try&rev=5c7afd07c842

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8405900 [details] [diff] [review]
Waiting for entire PDF data during testing

LGTM :)
https://tbpl.mozilla.org/?tree=Try&rev=96e6e3f81065

Comment 11

[Yury Delendik (:yury)]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #10)
> Comment on attachment 8405900 [details] [diff] [review]
> Waiting for entire PDF data during testing
> 
> LGTM :)
> https://tbpl.mozilla.org/?tree=Try&rev=96e6e3f81065

Glad to hear it works, however we need to file a follow up bug to investigate why JS engine crashes or memory leaks during testing when entire PDF data is not downloaded. 

Ryan, shall I checkin-needed the bug? Not sure who can review this work around. Will r=me work?

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Maybe Mossop? Also, did you want to land it upstream and just update the rev here?

Comment 13

[Yury Delendik (:yury)]

Attachment: Update pdf.js to version 1.0.21

Additional changes:
#4611 Exclude B2G stubs from linting
#4612 Fixes number of glyphs in the generated font
#4606 Makes PDF files load when xrefEntry is undefined
#4614 Refactor - remove redundant function and all references
#4617 Changes 'documentload' event behavior.
#4618 Refactoring - remove duplicated code in TextLayerBuilder
#4616 Creates make.js code to build jsdoc.
#4613 Refactors buildImage to use Promise; don't draw bad images
#4581 Check that images have valid dimensions (issue 4575)

Tests changes added to this attachment as well.

https://tbpl.mozilla.org/?tree=Try&rev=03d62dabf63c

Comment 14

[Yury Delendik (:yury)]

Comment on attachment 8406406 [details] [diff] [review]
Update pdf.js to version 1.0.21

Removed blocking for browser_pdfjs_{view|main} for Mac as well.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8406406 [details] [diff] [review]
Update pdf.js to version 1.0.21

Looks good! I'll land this in a couple hours.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/076498ab4476

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/076498ab4476