Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi] Reporter
	Description • 9 years ago

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

	Dimi Lee [:dimi] Assignee
	Comment 1 • 9 years ago

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

	Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi] Reporter
	Comment 2 • 9 years ago

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.

	Ben Kelly [:bkelly, not reviewing]
	Comment 4 • 9 years ago

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.

	Dimi Lee [:dimi] Assignee
	Comment 5 • 9 years ago

Address ben's comment

	Dimi Lee [:dimi] Assignee
	Comment 6 • 9 years ago

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

I am figuring out the root cause.

	Dimi Lee [:dimi] Assignee
	Comment 7 • 9 years ago

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

	Dimi Lee [:dimi] Assignee
	Comment 8 • 9 years ago

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

	Pulsebot
	Comment 9 • 9 years ago

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

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 10 • 9 years ago

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

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 11 • 9 years ago

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

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 12 • 9 years ago

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

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 13 • 9 years ago

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

	Ryan VanderMeulen [:RyanVM]
	Comment 14 • 9 years ago

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