Closed Bug 1387381 Opened 3 years ago Closed 3 years ago

Remove nsXPIDLString local variables

Categories

(Core :: General, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: njn, Assigned: njn)

References

Details

Attachments

(1 file)

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: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.