Add a mechanism to pause and resume geolocation requests.

content/browser/geolocation/geolocation_dispatcher_host.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/geolocation/geolocation_dispatcher_host.h"
 
 #include <map>
 #include <set>
 #include <utility>
 
@@ skipping 51 matching lines @@
                            int bridge_id,
                            const GURL& requesting_frame);
   void OnCancelPermissionRequest(int render_view_id,
                                  int bridge_id,
                                  const GURL& requesting_frame);
   void OnStartUpdating(int render_view_id,
                        const GURL& requesting_frame,
       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();
 
   void OnLocationUpdate(const Geoposition& position);
 
   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_;
 
   DISALLOW_COPY_AND_ASSIGN(GeolocationDispatcherHostImpl);
 };
 
 GeolocationDispatcherHostImpl::GeolocationDispatcherHostImpl(
     int render_process_id,
     GeolocationPermissionContext* geolocation_permission_context)
@@ skipping 17 matching lines @@
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   *msg_was_ok = true;
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP_EX(GeolocationDispatcherHostImpl, msg, *msg_was_ok)
     IPC_MESSAGE_HANDLER(GeolocationHostMsg_CancelPermissionRequest,
                         OnCancelPermissionRequest)
     IPC_MESSAGE_HANDLER(GeolocationHostMsg_RequestPermission,
                         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;
 }
 
 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));
   }
 }
 
 void GeolocationDispatcherHostImpl::OnRequestPermission(
     int render_view_id,
     int bridge_id,
     const GURL& requesting_frame) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DVLOG(1) << __FUNCTION__ << " " << render_process_id_ << ":"
            << render_view_id << ":" << bridge_id;
@@ skipping 33 matching lines @@
     const GURL& requesting_frame,
     bool enable_high_accuracy) {
   // StartUpdating() can be invoked as a result of high-accuracy mode
   // being enabled / disabled. No need to record the dispatcher again.
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DVLOG(1) << __FUNCTION__ << " " << render_process_id_ << ":"
            << render_view_id;
   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();
 }
 
 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;
     }
   } else {
     if (!geolocation_provider_)
       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;
       }
     }
     geolocation_provider_->AddLocationUpdateCallback(
         callback_, use_high_accuracy);
   }
 }
 }  // namespace
 
 
 // GeolocationDispatcherHost --------------------------------------------------
 
 // static
 GeolocationDispatcherHost* GeolocationDispatcherHost::New(
     int render_process_id,
     GeolocationPermissionContext* geolocation_permission_context) {
   return new GeolocationDispatcherHostImpl(
       render_process_id,
       geolocation_permission_context);
 }
 
 GeolocationDispatcherHost::GeolocationDispatcherHost() {
 }
 
 GeolocationDispatcherHost::~GeolocationDispatcherHost() {
 }
 
 }  // namespace content