> ========================================================================
> http://mondrian.corp.google.com/file/8305270///depot/googleclient/gears/opensource/gears/geolocation/gps_location_provider_wince.cc?a=1
> File
> //depot/googleclient/gears/opensource/gears/geolocation/gps_location_provider_wince.cc
> (snapshot 1)
> ------------------------------------
> Line 111: request_address_ |= request_address;
> Lazily create the reverse geocoder here? Or would this be a silly
> optimization?
Good point - done.
> ------------------------------------
> Line 221: // required. Otherwise, the new listener should continue to wait.
> Technically, we could update just the new listener if it's not interested in
> an
> address. However, this would complicate things. Fine as it is :)
Agreed.
> ------------------------------------
> Line 297: }
> is the above brace misaligned?
Fixed.
> ------------------------------------
> Line 353: // back, unless there is a reverse geocode in progress.
> But what good does it do to update position_ if we don't update the listeners?
> Shouldn't you change position_ only when you also decide to update the
> listeners
> (i.e. inside the if statement below) ?
The idea was to keep position_ up to date, but there was a bug - now fixed.
> ------------------------------------
> Line 393: LOG("Failed to make reverse geocode request.");
> If this fails, doesn't is_reverse_geocode_in_progress_ remain forever true?
Yes, fixed.
> http://mondrian.corp.google.com/file/8305270///depot/googleclient/gears/opensource/gears/geolocation/location_provider_pool.cc?a=1
> File
> //depot/googleclient/gears/opensource/gears/geolocation/location_provider_pool.cc
> (snapshot 1)
> ------------------------------------
> Line 154: // Therefore we must key network and GPD providers on server URL,
> host and
> GPS
Fixed.
> http://mondrian.corp.google.com/file/8305270///depot/googleclient/gears/opensource/gears/geolocation/reverse_geocoder.cc?a=1
> File
> //depot/googleclient/gears/opensource/gears/geolocation/reverse_geocoder.cc
> (snapshot 1)
> ------------------------------------
> Line 44: // which we'll call MakeRequest().
> Can you use ASSERT_SINGLE_THREAD() here?
Done.
New snapshot uploaded.
Steve
