Avoid back-and-forth UTF-16 to UTF-8 to UTF-16 conversions in xpcom/base/MacHelpers.mm

RESOLVED FIXED in Firefox 61

Status

()

enhancement
RESOLVED FIXED
Last year
Last year

People

(Reporter: hsivonen, Assigned: hsivonen)

Tracking

unspecified
mozilla61
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox61 fixed)

Details

Attachments

(1 attachment)

xpcom/base/MacHelpers.mm appears to ask an NSString (UTF-16) to provide a UTF-8 conversion and then immediately converts the result back to UTF-16.

We should instead ask the NSString for its (UTF-16) length, resize the nsAString to that length and ask the NSString to write its contents to aCountryCode.BeginWriting().

(Not that converting a couple of characters matters much perf-wise, but I'm changing the signature of AppendUTF8toUTF16 so *some* change to these lines is going to be needed anyway.)
Blocks: 1402247
Trying to write a patch without a Mac...
Assignee: nobody → hsivonen
Status: NEW → ASSIGNED
Looks like neither bug 346345 nor bug 334670 ever got fixed. :-(
Attachment #8962265 - Flags: review?(mstange)
Comment on attachment 8962265 [details]
Bug 1448772 - Avoid back-and-forth UTF-16 to UTF-8 to UTF-16 conversions in xpcom/base/MacHelpers.mm.

https://reviewboard.mozilla.org/r/231126/#review238420

Looks good, thank you!

::: toolkit/crashreporter/mac_utils.mm:22
(Diff revision 14)
> -  [name getCharacters:nameBuffer];
> -  [reason getCharacters:reasonBuffer];
> +  (void) mozilla::CopyCocoaStringToXPCOMString(name, nameStr);
> +  (void) mozilla::CopyCocoaStringToXPCOMString(reason, reasonStr);

You could also use mozilla::unused << here.
Attachment #8962265 - Flags: review?(mstange) → review+
Pushed by hsivonen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/24f434072ecd
Avoid back-and-forth UTF-16 to UTF-8 to UTF-16 conversions in xpcom/base/MacHelpers.mm. r=mstange
https://hg.mozilla.org/mozilla-central/rev/24f434072ecd
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.