Adding class WebWavSoundResource, downloads and decode wav audio data from a URL. Implements the

Adding class WebWavSoundResource, downloads and decode wav audio data from a URL. Implements the 
WebResourceLoader::Delegate methods to plug into that class. 
Committed: http://code.google.com/p/nativeclient/source/detail?r=241

[gwink1, 2011-04-07 20:00:50]

Here's class WebWavSoundResource. It's a delegate to WebResourceLoader. It
aggregates the data chunks received from the downloader. Once it's got the whole
resource data, it analyses it as a possible wav file format and extra the audio
data.

[David Springer, 2011-04-07 20:10:00]

LGTM

[bbudge, 2011-04-07 21:44:20]

Several memory leak concerns.

http://codereview.chromium.org/6801054/diff/1/experimental/life2011/life_stag...
File experimental/life2011/life_stage_5/web_wav_sound_resource.cc (right):

http://codereview.chromium.org/6801054/diff/1/experimental/life2011/life_stag...
experimental/life2011/life_stage_5/web_wav_sound_resource.cc:121:
loader->LoadURL(url);
This looks like a memory leak.

http://codereview.chromium.org/6801054/diff/1/experimental/life2011/life_stag...
File experimental/life2011/life_stage_5/web_wav_sound_resource.h (right):

http://codereview.chromium.org/6801054/diff/1/experimental/life2011/life_stag...
experimental/life2011/life_stage_5/web_wav_sound_resource.h:49: char* wav_data_;
I recommend you manage this memory with a smart pointer or use std::vector. If
you don't, then you MUST block the compiler's generation of copy constructor and
assignment operator, as they'll do the wrong thing.

[gwink1, 2011-04-07 21:44:26]

On 2011/04/07 20:10:00, David Springer wrote:
> LGTM

+ bbudge@google.com 
I just realized I'm not getting emails from my chromium.org account. Not sure
whether Bill is.

[bbudge, 2011-04-07 21:46:01]

My previous comment should have been, 1 mem leak and potential access of deleted
buffers.

[gwink1, 2011-04-08 16:33:16]

http://codereview.chromium.org/6801054/diff/1/experimental/life2011/life_stag...
File experimental/life2011/life_stage_5/web_wav_sound_resource.cc (right):

http://codereview.chromium.org/6801054/diff/1/experimental/life2011/life_stag...
experimental/life2011/life_stage_5/web_wav_sound_resource.cc:121:
loader->LoadURL(url);
On 2011/04/07 21:44:21, bbudge1 wrote:
> This looks like a memory leak.

Yes and no.  The loader is deleted in the delegate method
OnWebResourceLoaderDone below. Class WebResourceLoader guarantees that it will
always call OnWebResourceLoaderDone and always last. That ties the lifetime of
the loader to the download process. Seems fragile though; I changed it to have
this class own its loader.

http://codereview.chromium.org/6801054/diff/1/experimental/life2011/life_stag...
File experimental/life2011/life_stage_5/web_wav_sound_resource.h (right):

http://codereview.chromium.org/6801054/diff/1/experimental/life2011/life_stag...
experimental/life2011/life_stage_5/web_wav_sound_resource.h:49: char* wav_data_;
On 2011/04/07 21:44:21, bbudge1 wrote:
> I recommend you manage this memory with a smart pointer or use std::vector. If
> you don't, then you MUST block the compiler's generation of copy constructor
and
> assignment operator, as they'll do the wrong thing.

Ended up doing both. Disallowing copy and assign because they don't really make
sense for this class. Switched to std::vector for the buffer, for easier
management.

[bbudge-google, 2011-04-08 17:08:29]

LGTM with a NULL check

http://codereview.chromium.org/6801054/diff/7001/experimental/life2011/life_s...
File experimental/life2011/life_stage_5/web_wav_sound_resource.cc (right):

http://codereview.chromium.org/6801054/diff/7001/experimental/life2011/life_s...
experimental/life2011/life_stage_5/web_wav_sound_resource.cc:200:
loader_->CloseAndDeleteSelf();
This needs a NULL check. Someone could instantiate and then immediately delete
an instance, causing a crash.