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)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
FxOS-S3 (24Jul)
Tracking Status
e10s - ---
firefox42 --- fixed

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
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
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)
See Also: → 1171917
> 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)
(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: nobody → jaoo
Target Milestone: --- → FxOS-S2 (10Jul)
Bug 1178233 - [non-e10s] The update process doesn't work within about:serviceworkers in non-e10s mode. r=baku
Attachment #8628809 - Flags: review?(amarchesini)
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)
Attachment #8628810 - Flags: review?(amarchesini) → review+
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
Attachment #8628809 - Flags: review?(amarchesini)
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(...);
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)
Attachment #8628810 - Flags: review+ → review?(amarchesini)
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
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)
Status: NEW → ASSIGNED
Attachment #8628810 - Flags: review+
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!
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
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 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+
(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.
(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
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)
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
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 ;).
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
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 #8628810 - Flags: review?(amarchesini)
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
(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.
(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
Target Milestone: FxOS-S2 (10Jul) → FxOS-S3 (24Jul)
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
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 #8628809 - Flags: review?(amarchesini) → review+
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 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+
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!
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)
Depends on: 1200906
Depends on: 1200912
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: