1320651 - Do not crash in ErrorLoadingBuiltinSheet for every error

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Wei-Cheng Pan [:wcpan] [:wcp] [:legnaleurc] (left Mozilla)]

Maybe ErrorLoadingBuiltinSheet should not crash for non-critical error.

For example, in bug 1046166, content process may have no permission to read userContent.css, but instead of crashing the process, sending the error message to console service should be enough.

Maybe we can only crash for eAgentSheetFeatures. As for other parsing mode, just send the error message to nsIConsoleService.

Comment 1

[Cameron McCormack (:heycam)]

That sounds fine.  Failing to load userContent.css and userChrome.css is not critical, though the UA sheets failing to load is.  I probably prefer a separate argument that explicitly indicates whether this is a critical style sheet load, rather than using eAgentSheetFeatures, though.

Comment 2

[Wei-Cheng Pan [:wcpan] [:wcp] [:legnaleurc] (left Mozilla)]

Attachment: 0001-Bug-1320651-Only-crash-for-real-builtin-sheet-error..patch

try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3145ec7cdb4265f24b5f5c1d18dd260752b0d6d3

Add a flag to LoadSheet() to determine the error reporting action.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8817079 [details] [diff] [review]
0001-Bug-1320651-Only-crash-for-real-builtin-sheet-error..patch

r=me, though using an enum (e.g. a FailureAction enum with "Crash" and "LogToConsole" as values) instead of a boolean might be nice.

Comment 4

[Wei-Cheng Pan [:wcpan] [:wcp] [:legnaleurc] (left Mozilla)]

Attachment: 0001-Bug-1320651-Only-crash-for-real-builtin-sheet-error.-v2.patch

Change the diff context to 8 lines.

Comment 5

[Wei-Cheng Pan [:wcpan] [:wcp] [:legnaleurc] (left Mozilla)]

Attachment: 0001-Bug-1320651-Only-crash-for-real-builtin-sheet-error.-v3.patch

Update for comment 3.

Try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=48d906aa48cf1cb13f93d97bd59ef7bd026d222f

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1e325da15c78
Only crash for real builtin sheet error. r=bz

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1e325da15c78

Comment 8

[Wei-Cheng Pan [:wcpan] [:wcp] [:legnaleurc] (left Mozilla)]

Comment on attachment 8817091 [details] [diff] [review]
0001-Bug-1320651-Only-crash-for-real-builtin-sheet-error.-v3.patch

Approval Request Comment
[Feature/Bug causing the regression]: bug 1046166
[User impact if declined]: Their tabs might crash immediately because sandbox refuses to load the sheet.
[Is this code covered by automated tests?]: Profiles without userChrome.css/userContent.css has been covered. Manually tested with userChrome.css/userContent.css.
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: bug 1046166
[Is the change risky?]: no
[Why is the change risky/not risky?]: It prevents crashes from some cases, which will improve user experience.
[String changes made/needed]:

Comment 9

[Julien Cristau [:jcristau]]

Comment on attachment 8817091 [details] [diff] [review]
0001-Bug-1320651-Only-crash-for-real-builtin-sheet-error.-v3.patch

don't crash on failure to load user stylesheet, for aurora52

Comment 10

[Carsten Book [:Tomcat]]

needs rebasing for aurora

Comment 11

[Wei-Cheng Pan [:wcpan] [:wcp] [:legnaleurc] (left Mozilla)]

Attachment: 0002-Bug-1320651-Only-crash-for-real-builtin-sheet-error..patch

Try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cfcaa3dd201359300e6067b25a3732f973985387

Note:
Please land this after bug 1046166.

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f6dc38c29beb