Comment 41 for bug 732572

Comment on attachment 570562
Bug 635918 Part 1 - Make nsISound::Play use libcanberra on Linux rather than esound (v2)

Review of attachment 570562:
-----------------------------------------------------------------

Thanks, looks good. I do think an nsAutoPtr should be used for the callback data, comments below.

::: widget/src/gtk2/nsSound.cpp
@@ +152,5 @@
> + if (!data) {
> + return;
> + }
> + PR_Close(data->mFD);
> + PR_Delete(data->mPath.get());

These two lines can also be removed, since the destructor will take care of this when delete is called.

@@ +269,2 @@
> }
>

Allocate the CanberraPlayCBData here, like so:
  nsAutoPtr<CanberraPlayCBData> cbdata(new CanberraPlayCBData(fd, path));

@@ +273,5 @@
> + while (length > 0) {
> + PRInt32 amount = PR_Write(fd, data, length);
> + if (amount < 0) {
> + PR_Close(fd);
> + PR_Delete(path.get());

Move the close and delete into a destructor for CanberraPlayCBData, then remove the code from here and the two error paths below.

@@ +300,3 @@
>
> + ca_proplist_sets(p, "media.filename", path.get());
> + ca_context_play_full(ctx, 0, p, ca_finish_cb, cbdata);

Pass cbdata here using |cbdata.forget()|.