Closed Bug 1089778 Opened 5 years ago Closed 5 years ago

ServiceWorkerRegistration should be keyed by scope for event dispatch and invalidation

Categories

(Core :: DOM: Workers, defect)

36 Branch
x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla38

People

(Reporter: nsm, Assigned: nsm)

References

Details

Attachments

(1 file)

When the code was adapted from ServiceWorkerContainer, ServiceWorkerRegistrations continued to be keyed on the URI of the document owning the ServiceWorkerRegistration. ServiceWorkerRegistrations should be keyed by scope instead when firing events such as updatefound and invalidating their ServiceWorker instances.
Assignee: nobody → nsm.nikhil
Status: NEW → ASSIGNED
Comment on attachment 8512136 [details] [diff] [review]
ServiceWorkerRegistration is keyed by scope for event dispatch and invalidation

Review of attachment 8512136 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/workers/ServiceWorkerManager.cpp
@@ +1785,2 @@
>    AssertIsOnMainThread();
> +  nsCString scope = NS_ConvertUTF16toUTF8(aScope);

nsAutoCString

@@ +1807,5 @@
>    ServiceWorkerRegistration* registration = static_cast<ServiceWorkerRegistration*>(aListener);
>    MOZ_ASSERT(!domainInfo->mServiceWorkerRegistrations.Contains(registration));
> +#ifdef DEBUG
> +  // Ensure a registration is only listening for it's own scope.
> +  nsString regScope;

nsAutoString

@@ +1822,3 @@
>  {
>    AssertIsOnMainThread();
> +  nsCString scope = NS_ConvertUTF16toUTF8(aScope);

nsAutoCString

@@ +1830,5 @@
>    ServiceWorkerRegistration* registration = static_cast<ServiceWorkerRegistration*>(aListener);
>    MOZ_ASSERT(domainInfo->mServiceWorkerRegistrations.Contains(registration));
> +#ifdef DEBUG
> +  // Ensure a registration is unregistering for it's own scope.
> +  nsString regScope;

nsAutoString

@@ +1852,5 @@
>    if (domainInfo) {
>      nsTObserverArray<ServiceWorkerRegistration*>::ForwardIterator it(domainInfo->mServiceWorkerRegistrations);
>      while (it.HasMore()) {
>        nsRefPtr<ServiceWorkerRegistration> target = it.GetNext();
> +      nsString regScope;

nsAutoString

@@ +1856,5 @@
> +      nsString regScope;
> +      target->GetScope(regScope);
> +      MOZ_ASSERT(!regScope.IsEmpty());
> +
> +      nsCString utf8Scope = NS_ConvertUTF16toUTF8(regScope);

NS_ConvertUTF16toUTF8 utf8Scope(regScope);

@@ +1859,5 @@
> +
> +      nsCString utf8Scope = NS_ConvertUTF16toUTF8(regScope);
> +      if (utf8Scope.Equals(aRegistration->mScope)) {
> +        nsresult rv = target->DispatchTrustedEvent(aName);
> +        if (NS_WARN_IF(NS_FAILED(rv))) {

then just do: NS_WARN_IF(NS_FAILED(rv));

@@ +2256,5 @@
>    if (domainInfo) {
>      nsTObserverArray<ServiceWorkerRegistration*>::ForwardIterator it(domainInfo->mServiceWorkerRegistrations);
>      while (it.HasMore()) {
>        nsRefPtr<ServiceWorkerRegistration> target = it.GetNext();
> +      nsString regScope;

nsAutoString

@@ +2263,3 @@
>  
> +      nsCString utf8Scope = NS_ConvertUTF16toUTF8(regScope);
> +

NS_ConvertUTF16toUTF8 utf8Scope(regScope);
Attachment #8512136 - Flags: review?(amarchesini) → review+
https://hg.mozilla.org/mozilla-central/rev/180805605d91
https://hg.mozilla.org/mozilla-central/rev/4a3216caf471
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.