Client-based geolocation support.

chrome/renderer/geolocation_dispatcher.cc

+// Copyright (c) 2010 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.
+
+#if defined(ENABLE_CLIENT_BASED_GEOLOCATION)
+
+#include "third_party/WebKit/WebKit/chromium/public/WebGeolocationPermissionRequest.h"
+#include "third_party/WebKit/WebKit/chromium/public/WebGeolocationPermissionRequestContainer.h"
+#include "third_party/WebKit/WebKit/chromium/public/WebGeolocationClient.h"
+#include "third_party/WebKit/WebKit/chromium/public/WebGeolocationPosition.h"
+#include "third_party/WebKit/WebKit/chromium/public/WebGeolocationError.h"
+#include "third_party/WebKit/WebKit/chromium/public/WebSecurityOrigin.h"
+#include "chrome/renderer/geolocation_dispatcher.h"

[joth, 2010/12/07 10:24:51]

alpha order

+#include "chrome/renderer/render_view.h"
+#include "ipc/ipc_message.h"
+
+using namespace WebKit;
+
+GeolocationDispatcher::GeolocationDispatcher(RenderView* render_view)
+    : render_view_(render_view),
+      pending_permissions_(new WebGeolocationPermissionRequestContainer),

[bulach, 2010/12/07 11:18:42]

we normally add "()", as new Foo()

