Geolocation: move creation of the GeolocationDelegate to the embedders

Geolocation: move creation of the GeolocationDelegate to the embedders

This CL nukes ContentBrowserClient::CreateGeolocationDelegate()
and their overrides, and transfers each embedder's callsite to the
appropriate correspondent BrowserMainParts. These embedders are
namely: android_webview,  blimp, cast, chrome and content_shell.

Continuation of https://crrev.com/2127973002/.

BUG=612334
Committed: https://crrev.com/80ef5237b1c8089848df230b25dad099acf0dff6
Cr-Commit-Position: refs/heads/master@{#407291}

[mcasas, 2016-07-22 02:08:26]

Patchset #1 (id:1) has been deleted

[mcasas, 2016-07-22 04:13:21]

Patchset #1 (id:20001) has been deleted

[mcasas, 2016-07-22 04:28:01]

Description was changed from

==========
[wip] Geolocation: move creation of the GeolocationDelegate to the embedders

Warning: has https://codereview.chromium.org/2127973002/ patched in.

BUG=612334
==========

to

==========
Geolocation: move creation of the GeolocationDelegate to the embedders

This CL nukes ContentBrowserClient::CreateGeolocationDelegate
and its callsite in BrowserMainLoop, and transfer each to the 
individual embedders, namely: android_webview, blimp, cast,
shell and, obviously, chrome.

Continuation of https://crrev.com/2127973002/.

BUG=612334
==========

[mcasas, 2016-07-22 04:28:48]

Description was changed from

==========
Geolocation: move creation of the GeolocationDelegate to the embedders

This CL nukes ContentBrowserClient::CreateGeolocationDelegate
and its callsite in BrowserMainLoop, and transfer each to the 
individual embedders, namely: android_webview, blimp, cast,
shell and, obviously, chrome.

Continuation of https://crrev.com/2127973002/.

BUG=612334
==========

to

==========
Geolocation: move creation of the GeolocationDelegate to the embedders

This CL nukes ContentBrowserClient::CreateGeolocationDelegate
and its callsite in BrowserMainLoop, and transfer each to the 
individual appropriate BrowserMainParts, namely: android_webview, 
blimp, cast, chrome and content_shell.

Continuation of https://crrev.com/2127973002/.

BUG=612334
==========

[mcasas, 2016-07-22 04:38:59]

Description was changed from

==========
Geolocation: move creation of the GeolocationDelegate to the embedders

This CL nukes ContentBrowserClient::CreateGeolocationDelegate
and its callsite in BrowserMainLoop, and transfer each to the 
individual appropriate BrowserMainParts, namely: android_webview, 
blimp, cast, chrome and content_shell.

Continuation of https://crrev.com/2127973002/.

BUG=612334
==========

to

==========
Geolocation: move creation of the GeolocationDelegate to the embedders

This CL nukes ContentBrowserClient::CreateGeolocationDelegate()
and its callsite in BrowserMainLoop, and transfer each to the 
individual appropriate BrowserMainParts, namely: android_webview, 
blimp, cast, chrome and content_shell.

Continuation of https://crrev.com/2127973002/.

BUG=612334
==========

[mcasas, 2016-07-22 04:40:24]

Description was changed from

==========
Geolocation: move creation of the GeolocationDelegate to the embedders

This CL nukes ContentBrowserClient::CreateGeolocationDelegate()
and its callsite in BrowserMainLoop, and transfer each to the 
individual appropriate BrowserMainParts, namely: android_webview, 
blimp, cast, chrome and content_shell.

Continuation of https://crrev.com/2127973002/.

BUG=612334
==========

to

==========
Geolocation: move creation of the GeolocationDelegate to the embedders

This CL nukes ContentBrowserClient::CreateGeolocationDelegate()
and their overrides, and transfers each embedder's callsite to the 
appropriate correspondent BrowserMainParts. These embedders are 
namely: android_webview,  blimp, cast, chrome and content_shell.

Continuation of https://crrev.com/2127973002/.

BUG=612334
==========

[mcasas, 2016-07-22 15:52:13]

mcasas@chromium.org changed reviewers:
+ jam@chromium.org

[mcasas, 2016-07-22 15:52:14]

jam@, second part, PTAL

[jam, 2016-07-22 21:23:19]

lgtm with one comment: please add debug checks to geolocation_provider_impl.cc
to ensure that the delegate isn't set after it's used (i.e. to guard against
embedders setting this too late).

[mcasas, 2016-07-22 21:27:10]

On 2016/07/22 21:23:19, jam wrote:
> lgtm with one comment: please add debug checks to geolocation_provider_impl.cc
> to ensure that the delegate isn't set after it's used (i.e. to guard against
> embedders setting this too late).

There's one already in [1]:

void GeolocationProvider::SetGeolocationDelegate(
    GeolocationDelegate* delegate) {
  DCHECK(!g_delegate.Get());  
  g_delegate.Get().reset(delegate);
}

that protects against SetGeolocationDelegate() being
called after [2] (which is the only other point where
|g_delegate| can be set.


[1]
https://cs.chromium.org/chromium/src/content/browser/geolocation/geolocation_...
[2]
https://cs.chromium.org/chromium/src/content/browser/geolocation/geolocation_...

[jam, 2016-07-22 21:38:19]

On Fri, Jul 22, 2016 at 2:27 PM, <mcasas@chromium.org> wrote:

> On 2016/07/22 21:23:19, jam wrote:
> > lgtm with one comment: please add debug checks to
> geolocation_provider_impl.cc
> > to ensure that the delegate isn't set after it's used (i.e. to guard
> against
> > embedders setting this too late).
>
> There's one already in [1]:
>
> void GeolocationProvider::SetGeolocationDelegate(
> GeolocationDelegate* delegate) {
> DCHECK(!g_delegate.Get());
> g_delegate.Get().reset(delegate);
> }
>
> that protects against SetGeolocationDelegate() being
> called after [2] (which is the only other point where
> |g_delegate| can be set.
>

ah, ok nvm then :)


>
>
> [1]
>
>
https://cs.chromium.org/chromium/src/content/browser/geolocation/geolocation_...
> [2]
>
>
https://cs.chromium.org/chromium/src/content/browser/geolocation/geolocation_...
>
> https://codereview.chromium.org/2174613002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[mcasas, 2016-07-22 21:41:47]

The CQ bit was checked by mcasas@chromium.org

[commit-bot: I haz the power, 2016-07-22 21:42:20]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-22 23:01:03]

Description was changed from

==========
Geolocation: move creation of the GeolocationDelegate to the embedders

This CL nukes ContentBrowserClient::CreateGeolocationDelegate()
and their overrides, and transfers each embedder's callsite to the 
appropriate correspondent BrowserMainParts. These embedders are 
namely: android_webview,  blimp, cast, chrome and content_shell.

Continuation of https://crrev.com/2127973002/.

BUG=612334
==========

to

==========
Geolocation: move creation of the GeolocationDelegate to the embedders

This CL nukes ContentBrowserClient::CreateGeolocationDelegate()
and their overrides, and transfers each embedder's callsite to the 
appropriate correspondent BrowserMainParts. These embedders are 
namely: android_webview,  blimp, cast, chrome and content_shell.

Continuation of https://crrev.com/2127973002/.

BUG=612334
==========

[commit-bot: I haz the power, 2016-07-22 23:01:04]

Committed patchset #1 (id:40001)

[commit-bot: I haz the power, 2016-07-22 23:04:04]

Description was changed from

==========
Geolocation: move creation of the GeolocationDelegate to the embedders

This CL nukes ContentBrowserClient::CreateGeolocationDelegate()
and their overrides, and transfers each embedder's callsite to the 
appropriate correspondent BrowserMainParts. These embedders are 
namely: android_webview,  blimp, cast, chrome and content_shell.

Continuation of https://crrev.com/2127973002/.

BUG=612334
==========

to

==========
Geolocation: move creation of the GeolocationDelegate to the embedders

This CL nukes ContentBrowserClient::CreateGeolocationDelegate()
and their overrides, and transfers each embedder's callsite to the
appropriate correspondent BrowserMainParts. These embedders are
namely: android_webview,  blimp, cast, chrome and content_shell.

Continuation of https://crrev.com/2127973002/.

BUG=612334

Committed: https://crrev.com/80ef5237b1c8089848df230b25dad099acf0dff6
Cr-Commit-Position: refs/heads/master@{#407291}
==========

[commit-bot: I haz the power, 2016-07-22 23:04:06]

Patchset 1 (id:??) landed as
https://crrev.com/80ef5237b1c8089848df230b25dad099acf0dff6
Cr-Commit-Position: refs/heads/master@{#407291}