Cleanup geofence registrations when a service worker is unregistered.

Cleanup geofence registrations when a service worker is unregistered.

This changes ServiceWorkerContextObserver::OnRegistrationDeleted to include the
registration ID.
Also modify GeofencingManager unit tests to have an actual
ServiceWorkerContextWrapper instance to test this behavior.

BUG=444136, 383125
Committed: https://crrev.com/05b26f88e6d0a7559a050c12ae01cf00917bd0b2
Cr-Commit-Position: refs/heads/master@{#310905}

[Marijn Kruisselbrink, 2014-12-19 20:29:02]

Patchset #1 (id:1) has been deleted

[Marijn Kruisselbrink, 2014-12-19 21:41:47]

Patchset #1 (id:20001) has been deleted

[Marijn Kruisselbrink, 2014-12-19 21:42:04]

mek@chromium.org changed reviewers:
+ michaeln@chromium.org

[Marijn Kruisselbrink, 2015-01-09 19:20:34]

michaeln: could you please take a look?

[michaeln, 2015-01-09 19:38:19]

yep... looking

[michaeln, 2015-01-09 22:58:41]

lgtm

https://codereview.chromium.org/788073007/diff/60001/content/browser/geofenci...
File content/browser/geofencing/geofencing_manager.cc (right):

https://codereview.chromium.org/788073007/diff/60001/content/browser/geofenci...
content/browser/geofencing/geofencing_manager.cc:333: int64
service_worker_registration_id) {
id here can be 'invalid', not sure if you want to test for that specifically and
return early?

https://codereview.chromium.org/788073007/diff/60001/content/browser/service_...
File content/browser/service_worker/service_worker_context_core.cc (right):

https://codereview.chromium.org/788073007/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_context_core.cc:317:
observer_list_->Notify(&ServiceWorkerContextObserver::OnRegistrationDeleted,
Seems odd that this notif can happen even if nothing was deleted (unlike the
Stored notif)? In that case the ID kInvalidServiceWorkerRegistrationId.

[Marijn Kruisselbrink, 2015-01-09 23:13:15]

https://codereview.chromium.org/788073007/diff/60001/content/browser/geofenci...
File content/browser/geofencing/geofencing_manager.cc (right):

https://codereview.chromium.org/788073007/diff/60001/content/browser/geofenci...
content/browser/geofencing/geofencing_manager.cc:333: int64
service_worker_registration_id) {
On 2015/01/09 22:58:41, michaeln wrote:
> id here can be 'invalid', not sure if you want to test for that specifically
and
> return early?

I'm not sure if that's worth the extra code. It would save one map lookup, but
nothing bad happens when it is invalid anyway.

https://codereview.chromium.org/788073007/diff/60001/content/browser/service_...
File content/browser/service_worker/service_worker_context_core.cc (right):

https://codereview.chromium.org/788073007/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_context_core.cc:317:
observer_list_->Notify(&ServiceWorkerContextObserver::OnRegistrationDeleted,
On 2015/01/09 22:58:41, michaeln wrote:
> Seems odd that this notif can happen even if nothing was deleted (unlike the
> Stored notif)? In that case the ID kInvalidServiceWorkerRegistrationId.

Yes, that does seem odd (but not anything introduced by my change; at least not
it will be possible for an observer to detect this cast). Should I file a bug
for this?

[Marijn Kruisselbrink, 2015-01-09 23:13:20]

The CQ bit was checked by mek@chromium.org

[commit-bot: I haz the power, 2015-01-09 23:13:57]

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

[commit-bot: I haz the power, 2015-01-10 00:10:40]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2015-01-10 00:11:31]

Patchset 2 (id:??) landed as
https://crrev.com/05b26f88e6d0a7559a050c12ae01cf00917bd0b2
Cr-Commit-Position: refs/heads/master@{#310905}