Some simplifications, as a pre-step to http://crbug.com/40103

- simplify the y geolocation IPC protocol, by using the error code already in Geoposition
- convert Geoposition's error_message to UTF8 as this is easier alround
- simplify the dispatcher host render id set (as the process_id part is implicit from the dispatcher host instance)

BUG=40103
TEST=unit_tests --gtest_filter=*Geol*

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

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

5 files changed, 30 insertions, 66 deletions

diff --git a/chrome/browser/geolocation/geolocation_dispatcher_host.cc b/chrome/browser/geolocation/geolocation_dispatcher_host.cc
index f937476..33d5c2b 100644
--- a/chrome/browser/geolocation/geolocation_dispatcher_host.cc
+++ b/chrome/browser/geolocation/geolocation_dispatcher_host.cc
@@ -12,25 +12,6 @@
 #include "chrome/browser/renderer_host/resource_message_filter.h"
 #include "chrome/common/render_messages.h"
 
-struct GeolocationDispatcherHost::GeolocationServiceRenderId {
-  int process_id;
-  int render_view_id;
-  GeolocationServiceRenderId(
-      int process_id, int render_view_id)
-      : process_id(process_id),
-        render_view_id(render_view_id) {
-  }
-  bool operator==(const GeolocationServiceRenderId& rhs) const {
-    return process_id == rhs.process_id &&
-           render_view_id == rhs.render_view_id;
-  }
-  bool operator<(const GeolocationServiceRenderId& rhs) const {
-    if (process_id == rhs.process_id)
-      return render_view_id < rhs.render_view_id;
-    return process_id < rhs.process_id;
-  }
-};
-
 GeolocationDispatcherHost::GeolocationDispatcherHost(
     int resource_message_filter_process_id,
     GeolocationPermissionContext* geolocation_permission_context)
@@ -76,29 +57,16 @@ void GeolocationDispatcherHost::OnLocationUpdate(
     const Geoposition& geoposition) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
 
-  for (std::set<GeolocationServiceRenderId>::iterator geo =
-       geolocation_renderers_.begin();
-       geo != geolocation_renderers_.end();
-       ++geo) {
-    IPC::Message* message;
-    if (geoposition.IsValidFix()) {
-      message = new ViewMsg_Geolocation_PositionUpdated(
-          geo->render_view_id, geoposition);
-    } else {
-      DCHECK(geoposition.IsInitialized());
-      message = new ViewMsg_Geolocation_Error(
-          geo->render_view_id, geoposition.error_code,
-          WideToUTF8(geoposition.error_message));
-    }
-    CallRenderViewHost(
-        geo->process_id, geo->render_view_id,
-        &RenderViewHost::Send, message);
+  for (std::set<int>::iterator it = geolocation_renderer_ids_.begin();
+       it != geolocation_renderer_ids_.end(); ++it) {
+    IPC::Message* message =
+        new ViewMsg_Geolocation_PositionUpdated(*it, geoposition);
+    CallRenderViewHost(resource_message_filter_process_id_, *it,
+                       &RenderViewHost::Send, message);
   }
 }
 
 void GeolocationDispatcherHost::OnRegisterDispatcher(int render_view_id) {
-  // TODO(bulach): is this the right way to get the RendererViewHost process id
-  // to be used by RenderViewHost::FromID?
   RegisterDispatcher(
       resource_message_filter_process_id_, render_view_id);
 }
