If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

nsPicoService is instantiated in content processes as well.

RESOLVED FIXED in mozilla27

Status

()

Core
DOM
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: eeejay, Assigned: eeejay)

Tracking

unspecified
mozilla27
All
Gonk (Firefox OS)
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

4 years ago
It should only be in the top-level gecko process.
(Assignee)

Comment 1

4 years ago
Created attachment 807941 [details] [diff] [review]
Have nsPicoService only start in main gecko process.
Attachment #807941 - Flags: review?(bugs)

Comment 2

4 years ago
Hmm, I was told on IRC when I reviewed the original patch that app-startup doesn't happen in content processes.
(Assignee)

Comment 3

4 years ago
I can confirm that it does :P

Comment 4

4 years ago
Comment on attachment 807941 [details] [diff] [review]
Have nsPicoService only start in main gecko process.

Also, shouldn't we make sure the service isn't ever started in child processes?
Could you add some process type check to the service creation?
Attachment #807941 - Flags: review?(bugs) → review-
For Push, I used a component [1] to ensure parent process only startup. I don't know if the PicoService is similar to a JSM, but that concept might be useful. Surprisingly, components seem to respond to app-startup only in the parent on b2g. I don't really know why.

[1] https://mxr.mozilla.org/mozilla-central/source/dom/push/src/PushServiceLauncher.js
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=871372#c5
(Assignee)

Comment 6

4 years ago
(In reply to Nikhil Marathe [:nsm] from comment #5)
> For Push, I used a component [1] to ensure parent process only startup. I
> don't know if the PicoService is similar to a JSM, but that concept might be
> useful. Surprisingly, components seem to respond to app-startup only in the
> parent on b2g. I don't really know why.
> 
> [1]
> https://mxr.mozilla.org/mozilla-central/source/dom/push/src/
> PushServiceLauncher.js
> [2] https://bugzilla.mozilla.org/show_bug.cgi?id=871372#c5

I took some inspiration from the bluetooth service that uses profile-after-change. It  seesm to only happen on the main process. You don't think that is a good option?

Comment 7

4 years ago
If some code explicitly calls getService(...speech-contract-id) on child process, the
call should fail, IMO.
(Assignee)

Comment 8

4 years ago
Created attachment 808699 [details] [diff] [review]
Have nsPicoService only start in main gecko process.

Added an assert here, is this what you meant? From my local testing this patch does the trick. But I Would love to be certain.
Attachment #807941 - Attachment is obsolete: true
Attachment #808699 - Flags: feedback?(bugs)
(Assignee)

Comment 9

4 years ago
Gregor, does this patch work for you?
Flags: needinfo?(anygregor)
Comment on attachment 808699 [details] [diff] [review]
Have nsPicoService only start in main gecko process.

I would return null there if the process type is wrong.
Attachment #808699 - Flags: feedback?(bugs)
(Assignee)

Comment 11

4 years ago
Created attachment 808736 [details] [diff] [review]
Have nsPicoService only start in main gecko process.
Attachment #808699 - Attachment is obsolete: true
Attachment #808736 - Flags: review?(bugs)
Comment on attachment 808736 [details] [diff] [review]
Have nsPicoService only start in main gecko process.

Don't you want XRE_GetProcessType() != GeckoProcessType_Default
Attachment #808736 - Flags: review?(bugs) → review+
I still see following with this patch:
F/MOZ_Assert( 1283): Assertion failure: false (nsPicoService can only be started on main gecko process), at ../../../../../../content/media/webspeech/synth/pico/nsPicoService.cpp:703
Flags: needinfo?(anygregor)
And it gets stuck in an endless loop and we can't start the phone.
(Assignee)

Comment 15

4 years ago
Oops, I didn't upload the right patch.
(Assignee)

Comment 16

4 years ago
Created attachment 809586 [details] [diff] [review]
Have nsPicoService only start in main gecko process. r=smaug
(Assignee)

Updated

4 years ago
Attachment #808736 - Attachment is obsolete: true
(Assignee)

Comment 17

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a76652f60ecb
Assignee: nobody → eitan
(Assignee)

Comment 18

4 years ago
Tested on an unagi device locally
https://hg.mozilla.org/mozilla-central/rev/a76652f60ecb
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.