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)

x86_64
macOS
defect
Not set
normal

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
Component: Printing: Output → Widget: Cocoa
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.
Also, this can resolve by MOZ_DISABLE_CONTENT_SANDBOX=1 although there is some bugs such as comment #1.
Summary: A debug build crash at nsPrintSettingsX::SetToFileName on macOS. → [e10s] A debug build crash at nsPrintSettingsX::SetToFileName on macOS.
When content sandbox is turned on, file access will be denied.  So we don't set NSProintJobSavingURL on content process.
Assignee: nobody → m_kato
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 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()?
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 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 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.
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
https://hg.mozilla.org/mozilla-central/rev/e3f4a1ad4928
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.