Add a mechanism to pause and resume geolocation requests.

content/browser/geolocation/geolocation_dispatcher_host.cc

Index: content/browser/geolocation/geolocation_dispatcher_host.cc
diff --git a/content/browser/geolocation/geolocation_dispatcher_host.cc b/content/browser/geolocation/geolocation_dispatcher_host.cc
index 96071c748f5e72d5e00dce006be77a7ee903e730..358bdddee6ed6d37e6b2dc6bc18a4527729ab019 100644
--- a/content/browser/geolocation/geolocation_dispatcher_host.cc
+++ b/content/browser/geolocation/geolocation_dispatcher_host.cc
@@ -69,7 +69,9 @@ class GeolocationDispatcherHostImpl : public GeolocationDispatcherHost {
       bool enable_high_accuracy);
   void OnStopUpdating(int render_view_id);
 
-  // Updates the |geolocation_provider_| with the currently required update
+  void OnPauseOrResume(int render_view_id, bool should_pause);
+
+  // Updates the |location_arbitrator_| with the currently required update

[benm (inactive), 2013/11/07 21:07:50]

The geolocation_provider/arbitrator comment changes here are artifacts of
merging M30 -> trunk. Will revert them.

   // options, based on |renderer_high_accuracy_|.
   void RefreshHighAccuracy();
 
