Closed
Bug 1397986
Opened 7 years ago
Closed 7 years ago
[e10s] Save to PDF doesn't work with content sandbox
Categories
(Core :: Widget: Cocoa, defect)
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: mantaroh, Assigned: m_kato)
References
Details
Attachments
(1 file)
The macOS debug build will crash when printing something.
If we remove the code of setting 'NSPrintSaveJob' from 'nsPrintSettingsX::SetToFileName'[1], we can prevent this crash.
[1] http://searchfox.org/mozilla-central/rev/44c693914255638d74bcf1ec3b0bcd448dfab2fd/widget/cocoa/nsPrintSettingsX.mm#342
Stack:
--------------------------------------------------------------------
[Parent 2781] WARNING: '!aEvent.mSucceeded', file /Volumes/External/mozilla-central/dom/ipc/TabParent.cpp, line 2284
2017-09-08 10:12:26.718 plugin-container[2782:34889] *** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '*** -[__NSPlaceholderDictionary initWithObjects:forKeys:count:]: attempt to insert nil object from objects[30]'
*** First throw call stack:
(
0 CoreFoundation 0x00007fffa20f057b __exceptionPreprocess + 171
1 libobjc.A.dylib 0x00007fffb74b91da objc_exception_throw + 48
2 CoreFoundation 0x00007fffa1ff109f -[__NSPlaceholderDictionary initWithObjects:forKeys:count:] + 351
3 CoreFoundation 0x00007fffa205b878 -[NSDictionary initWithDictionary:copyItems:] + 504
4 AppKit 0x00007fff9fe02077 -[NSPrintInfo initWithDictionary:] + 29
5 XUL 0x00000001055910ea _ZN16nsPrintSettingsX13SetToFileNameEPKDs + 458
6 XUL 0x000000010550f0bf _ZN14nsPrintOptions26DeserializeToPrintSettingsERKN7mozilla9embedding9PrintDataEP16nsIPrintSettings + 943
7 XUL 0x000000010558f4ac _ZN15nsPrintOptionsX26DeserializeToPrintSettingsERKN7mozilla9embedding9PrintDataEP16nsIPrintSettings + 28
8 XUL 0x0000000106d6740b _ZN15nsPrintingProxy15ShowPrintDialogEP18mozIDOMWindowProxyP18nsIWebBrowserPrintP16nsIPrintSettings + 1083
9 XUL 0x0000000105c5b474 _ZN13nsPrintEngine13DoCommonPrintEbP16nsIPrintSettingsP22nsIWebProgressListenerP14nsIDOMDocument + 3812
10 XUL 0x0000000105c5ef83 _ZN13nsPrintEngine5PrintEP16nsIPrintSettingsP22nsIWebProgressListener + 131
11 XUL 0x00000001058d9b07 _ZN16nsDocumentViewer5PrintEP16nsIPrintSettingsP22nsIWebProgressListener + 1399
12 XUL 0x0000000102b1663e NS_InvokeByIndex + 142
13 XUL 0x00000001035a2225 _ZN16CallMethodHelper4CallEv + 453
14 XUL 0x00000001035a4330 _Z17XPC_WN_CallMethodP9JSContextjPN2JS5ValueE + 688
15 XUL 0x0000000106fe4d03 _ZN2js12CallJSNativeEP9JSContextPFbS1_jPN2JS5ValueEERKNS2_8CallArgsE + 211
16 XUL 0x0000000106fe49e7 _ZN2js23InternalCallOrConstructEP9JSContextRKN2JS8CallArgsENS_14MaybeConstructE + 791
17 XUL 0x0000000106fdce14 _ZL9InterpretP9JSContextRN2js8RunStateE + 33732
18 XUL 0x0000000106fd47e3 _ZN2js9RunScriptEP9JSContextRNS_8RunStateE + 595
19 XUL 0x0000000106fe498b _ZN2js23InternalCallOrConstructEP9JSContextRKN2JS8CallArgsENS_14MaybeConstructE + 699
20 XUL 0x0000000106fdce14 _ZL9InterpretP9JSContextRN2js8RunStateE + 33732
--------------------------------------------------------------------
Regression is here:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd49b1d1bf8f
Reporter | ||
Updated•7 years ago
|
Component: Printing: Output → Widget: Cocoa
Assignee | ||
Comment 1•7 years ago
|
||
This has 2 bugs.
- nsPrintSettingsX::SetToFileName doesn't consider filename is empty (nullptr or ""). This method should accept it.
- nsPrintSettingsX::SetToFileName doesn't set mtoFileName. So nsIPrintSettings::getToFileName returns incorrect filename.
Assignee | ||
Comment 2•7 years ago
|
||
Also, this can resolve by MOZ_DISABLE_CONTENT_SANDBOX=1 although there is some bugs such as comment #1.
Assignee | ||
Updated•7 years ago
|
Summary: A debug build crash at nsPrintSettingsX::SetToFileName on macOS. → [e10s] A debug build crash at nsPrintSettingsX::SetToFileName on macOS.
Assignee | ||
Comment 3•7 years ago
|
||
When content sandbox is turned on, file access will be denied. So we don't set NSProintJobSavingURL on content process.
Assignee: nobody → m_kato
Comment hidden (mozreview-request) |
Assignee | ||
Comment 5•7 years ago
|
||
change to correct title.
Summary: [e10s] A debug build crash at nsPrintSettingsX::SetToFileName on macOS. → [e10s] Save to PDF doesn't work with content sandbox
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8906349 [details]
Bug 1397986 - Save to PDF doesn't work with content sandbox.
https://reviewboard.mozilla.org/r/178074/#review184060
Thanks for fixing this. Looks good to me. I just have one question below.
::: widget/cocoa/nsPrintSettingsX.mm:354
(Diff revision 1)
> - [printInfoDict setObject: NSPrintSaveJob forKey: NSPrintJobDisposition];
> + [printInfoDict setObject: NSPrintSaveJob forKey: NSPrintJobDisposition];
> - [printInfoDict setObject: jobSavingURL forKey: NSPrintJobSavingURL];
> + [printInfoDict setObject: jobSavingURL forKey: NSPrintJobSavingURL];
> - }
> + }
> + mToFileName = aToFileName;
> + } else {
> + mToFileName.Truncate();
Could you explain wny we need the call to mToFileName.Truncate()?
Assignee | ||
Comment 7•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8906349 [details]
Bug 1397986 - Save to PDF doesn't work with content sandbox.
https://reviewboard.mozilla.org/r/178074/#review184060
> Could you explain wny we need the call to mToFileName.Truncate()?
SetToFileName(null or empty) has to clear filename as design. See nsPrintSettings::SetToFileName implementation.
Ex.
```
settings->SetToFileName("file:///abc.pdf");
settting->GetToFileName(xxxx); -> xxx is "file:///abc.pdf"
settings->SetToFileName(nullptr);
settings->GetToFileName(xxx); -> xxx is empty.
```
Before landing this even if non-e10s, GetToFileName doesn't return correct filename like the example.
Comment 8•7 years ago
|
||
mozreview-review |
Comment on attachment 8906349 [details]
Bug 1397986 - Save to PDF doesn't work with content sandbox.
https://reviewboard.mozilla.org/r/178074/#review184572
r+. Please do as much manual testing as you can because we have very little automated test coverage for printing.
Attachment #8906349 -
Flags: review?(haftandilian) → review+
Comment 9•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8906349 [details]
Bug 1397986 - Save to PDF doesn't work with content sandbox.
https://reviewboard.mozilla.org/r/178074/#review184060
> SetToFileName(null or empty) has to clear filename as design. See nsPrintSettings::SetToFileName implementation.
>
> Ex.
>
> ```
> settings->SetToFileName("file:///abc.pdf");
> settting->GetToFileName(xxxx); -> xxx is "file:///abc.pdf"
> settings->SetToFileName(nullptr);
> settings->GetToFileName(xxx); -> xxx is empty.
> ```
>
> Before landing this even if non-e10s, GetToFileName doesn't return correct filename like the example.
Thanks for the explanation.
Comment 10•7 years ago
|
||
Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/e3f4a1ad4928
Save to PDF doesn't work with content sandbox. r=haik
Comment 11•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•