@@ -163,6 +131,7 @@ void GeolocationDispatcherHost::OnResume(
 
 void GeolocationDispatcherHost::RegisterDispatcher(
     int process_id, int render_view_id) {
+  DCHECK_EQ(resource_message_filter_process_id_, process_id);
   if (!ChromeThread::CurrentlyOn(ChromeThread::IO)) {
     ChromeThread::PostTask(
         ChromeThread::IO, FROM_HERE,
@@ -173,16 +142,13 @@ void GeolocationDispatcherHost::RegisterDispatcher(
   }
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
 
-  GeolocationServiceRenderId geolocation_render_id(
-      process_id, render_view_id);
-  std::set<GeolocationServiceRenderId>::const_iterator existing =
-      geolocation_renderers_.find(geolocation_render_id);
-  DCHECK(existing == geolocation_renderers_.end());
-  geolocation_renderers_.insert(geolocation_render_id);
+  DCHECK_EQ(0u, geolocation_renderer_ids_.count(render_view_id));
+  geolocation_renderer_ids_.insert(render_view_id);
 }
 
 void GeolocationDispatcherHost::UnregisterDispatcher(
     int process_id, int render_view_id) {
+  DCHECK_EQ(resource_message_filter_process_id_, process_id);
   if (!ChromeThread::CurrentlyOn(ChromeThread::IO)) {
     ChromeThread::PostTask(
         ChromeThread::IO, FROM_HERE,
@@ -193,10 +159,6 @@ void GeolocationDispatcherHost::UnregisterDispatcher(
   }
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
 
-  GeolocationServiceRenderId geolocation_render_id(
-      process_id, render_view_id);
-  std::set<GeolocationServiceRenderId>::iterator existing =
-      geolocation_renderers_.find(geolocation_render_id);
-  DCHECK(existing != geolocation_renderers_.end());
-  geolocation_renderers_.erase(existing);
+  DCHECK_EQ(1u, geolocation_renderer_ids_.count(render_view_id));
+  geolocation_renderer_ids_.erase(render_view_id);
 }
diff --git a/chrome/browser/geolocation/geolocation_dispatcher_host.h b/chrome/browser/geolocation/geolocation_dispatcher_host.h
index 8d1e0d9..6cff179 100644
--- a/chrome/browser/geolocation/geolocation_dispatcher_host.h
+++ b/chrome/browser/geolocation/geolocation_dispatcher_host.h
@@ -37,7 +37,6 @@ class GeolocationDispatcherHost
   virtual void OnLocationUpdate(const Geoposition& position);
 
  private:
-  struct GeolocationServiceRenderId;
   friend class base::RefCountedThreadSafe<GeolocationDispatcherHost>;
   virtual ~GeolocationDispatcherHost();
 
@@ -62,8 +61,11 @@ class GeolocationDispatcherHost
   int resource_message_filter_process_id_;
   scoped_refptr<GeolocationPermissionContext> geolocation_permission_context_;
 
+  // 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.
-  std::set<GeolocationServiceRenderId> geolocation_renderers_;
+  std::set<int> geolocation_renderer_ids_;
   // Only set whilst we are registered with the arbitrator.
   scoped_refptr<GeolocationArbitrator> location_arbitrator_;
 
diff --git a/chrome/browser/geolocation/geolocation_permission_context.cc b/chrome/browser/geolocation/geolocation_permission_context.cc
index 4659fef..a040714 100644
--- a/chrome/browser/geolocation/geolocation_permission_context.cc
+++ b/chrome/browser/geolocation/geolocation_permission_context.cc
@@ -403,7 +403,7 @@ GeolocationArbitrator* GeolocationPermissionContext::StartUpdatingRequested(
   // Note we cannot store the arbitrator as a member as it is not thread safe.
   GeolocationArbitrator* arbitrator = GeolocationArbitrator::GetInstance();
 
-  // WebKit will not request permsission until it has received a valid
+  // WebKit will not request permission until it has received a valid
   // location, but the google network location provider will not give a
   // valid location until the user has granted permission. So we cut the Gordian
   // Knot by reusing the the 'start updating' request to also trigger
diff --git a/chrome/browser/geolocation/libgps_wrapper_linux.cc b/chrome/browser/geolocation/libgps_wrapper_linux.cc
index 977a086..141ad2f 100644
--- a/chrome/browser/geolocation/libgps_wrapper_linux.cc
+++ b/chrome/browser/geolocation/libgps_wrapper_linux.cc
@@ -119,11 +119,11 @@ bool LibGps::GetPosition(Geoposition* position) {
   DCHECK(position);
   position->error_code = Geoposition::ERROR_CODE_POSITION_UNAVAILABLE;
   if (!library().is_open()) {
-    position->error_message = L"No gpsd connection";
+    position->error_message = "No gpsd connection";
     return false;
   }
   if (!GetPositionIfFixed(position)) {
-    position->error_message = ASCIIToWide(last_error_);
+    position->error_message = last_error_;
     return false;
   }
   position->error_code = Geoposition::ERROR_CODE_NONE;
@@ -136,7 +136,7 @@ bool LibGps::GetPosition(Geoposition* position) {
                  << " accuracy " << position->accuracy << " time "
                  << position->timestamp.ToDoubleT();
     position->error_code = Geoposition::ERROR_CODE_POSITION_UNAVAILABLE;
-    position->error_message = L"Bad fix from gps";
+    position->error_message = "Bad fix from gps";
     return false;
   }
   return true;
diff --git a/chrome/browser/geolocation/network_location_request.cc b/chrome/browser/geolocation/network_location_request.cc
index 54018af..c278b74 100644
--- a/chrome/browser/geolocation/network_location_request.cc
+++ b/chrome/browser/geolocation/network_location_request.cc
@@ -170,14 +170,14 @@ void FormRequestBody(const std::string& host_name,
 }
 
 void FormatPositionError(const GURL& server_url,
-                         const std::wstring& message,
+                         const std::string& message,
                          Geoposition* position) {
     position->error_code = Geoposition::ERROR_CODE_POSITION_UNAVAILABLE;
-    position->error_message = L"Network location provider at '";
-    position->error_message += ASCIIToWide(server_url.possibly_invalid_spec());
-    position->error_message += L"' : ";
+    position->error_message = "Network location provider at '";
+    position->error_message += server_url.possibly_invalid_spec();
+    position->error_message += "' : ";
     position->error_message += message;
-    position->error_message += L".";
+    position->error_message += ".";
     LOG(INFO) << "NetworkLocationRequest::GetLocationFromResponse() : "
               << position->error_message;
 }
@@ -195,12 +195,12 @@ void GetLocationFromResponse(bool http_post_result,
   // HttpPost can fail for a number of reasons. Most likely this is because
   // we're offline, or there was no response.
   if (!http_post_result) {
-    FormatPositionError(server_url, L"No response received", position);
+    FormatPositionError(server_url, "No response received", position);
     return;
   }
   if (status_code != 200) {  // HTTP OK.
-    std::wstring message = L"Returned error code ";
-    message += IntToWString(status_code);
+    std::string message = "Returned error code ";
+    message += IntToString(status_code);
     FormatPositionError(server_url, message, position);
     return;
   }
@@ -208,14 +208,14 @@ void GetLocationFromResponse(bool http_post_result,
   // this position fix.
   if (!ParseServerResponse(response_body, timestamp, position, access_token)) {
     // We failed to parse the repsonse.
-    FormatPositionError(server_url, L"Response was malformed", position);
+    FormatPositionError(server_url, "Response was malformed", position);
     return;
   }
   // The response was successfully parsed, but it may not be a valid
   // position fix.
   if (!position->IsValidFix()) {
     FormatPositionError(server_url,
-                        L"Did not provide a good position fix", position);
+                        "Did not provide a good position fix", position);
     return;
   }
 }