Closed
Bug 1178233
Opened 9 years ago
Closed 9 years ago
[non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode
Categories
(Core :: DOM: Service Workers, defect)
Core
DOM: Service Workers
Tracking
()
RESOLVED
FIXED
FxOS-S3 (24Jul)
People
(Reporter: noemi, Assigned: jaoo)
References
Details
Attachments
(2 files)
Hi,
scenario checked with today's (6/29) master build in Nightly Debug (eaf4f9b45117 revision)
STRs:
1- register a sw
2- open about:sw in another tab (the sw is properly shown)
3- change the sw
3- click on the "update" button within about:sw tab
Actual result:
nothing happens
Expected result:
The update process is properly initiated and completed
Note: you can run step 1 and then step 2 or step 2 and then step 1 the result is exactly the same
Reporter | ||
Updated•9 years ago
|
Summary: [non-e10s] Update process within about:serviceworkers doesn't work in non-e10s mode → [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode
Updated•9 years ago
|
tracking-e10s:
--- → -
Assignee | ||
Comment 1•9 years ago
|
||
I'm suffering this in the tests I'm adding on bug 1171917. If I'm not wrong this is because about:sw is chrome process and when propagating the update the ServiceWorkerManagerService object doesn't notify the update because of the check at [1]. Andrea, is that correct? Thanks.
[1] https://mxr.mozilla.org/mozilla-central/source/dom/workers/ServiceWorkerManagerService.cpp#170
Flags: needinfo?(amarchesini)
Comment 2•9 years ago
|
||
> https://mxr.mozilla.org/mozilla-central/source/dom/workers/
> ServiceWorkerManagerService.cpp#170
This check means that we don't dispatch a notification for update at the parent who triggered it.
Why do you think this is wrong?
Flags: needinfo?(amarchesini)
Assignee | ||
Comment 3•9 years ago
|
||
(In reply to Andrea Marchesini (:baku) from comment #2)
> > https://mxr.mozilla.org/mozilla-central/source/dom/workers/
> > ServiceWorkerManagerService.cpp#170
>
> This check means that we don't dispatch a notification for update at the
> parent who triggered it.
> Why do you think this is wrong?
Because the update is not notified and the service worker is never updated.
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → jaoo
Blocks: nga-toolkit-service-workers, 1171917
Target Milestone: --- → FxOS-S2 (10Jul)
Assignee | ||
Comment 4•9 years ago
|
||
Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. r=baku
Attachment #8628809 -
Flags: review?(amarchesini)
Assignee | ||
Comment 5•9 years ago
|
||
Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. Test. r=baku
Attachment #8628810 -
Flags: review?(amarchesini)
Updated•9 years ago
|
Attachment #8628810 -
Flags: review?(amarchesini) → review+
Comment 6•9 years ago
|
||
Comment on attachment 8628810 [details]
MozReview Request: Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. Test. r=baku
https://reviewboard.mozilla.org/r/12477/#review10923
Ship It!
::: dom/workers/test/serviceworkers/test_aboutserviceworkers.html:45
(Diff revision 1)
> +
extra spaces
Updated•9 years ago
|
Attachment #8628809 -
Flags: review?(amarchesini)
Comment 7•9 years ago
|
||
Comment on attachment 8628809 [details]
MozReview Request: Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. r=baku
https://reviewboard.mozilla.org/r/12475/#review10921
::: dom/workers/ServiceWorkerManager.cpp:4714
(Diff revision 1)
> + }
I think you just need to do:
SoftUpdate(aOriginAttributes, NS_ConvertUTF16toUTF8(aScope));
mActor->SendPropagateSoftUpdate(...);
Assignee | ||
Comment 8•9 years ago
|
||
Comment on attachment 8628809 [details]
MozReview Request: Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. r=baku
Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. r=baku
Attachment #8628809 -
Flags: review?(amarchesini)
Assignee | ||
Updated•9 years ago
|
Attachment #8628810 -
Flags: review+ → review?(amarchesini)
Assignee | ||
Comment 9•9 years ago
|
||
Comment on attachment 8628810 [details]
MozReview Request: Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. Test. r=baku
Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. Test. r=baku
Assignee | ||
Comment 10•9 years ago
|
||
Comment on attachment 8628810 [details]
MozReview Request: Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. Test. r=baku
Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. Test. r=baku
Attachment #8628810 -
Flags: review?(amarchesini)
Assignee | ||
Updated•9 years ago
|
Status: NEW → ASSIGNED
Updated•9 years ago
|
Attachment #8628810 -
Flags: review+
Comment 11•9 years ago
|
||
Comment on attachment 8628810 [details]
MozReview Request: Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. Test. r=baku
https://reviewboard.mozilla.org/r/12477/#review11065
Ship It!
Assignee | ||
Comment 12•9 years ago
|
||
Comment on attachment 8628809 [details]
MozReview Request: Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. r=baku
Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. r=baku
Assignee | ||
Comment 13•9 years ago
|
||
Comment on attachment 8628810 [details]
MozReview Request: Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. Test. r=baku
Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. Test. r=baku
Attachment #8628810 -
Flags: review+
Comment 14•9 years ago
|
||
Comment on attachment 8628809 [details]
MozReview Request: Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. r=baku
https://reviewboard.mozilla.org/r/12475/#review11405
Ship It!
Attachment #8628809 -
Flags: review?(amarchesini) → review+
Assignee | ||
Comment 15•9 years ago
|
||
Comment 16•9 years ago
|
||
Assignee | ||
Comment 17•9 years ago
|
||
(In reply to Pulsebot from comment #16)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/2db7f8ad8c8f
> https://hg.mozilla.org/integration/mozilla-inbound/rev/ab0df10f86e2
The patches landed here will be backed out as I've just noticed an issue. The tests will pass but a new issue will be added. :tomcat will back them out. Sorry for the noise.
Comment 18•9 years ago
|
||
Comment 19•9 years ago
|
||
(In reply to José Antonio Olivera Ortega [:jaoo] from comment #17)
> (In reply to Pulsebot from comment #16)
> > https://hg.mozilla.org/integration/mozilla-inbound/rev/2db7f8ad8c8f
> > https://hg.mozilla.org/integration/mozilla-inbound/rev/ab0df10f86e2
>
> The patches landed here will be backed out as I've just noticed an issue.
> The tests will pass but a new issue will be added. :tomcat will back them
> out. Sorry for the noise.
np, backed out now from mozilla-inbound
Assignee | ||
Comment 20•9 years ago
|
||
Comment on attachment 8628809 [details]
MozReview Request: Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. r=baku
Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. r=baku
Attachment #8628809 -
Flags: review+ → review?(amarchesini)
Assignee | ||
Comment 21•9 years ago
|
||
Comment on attachment 8628810 [details]
MozReview Request: Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. Test. r=baku
Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. Test. r=baku
Assignee | ||
Comment 22•9 years ago
|
||
https://reviewboard.mozilla.org/r/12475/#review11497
::: dom/workers/ServiceWorkerManagerService.cpp:169
(Diff revision 4)
>
Andrea, I've changed the check from the child to the parent. It seems the right approach (IIRC you said something similar during our IRC discussion yesterday). The previous version of it was backed out because I wanted to double check whether everything was fine). This version fixes the issue reported in this bug and doesn't add anything wrong ;).
Comment 23•9 years ago
|
||
Hey Jaoo, seems one test also problems like https://treeherder.mozilla.org/logviewer.html#?job_id=11519983&repo=mozilla-inbound - that is also in the try run here
Assignee | ||
Comment 24•9 years ago
|
||
Comment on attachment 8628809 [details]
MozReview Request: Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. r=baku
Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. r=baku
Assignee | ||
Updated•9 years ago
|
Attachment #8628810 -
Flags: review?(amarchesini)
Assignee | ||
Comment 25•9 years ago
|
||
Comment on attachment 8628810 [details]
MozReview Request: Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. Test. r=baku
Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. Test. r=baku
Assignee | ||
Comment 26•9 years ago
|
||
(In reply to Carsten Book [:Tomcat] from comment #23)
> Hey Jaoo, seems one test also problems like
> https://treeherder.mozilla.org/logviewer.html#?job_id=11519983&repo=mozilla-
> inbound - that is also in the try run here
Thanks, there was a race condition I should be fixed now. Waiting a try to verify that.
Assignee | ||
Comment 27•9 years ago
|
||
(In reply to José Antonio Olivera Ortega [:jaoo] from comment #26)
> (In reply to Carsten Book [:Tomcat] from comment #23)
> > Hey Jaoo, seems one test also problems like
> > https://treeherder.mozilla.org/logviewer.html#?job_id=11519983&repo=mozilla-
> > inbound - that is also in the try run here
>
> Thanks, there was a race condition I should be fixed now. Waiting a try to
> verify that.
Green! https://treeherder.mozilla.org/#/jobs?repo=try&revision=a4d398733383
Reporter | ||
Updated•9 years ago
|
Target Milestone: FxOS-S2 (10Jul) → FxOS-S3 (24Jul)
Assignee | ||
Comment 28•9 years ago
|
||
Comment on attachment 8628809 [details]
MozReview Request: Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. r=baku
Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. r=baku
Assignee | ||
Comment 29•9 years ago
|
||
Comment on attachment 8628810 [details]
MozReview Request: Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. Test. r=baku
Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. Test. r=baku
Updated•9 years ago
|
Attachment #8628809 -
Flags: review?(amarchesini) → review+
Comment 30•9 years ago
|
||
Comment on attachment 8628809 [details]
MozReview Request: Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. r=baku
https://reviewboard.mozilla.org/r/12475/#review11575
Ship It!
::: dom/workers/ServiceWorkerManagerService.h:56
(Diff revision 5)
> + uint32_t AgentsCount();
const
::: dom/workers/ServiceWorkerManagerService.cpp:174
(Diff revision 6)
> - } else {
> + data->mParentFound = data->mParentFound || parent->ID() == data->mParentID;
if (parent->ID() == data->mParentID) {
data->mParentFound = true;
}
Comment 31•9 years ago
|
||
Comment on attachment 8628810 [details]
MozReview Request: Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. Test. r=baku
https://reviewboard.mozilla.org/r/12477/#review11717
Ship It!
Attachment #8628810 -
Flags: review?(amarchesini) → review+
Comment 32•9 years ago
|
||
Reporter | ||
Comment 33•9 years ago
|
||
Hi,
just checked scenario in comment 0 with m-i build in Nightly Debug (7b876dcebaeb revision) and the update process is properly launched. Once this patch lands in m-c the update process will be checked both in desktop and b2g. Thanks!
https://hg.mozilla.org/mozilla-central/rev/c05f3df663bf
https://hg.mozilla.org/mozilla-central/rev/7b876dcebaeb
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 35•9 years ago
|
||
Hi,
just checked with m-c Nightly Debug (c95ebeebbc5d revision) and the update process is launched under a non-e10s scenario although the activate event is fired even when the sw is controlling the client, which shouldn't. It seems we should wait for SWM refactor (Bug 1182117)
You need to log in
before you can comment on or make changes to this bug.
Description
•