Geolocation request for Google Now

chrome/browser/ui/google_now/google_now_service.cc

Index: chrome/browser/ui/google_now/google_now_service.cc
diff --git a/chrome/browser/ui/google_now/google_now_service.cc b/chrome/browser/ui/google_now/google_now_service.cc
index 1f45ea0d5cea71c02b4c016a9bef1d6abdbef420..dda5b541df7e8788030ef69ccaf1b03e14a4c5cb 100644
--- a/chrome/browser/ui/google_now/google_now_service.cc
+++ b/chrome/browser/ui/google_now/google_now_service.cc
@@ -3,20 +3,26 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/google_now/google_now_service.h"
+#include "content/public/browser/browser_thread.h"

[rgustafson, 2012/12/06 20:10:34]

Is this used?

[vadimt, 2012/12/06 21:05:18]

Done.

+#include "content/public/browser/geolocation.h"
 #include "content/public/common/geoposition.h"
 
+using base::Bind;
 using base::TimeDelta;
 using content::Geoposition;
 using net::URLRequest;
 
 namespace {
+// TODO(vadimt): Figure out the values of the constants.
+
 // Period for polling for Google Now cards to use when the period from the
 // server is not available.
-// TODO(vadimt): Figure out the value.
 // TODO(vadimt): Figure out the consequences for LBS.
 // TODO(vadimt): Consider triggers other than the timer for refreshing the
 // position, such as waking from sleep.
 const int kDefaultPollingPeriodMs = 300000; // 5 minutes
+// Time allocated to a geolocation request.
+const int kGeolocationRequestTimeoutMs = 60000; // 1 min
 }  // namespace
 
 struct GoogleNowService::ServerResponse {
@@ -25,7 +31,8 @@ struct GoogleNowService::ServerResponse {
 };
 
 GoogleNowService::GoogleNowService(Profile* profile)
-    : profile_(profile) {
+    : profile_(profile),
+      ALLOW_THIS_IN_INITIALIZER_LIST(geolocation_request_weak_factory_(this)) {
   DCHECK(profile_);
 }
 
@@ -49,6 +56,8 @@ void GoogleNowService::StartWaitingForNextUpdate(int64 delay_ms) {
 void GoogleNowService::OnWaitingForNextUpdateEnds() {
   DCHECK(IsGoogleNowEnabled());
   DCHECK(!next_update_timer_.IsRunning());
+  DCHECK(!geolocation_request_timer_.IsRunning());
+  DCHECK(!geolocation_request_weak_factory_.HasWeakPtrs());
 
   UpdateCards();
 }
@@ -67,17 +76,37 @@ void GoogleNowService::UpdateCards() {
 }
 
 void GoogleNowService::StartObtainingGeolocation() {
-  // TODO(vadimt): Implement via making a geolocation request.
-  OnLocationObtained(Geoposition());
+  DCHECK(!geolocation_request_weak_factory_.HasWeakPtrs());
+  content::RequestLocationUpdate(Bind(
+      &GoogleNowService::OnLocationObtained,
+      geolocation_request_weak_factory_.GetWeakPtr()));
+
+  DCHECK(!geolocation_request_timer_.IsRunning());
+  geolocation_request_timer_.Start(FROM_HERE,
+      TimeDelta::FromMilliseconds(kGeolocationRequestTimeoutMs),
+      this, &GoogleNowService::OnLocationRequestTimeout);
 }
 
 void GoogleNowService::OnLocationObtained(const Geoposition& position) {
   DCHECK(IsGoogleNowEnabled());
   DCHECK(!next_update_timer_.IsRunning());
+  DCHECK(geolocation_request_timer_.IsRunning());
+  DCHECK(geolocation_request_weak_factory_.HasWeakPtrs());

[rgustafson, 2012/12/06 20:10:34]

If the location is obtained, where are the ptrs invalidated?

[vadimt, 2012/12/06 21:05:18]

Another great catch! It's done by the framework after OnLocationObtained, but
asynchronously, so in theory, this may not happen before
OnServerRequestCompleted. I've made invalidation explicit.

 
+  geolocation_request_timer_.Stop();
   StartServerRequest(position);
 }
 
+void GoogleNowService::OnLocationRequestTimeout() {
+  DCHECK(IsGoogleNowEnabled());
+  DCHECK(!next_update_timer_.IsRunning());
+  DCHECK(!geolocation_request_timer_.IsRunning());
+  DCHECK(geolocation_request_weak_factory_.HasWeakPtrs());
+
+  geolocation_request_weak_factory_.InvalidateWeakPtrs();
+  StartServerRequest(Geoposition());
+}
+
 void GoogleNowService::StartServerRequest(
     const content::Geoposition& position) {
   // TODO(vadimt): Implement via making URLRequest to the server.
@@ -88,6 +117,10 @@ void GoogleNowService::OnServerRequestCompleted(URLRequest* request,
                                                 int num_bytes) {
   DCHECK(IsGoogleNowEnabled());
   DCHECK(!next_update_timer_.IsRunning());
+  DCHECK(!geolocation_request_timer_.IsRunning());
+  // TODO(vadimt): Uncomment the check below once OnServerRequestCompleted is
+  // called asynchronously.
+  // DCHECK(!geolocation_request_weak_factory_.HasWeakPtrs());
 
   ServerResponse server_response;
   // TODO(vadimt): Check request's status.