Geolocation: add UMA histograms for events and response codes.

content/browser/geolocation/network_location_request.cc

Index: content/browser/geolocation/network_location_request.cc
diff --git a/content/browser/geolocation/network_location_request.cc b/content/browser/geolocation/network_location_request.cc
index 94074be561d9c7b4acde2e456099f9ede99d04d0..98e1cf0d6fd153c12e62647f23c987cd0ae14280 100644
--- a/content/browser/geolocation/network_location_request.cc
+++ b/content/browser/geolocation/network_location_request.cc
@@ -31,6 +31,35 @@ const char kLatitudeString[] = "lat";
 const char kLongitudeString[] = "lng";
 const char kAccuracyString[] = "accuracy";
 
+// NOTE: Never remove or reorder any of these as that would confuse
+// interpretation of previously logged data. Make sure to update the enum list
+// in tools/metrics/histograms/histograms.xml to keep it in sync.

[timvolodine, 2013/09/30 16:38:49]

side-note: to slightly help this problem we could assign values to them like =0,
=1.. That way deleting entries would be ok. Also, it looks like a certain
numbering is given in the .xml file associated with this patch.

[Michael van Ouwerkerk, 2013/09/30 17:02:22]

Done.

+enum NetworkLocationRequestEvent {
+  NETWORK_LOCATION_REQUEST_EVENT_REQUEST_START,
+  NETWORK_LOCATION_REQUEST_EVENT_REQUEST_CANCEL,
+  NETWORK_LOCATION_REQUEST_EVENT_RESPONSE_SUCCESS,
+  NETWORK_LOCATION_REQUEST_EVENT_RESPONSE_NOT_OK,
+  NETWORK_LOCATION_REQUEST_EVENT_RESPONSE_EMPTY,
+  NETWORK_LOCATION_REQUEST_EVENT_RESPONSE_MALFORMED,
+  NETWORK_LOCATION_REQUEST_EVENT_RESPONSE_INVALID_FIX,
+
+  // NOTE: Add entries only immediately above this line.
+  NETWORK_LOCATION_REQUEST_EVENT_COUNT
+};
+
+void RecordUmaEvent(NetworkLocationRequestEvent event) {
+  UMA_HISTOGRAM_ENUMERATION("Geolocation.NetworkLocationRequest.Event",
+                            event, NETWORK_LOCATION_REQUEST_EVENT_COUNT);

[timvolodine, 2013/09/30 16:38:49]

indent

[Michael van Ouwerkerk, 2013/09/30 17:02:22]

Done.

+}
+
+const int kCodeRangeStart = 100; // Range start is inclusive.
+const int kCodeRangeEnd = 600; // Range end is exclusive.
+const int kCodeBuckets = 502; // 500 codes, underflow, and overflow.
+void RecordUmaResponseCode(int code) {
+  UMA_HISTOGRAM_CUSTOM_COUNTS("Geolocation.NetworkLocationRequest.ResponseCode",
+      code, kCodeRangeStart, kCodeRangeEnd, kCodeBuckets);
+}
+
 // Local functions
 // Creates the request url to send to the server.
 GURL FormRequestURL(const GURL& url);
@@ -40,7 +69,8 @@ void FormUploadData(const WifiData& wifi_data,
                     const string16& access_token,
                     std::string* upload_data);
 
-// Parsers the server response.
+// Attempts to extract a position from the response. Detects and indicates
+// various failure cases.
 void GetLocationFromResponse(bool http_post_result,
                              int status_code,
                              const std::string& response_body,
@@ -78,8 +108,10 @@ NetworkLocationRequest::~NetworkLocationRequest() {
 bool NetworkLocationRequest::MakeRequest(const string16& access_token,
                                          const WifiData& wifi_data,
                                          const base::Time& timestamp) {
+  RecordUmaEvent(NETWORK_LOCATION_REQUEST_EVENT_REQUEST_START);
   if (url_fetcher_ != NULL) {
     DVLOG(1) << "NetworkLocationRequest : Cancelling pending request";
+    RecordUmaEvent(NETWORK_LOCATION_REQUEST_EVENT_REQUEST_CANCEL);
     url_fetcher_.reset();
   }
   wifi_data_ = wifi_data;
@@ -108,6 +140,7 @@ void NetworkLocationRequest::OnURLFetchComplete(
 
   net::URLRequestStatus status = source->GetStatus();
   int response_code = source->GetResponseCode();
+  RecordUmaResponseCode(response_code);
 
   Geoposition position;
   string16 access_token;
@@ -259,12 +292,14 @@ void GetLocationFromResponse(bool http_post_result,
   // we're offline, or there was no response.
   if (!http_post_result) {
     FormatPositionError(server_url, "No response received", position);
+    RecordUmaEvent(NETWORK_LOCATION_REQUEST_EVENT_RESPONSE_EMPTY);
     return;
   }
   if (status_code != 200) {  // HTTP OK.
     std::string message = "Returned error code ";
     message += base::IntToString(status_code);
     FormatPositionError(server_url, message, position);
+    RecordUmaEvent(NETWORK_LOCATION_REQUEST_EVENT_RESPONSE_NOT_OK);
     return;
   }
   // We use the timestamp from the wifi data that was used to generate
@@ -272,6 +307,7 @@ void GetLocationFromResponse(bool http_post_result,
   if (!ParseServerResponse(response_body, timestamp, position, access_token)) {
     // We failed to parse the repsonse.
     FormatPositionError(server_url, "Response was malformed", position);
+    RecordUmaEvent(NETWORK_LOCATION_REQUEST_EVENT_RESPONSE_MALFORMED);
     return;
   }
   // The response was successfully parsed, but it may not be a valid
@@ -279,8 +315,11 @@ void GetLocationFromResponse(bool http_post_result,
   if (!position->Validate()) {
     FormatPositionError(server_url,
                         "Did not provide a good position fix", position);
+    RecordUmaEvent(NETWORK_LOCATION_REQUEST_EVENT_RESPONSE_INVALID_FIX);
     return;
   }
+  RecordUmaEvent(NETWORK_LOCATION_REQUEST_EVENT_RESPONSE_SUCCESS);
+  return;

[bulach, 2013/09/30 16:27:21]

nit: remove return

[Michael van Ouwerkerk, 2013/09/30 17:02:22]

Oops, experiment leftover. Done.

 }
 
 // Numeric values without a decimal point have type integer and IsDouble() will