Closed Bug 1320651 Opened 3 years ago Closed 3 years ago
Do not crash in Error
Loading Builtin Sheet for every error
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.
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.
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3145ec7cdb4265f24b5f5c1d18dd260752b0d6d3 Add a flag to LoadSheet() to determine the error reporting action.
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.
Attachment #8817079 - Flags: review+
Change the diff context to 8 lines.
Attachment #8817079 - Attachment is obsolete: true
Update for comment 3. Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=48d906aa48cf1cb13f93d97bd59ef7bd026d222f
Attachment #8817082 - Attachment is obsolete: true
3 years ago
3 years ago
Assignee: nobody → wpan
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/1e325da15c78 Only crash for real builtin sheet error. r=bz
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]:
Attachment #8817091 - Flags: approval-mozilla-aurora?
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
Attachment #8817091 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
needs rebasing for aurora
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cfcaa3dd201359300e6067b25a3732f973985387 Note: Please land this after bug 1046166.
Flags: needinfo?(wpan) → needinfo?(cbook)
You need to log in before you can comment on or make changes to this bug.