Geolocation code cleanup following switch to client-based Geolocation.

Remove unnecessary methods
  GeolocationPermissionContext::StartUpdatingRequested and
  GeolocationPermissionContext::StopUpdatingRequested

Remove the unused bridge_id from IPC messages
  ViewHostMsg_Geolocation_StartUpdating
  ViewHostMsg_Geolocation_StopUpdating.

Remove unnecessary / unused IPC messages:
  ViewHostMsg_Geolocation_RegisterDispatcher
  ViewHostMsg_Geolocation_UnregisterDispatcher
  ViewHostMsg_Geolocation_Suspend
  ViewHostMsg_Geolocation_Resume

Rename GeolocationDispatcherHost::bridge_update_options_ to
renderer_update_options, and simplify map type to key only on
render id, since there is no relevant bridge id.

BUG=59907
TEST=Existing

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

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

5 files changed, 26 insertions, 142 deletions

diff --git a/chrome/browser/geolocation/geolocation_dispatcher_host.cc b/chrome/browser/geolocation/geolocation_dispatcher_host.cc
index 3b824c2..dbe6108 100644
--- a/chrome/browser/geolocation/geolocation_dispatcher_host.cc
+++ b/chrome/browser/geolocation/geolocation_dispatcher_host.cc
@@ -35,21 +35,17 @@ class GeolocationDispatcherHostImpl : public GeolocationDispatcherHost,
  private:
   virtual ~GeolocationDispatcherHostImpl();
 
-  void OnRegisterDispatcher(int render_view_id);
-  void OnUnregisterDispatcher(int render_view_id);
   void OnRequestPermission(
       int render_view_id, 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, int bridge_id, const GURL& requesting_frame,
+      int render_view_id, const GURL& requesting_frame,
       bool enable_high_accuracy);
-  void OnStopUpdating(int render_view_id, int bridge_id);
-  void OnSuspend(int render_view_id, int bridge_id);
-  void OnResume(int render_view_id, int bridge_id);
+  void OnStopUpdating(int render_view_id);
 
   // Updates the |location_arbitrator_| with the currently required update
-  // options, based on |bridge_update_options_|.
+  // options, based on |renderer_update_options_|.
   void RefreshGeolocationObserverOptions();
 
   int render_process_id_;
@@ -60,10 +56,9 @@ class GeolocationDispatcherHostImpl : public GeolocationDispatcherHost,
   // context switches.
   // Only used on the IO thread.
   std::set<int> geolocation_renderer_ids_;
-  // Maps <renderer_id, bridge_id> to the location arbitrator update options
-  // that correspond to this particular bridge.
-  std::map<std::pair<int, int>, GeolocationObserverOptions>
-      bridge_update_options_;
+  // Maps renderer_id to the location arbitrator update options that correspond
+  // to this particular bridge.
+  std::map<int, GeolocationObserverOptions> renderer_update_options_;
   // Only set whilst we are registered with the arbitrator.
   GeolocationProvider* location_provider_;
 
