Closed Bug 1336350 Opened 7 years ago Closed 7 years ago

Some PDF crashes Firefox when using internal PDF Viewer

Categories

(Core :: Graphics, defect)

48 Branch
x86
Windows
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla54
Tracking Status
firefox51 --- wontfix
firefox52 --- verified
firefox-esr52 --- verified
firefox53 --- verified
firefox54 --- verified

People

(Reporter: 5.5, Assigned: milan)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0
Build ID: 20170125094131

Steps to reproduce:

[1] Set the [PDF Document] option to use internal PDF Viewer (default)

[2] Access the following URL.
http://moji.gr.jp/firefox/sample.pdf


Actual results:

Firefox crashed.
Component: Layout → PDF Viewer
OS: Unspecified → Windows 10
Product: Core → Firefox
Hardware: Unspecified → x86_64
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
Resolution: --- → DUPLICATE
i'm reopening this bug since the particular issue with pdfs seems to be regressing more recently than bug 1198921, which goes back further...

the regression range for these STR is https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=24c5fbde4488e06ef79905e1c520027cddcd1189&tochange=b942c98f56c4c2926b8b81b98425072a091bbf7b - so likely the switch to msvc 2015 to compile firefox

64bit builds of firefox don't seem to crash in this scenario.

(the following pdf exhibits the same pattern: www.mysoft.hu/docs/Dell/E-PORTPLUS.pdf)
Status: RESOLVED → REOPENED
Crash Signature: [@ _moz_cairo_scaled_font_get_font_options ]
Ever confirmed: true
Keywords: crash, regression
OS: Windows 10 → Windows
Hardware: x86_64 → x86
Resolution: DUPLICATE → ---
Version: 51 Branch → 48 Branch
hi, would this bug be better placed in a gfx component?
Flags: needinfo?(msreckovic)
See Also: → 1198921
Component: PDF Viewer → Graphics
Product: Firefox → Core
(In reply to [:philipp] from comment #3)
> hi, would this bug be better placed in a gfx component?

Yes, pdf.js is mostly a regular web page, it shouldn't be able to crash firefox.
Status: REOPENED → NEW
Flags: needinfo?(msreckovic)
PDFium should make all this obsolete, but in the meantime:

_cairo_dwrite_scaled_font_init_glyph_metrics returns status 100 (CAIRO_INT_STATUS_UNSUPPORTED) and all goes bad after that.

This happens since HRESULT hr = font_face->dwriteface->GetDesignGlyphMetrics(&charIndex, 1, &metrics) returns ERROR_ARITHMETIC_OVERFLOW - (Arithmetic result exceeds 32 bits.)
(In reply to Milan Sreckovic [:milan] from comment #5)
> PDFium should make all this obsolete, but in the meantime:
> 

Like I mentioned above, the majority of PDF.js runs as a regular content page, so it's possible other sites could trigger this issue too.
Comment on attachment 8841113 [details]
Bug 1336350: CAIRO_INT_STATUS_UNSUPPORTED is a special case of an error.

https://reviewboard.mozilla.org/r/115452/#review117500
Attachment #8841113 - Flags: review?(jmuizelaar) → review+
Assignee: nobody → milan
Pushed by msreckovic@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ce4d5be6f3fb
CAIRO_INT_STATUS_UNSUPPORTED is a special case of an error.  r=jrmuizel
Comment on attachment 8841113 [details]
Bug 1336350: CAIRO_INT_STATUS_UNSUPPORTED is a special case of an error.

Approval Request Comment
[Feature/Bug causing the regression]: Most likely, the switch to the new compiler (bug 1186060) as what triggers the "Arithmetic result exceeds 32 bits" error, which then gets us into this code path.
[User impact if declined]: ~1000 weekly crashes on beta
[Is this code covered by automated tests?]: No, but we have a reproducible test case.
[Has the fix been verified in Nightly?]: The approval should wait until it has been.
[Needs manual test from QE? If yes, steps to reproduce]: Probably a good idea, given the timing of the uplifts - the crashing test case is attached to this bug.
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: Low risk, other code does similar things.
[Why is the change risky/not risky?]:
[String changes made/needed]:
Attachment #8841113 - Flags: approval-mozilla-beta?
Attachment #8841113 - Flags: approval-mozilla-aurora?
i can confirm that the crash is no longer reproducible with the tinderbox build from https://archive.mozilla.org/pub/firefox/tinderbox-builds/autoland-win32/1488304835/ containing the patch.
https://hg.mozilla.org/mozilla-central/rev/ce4d5be6f3fb
Status: NEW → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment on attachment 8841113 [details]
Bug 1336350: CAIRO_INT_STATUS_UNSUPPORTED is a special case of an error.

last minute crash fix for 52 rc2
Attachment #8841113 - Flags: approval-mozilla-release+
Attachment #8841113 - Flags: approval-mozilla-beta?
Attachment #8841113 - Flags: approval-mozilla-beta+
Attachment #8841113 - Flags: approval-mozilla-aurora?
Attachment #8841113 - Flags: approval-mozilla-aurora+
Flagging this for manual testing, str in Comment 0, testcases in Comment 0 and Comment 2.
Flags: qe-verify+
Verified fixed FX 52.0-build2 on Win 7 x64, Win 10 x64, OS X 10.11, Ubuntu 16.04 x86.
Verified fixed FX 52 ESR, 53.0a2 (2017-03-06), 54.0a1 (2017-03-06) Win 10
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.