nsStringBundle preload can fail in content processes leaving the process without localized strings

RESOLVED FIXED in Firefox 57

Status

()

RESOLVED FIXED
a year ago
10 months ago

People

(Reporter: gkrizsanits, Assigned: gkrizsanits)

Tracking

unspecified
mozilla58
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed, firefox58 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

a year ago
This is a fall out from bug 1385249.
(Assignee)

Updated

a year ago
Blocks: 1385249
(Assignee)

Comment 1

a year ago
Created attachment 8913739 [details] [diff] [review]
Delay nsStringBundle preloading in content processes. v2

r+ from Olli from bug 1385249 comment 30.

related try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f7afe21d28333c1a791fa9a43ce338d52ac8003d&selectedJob=134041589
Assignee: nobody → gkrizsanits
Attachment #8913739 - Flags: review+
(Assignee)

Updated

a year ago
Blocks: 1401599
(Assignee)

Comment 3

a year ago
Comment on attachment 8913739 [details] [diff] [review]
Delay nsStringBundle preloading in content processes. v2

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1363482 (preloading strings)

[User impact if declined]: Preloading localized string bundles might fail, leaving the content process without localized strings forever. This can lead to a wide variety of failures from incorrectly rendering buttons to failing reporting errors and who knows what else. 

[Is this code covered by automated tests?]: Mostly. There can be still edge cases we have not considered, but this patch fixes the ones I can think of and fixed the intermittent failures I know about.

[Has the fix been verified in Nightly?]: Not yet.

[Needs manual test from QE? If yes, steps to reproduce]: I don't think manual testing is possible. Landing the patch under bug 1385249 will do some validation and bug 1401599 should be fixed by this patch as well.

[List of other uplifts needed for the feature/fix]: None.

[Is the change risky?]: It is more risky not taking it.

[Why is the change risky/not risky?]: The worst it can cause is that by delaying the preloading of these strings, it might come with a tiny startup time regression. Not taking it can result misbehaving content processes.

[String changes made/needed]: None.
Attachment #8913739 - Flags: approval-mozilla-beta?
status-firefox57: --- → affected
status-firefox58: --- → affected

Comment 4

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/53f147e1524e
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox58: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Comment on attachment 8913739 [details] [diff] [review]
Delay nsStringBundle preloading in content processes. v2

Seems like a must fix for 57, Beta57+
Attachment #8913739 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 6

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/a88f9115bb9c
status-firefox57: affected → fixed
You need to log in before you can comment on or make changes to this bug.