@@ -92,10 +87,6 @@ bool GeolocationDispatcherHostImpl::OnMessageReceived(
   *msg_was_ok = true;
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP_EX(GeolocationDispatcherHostImpl, msg, *msg_was_ok)
-    IPC_MESSAGE_HANDLER(ViewHostMsg_Geolocation_RegisterDispatcher,
-                        OnRegisterDispatcher)
-    IPC_MESSAGE_HANDLER(ViewHostMsg_Geolocation_UnregisterDispatcher,
-                        OnUnregisterDispatcher)
     IPC_MESSAGE_HANDLER(ViewHostMsg_Geolocation_CancelPermissionRequest,
                         OnCancelPermissionRequest)
     IPC_MESSAGE_HANDLER(ViewHostMsg_Geolocation_RequestPermission,
@@ -104,10 +95,6 @@ bool GeolocationDispatcherHostImpl::OnMessageReceived(
                         OnStartUpdating)
     IPC_MESSAGE_HANDLER(ViewHostMsg_Geolocation_StopUpdating,
                         OnStopUpdating)
-    IPC_MESSAGE_HANDLER(ViewHostMsg_Geolocation_Suspend,
-                        OnSuspend)
-    IPC_MESSAGE_HANDLER(ViewHostMsg_Geolocation_Resume,
-                        OnResume)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
@@ -125,20 +112,6 @@ void GeolocationDispatcherHostImpl::OnLocationUpdate(
   }
 }
 
-void GeolocationDispatcherHostImpl::OnRegisterDispatcher(
-    int render_view_id) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  DCHECK_EQ(0u, geolocation_renderer_ids_.count(render_view_id));
-  geolocation_renderer_ids_.insert(render_view_id);
-}
-
-void GeolocationDispatcherHostImpl::OnUnregisterDispatcher(
-    int render_view_id) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  DCHECK_EQ(1u, geolocation_renderer_ids_.count(render_view_id));
-  geolocation_renderer_ids_.erase(render_view_id);
-}
-
 void GeolocationDispatcherHostImpl::OnRequestPermission(
     int render_view_id,
     int bridge_id,
@@ -165,58 +138,35 @@ void GeolocationDispatcherHostImpl::OnCancelPermissionRequest(
 
 void GeolocationDispatcherHostImpl::OnStartUpdating(
     int render_view_id,
-    int bridge_id,
     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 register the dispatcher again.
-  if (!geolocation_renderer_ids_.count(render_view_id))
-    OnRegisterDispatcher(render_view_id);
-  // WebKit sends the startupdating request before checking permissions, to
-  // optimize the no-location-available case and reduce latency in the success
-  // case (location lookup happens in parallel with the permission request).
+  // being enabled / disabled. No need to record the dispatcher again.
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DVLOG(1) << __FUNCTION__ << " " << render_process_id_ << ":"
-           << render_view_id << ":" << bridge_id;
-  bridge_update_options_[std::make_pair(render_view_id, bridge_id)] =
+           << render_view_id;
+  if (!geolocation_renderer_ids_.count(render_view_id))
+    geolocation_renderer_ids_.insert(render_view_id);
+
+  renderer_update_options_[render_view_id] =
       GeolocationObserverOptions(enable_high_accuracy);
-  geolocation_permission_context_->StartUpdatingRequested(
-      render_process_id_, render_view_id, bridge_id,
-      requesting_frame);
   RefreshGeolocationObserverOptions();
 }
 
-void GeolocationDispatcherHostImpl::OnStopUpdating(int render_view_id,
-                                                      int bridge_id) {
+void GeolocationDispatcherHostImpl::OnStopUpdating(int render_view_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DVLOG(1) << __FUNCTION__ << " " << render_process_id_ << ":"
-           << render_view_id << ":" << bridge_id;
-  if (bridge_update_options_.erase(std::make_pair(render_view_id, bridge_id)))
+           << render_view_id;
+  if (renderer_update_options_.erase(render_view_id))
     RefreshGeolocationObserverOptions();
-  geolocation_permission_context_->StopUpdatingRequested(
-      render_process_id_, render_view_id, bridge_id);
-  OnUnregisterDispatcher(render_view_id);
-}
-
-void GeolocationDispatcherHostImpl::OnSuspend(int render_view_id,
-                                                 int bridge_id) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  DVLOG(1) << __FUNCTION__ << " " << render_process_id_ << ":"
-           << render_view_id << ":" << bridge_id;
-  // TODO(bulach): connect this with GeolocationArbitrator.
-}
 
-void GeolocationDispatcherHostImpl::OnResume(int render_view_id,
-                                                int bridge_id) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  DVLOG(1) << __FUNCTION__ << " " << render_process_id_ << ":"
-           << render_view_id << ":" << bridge_id;
-  // TODO(bulach): connect this with GeolocationArbitrator.
+  DCHECK_EQ(1U, geolocation_renderer_ids_.count(render_view_id));
+  geolocation_renderer_ids_.erase(render_view_id);
 }
 
 void GeolocationDispatcherHostImpl::RefreshGeolocationObserverOptions() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  if (bridge_update_options_.empty()) {
+  if (renderer_update_options_.empty()) {
     if (location_provider_) {
       location_provider_->RemoveObserver(this);
       location_provider_ = NULL;
@@ -227,7 +177,7 @@ void GeolocationDispatcherHostImpl::RefreshGeolocationObserverOptions() {
     // Re-add to re-establish our options, in case they changed.
     location_provider_->AddObserver(
         this,
-        GeolocationObserverOptions::Collapse(bridge_update_options_));
+        GeolocationObserverOptions::Collapse(renderer_update_options_));
   }
 }
 }  // namespace
diff --git a/chrome/browser/geolocation/geolocation_permission_context.cc b/chrome/browser/geolocation/geolocation_permission_context.cc
index cd2d0c7..76bc3d7 100644
--- a/chrome/browser/geolocation/geolocation_permission_context.cc
+++ b/chrome/browser/geolocation/geolocation_permission_context.cc
@@ -432,23 +432,6 @@ void GeolocationPermissionContext::CancelGeolocationPermissionRequest(
   CancelPendingInfoBarRequest(render_process_id, render_view_id, bridge_id);
 }
 
-void GeolocationPermissionContext::StartUpdatingRequested(
-    int render_process_id, int render_view_id, int bridge_id,
-    const GURL& requesting_frame) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  // Note we cannot store the arbitrator as a member as it is not thread safe.
-  GeolocationProvider* provider = GeolocationProvider::GetInstance();
-
-  // Client-based Geolocation uses a preemptive permission model, so permission
-  // ought to have been requested and granted before the controller requests
-  // the client to start updating.
-  DCHECK(provider->HasPermissionBeenGranted());
-}
-
-void GeolocationPermissionContext::StopUpdatingRequested(
-    int render_process_id, int render_view_id, int bridge_id) {
-}
-
 void GeolocationPermissionContext::NotifyPermissionSet(
     int render_process_id, int render_view_id, int bridge_id,
     const GURL& requesting_frame, bool allowed) {
diff --git a/chrome/browser/geolocation/geolocation_permission_context.h b/chrome/browser/geolocation/geolocation_permission_context.h
index c2a08e7..cce7090 100644
--- a/chrome/browser/geolocation/geolocation_permission_context.h
+++ b/chrome/browser/geolocation/geolocation_permission_context.h
@@ -44,22 +44,6 @@ class GeolocationPermissionContext
       int render_process_id, int render_view_id, int bridge_id,
       const GURL& requesting_frame, bool allowed);
 
