Bug 1740551 Comment 10 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

Original comment by
on 
My high-level understanding is that we get somehow in a situation where:

**Context**
- we have a valid `ServiceWorkerManager::RegistrationDataPerPrincipal` entry *RDpP* in `ServiceWorkerManager::mRegistrationInfos` for a given scope
  - but *RDpP* contains an empty list `mInfos` and it is not (yet) clear to me if this is an expected state and how we got here

** Crash path**
- we enter `ServiceWorkerRegistrationInfo::MaybeCheckNavigationUpdate` and find a living `ControlledClientData` that contains a living strong reference `mRegistrationInfo` referring to the same scope as *RDpP* above and we call `MaybeScheduleUpdate` on it
  - we reach the navigation fault threshold inside `ServiceWorkerRegistrationInfo::MaybeScheduleUpdate`
  - this triggers a `CheckQuotaUsage` on the same registration info
    - there we take the `mPrincipal` and `Scope()` of this registratiuon info to call `swm->CheckPrincipalQuotaUsage`
    - Inside `ServiceWorkerManager::CheckPrincipalQuotaUsage` we use this this scope to first lookup the (existing) *RDpP* from above and then the registration info that we expect to find in `mInfo` of *RDpP*. But that is empty and therefore we have a `nullptr`

**Questions**
- Should we ever be in the configuration to have a living but empty * RDpP* for a given scope?
  - if *yes* we must deal with this possibility in `CheckPrincipalQuotaUsage` (and maybe other places)
    - there is a [comment here](https://searchfox.org/mozilla-central/rev/fb8d77331582639ea6848a61dd8ee812fac31b77/dom/serviceworkers/ServiceWorkerManager.cpp#1805-1808) that talks about re-use of *RDpP* instances, so it seems likely?
  - if *no* we need to continue investigations how we got there.
    - there are [places in the code](https://searchfox.org/mozilla-central/rev/5e15e00fa247cba5b765727496619bf9010ed162/dom/serviceworkers/ServiceWorkerManager.cpp#1683) where it seems we explicitly do not want this to happen, instead
- Is it expected that we still see a living `ControlledClientData` in `ServiceWorkerManagermControlledClients` for a given client id after we apparently already unregistered the service worker?
- Can't we directly use the already present registration info from `ControlledClientData` reducing the number of indirect (and error prone) lookups here?
  - I also assume the code could benefit from some wrapper lookup functions that handle well all possible `nullptr` constellations rather than having all this manual double lookups?
Revision 1 by
on 
My high-level understanding is that we get somehow in a situation where:

**Context**
- we have a valid `ServiceWorkerManager::RegistrationDataPerPrincipal` entry *RDpP* in `ServiceWorkerManager::mRegistrationInfos` for a given scope
  - but *RDpP* contains an empty list `mInfos` and it is not (yet) clear to me if this is an expected state and how we got here

**Crash path**
- we enter `ServiceWorkerRegistrationInfo::MaybeCheckNavigationUpdate` and find a living `ControlledClientData` that contains a living strong reference `mRegistrationInfo` referring to the same scope as *RDpP* above and we call `MaybeScheduleUpdate` on it
  - we reach the navigation fault threshold inside `ServiceWorkerRegistrationInfo::MaybeScheduleUpdate`
  - this triggers a `CheckQuotaUsage` on the same registration info
    - there we take the `mPrincipal` and `Scope()` of this registratiuon info to call `swm->CheckPrincipalQuotaUsage`
    - Inside `ServiceWorkerManager::CheckPrincipalQuotaUsage` we use this this scope to first lookup the (existing) *RDpP* from above and then the registration info that we expect to find in `mInfo` of *RDpP*. But that is empty and therefore we have a `nullptr`

**Questions**
- Should we ever be in the configuration to have a living but empty * RDpP* for a given scope?
  - if *yes* we must deal with this possibility in `CheckPrincipalQuotaUsage` (and maybe other places)
    - there is a [comment here](https://searchfox.org/mozilla-central/rev/fb8d77331582639ea6848a61dd8ee812fac31b77/dom/serviceworkers/ServiceWorkerManager.cpp#1805-1808) that talks about re-use of *RDpP* instances, so it seems likely?
  - if *no* we need to continue investigations how we got there.
    - there are [places in the code](https://searchfox.org/mozilla-central/rev/5e15e00fa247cba5b765727496619bf9010ed162/dom/serviceworkers/ServiceWorkerManager.cpp#1683) where it seems we explicitly do not want this to happen, instead
- Is it expected that we still see a living `ControlledClientData` in `ServiceWorkerManagermControlledClients` for a given client id after we apparently already unregistered the service worker?
- Can't we directly use the already present registration info from `ControlledClientData` reducing the number of indirect (and error prone) lookups here?
  - I also assume the code could benefit from some wrapper lookup functions that handle well all possible `nullptr` constellations rather than having all this manual double lookups?
Revision 2 by
on 
My high-level understanding is that we get somehow in a situation where:

**Context**
- we have a valid `ServiceWorkerManager::RegistrationDataPerPrincipal` entry *RDpP* in `ServiceWorkerManager::mRegistrationInfos` for a given scope
  - but *RDpP* contains an empty list `mInfos` and it is not (yet) clear to me if this is an expected state and how we got here

**Crash path**
- we enter `ServiceWorkerRegistrationInfo::MaybeCheckNavigationUpdate` and find a living `ControlledClientData` that contains a living strong reference `mRegistrationInfo` referring to the same scope as *RDpP* above and we call `MaybeScheduleUpdate` on it
  - we reach the navigation fault threshold inside `ServiceWorkerRegistrationInfo::MaybeScheduleUpdate`
  - this triggers a `CheckQuotaUsage` on the same registration info
    - there we take the `mPrincipal` and `Scope()` of this registratiuon info to call `swm->CheckPrincipalQuotaUsage`
    - Inside `ServiceWorkerManager::CheckPrincipalQuotaUsage` we use this scope to first lookup the (existing) *RDpP* from above and then the registration info that we expect to find in `mInfo` of *RDpP*. But that is empty and therefore we have a `nullptr`

**Questions**
- Should we ever be in the configuration to have a living but empty * RDpP* for a given scope?
  - if *yes* we must deal with this possibility in `CheckPrincipalQuotaUsage` (and maybe other places)
    - there is a [comment here](https://searchfox.org/mozilla-central/rev/fb8d77331582639ea6848a61dd8ee812fac31b77/dom/serviceworkers/ServiceWorkerManager.cpp#1805-1808) that talks about re-use of *RDpP* instances, so it seems likely?
  - if *no* we need to continue investigations how we got there.
    - there are [places in the code](https://searchfox.org/mozilla-central/rev/5e15e00fa247cba5b765727496619bf9010ed162/dom/serviceworkers/ServiceWorkerManager.cpp#1683) where it seems we explicitly do not want this to happen, instead
- Is it expected that we still see a living `ControlledClientData` in `ServiceWorkerManagermControlledClients` for a given client id after we apparently already unregistered the service worker?
- Can't we directly use the already present registration info from `ControlledClientData` reducing the number of indirect (and error prone) lookups here?
  - I also assume the code could benefit from some wrapper lookup functions that handle well all possible `nullptr` constellations rather than having all this manual double lookups?
Revision 3 by
on 
My high-level understanding is that we get somehow in a situation where:

**Context**
- we have a valid `ServiceWorkerManager::RegistrationDataPerPrincipal` entry *RDpP* in `ServiceWorkerManager::mRegistrationInfos` for a given scope
  - but *RDpP* contains an empty list `mInfos` and it is not (yet) clear to me if this is an expected state and how we got here

**Crash path**
- we enter `ServiceWorkerRegistrationInfo::MaybeCheckNavigationUpdate` and find a living `ControlledClientData` that contains a living strong reference `mRegistrationInfo` referring to the same scope as *RDpP* above and we call `MaybeScheduleUpdate` on it
  - we reach the navigation fault threshold inside `ServiceWorkerRegistrationInfo::MaybeScheduleUpdate`
  - this triggers a `CheckQuotaUsage` on the same registration info
    - there we take the `mPrincipal` and `Scope()` of this registratiuon info to call `swm->CheckPrincipalQuotaUsage`
    - Inside `ServiceWorkerManager::CheckPrincipalQuotaUsage` we use this scope to first lookup the (existing) *RDpP* from above and then the registration info that we expect to find in `mInfo` of *RDpP*. But that is empty and therefore we have a `nullptr`

**Questions**
- Should we ever be in the configuration to have a living but empty * RDpP* for a given scope?
  - if *yes* we must deal with this possibility in `CheckPrincipalQuotaUsage` (and maybe other places)
    - there is a [comment here](https://searchfox.org/mozilla-central/rev/fb8d77331582639ea6848a61dd8ee812fac31b77/dom/serviceworkers/ServiceWorkerManager.cpp#1805-1808) that talks about re-use of *RDpP* instances, so it seems likely?
  - if *no* we need to continue investigations how we got there.
    - there are [places in the code](https://searchfox.org/mozilla-central/rev/5e15e00fa247cba5b765727496619bf9010ed162/dom/serviceworkers/ServiceWorkerManager.cpp#1683) where it seems we explicitly do not want this to happen, instead
- Is it expected that we still see a living `ControlledClientData` in `ServiceWorkerManagerControlledClients` for a given client id after we apparently already unregistered the service worker?
- Can't we directly use the already present registration info from `ControlledClientData` reducing the number of indirect (and error prone) lookups here?
  - I also assume the code could benefit from some wrapper lookup functions that handle well all possible `nullptr` constellations rather than having all this manual double lookups?
Revision 4 by
on 
My high-level understanding is that we get somehow in a situation where:

**Context**
- we have a valid `ServiceWorkerManager::RegistrationDataPerPrincipal` entry *RDpP* in `ServiceWorkerManager::mRegistrationInfos` for a given scope
  - but *RDpP* contains an empty list `mInfos` and it is not (yet) clear to me if this is an expected state and how we got here

**Crash path**
- we enter `ServiceWorkerRegistrationInfo::MaybeCheckNavigationUpdate` and find a living `ControlledClientData` that contains a living strong reference `mRegistrationInfo` referring to the same scope as *RDpP* above and we call `MaybeScheduleUpdate` on it
  - we reach the navigation fault threshold inside `ServiceWorkerRegistrationInfo::MaybeScheduleUpdate`
  - this triggers a `CheckQuotaUsage` on the same registration info
    - there we take the `mPrincipal` and `Scope()` of this registratiuon info to call `swm->CheckPrincipalQuotaUsage`
    - Inside `ServiceWorkerManager::CheckPrincipalQuotaUsage` we use this scope to first lookup the (existing) *RDpP* from above and then the registration info that we expect to find in `mInfo` of *RDpP*. But that is empty and therefore we have a `nullptr`

**Questions**
- Should we ever be in the configuration to have a living but empty * RDpP* for a given scope?
  - if *yes* we must deal with this possibility in `CheckPrincipalQuotaUsage` (and maybe other places)
    - there is a [comment here](https://searchfox.org/mozilla-central/rev/fb8d77331582639ea6848a61dd8ee812fac31b77/dom/serviceworkers/ServiceWorkerManager.cpp#1805-1808) that talks about re-use of *RDpP* instances, so it seems likely?
  - if *no* we need to continue investigations how we got there.
    - there are [places in the code](https://searchfox.org/mozilla-central/rev/5e15e00fa247cba5b765727496619bf9010ed162/dom/serviceworkers/ServiceWorkerManager.cpp#1683) where it seems we explicitly do not want this to happen, instead
- Is it expected that we still see a living `ControlledClientData` in `ServiceWorkerManager::mControlledClients` for a given client id after we apparently already unregistered the service worker?
- Can't we directly use the already present registration info from `ControlledClientData` reducing the number of indirect (and error prone) lookups here?
  - I also assume the code could benefit from some wrapper lookup functions that handle well all possible `nullptr` constellations rather than having all this manual double lookups?
Revision 5 by
on 
My high-level understanding is that we get somehow in a situation where:

**Context**
- we have a valid `ServiceWorkerManager::RegistrationDataPerPrincipal` entry *RDpP* in `ServiceWorkerManager::mRegistrationInfos` for a given scope
  - but *RDpP* contains an empty list `mInfos` and it is not (yet) clear to me if this is an expected state and how we got here

**Crash path**
- we enter `ServiceWorkerRegistrationInfo::MaybeCheckNavigationUpdate` and find a living `ControlledClientData` that contains a living strong reference `mRegistrationInfo` referring to the same scope as *RDpP* above and we call `MaybeScheduleUpdate` on it
  - we reach the navigation fault threshold inside `ServiceWorkerRegistrationInfo::MaybeScheduleUpdate`
  - this triggers a `CheckQuotaUsage` on the same registration info
    - there we take the `mPrincipal` and `Scope()` of this registratiuon info to call `swm->CheckPrincipalQuotaUsage`
    - Inside `ServiceWorkerManager::CheckPrincipalQuotaUsage` we use this scope to first lookup the (existing) *RDpP* from above and then the registration info that we expect to find in `mInfo` of *RDpP*. But that is empty and therefore we have a `nullptr`

**Questions**
- Should we ever be in the configuration to have a living but empty * RDpP* for a given scope?
  - if *yes* we must deal with this possibility in `CheckPrincipalQuotaUsage` (and maybe other places)
    - there is a [comment here](https://searchfox.org/mozilla-central/rev/fb8d77331582639ea6848a61dd8ee812fac31b77/dom/serviceworkers/ServiceWorkerManager.cpp#1805-1808) that talks about re-use of *RDpP* instances, so it seems likely?
  - if *no* we need to continue investigations how we got there.
    - there are [places in the code](https://searchfox.org/mozilla-central/rev/5e15e00fa247cba5b765727496619bf9010ed162/dom/serviceworkers/ServiceWorkerManager.cpp#1683) where it seems we explicitly do not want this to happen, instead
- Is it expected that we still see a living `ControlledClientData` in `ServiceWorkerManager::mControlledClients` for a given client id after we apparently already unregistered the service worker?
- Can't we directly use the already present registration info from `ControlledClientData` reducing the number of indirect (and error prone) lookups here?
  - I also assume the code could benefit from some wrapper lookup functions that handle well all possible `nullptr` constellations rather than having all these manual double lookups?
Revision 6 by
on 
My high-level understanding is that we get somehow in a situation where:

**Context**
- we have a valid `ServiceWorkerManager::RegistrationDataPerPrincipal` entry *RDpP* in `ServiceWorkerManager::mRegistrationInfos` for a given scope
  - but *RDpP* contains an empty list `mInfos` and it is not (yet) clear to me if this is an expected state and how we got here

**Crash path**
- we enter `ServiceWorkerRegistrationInfo::MaybeCheckNavigationUpdate` and find a living `ControlledClientData` that contains a living strong reference `mRegistrationInfo` referring to the same scope as *RDpP* above and we call `MaybeScheduleUpdate` on it
  - we reach the navigation fault threshold inside `ServiceWorkerRegistrationInfo::MaybeScheduleUpdate`
  - this triggers a `CheckQuotaUsage` on the same registration info
    - there we take the `mPrincipal` and `Scope()` of this registratiuon info to call `swm->CheckPrincipalQuotaUsage`
    - Inside `ServiceWorkerManager::CheckPrincipalQuotaUsage` we use this scope to first lookup the (existing) *RDpP* from above and then the registration info that we expect to find in `mInfo` of *RDpP*. But that is empty and therefore we have a `nullptr`

**Questions**
- Should we ever be in the configuration to have a living but empty *RDpP* for a given scope?
  - if *yes* we must deal with this possibility in `CheckPrincipalQuotaUsage` (and maybe other places)
    - there is a [comment here](https://searchfox.org/mozilla-central/rev/fb8d77331582639ea6848a61dd8ee812fac31b77/dom/serviceworkers/ServiceWorkerManager.cpp#1805-1808) that talks about re-use of *RDpP* instances, so it seems likely?
  - if *no* we need to continue investigations how we got there.
    - there are [places in the code](https://searchfox.org/mozilla-central/rev/5e15e00fa247cba5b765727496619bf9010ed162/dom/serviceworkers/ServiceWorkerManager.cpp#1683) where it seems we explicitly do not want this to happen, instead
- Is it expected that we still see a living `ControlledClientData` in `ServiceWorkerManager::mControlledClients` for a given client id after we apparently already unregistered the service worker?
- Can't we directly use the already present registration info from `ControlledClientData` reducing the number of indirect (and error prone) lookups here?
  - I also assume the code could benefit from some wrapper lookup functions that handle well all possible `nullptr` constellations rather than having all these manual double lookups?
Revision 7 by
on 
My high-level understanding is that we get somehow in a situation where:

**Context**
- we have a valid `ServiceWorkerManager::RegistrationDataPerPrincipal` entry *RDpP* in `ServiceWorkerManager::mRegistrationInfos` for a given scope
  - but *RDpP* contains an empty list `mInfos` and it is not (yet) clear to me if this is an expected state and how we got here

**Crash path**
- we enter `ServiceWorkerRegistrationInfo::MaybeCheckNavigationUpdate` and find a living `ControlledClientData` that contains a living strong reference `mRegistrationInfo` referring to the same scope as *RDpP* above and we call `MaybeScheduleUpdate` on it
  - we reach the navigation fault threshold inside `ServiceWorkerRegistrationInfo::MaybeScheduleUpdate`
  - this triggers a `CheckQuotaUsage` on the same registration info
    - there we take the `mPrincipal` and `Scope()` of this registration info to call `swm->CheckPrincipalQuotaUsage`
    - Inside `ServiceWorkerManager::CheckPrincipalQuotaUsage` we use this scope to first lookup the (existing) *RDpP* from above and then the registration info that we expect to find in `mInfo` of *RDpP*. But that is empty and therefore we have a `nullptr`

**Questions**
- Should we ever be in the configuration to have a living but empty *RDpP* for a given scope?
  - if *yes* we must deal with this possibility in `CheckPrincipalQuotaUsage` (and maybe other places)
    - there is a [comment here](https://searchfox.org/mozilla-central/rev/fb8d77331582639ea6848a61dd8ee812fac31b77/dom/serviceworkers/ServiceWorkerManager.cpp#1805-1808) that talks about re-use of *RDpP* instances, so it seems likely?
  - if *no* we need to continue investigations how we got there.
    - there are [places in the code](https://searchfox.org/mozilla-central/rev/5e15e00fa247cba5b765727496619bf9010ed162/dom/serviceworkers/ServiceWorkerManager.cpp#1683) where it seems we explicitly do not want this to happen, instead
- Is it expected that we still see a living `ControlledClientData` in `ServiceWorkerManager::mControlledClients` for a given client id after we apparently already unregistered the service worker?
- Can't we directly use the already present registration info from `ControlledClientData` reducing the number of indirect (and error prone) lookups here?
  - I also assume the code could benefit from some wrapper lookup functions that handle well all possible `nullptr` constellations rather than having all these manual double lookups?

Back to Bug 1740551 Comment 10