======================================================================== 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 89: reverse_geocoder_.reset(new ReverseGeocoder(reverse_geocode_url_, Not a big deal, but it looks like we create the reverse geocoder even when the listener isn't interested in the address. ------------------------------------ Line 111: request_address_ |= request_address; Lazily create the reverse geocoder here? Or would this be a silly optimization? ------------------------------------ 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 :) ------------------------------------ Line 297: } is the above brace misaligned? ------------------------------------ Line 338: MutexLock lock(&position_mutex_); Hurray for finer granularity locking! ------------------------------------ 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) ? ------------------------------------ Line 393: LOG("Failed to make reverse geocode request."); If this fails, doesn't is_reverse_geocode_in_progress_ remain forever true? ======================================================================== 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 ======================================================================== 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? ========================================================================
-- To respond, reply to this email or visit http://mondrian.corp.google.com/8305270