+      enable_high_accuracy_(false),
+      updating_(false)
+{

[joth, 2010/12/07 10:24:51]

move { to previous line

+}
+
+GeolocationDispatcher::~GeolocationDispatcher()
+{
+}

[joth, 2010/12/07 10:24:51]

you are allowed to do this all on one line:
Foo::~Foo() {}

Anyway, put { on previous line

+
+bool GeolocationDispatcher::OnMessageReceived(const IPC::Message& message) {
+  bool handled = true;
+  IPC_BEGIN_MESSAGE_MAP(GeolocationDispatcher, message)
+    IPC_MESSAGE_HANDLER(ViewMsg_Geolocation_PermissionSet,
+                        OnGeolocationPermissionSet)
+    IPC_MESSAGE_HANDLER(ViewMsg_Geolocation_PositionUpdated,
+                        OnGeolocationPositionUpdated)
+    IPC_MESSAGE_UNHANDLED(handled = false)
+  IPC_END_MESSAGE_MAP()
+  return handled;
+}
+
+void GeolocationDispatcher::geolocationDestroyed() {
+  controller_.reset();
+  DCHECK(!updating_);
+}
+
+void GeolocationDispatcher::startUpdating() {
+  // TODO(jknotten): Remove url and bridge_id from StartUpdating message
+  GURL url;
+  render_view_->Send(new ViewHostMsg_Geolocation_StartUpdating(
+      render_view_->routing_id(), -1, url, enable_high_accuracy_));
+  updating_ = true;
+}
+
+void GeolocationDispatcher::stopUpdating() {
+  // TODO(jknotten): Remove url and bridge_id from StartUpdating message

[bulach, 2010/12/07 11:18:42]

nit: StopUpdating message

+  render_view_->Send(new ViewHostMsg_Geolocation_StopUpdating(
+      render_view_->routing_id(), -1));
+  updating_ = false;
+}
+
+void GeolocationDispatcher::setEnableHighAccuracy(bool enable_high_accuracy) {
+  enable_high_accuracy_ = enable_high_accuracy;

[joth, 2010/12/07 10:24:51]

do we not need to send IPC to the GeolocationDispatcherHost (if it changed)?
TODO?

[joth, 2010/12/08 10:47:40]

ping

[John Knottenbelt, 2010/12/08 12:28:34]

Sorry, missed this one. We don't need to send an IPC because
setEnableHighAccuracy is called by the GeolocationController:
 - with true, before it calls startUpdating in response to the first
high-accuracy Geolocation subscription.
 - with false, and after having called stopUpdating when the last high-accuracy
Geolocation unsubscribes.
I will add a comment about this.
On 2010/12/08 10:47:40, joth wrote:

[joth, 2010/12/08 12:54:29]

what if there are still low-accuracy subscriber when the last high accuracy one
leaves us? It can't call stopUpdating, so does it re-call startUpdating? Or does
it do a stop/start?

+}
+
+void GeolocationDispatcher::setController(
+    const WebGeolocationController& controller) {
+  controller_ = controller;

[joth, 2010/12/07 10:24:51]

this does a deep copy which feels a bit funky as normally a BlahController is an
identity object (i.e. DISALLOW_COPY_AND_ASSIGN or equiv) but presumably this is
all as per the WebKit design.

+}
+
+bool GeolocationDispatcher::lastPosition(WebGeolocationPosition&) {
+  return false;

[joth, 2010/12/07 10:24:51]

maybe comment why? (we don't want to fetch it synchronously from the browser,
and no need for yet-another layer of caching)

+}
+
+// TODO(jknotten): Change the messages to use a security origin, so no
+// conversion is necessary.
+void GeolocationDispatcher::requestPermission(
+    const WebGeolocationPermissionRequest& permissionRequest) {
+  int bridge_id = pending_permissions_->add(permissionRequest);
+  GURL url(static_cast<string16>(
+      permissionRequest.securityOrigin().toString()));

[joth, 2010/12/07 10:24:51]

this cast looks suspect. ah I see you're forcing it to invoke the conversion
operator.
Suggest using implicit_cast<> (defined in base) or make a temporary on the
stack:
string16 origin = permissionRequest.securityOrigin().toString();
rv_->Send(blah, blah, GURL(origin));

[John Knottenbelt, 2010/12/07 12:30:04]

Done.

+  render_view_->Send(new ViewHostMsg_Geolocation_RequestPermission(
+      render_view_->routing_id(), bridge_id, GURL(url)));

[joth, 2010/12/07 10:24:51]

nit: no need for the GURL() around url (modulo my previous comment)

[John Knottenbelt, 2010/12/07 12:30:04]

Done.

+}
+
+// TODO(jknotten): Change the messages to use a security origin, so no
+// conversion is necessary.
+void GeolocationDispatcher::cancelPermissionRequest(
+    const WebGeolocationPermissionRequest& permissionRequest) {
+  int bridge_id;
+  if (!pending_permissions_->remove(permissionRequest, bridge_id))
+    return;
+  GURL url(static_cast<string16>(
+      permissionRequest.securityOrigin().toString()));
+  render_view_->Send(new ViewHostMsg_Geolocation_CancelPermissionRequest(
+      render_view_->routing_id(), bridge_id, GURL(url)));

[joth, 2010/12/07 10:24:51]

dittos

[John Knottenbelt, 2010/12/07 12:30:04]

Done.

+}
+
+// Permission for using geolocation has been set.
+void GeolocationDispatcher::OnGeolocationPermissionSet(
+    int bridge_id, bool is_allowed) {
+  WebGeolocationPermissionRequest permissionRequest;
+  if (!pending_permissions_->remove(bridge_id, permissionRequest))
+    return;
+  permissionRequest.setIsAllowed(is_allowed);
+}
+
+// We have an updated geolocation position or error code.
+void GeolocationDispatcher::OnGeolocationPositionUpdated(
+    const Geoposition& geoposition) {
+  DCHECK(updating_);

[joth, 2010/12/07 10:24:51]

suggest adding
DCHECK(geoposition.IsInitialized());

[John Knottenbelt, 2010/12/07 12:30:04]

Done.

+  if (geoposition.IsValidFix()) {
+    controller_.positionChanged(
+        WebGeolocationPosition(
+            static_cast<int64>(geoposition.timestamp.ToDoubleT() * 1000),
+            geoposition.latitude, geoposition.longitude,
+            geoposition.accuracy,
+            geoposition.is_valid_altitude(), geoposition.altitude,
+            geoposition.is_valid_altitude_accuracy(),
+            geoposition.altitude_accuracy,
+            geoposition.is_valid_heading(), geoposition.heading,
+            geoposition.is_valid_speed(), geoposition.speed));
+  } else {
+    WebGeolocationError::Error code;
+    switch (geoposition.error_code) {
+      case Geoposition::ERROR_CODE_PERMISSION_DENIED:
+        code = WebGeolocationError::ErrorPermissionDenied;
+        break;
+      case Geoposition::ERROR_CODE_POSITION_UNAVAILABLE:
+        code = WebGeolocationError::ErrorPositionUnavailable;
+        break;
+      default:
+        DCHECK(false);

[joth, 2010/12/07 10:24:51]

NOTREACHED() << geoposition.error_code;

[joth, 2010/12/08 10:47:40]

ping

[John Knottenbelt, 2010/12/08 12:28:34]

Done.

+    }
+    controller_.errorOccurred(
+        WebGeolocationError(
+            code, WebKit::WebString::fromUTF8(geoposition.error_message)));
+  }
+}
+
+#endif // CLIENT_BASED_GEOLOCATION