781133 - Fix locale specific numbers in hung plugin CPU usage reporting

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Comment 1

[Georg Fritzsche [:gfritzsche]]

Attachment: Fix locale specific CPU usage numbers

Avoid writing locale specific numbers for the plugin CPU usage report.
Stop fixing locale specific numbers in the associated test.

Comment 2

[Benjamin Smedberg]

Comment on attachment 650117 [details] [diff] [review]
Fix locale specific CPU usage numbers

I'm surprised that nsPrintfCString was locale-dependent (and that this function would be different). But if this works, let's do it!

Comment 3

[Georg Fritzsche [:gfritzsche]]

(In reply to Benjamin Smedberg  [:bsmedberg] [away 27-July until 7-Aug] from comment #2)
> I'm surprised that nsPrintfCString was locale-dependent (and that this
> function would be different). But if this works, let's do it!

Apparently nsPrintfCString behaves like printf() locale-wise, while PR_cnvtf() behaves like specified for ECMA script.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ac8d6c79a074

Comment 5

[Ryan VanderMeulen [:RyanVM]]

This got caught up in the fray with all the other backouts today.
https://hg.mozilla.org/integration/mozilla-inbound/rev/40e59c86c2b3

Re-landed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ecb6aaee9601

Comment 6

[Phil Ringnalda (:philor)]

Backed out yet again in https://hg.mozilla.org/integration/mozilla-inbound/rev/beb7a4c0d06f

https://tbpl.mozilla.org/php/getParsedLog.php?id=14247588&tree=Mozilla-Inbound
4453 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/dom/plugins/test/test_busy_hang.xul | got extra field for plugin cpu usage
4454 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/dom/plugins/test/test_busy_hang.xul | plugin cpu usage is >0%

Comment 7

[Georg Fritzsche [:gfritzsche]]

Attachment: Fix locale specific CPU usage numbers

Use nsCString::AppendFloat() to avoid local buffer management, round manually.

Try: https://tbpl.mozilla.org/?tree=Try&rev=7b06c9be10a3

Comment 8

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 650475 [details] [diff] [review]
Fix locale specific CPU usage numbers

Try build results are fine. Benjamin, can you do a follow review?

Comment 9

[Benjamin Smedberg]

Comment on attachment 650475 [details] [diff] [review]
Fix locale specific CPU usage numbers

Why std::ceilf instead of ceilf? I'm not sure it matters either way, though.

Comment 10

[Benjamin Smedberg]

From IRC discussion, this should either be ::ceilf or std::ceil, since std::ceilf is apparently a Windows-only weirdism.

Comment 11

[Georg Fritzsche [:gfritzsche]]

Attachment: Fix locale specific CPU usage numbers

Fixed std::ceilf() oversight.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Green on Try (the Android R2 failure is unrelated).
https://tbpl.mozilla.org/?tree=Try&rev=cfba471376c9

https://hg.mozilla.org/integration/mozilla-inbound/rev/6cfd16fe1cb3

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6cfd16fe1cb3