@@ -78,15 +80,17 @@ class GeolocationDispatcherHostImpl : public GeolocationDispatcherHost {
   int render_process_id_;
   scoped_refptr<GeolocationPermissionContext> geolocation_permission_context_;
 
+  struct RendererGeolocationOptions {
+    bool high_accuracy;
+    bool is_paused;
+  };
   // Iterated when sending location updates to renderer processes. The fan out
   // to individual bridge IDs happens renderer side, in order to minimize
   // context switches.
   // Only used on the IO thread.

[joth, 2013/11/07 21:52:00]

(update comment)

-  std::set<int> geolocation_renderer_ids_;
-  // Maps renderer_id to whether high accuracy is requested for this particular
-  // bridge.
-  std::map<int, bool> renderer_high_accuracy_;
-  // Only set whilst we are registered with the geolocation provider.
+  std::map<int, RendererGeolocationOptions> geolocation_renderers_;
+
+  // Only set whilst we are registered with the arbitrator.
   GeolocationProviderImpl* geolocation_provider_;
 
   GeolocationProviderImpl::LocationUpdateCallback callback_;
@@ -124,6 +128,7 @@ bool GeolocationDispatcherHostImpl::OnMessageReceived(
                         OnRequestPermission)
     IPC_MESSAGE_HANDLER(GeolocationHostMsg_StartUpdating, OnStartUpdating)
     IPC_MESSAGE_HANDLER(GeolocationHostMsg_StopUpdating, OnStopUpdating)
+    IPC_MESSAGE_HANDLER(GeolocationHostMsg_PauseOrResume, OnPauseOrResume)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
@@ -132,9 +137,10 @@ bool GeolocationDispatcherHostImpl::OnMessageReceived(
 void GeolocationDispatcherHostImpl::OnLocationUpdate(
     const Geoposition& geoposition) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  for (std::set<int>::iterator it = geolocation_renderer_ids_.begin();
-       it != geolocation_renderer_ids_.end(); ++it) {
-    Send(new GeolocationMsg_PositionUpdated(*it, geoposition));
+  for (std::map<int, RendererGeolocationOptions>::iterator it =
+       geolocation_renderers_.begin();
+       it != geolocation_renderers_.end(); ++it) {
+    Send(new GeolocationMsg_PositionUpdated(it->first, geoposition));
   }
 }
 
@@ -188,10 +194,8 @@ void GeolocationDispatcherHostImpl::OnStartUpdating(
   UMA_HISTOGRAM_BOOLEAN(
       "Geolocation.GeolocationDispatcherHostImpl.EnableHighAccuracy",
       enable_high_accuracy);
-  if (!geolocation_renderer_ids_.count(render_view_id))
-    geolocation_renderer_ids_.insert(render_view_id);
-
-  renderer_high_accuracy_[render_view_id] = enable_high_accuracy;
+  RendererGeolocationOptions opts = { enable_high_accuracy, false };
+  geolocation_renderers_[render_view_id] = opts;

[joth, 2013/11/07 21:52:00]

if we were previously paused we should now unpause

Should cover this with a test

   RefreshHighAccuracy();
 }
 
@@ -199,16 +203,45 @@ void GeolocationDispatcherHostImpl::OnStopUpdating(int render_view_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DVLOG(1) << __FUNCTION__ << " " << render_process_id_ << ":"
            << render_view_id;
-  if (renderer_high_accuracy_.erase(render_view_id))
-    RefreshHighAccuracy();
+  DCHECK_EQ(1U, geolocation_renderers_.count(render_view_id));
+  geolocation_renderers_.erase(render_view_id);

[joth, 2013/11/07 21:52:00]

if we just erased the only unpaused provider is should now pause

(again, cover in a test)

+  RefreshHighAccuracy();
+}
 
-  DCHECK_EQ(1U, geolocation_renderer_ids_.count(render_view_id));
-  geolocation_renderer_ids_.erase(render_view_id);
+void GeolocationDispatcherHostImpl::OnPauseOrResume(int render_view_id,
+                                                    bool should_pause) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  if (geolocation_renderers_.find(render_view_id) ==
+      geolocation_renderers_.end())
+    return;
+
+  if (should_pause) {
+    RendererGeolocationOptions* opts =
+        &(geolocation_renderers_[render_view_id]);
+    opts->is_paused = true;
+
+    bool pause_all = true;
+    std::map<int, RendererGeolocationOptions>::iterator i =
+        geolocation_renderers_.begin();
+    for (; i != geolocation_renderers_.end(); ++i) {
+      if (!i->second.is_paused) {
+        pause_all = false;
+        break;
+      }
+    }
+    if (pause_all)
+      GeolocationProvider::GetInstance()->PauseAllLocationUpdateCallbacks();
+  } else {
+    RendererGeolocationOptions* opts =
+        &(geolocation_renderers_[render_view_id]);
+    opts->is_paused = false;
+    GeolocationProvider::GetInstance()->ResumeAllLocationUpdateCallbacks();
+  }
 }
 
 void GeolocationDispatcherHostImpl::RefreshHighAccuracy() {

[joth, 2013/11/07 21:52:00]

this should probably be RefeshProviederOptions and have it resolve both the high
accuracy and paused-ness state

   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  if (renderer_high_accuracy_.empty()) {
+  if (geolocation_renderers_.empty()) {
     if (geolocation_provider_) {
       geolocation_provider_->RemoveLocationUpdateCallback(callback_);

[joth, 2013/11/07 21:52:00]

can we just use RemoveLocationUpdateCallback instead of the new 
PauseAllCallbacks call, and AddLocationUpdateCallback instead of resume?

then your edit to the specific location providers can be reverted.

code here would become simples:-

bool register_for_updates = false;
bool use_high_accuracy = false;

for (it : geolocation_renderers_) {
  register_for_updates |= !(it->is_pause);
  use_high_accuracy |= it->high_accuracy;
  if (register_for_updates && use_high_accuracy)
    break;  // optimization
}

if (register_for_updates) {
  geolocation_provider_->AddLocationUpdateCallback(callback, use_high_accuract);
} else {
  geolocation_provider_->RemoveLocationUpdateCallback(callback_);
}

[joth, 2013/11/07 21:58:15]

Actually my change above is needed otherwise this introduces a bug:
there is one GeolocationDispatcherHostImpl instance per render *process*. each
one can independently register for updates.

We can't call a global "pause all providers" from in here, as even if one render
process no longer needs updates, other render processes may still have
non-paused render views.

       geolocation_provider_ = NULL;
@@ -218,9 +251,10 @@ void GeolocationDispatcherHostImpl::RefreshHighAccuracy() {
       geolocation_provider_ = GeolocationProviderImpl::GetInstance();
     // Re-add to re-establish our options, in case they changed.
     bool use_high_accuracy = false;
-    std::map<int, bool>::iterator i = renderer_high_accuracy_.begin();
-    for (; i != renderer_high_accuracy_.end(); ++i) {
-      if (i->second) {
+    std::map<int, RendererGeolocationOptions>::iterator i =
+        geolocation_renderers_.begin();
+    for (; i != geolocation_renderers_.end(); ++i) {
+      if (i->second.high_accuracy) {
         use_high_accuracy = true;
         break;
       }