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
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.

Attachment

General

Created:
Updated:
Size: