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()|. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/732572 Title: New Mail Notification Sound does not play in Natty To manage notifications about this bug go to: https://bugs.launchpad.net/thunderbird/+bug/732572/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs