Split out of reverted change http://src.chromium.org/viewvc/chrome?view=rev&revision=38207

Commit fixes to code under test, whilst tests themselves are still pending.

BUG=http://crbug.com/11246
TEST=see http://codereview.chromium.org/578006

Review URL: http://codereview.chromium.org/571014

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@38213 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 23 insertions, 13 deletions

diff --git a/chrome/browser/geolocation/location_provider.h b/chrome/browser/geolocation/location_provider.h
index 762ccb1..a9960805 100644
--- a/chrome/browser/geolocation/location_provider.h
+++ b/chrome/browser/geolocation/location_provider.h
@@ -49,14 +49,14 @@ class LocationProviderBase
     // Used to inform listener that a new position fix is available or that a
     // fatal error has occurred. Providers should call this for new listeners
     // as soon as a position is available.
-    virtual bool LocationUpdateAvailable(LocationProviderBase* provider) = 0;
+    virtual void LocationUpdateAvailable(LocationProviderBase* provider) = 0;
     // Used to inform listener that movement has been detected. If obtaining the
     // position succeeds, this will be followed by a call to
     // LocationUpdateAvailable. Some providers may not be able to detect
     // movement before a new fix is obtained, so will never call this method.
     // Note that this is not called in response to registration of a new
     // listener.
-    virtual bool MovementDetected(LocationProviderBase* provider) = 0;
+    virtual void MovementDetected(LocationProviderBase* provider) = 0;
 
    protected:
     virtual ~ListenerInterface() {}
diff --git a/chrome/browser/geolocation/network_location_provider.cc b/chrome/browser/geolocation/network_location_provider.cc
index 948207d..0fb5fe1 100644
--- a/chrome/browser/geolocation/network_location_provider.cc
+++ b/chrome/browser/geolocation/network_location_provider.cc
@@ -80,7 +80,7 @@ class NetworkLocationProvider::PositionCache {
     const string16 separator(ASCIIToUTF16("|"));
     for (WifiData::AccessPointDataSet::const_iterator iter =
          wifi_data.access_point_data.begin();
-         iter != wifi_data.access_point_data.begin();
+         iter != wifi_data.access_point_data.end();
          iter++) {
       *key += separator;
       *key += iter->mac_address;
diff --git a/chrome/browser/geolocation/network_location_request.cc b/chrome/browser/geolocation/network_location_request.cc
index 7fff715..65e3f37 100644
--- a/chrome/browser/geolocation/network_location_request.cc
+++ b/chrome/browser/geolocation/network_location_request.cc
@@ -66,7 +66,6 @@ NetworkLocationRequest::NetworkLocationRequest(URLRequestContextGetter* context,
                                                ListenerInterface* listener)
     : url_context_(context), timestamp_(kint64min), listener_(listener),
       url_(url), host_name_(host_name) {
-//  DCHECK(url_context_);
   DCHECK(listener);
 }
 
@@ -89,7 +88,7 @@ bool NetworkLocationRequest::MakeRequest(const string16& access_token,
   timestamp_ = timestamp;
 
   url_fetcher_.reset(URLFetcher::Create(
-      host_name_.size(),  // Used for testing
+      wifi_data.access_point_data.size(),  // Used for testing
       url_, URLFetcher::POST, this));
   url_fetcher_->set_upload_data(kMimeApplicationJson, post_body);
   url_fetcher_->set_request_context(url_context_);
@@ -312,20 +311,31 @@ bool ParseServerResponse(const std::string& response_body,
   response_object->GetStringAsUTF16(kAccessTokenString, access_token);
 
   // Get the location
-  DictionaryValue* location_object;
-  if (!response_object->GetDictionary(kLocationString, &location_object)) {
-    Value* value = NULL;
-    // If the network provider was unable to provide a position fix, it should
-    // return a 200 with location == null. Otherwise it's an error.
-    return response_object->Get(kLocationString, &value)
-           && value && value->IsType(Value::TYPE_NULL);
+  Value* location_value = NULL;
+  if (!response_object->Get(kLocationString, &location_value)) {
+    LOG(INFO) << "ParseServerResponse() : Missing location attribute.\n";
+    return false;
+  }
+  DCHECK(location_value);
+
+  if (!location_value->IsType(Value::TYPE_DICTIONARY)) {
+    if (!location_value->IsType(Value::TYPE_NULL)) {
+      LOG(INFO) << "ParseServerResponse() : Unexpected location type"
+                << location_value->GetType() << ".\n";
+      // If the network provider was unable to provide a position fix, it should
+      // return a HTTP 200, with "location" : null. Otherwise it's an error.
+      return false;
+    }
+    return true;  // Successfully parsed response containing no fix.
   }
-  DCHECK(location_object);
+  DictionaryValue* location_object =
+      static_cast<DictionaryValue*>(location_value);
 
   // latitude and longitude fields are always required.
   double latitude, longitude;
   if (!GetAsDouble(*location_object, kLatitudeString, &latitude) ||
       !GetAsDouble(*location_object, kLongitudeString, &longitude)) {
+    LOG(INFO) << "ParseServerResponse() : location lacks lat and/or long.\n";
     return false;
   }
   // All error paths covered: now start actually modifying postion.