Closed Bug 835612 Opened 11 years ago Closed 11 years ago

Fix nsVolumeService to use strings better

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: dhylands, Assigned: baku)

References

Details

Attachments

(1 file)

+++ This bug was initially created as a clone of Bug #835408 +++

In particular bug 835408 comment 7:

>   nsString stateStr(NS_ConvertUTF8toUTF16(vol->StateStr()));
>   obs->NotifyObservers(vol, NS_VOLUME_STATE_CHANGED, stateStr.get());
On an unrelated note, I believe NS_ConvertUTF8toUTF16 derives from nsAutoString, so if the conversion results in a short enough string it gets stored in the auto buffer and then promptly copied to stateStr. There are two nicer ways to write this:

NS_ConvertUTF8toUTF16 stateStr(vol->StateStr());
obs->NotifyObservers(vol, NS_VOLUME_STATE_CHANGED, stateStr.get());

or

obs->NotifyObservers(vol, NS_VOLUME_STATE_CHANGED,
                     NS_ConvertUTF8toUTF16(vol->StateStr()).get());
Attached patch patchSplinter Review
Attachment #707580 - Flags: review?(justin.lebar+bug)
Assignee: nobody → amarchesini
Comment on attachment 707580 [details] [diff] [review]
patch

Sure.

You could even do

  NotifyObservers(..., NS_ConvertUTF8toUTF16(volume->StateStr()).get());

if you like.  Either way is fine with me.
Attachment #707580 - Flags: review?(justin.lebar+bug) → review+
Should this patch land to b2g18?
Keywords: checkin-needed
(In reply to Andrea Marchesini (:baku) from comment #3)
> Should this patch land to b2g18?

Does this patch solve some immediate problem?
Not really :) It's just a small optimization.
https://hg.mozilla.org/integration/mozilla-inbound/rev/7b03b419446f

Please post changeset links and remove checkin-needed when landing your patches on inbound so others don't waste their time attempting to do the same.
Keywords: checkin-needed
I forgot to remove checkin-needed. Sorry for this.
https://hg.mozilla.org/mozilla-central/rev/7b03b419446f
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: