Fix locale specific numbers in hung plugin CPU usage reporting

RESOLVED FIXED in mozilla17

Status

()

Core
Plug-ins
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: gfritzsche, Assigned: gfritzsche)

Tracking

17 Branch
mozilla17
All
Windows 7
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Comment hidden (empty)
Created attachment 650117 [details] [diff] [review]
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.
Attachment #650117 - Flags: review?(benjamin)
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!
Attachment #650117 - Flags: review?(benjamin) → review+
Keywords: checkin-needed
(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.
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac8d6c79a074
Flags: in-testsuite+
Keywords: checkin-needed
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
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%
Created attachment 650475 [details] [diff] [review]
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
Attachment #650117 - Attachment is obsolete: true
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?
Attachment #650475 - Flags: review?(benjamin)
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.
Attachment #650475 - Flags: review?(benjamin) → review+
From IRC discussion, this should either be ::ceilf or std::ceil, since std::ceilf is apparently a Windows-only weirdism.
Created attachment 653426 [details] [diff] [review]
Fix locale specific CPU usage numbers

Fixed std::ceilf() oversight.
Attachment #650475 - Attachment is obsolete: true
Keywords: checkin-needed
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
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/6cfd16fe1cb3
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.