Remove nsXPIDLString local variables

RESOLVED FIXED in Firefox 57

Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

unspecified
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

Attachments

(1 attachment)

This will remove a big chunk of the remaining nsXPIDLString occurrences.
nsXPIDLStrings are marked as VOIDED upon initialization. Most of these local
nsXPIDLString variables are immediately set via getter_Copies(), which will
either assign a string value (using Adopt()) or do SetIsVoid(). These can be
trivially converted to nsString, which will get the same treatment.

The patch suitably converts the remaining nsXPIDLString local variable as well.
Attachment #8893741 - Flags: review?(erahm)
Comment on attachment 8893741 [details] [diff] [review]
Remove nsXPIDLString local variables

Review of attachment 8893741 [details] [diff] [review]:
-----------------------------------------------------------------

r=me, just one small followup.

::: extensions/spellcheck/src/mozSpellChecker.cpp
@@ +419,3 @@
>        nsCOMPtr<mozISpellI18NManager> serv(do_GetService("@mozilla.org/spellchecker/i18nmanager;1", &rv));
>        NS_ENSURE_SUCCESS(rv, rv);
> +      return serv->GetUtil(nullptr, getter_AddRefs(mConverter));

This is kind of sketchy, I can look at the code and confirm the param isn't used but we don't know that will always be the case. Can you just convert the idl to take an AString instead of wstring?

Or maybe all this is deprecated so it's not worth dealing with.
Attachment #8893741 - Flags: review?(erahm) → review+
> This is kind of sketchy, I can look at the code and confirm the param isn't
> used but we don't know that will always be the case. Can you just convert
> the idl to take an AString instead of wstring?
> 
> Or maybe all this is deprecated so it's not worth dealing with.

C++ methods like this generally check their arguments are null and fail if so. (NS_ENSURE_ARG() is the standard way.) So I believe the responsibility lies with the callee, not the caller. So I'll leave it as is.
Pushed by nnethercote@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3984a9f49bcf
Remove nsXPIDLString local variables. r=erahm.
https://hg.mozilla.org/mozilla-central/rev/3984a9f49bcf
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.