1381410 - Add missing monitor enter in nsStringBundle::Format*()

Status: RESOLVED FIXED

(Core :: Internationalization, enhancement)

Description

[Nicholas Nethercote [inactive]]

nsStringBundle::Get*() enter a monitor, but nsStringBundle::Format*() do not.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: (part 1) - Rename two protected methods in nsStringBundle

nsStringBundle has two public methods GetStringFrom{ID,Name}() and two
very similar helper methods with the same name. It's easy to get confused
between the two -- indeed, it may be the reason for the data race that this bug
is about -- so this patch renames the helper functions by adding a "Helper"
suffix.

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Remove nsStringBundle::GetStringFromIDHelper()

GetStringFromIDHelper() is very similar to GetSringFromNameHelper(). This patch
removes the former, and changes GetStringFromID() to call GetStringFromName(),
similar to how FormatStringFromID() calls FormatStringFromName().

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: (part 3) - Simplify nsExtensibleStringBundle::GetStringFromID()

Similar to the previous patch, this patch changes GetStringFromID() to call
GetStringFromName(), but this time in nsExtensibleStringBundle.

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: (part 4) - Add missing monitor enter in nsStringBundle::Format*() functions

GetStringFromName() enters mReentrantMonitor(), and the other Get*() functions
go through GetStringFromName(). But none of the Format*() functions enter the
monitor.

This patch moves some repeated code from GetStringFromName() and
FormatStringFromName() into GetStringFromNameHelper(), including the monitor
entry. All the Get*() and Format*() functions now use
GetStringFromNameHelper(), which means they now all enter the monitor.

Comment 5

[Nicholas Nethercote [inactive]]

Attachment: (part 5) - Reorder nsStringBundle

This patch moves the destructor after the constructor, puts the
NS_IMPL_ISUPPORTS line in a more sensible spot, and moves the Get*() functions
before the Format*() functions in order to match the order in
nsIStringBundle.idl.

Comment 6

[Masatoshi Kimura [:emk]]

Comment on attachment 8886955 [details] [diff] [review]
(part 1) - Rename two protected methods in nsStringBundle

Nice cleanup.

Comment 7

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/048682b66e04eb7137241c9914f250f9a979a15a
Bug 1381410 (part 1) - Rename two protected methods in nsStringBundle. r=emk.

https://hg.mozilla.org/integration/mozilla-inbound/rev/c200c517cd5b0b3005f914654751c35ef66a592f
Bug 1381410 (part 2) - Remove nsStringBundle::GetStringFromIDHelper(). r=emk.

https://hg.mozilla.org/integration/mozilla-inbound/rev/c528ff7eb1174dfc33d3d30b9da4b176fbacb5ff
Bug 1381410 (part 3) - Simplify nsExtensibleStringBundle::GetStringFromID(). r=emk.

https://hg.mozilla.org/integration/mozilla-inbound/rev/3756382ef3b77ecfbd89d142c21e0ed202dfbf6b
Bug 1381410 (part 4) - Add missing monitor enter in nsStringBundle::Format*() functions. r=emk.

https://hg.mozilla.org/integration/mozilla-inbound/rev/8cd4bd9ec8ae107a4c6a178c53aae56668804fbe
Bug 1381410 (part 5) - Reorder nsStringBundle. r=emk.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/048682b66e04
https://hg.mozilla.org/mozilla-central/rev/c200c517cd5b
https://hg.mozilla.org/mozilla-central/rev/c528ff7eb117
https://hg.mozilla.org/mozilla-central/rev/3756382ef3b7
https://hg.mozilla.org/mozilla-central/rev/8cd4bd9ec8ae