Do not send IPC message to cancel Geolocation permission.

Do not send IPC message to cancel Geolocation permission.

The primary goal of that CL is to be able to assess which IPC
are actually required in order to handle permissions. Cancel
is one of the calls that does not seem needed.

This CL depends on: https://codereview.chromium.org/600043004/

BUG=421085
Committed: https://crrev.com/7a800db3e8cb826f6ad6120ca73c79135567b356
Cr-Commit-Position: refs/heads/master@{#298736}

[mlamouri (slow - plz ping), 2014-10-03 15:54:32]

mlamouri@chromium.org changed reviewers:
+ mvanouwerkerk@chromium.org

[mlamouri (slow - plz ping), 2014-10-03 15:54:32]

Michael, can you have a look at this?

[mlamouri (slow - plz ping), 2014-10-07 15:15:17]

mlamouri@chromium.org changed reviewers:
+ creis@chromium.org

[mlamouri (slow - plz ping), 2014-10-07 15:15:18]

Michael, could you review this CL as the feature owner?
Charlie, could you review this cL as the file OWNER?

Thanks :)

[Mike West, 2014-10-07 15:30:56]

mkwst@chromium.org changed reviewers:
+ mkwst@chromium.org

[Mike West, 2014-10-07 15:30:56]

Drive-by LGTM. This looks like a reasonable transition away from IPC. The effect
will be that the browser holds onto requests a little bit longer, but that seems
like a reasonable tradeoff. +tsepez for confirmation.

[Mike West, 2014-10-07 15:31:13]

mkwst@chromium.org changed reviewers:
+ tsepez@chromium.org

[Mike West, 2014-10-07 15:31:13]

actually +tsepez for confirmation. :)

[Charlie Reis, 2014-10-07 17:33:11]

[+site-isolation-reviews]

This depends on https://codereview.chromium.org/600043004/, which I have some
concerns about.  We can work through them there first.

[Tom Sepez, 2014-10-07 17:42:16]

Messages themselves LGTM, but do address the other concerns.

[Charlie Reis, 2014-10-07 21:35:36]

Concerns resolved.  LGTM with nits.

https://codereview.chromium.org/625963003/diff/20001/content/browser/geolocat...
File content/browser/geolocation/geolocation_dispatcher_host.h (right):

https://codereview.chromium.org/625963003/diff/20001/content/browser/geolocat...
content/browser/geolocation/geolocation_dispatcher_host.h:47: const GURL&
requesting_origin,
nit: Can you add a similar TODO as the one above
GeolocationDispatcher::requestPermission?  This isn't actually guaranteed to be
an origin yet, since the renderer could pass any URL.  (I do agree with the name
change, though.)

https://codereview.chromium.org/625963003/diff/20001/content/common/geolocati...
File content/common/geolocation_messages.h (right):

https://codereview.chromium.org/625963003/diff/20001/content/common/geolocati...
content/common/geolocation_messages.h:47:
IPC_MESSAGE_ROUTED3(GeolocationHostMsg_RequestPermission,
nit: Same TODO.  We should be using url::Origin or something similar here.

[mlamouri (slow - plz ping), 2014-10-08 09:24:33]

Michael, could you have a look?

https://codereview.chromium.org/625963003/diff/20001/content/browser/geolocat...
File content/browser/geolocation/geolocation_dispatcher_host.h (right):

https://codereview.chromium.org/625963003/diff/20001/content/browser/geolocat...
content/browser/geolocation/geolocation_dispatcher_host.h:47: const GURL&
requesting_origin,
On 2014/10/07 21:35:35, Charlie Reis wrote:
> nit: Can you add a similar TODO as the one above
> GeolocationDispatcher::requestPermission?  This isn't actually guaranteed to
be
> an origin yet, since the renderer could pass any URL.  (I do agree with the
name
> change, though.)

Done.

https://codereview.chromium.org/625963003/diff/20001/content/common/geolocati...
File content/common/geolocation_messages.h (right):

https://codereview.chromium.org/625963003/diff/20001/content/common/geolocati...
content/common/geolocation_messages.h:47:
IPC_MESSAGE_ROUTED3(GeolocationHostMsg_RequestPermission,
On 2014/10/07 21:35:35, Charlie Reis wrote:
> nit: Same TODO.  We should be using url::Origin or something similar here.

Done.

[Michael van Ouwerkerk, 2014-10-08 12:46:27]

lgtm

[mlamouri (slow - plz ping), 2014-10-08 16:10:54]

The CQ bit was checked by mlamouri@chromium.org

[commit-bot: I haz the power, 2014-10-08 16:11:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/625963003/40001

[commit-bot: I haz the power, 2014-10-08 17:03:18]

Committed patchset #3 (id:40001) as c25d8ece789a5990abda308e556b4e4e1e032c22

[commit-bot: I haz the power, 2014-10-08 17:04:55]

Patchset 3 (id:??) landed as
https://crrev.com/7a800db3e8cb826f6ad6120ca73c79135567b356
Cr-Commit-Position: refs/heads/master@{#298736}