1191647 - Listen to clear-origin-data in ServiceWorkerManager.cpp

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, defect)

Description

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

We should replace webapps-clear-data with clear-origin-data.

Comment 1

[Dimi Lee [:dimi]]

Attachment: Patch v1

This patch change "webapps-clear-data" to "clear-origin-data"

Comment 2

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 8645574 [details] [diff] [review]
Patch v1

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

::: dom/workers/ServiceWorkerManager.cpp
@@ +78,1 @@
>  using namespace mozilla;

#include "mozIApplicationClearPrivateDataParams.h" should be removed.

@@ +4505,1 @@
>  UnregisterIfMatchesClearPrivateDataParams(const nsACString& aScope,

This could be renamed to a shorter name,

@@ +4509,5 @@
>    UnregisterIfMatchesUserData* data =
>      static_cast<UnregisterIfMatchesUserData*>(aPtr);
>  
>    if (data->mUserData) {
> +    OriginAttributes *params = static_cast<OriginAttributes*>(data->mUserData);

OriginAttributes* params

@@ +4514,5 @@
>      MOZ_ASSERT(params);
>      MOZ_ASSERT(aReg->mPrincipal);
>  
> +    uint32_t appId = params->mAppId;
> +    bool browserOnly = params->mInBrowser;

I didn't understand how service-worker work on this part, 
but in general we should use 
OriginAttributes A == OriginAttributes B,
or using nsIPrincipal.equals/subsumes.

Comment 3

[Dimi Lee [:dimi]]

Attachment: Patch v2

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8646211 [details] [diff] [review]
Patch v2

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

r=me with comments addressed.  Thanks.

::: dom/workers/ServiceWorkerManager.cpp
@@ +4507,3 @@
>  {
>    UnregisterIfMatchesUserData* data =
>      static_cast<UnregisterIfMatchesUserData*>(aPtr);

A bit unrelated to your patch, but can you add MOZ_ASSERT(data) here?

@@ +4513,1 @@
>      MOZ_ASSERT(params);

Please add a MOZ_ASSERT(aReg) here.  And this assert can go because its inside the if(data->mUserData).

@@ +4840,3 @@
>      MOZ_ASSERT(XRE_IsParentProcess());
> +    OriginAttributes attrs;
> +    attrs.Init(nsAutoString(aData));

Please check this return value.  If we think this can never fail then MOZ_ALWAYS_TRUE() should be fine.

Comment 5

[Dimi Lee [:dimi]]

Attachment: Patch V3

Address ben's comment

Comment 6

[Dimi Lee [:dimi]]

There is error when running try.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5eee4a87e143

I am figuring out the root cause.

Comment 7

[Dimi Lee [:dimi]]

Attachment: Patch v4

- Rebase to latest code
- Fix try error by using MOZ_ALWAYS_TRUE

Comment 8

[Dimi Lee [:dimi]]

try result of patch v4 :
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8530df874b99

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ee8d28602bc2

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ee8d28602bc2

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/43f5881032ae for apparently breaking all of the builds like https://treeherder.mozilla.org/logviewer.html#?job_id=13062845&repo=mozilla-inbound

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Looks like it was a different bug in that checkin-needed push that caused the bustage. This can probably reland as-is.

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/60b0b9b99e16

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/60b0b9b99e16