-  // Called when a geolocation object wants to start receiving location updates.
-  // This also applies global policy around which location providers may be
-  // enabled at a given time (e.g. prior to the user agreeing to any geolocation
-  // permission requests).
-  void StartUpdatingRequested(
-      int render_process_id, int render_view_id, int bridge_id,
-      const GURL& requesting_frame);
-
-  // Called when a geolocation object has stopped. Because we are
-  // short-circuiting permission request (see StartUpdatingRequested above), we
-  // cancel any pending permission in here, since WebKit doesn't know about the
-  // pending permission request and will never call
-  // CancelGeolocationPermissionRequest().
-  void StopUpdatingRequested(
-      int render_process_id, int render_view_id, int bridge_id);
-
  private:
   friend class base::RefCountedThreadSafe<GeolocationPermissionContext>;
   virtual ~GeolocationPermissionContext();
diff --git a/chrome/common/render_messages_internal.h b/chrome/common/render_messages_internal.h
index ce0851d..9436e1a 100644
--- a/chrome/common/render_messages_internal.h
+++ b/chrome/common/render_messages_internal.h
@@ -2410,17 +2410,6 @@ IPC_SYNC_MESSAGE_CONTROL1_1(
 //---------------------------------------------------------------------------
 // Geolocation services messages
 
-// A GeolocationServiceBridgeImpl in the renderer process has been created.
-// This is used to lazily initialize the host dispatchers and related
-// Geolocation infrastructure in the browser process.
-IPC_MESSAGE_CONTROL1(ViewHostMsg_Geolocation_RegisterDispatcher,
-                     int /* render_view_id */)
-
-// A GeolocationServiceBridgeImpl has been destroyed.
-// This is used to let the Geolocation infrastructure do its cleanup.
-IPC_MESSAGE_CONTROL1(ViewHostMsg_Geolocation_UnregisterDispatcher,
-                     int /* render_view_id */)
-
 // The |render_view_id| and |bridge_id| representing |host| is requesting
 // permission to access geolocation position.
 // This will be replied by ViewMsg_Geolocation_PermissionSet.
@@ -2436,38 +2425,20 @@ IPC_MESSAGE_CONTROL3(ViewHostMsg_Geolocation_CancelPermissionRequest,
                      int /* bridge_id */,
                      GURL /* GURL of the frame */)
 
-// The |render_view_id| and |bridge_id| requests Geolocation service to start
-// updating.
+// The |render_view_id| requests Geolocation service to start updating.
 // This is an asynchronous call, and the browser process may eventually reply
 // with the updated geoposition, or an error (access denied, location
 // unavailable, etc.)
-IPC_MESSAGE_CONTROL4(ViewHostMsg_Geolocation_StartUpdating,
+IPC_MESSAGE_CONTROL3(ViewHostMsg_Geolocation_StartUpdating,
                      int /* render_view_id */,
-                     int /* bridge_id */,
                      GURL /* GURL of the frame requesting geolocation */,
                      bool /* enable_high_accuracy */)
 
-// The |render_view_id| and |bridge_id| requests Geolocation service to stop
-// updating.
+// The |render_view_id| requests Geolocation service to stop updating.
 // Note that the geolocation service may continue to fetch geolocation data
 // for other origins.
-IPC_MESSAGE_CONTROL2(ViewHostMsg_Geolocation_StopUpdating,
-                     int /* render_view_id */,
-                     int /* bridge_id */)
-
-// The |render_view_id| and |bridge_id| requests Geolocation service to
-// suspend.
-// Note that the geolocation service may continue to fetch geolocation data
-// for other origins.
-IPC_MESSAGE_CONTROL2(ViewHostMsg_Geolocation_Suspend,
-                     int /* render_view_id */,
-                     int /* bridge_id */)
-
-// The |render_view_id| and |bridge_id| requests Geolocation service to
-// resume.
-IPC_MESSAGE_CONTROL2(ViewHostMsg_Geolocation_Resume,
-                     int /* render_view_id */,
-                     int /* bridge_id */)
+IPC_MESSAGE_CONTROL1(ViewHostMsg_Geolocation_StopUpdating,
+                     int /* render_view_id */)
 
 // Updates the minimum/maximum allowed zoom percent for this tab from the
 // default values.  If |remember| is true, then the zoom setting is applied to
diff --git a/chrome/renderer/geolocation_dispatcher.cc b/chrome/renderer/geolocation_dispatcher.cc
index daf0856..e944673 100644
--- a/chrome/renderer/geolocation_dispatcher.cc
+++ b/chrome/renderer/geolocation_dispatcher.cc
@@ -42,19 +42,15 @@ void GeolocationDispatcher::geolocationDestroyed() {
 }
 
 void GeolocationDispatcher::startUpdating() {
-  // TODO(jknotten): Remove url and bridge_id from StartUpdating message
-  // once we have switched over to client-based geolocation.
   GURL url;
   render_view_->Send(new ViewHostMsg_Geolocation_StartUpdating(
-      render_view_->routing_id(), -1, url, enable_high_accuracy_));
+      render_view_->routing_id(), url, enable_high_accuracy_));
   updating_ = true;
 }
 
 void GeolocationDispatcher::stopUpdating() {
-  // TODO(jknotten): Remove url and bridge_id from StopUpdating message
-  // once we have switched over to client-based geolocation.
   render_view_->Send(new ViewHostMsg_Geolocation_StopUpdating(
-      render_view_->routing_id(), -1));
+      render_view_->routing_id()));
   updating_ = false;
 }