Convert geolocation code to use RenderFrame instead of RenderView.

Convert geolocation code to use RenderFrame instead of RenderView.

BUG=304341
R=bulach@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=274339

[jam, 2014-05-30 08:16:32]

I was originally planning on committing this after the Blink side, but
https://codereview.chromium.org/256843004/ got reverted because it broke some
browser tests. So now I've made the content side change, and I have an updated
version of the blink cl that will land after. I verified that this patch doesn't
break layout tests with and without the blink side cl.

Before this lands, it depends on https://codereview.chromium.org/256843004/.

[bulach, 2014-05-30 15:03:12]

lgtm, but I wanted to highlight a potential issue with main_render_frame_.. I'm
not too familiar with it, so feel free to go ahead if you think it's fine! :)

+mvanouwerkerk

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

https://codereview.chromium.org/303503008/diff/20001/content/browser/geolocat...
content/browser/geolocation/geolocation_dispatcher_host.cc:206: if
(!updating_frames_.empty() && !paused_) {
nit: while at it, it may be better to invert with an early return:

if (updating_frames_.empty() || paused_) {
  geolocation_subscription_.reset();
  return;
}
// Subscribe for location updates.
bool high_accuracy = false;
...

https://codereview.chromium.org/303503008/diff/20001/content/renderer/render_...
File content/renderer/render_view_impl.cc (right):

https://codereview.chromium.org/303503008/diff/20001/content/renderer/render_...
content/renderer/render_view_impl.cc:2320: return main_render_frame_.get();
just checking....
this would've returned NULL during initialization.
now that main_render_frame_ is being set earlier, it'll return a non-NULL
pointer.
I don't know if the previous implementation was intentional (it looks so, since
main_render_frame_->Initialize() is called much later on), but it seems it'd be
safer to change this semantic on its own patch rather than here? wdyt?

I suppose the geolocation_dispatcher_ initialization could then CHECK to ensure
the main_render_frame_ isn't NULL.

[jam, 2014-05-30 16:36:54]

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

https://codereview.chromium.org/303503008/diff/20001/content/browser/geolocat...
content/browser/geolocation/geolocation_dispatcher_host.cc:206: if
(!updating_frames_.empty() && !paused_) {
On 2014/05/30 15:03:12, bulach wrote:
> nit: while at it, it may be better to invert with an early return:
> 
> if (updating_frames_.empty() || paused_) {
>   geolocation_subscription_.reset();
>   return;
> }
> // Subscribe for location updates.
> bool high_accuracy = false;
> ...

Done.

https://codereview.chromium.org/303503008/diff/20001/content/renderer/render_...
File content/renderer/render_view_impl.cc (right):

https://codereview.chromium.org/303503008/diff/20001/content/renderer/render_...
content/renderer/render_view_impl.cc:2320: return main_render_frame_.get();
On 2014/05/30 15:03:12, bulach wrote:
> just checking....
> this would've returned NULL during initialization.
> now that main_render_frame_ is being set earlier, it'll return a non-NULL
> pointer.
> I don't know if the previous implementation was intentional (it looks so,
since
> main_render_frame_->Initialize() is called much later on), but it seems it'd
be
> safer to change this semantic on its own patch rather than here? wdyt?
> 
> I suppose the geolocation_dispatcher_ initialization could then CHECK to
ensure
> the main_render_frame_ isn't NULL.

My hunch is it's not intentional; so far I haven't seen any breakages in my
testing, but a full try run will make me more confident.

Regarding separating it out, since this cl is not too big, and that change is
only done because of this I prefer to not do the extra work of landing them
separately.