> ========================================================================
> http://mondrian.corp.google.com/file/8654410///depot/googleclient/gears/opensource/gears/geolocation/network_location_request.cc?a=1
> File 
> //depot/googleclient/gears/opensource/gears/geolocation/network_location_request.cc
>  (snapshot 1)
> ------------------------------------
> Line 84: return NULL;
> assert?
Done

> ------------------------------------
> Line 87: delete request;
> Use a scoped_ptr for request?
Done

> ------------------------------------
> Line 297: is_processing_response_mutex_.Lock();
> It's not clear to me why we need to hold this mutex before setting the
> is_shutdown_ flag. When we check this flag we don't lock any mutex.
We don't need to lock the mutex. I'd put the two new lines next to the
all to Abort because they all act to shut down the thread. I've moved
them outside the mutex to make things clear.

New snapshot uploaded.

Steve

Reply via email to