Closed Bug 778300 Opened 12 years ago Closed 12 years ago

Clock App alarm doesn't sound when run OOP

Categories

(Core :: Audio/Video, defect)

Product:
Core
Component:
Audio/Video
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED DUPLICATE of bug 784274
Project Flags:
blocking-kilimanjaro +
blocking-basecamp +

People

(Reporter: dhylands, Assigned: cjones)

References

Details

(Whiteboard: [LOE:M]) 

If I set an alarm shortly in the future and stay in the clock app, then the alarm doesn't sound if the clock app is run OOP.

Instead, I see the following errors in logcat:

[JavaScript Error: "Exposing chrome JS objects to content without __exposedProps__ is insecure and deprecated. See https://developer.mozilla.org/en/XPConnect_wrappers for more information." {file: "app://clock.b2g.hylands.org/js/alarm.js" line: 387}]

and

[JavaScript Error: "redeclaration of const ContentPanning" {file: "chrome://global/content/BrowserElementChild.js" line: 647}]

I was running on otoro.
blocking-basecamp: --- → ?
Maybe a problem of system messages / activities or the alarm API?

I think there might be a dup of this, not sure though.
Hi Ian,

(sorry for the wrong typing in the previous comment)

According the the log, it seems there might be something wrong in the Gaia end. Is that possible for you to replicate this issue? I'm glad to work with you to figure out the root cause at any time.
blocking-basecamp: ? → +
blocking-kilimanjaro: --- → +
Hi Gene,
I cannot reproduce the issue again on otoro or S2 with newest version of gaia/gecko.
But there is still JavaScript Error log with no *_exposedProps_*.

Hi Dave,
I don't see the log about *JavaScript Error: "redeclaration of const ContentPanning"*.
Could you help to check the newest version of gaia/gecko?
And the alarm work fine on it.

Thanks.
(In reply to Dave Hylands [:dhylands] from comment #0)
> If I set an alarm shortly in the future and stay in the clock app, then the
> alarm doesn't sound if the clock app is run OOP.
> 
> Instead, I see the following errors in logcat:
> 
> [JavaScript Error: "Exposing chrome JS objects to content without
> __exposedProps__ is insecure and deprecated. See
> https://developer.mozilla.org/en/XPConnect_wrappers for more information."
> {file: "app://clock.b2g.hylands.org/js/alarm.js" line: 387}]

Hi Fabrice,

Based on Ivan's experiments, this error occurs when the app attempts to access the returned system message in the handler. However, this error doesn't stop anything. We can still retrieve values from the system message.

Do you think we need to add __exposedProps__ in the System Message API or Alarm API to avoid this type of error? I think adding it in the System Message API seems more reasonable because the App should be able to read any types of system messages returned from platform no matter it comes from Alarm API or not.

> 
> and
> 
> [JavaScript Error: "redeclaration of const ContentPanning" {file:
> "chrome://global/content/BrowserElementChild.js" line: 647}]
> 
> I was running on otoro.

Hi Dave,

As mentioned by Ian, the Alarm-Clock works well on our Otoro phones and we didn't see the above-mentioned error in our logs. I'm wondering it's due to other issues instead of the Alarm-Clock. I'd appreciate if you could please check out the latest B2G (both Gaia and Gecko) and try it again? The latest codes work for us.
(In reply to Gene Lian [:gene] from comment #5) 
> Based on Ivan's experiments, this error occurs when the app attempts to
> access the returned system message in the handler. However, this error
> doesn't stop anything. We can still retrieve values from the system message.

Yes, this is a warning for now.

> Do you think we need to add __exposedProps__ in the System Message API or
> Alarm API to avoid this type of error? I think adding it in the System
> Message API seems more reasonable because the App should be able to read any
> types of system messages returned from platform no matter it comes from
> Alarm API or not.

We need to fix that properly. My guess is that this comes from the JS object that is sent back from the Alarm API to the System Message API. Can you try adding the __exposedProps__ there?
(In reply to Gene Lian [:gene] from comment #5)
> (In reply to Dave Hylands [:dhylands] from comment #0)
> > If I set an alarm shortly in the future and stay in the clock app, then the
> > alarm doesn't sound if the clock app is run OOP.
> > 
> > Instead, I see the following errors in logcat:
> > 
> > [JavaScript Error: "Exposing chrome JS objects to content without
> > __exposedProps__ is insecure and deprecated. See
> > https://developer.mozilla.org/en/XPConnect_wrappers for more information."
> > {file: "app://clock.b2g.hylands.org/js/alarm.js" line: 387}]
> 
> Hi Fabrice,
> 
> Based on Ivan's experiments, this error occurs when the app attempts to
> access the returned system message in the handler. However, this error
> doesn't stop anything. We can still retrieve values from the system message.
> 
> Do you think we need to add __exposedProps__ in the System Message API or
> Alarm API to avoid this type of error? I think adding it in the System
> Message API seems more reasonable because the App should be able to read any
> types of system messages returned from platform no matter it comes from
> Alarm API or not.
> 
> > 
> > and
> > 
> > [JavaScript Error: "redeclaration of const ContentPanning" {file:
> > "chrome://global/content/BrowserElementChild.js" line: 647}]
> > 
> > I was running on otoro.
> 
> Hi Dave,
> 
> As mentioned by Ian, the Alarm-Clock works well on our Otoro phones and we
> didn't see the above-mentioned error in our logs. I'm wondering it's due to
> other issues instead of the Alarm-Clock. I'd appreciate if you could please
> check out the latest B2G (both Gaia and Gecko) and try it again? The latest
> codes work for us.

I also don't see the above errors if I run on the otoro as-is. This is because the Clock app is currently black-listed from running OOP.

If, however, you edit apps/system/js/window_manager.js and comment out the 'Clock', line around line 388, this makes Clock run OOP. When Clock is running OOP, then I see the messages indicated, and the alarm doesn't sound.

I reconfirmed with the latest gecko/gaia fetched today.
(In reply to Dave Hylands [:dhylands] from comment #7)
> (In reply to Gene Lian [:gene] from comment #5)
> I also don't see the above errors if I run on the otoro as-is. This is
> because the Clock app is currently black-listed from running OOP.
> 
> If, however, you edit apps/system/js/window_manager.js and comment out the
> 'Clock', line around line 388, this makes Clock run OOP. When Clock is
> running OOP, then I see the messages indicated, and the alarm doesn't sound.
> 
> I reconfirmed with the latest gecko/gaia fetched today.

Hi Dave,

Got it! Thanks for clarifying this :D We'll try to figure out the root cause. Btw, it seems there are also lots of apps (almost all...) included in the OOP black-list. Do you have any idea that do all these bugs come from the same root cause or they could be very different case by case?
Actually, about 50% of the apps aren't listed (it used to be a white-list). Once over 50% were working under OOP, we swapped it and changed it to a black-list.

The root cause for each of them seems to be slightly different, although the media apps (camera, music, video) are all failing due to DeviceStorage.

Most of the time, it's because the app is running on a different process from the service it's trying to use, and the code is doing stuff that doesn't work when the caller/callee are in different processes.
I had some progresses to narrow down this problem. I think both Alarm API and System Message API work fine. The root cause is: the AlarmManager.init() in the alarm.js would run twice, thus registering the system message handler twice. When an alarm message is fired, some internal errors would occur, which is not what we expected.

Ian, I think it's more like an issue on the Gaia's end, could your please look into this when you're available?
I try to remove Clock App from the OOP list.
It will let Clock App be launched twice.
So, there are two callbacks per each event.
(Such as 'localized', 'load', 'DOMContentLoaded').
It's should be platform issue for multiple process.
Hi Dave and Cjones,

Do you have any idea why the Clock app would be initialized twice when running on OOP? I'm sure both the Alarm API and System Message API were launched only one time per navigator but the Clock app seems to be created and initialized with 2 instances (please see more at https://github.com/mozilla-b2g/gaia/blob/master/apps/clock/js/alarm.js). Any quick thoughts are highly appreciated.
Nothing comes to mind.  What code is being called twice?  What code calls the initialization code?
Whiteboard: [LOE:M]
Assignee: nobody → clian
(In reply to Chris Jones [:cjones] [:warhammer] from comment #13)
> Nothing comes to mind.  What code is being called twice?  What code calls
> the initialization code?

After investigating more, I think the root cause is not the double initialization issue (sorry I misunderstood this because console.log() would print twice for both child/parent processes but it's actually processed once).

The root cause seems to be the window.open() when running OOP. The App need that to open a pop-up page to notify users when alarm goes off. However, it doesn't work properly, which would even crash the app: https://github.com/mozilla-b2g/gaia/blob/master/apps/clock/js/alarm.js#L433. Although Bug 776129 seems to solve that, it still doesn't work for this issue.

If we temporally remove the open of pop-up page and just use log to display the alarm-firing results, everything works well, which proves the Alarm API and System Message API should be fine when running OOP (alarm can be set and fired across processes).

:cjones and :jlebar, may I have your comments or suggestions regarding this?
Depends on: 776129
Is there a particular reason we're using window.open for the alert dialog?  What does the dialer application do when it receives an incoming call?
So...is the exposedProps issue solved?  That's a real issue.  exposedProps is mandatory now, and I don't believe it's going to be backed out.

Then there's a separate issue with window.open:

> However, it doesn't work properly, which would even crash the app: https://github.com
> /mozilla-b2g/gaia/blob/master/apps/clock/js/alarm.js#L433. Although Bug 776129 seems to 
> solve that, it still doesn't work for this issue.

Can you please clarify what the failure mode is, after bug 776129?

Does the content process crash?  Does the main b2g process crash?  Can you get a stack trace?  Does the window merely not open?  Do you see any errors in logcat?  Is the failure reproducible on desktop builds, or only on the device?
On my desktop build, when I make the clock app OOP and set an alarm, I see

[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIAlarmHalService.setAlarm]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/AlarmService.jsm :: <TOP_LEVEL> :: line 71"  data: no]

Then the alarm never seems to fire.
On

changeset:   102904:f9a8fdb08193
tag:         tip
user:        Eric Chou <echou@mozilla.com>
date:        Tue Aug 21 14:24:05 2012 +0800
summary:     Bug 784255 - [b2g-bluetooth] devicefound event does not come up correctly, r=qdot

I don't see any crashes or exceptions on device, but I still don't hear audio from the notification :/.
Assignee: clian → nobody
Component: General → Video/Audio
Product: Boot2Gecko → Core
Around the time we try to play the audio, I see

W/AudioFlinger( 1658): Thread AudioOut_1 cannot connect to the power manager service

and also an error about parsing "AudioFilter.csv".

Any ideas why the audio backend might behave differently in-process vs. OOP now that both are trying to connect to mediaserver?
And relatedly, why the audio data API would work (for Dialer), but <audio> here wouldn't?
(In reply to Justin Lebar [:jlebar] from comment #17)

Hi Justin! Appreciate your concerns on this! :)

> So...is the exposedProps issue solved?  That's a real issue.  exposedProps
> is mandatory now, and I don't believe it's going to be backed out.

Yes, this one has been fixed (by Fabrice) so it's no longer an issue now. ;)

> 
> Then there's a separate issue with window.open:
> 
> > However, it doesn't work properly, which would even crash the app: https://github.com
> > /mozilla-b2g/gaia/blob/master/apps/clock/js/alarm.js#L433. Although Bug 776129 seems to 
> > solve that, it still doesn't work for this issue.
> 
> Can you please clarify what the failure mode is, after bug 776129?
> 
> Does the content process crash?  Does the main b2g process crash?  Can you
> get a stack trace?  Does the window merely not open?  Do you see any errors
> in logcat?  Is the failure reproducible on desktop builds, or only on the
> device?

I'm using Galaxy-S2-ICS to test everything so far (currently the Alarm API is only available in gonk-specific platforms). The OOP issue is very complicated here. I tried to summarize them as below:

1. First of all, the pop-up page should vibrate and play an audio when firing the alarm. However, I cannot hear any sound and the pop-up page would reboot the device when pushing the close button. No matter it would reboot or not, at least we're sure there must be something wrong when playing audio in OOP.

2. To further narrow it down, I tried to disable the vibration and audio logics to test the pop-up page only. The pop-up page seems to work well. However, after closing it (the content process is still alive so far), we use the editing page to set another alarm and go back to the main Clock page. At this moment, the content process will die immediately (b2g process it still alive).

3. Finally, I found out the root cause of crashing the content process is due to the following statement in the Clock app (alarm.js):

  navigator.mozSettings.getLock().set({'alarm.enabled': hasAlarmEnabled});

If we comment it out, then everything works well ;)

4. However, the most interesting thing is the above-mentioned root cause only happens when the pop-up page used to be shown and closed. That is, if we don't attempt to open the pop-up window then navigator.mozSettings.getLock().set still works. That's why I was wondering if there could be an implicit relation between navigator.mozSettings and window.open (very weird I know...).

I'm afraid there might be lots of OOP issues concurrently exist in the Clock-Alarm App. What I can help to confirm so far is Alarm API and System Message API should be working. We probably need the Gaia's guy to support clarify which APIs are not properly working and point them out (I'm not a real expert to examine all the Clock App logic, which really crashes me ^^").
(In reply to Chris Jones [:cjones] [:warhammer] from comment #20)
> Around the time we try to play the audio, I see
> 
> W/AudioFlinger( 1658): Thread AudioOut_1 cannot connect to the power manager
> service
> 
> and also an error about parsing "AudioFilter.csv".
> 

These warnings are expected.
> 3. Finally, I found out the root cause of crashing the content process is due to the 
> following statement in the Clock app (alarm.js):
> 
>   navigator.mozSettings.getLock().set({'alarm.enabled': hasAlarmEnabled});

If this is causing the content process to crash, I think the next step is to get a stacktrace.  I haven't had much luck attaching GDB to content processes on the phone, so my recommendation would be to try to attach GDB on the desktop, if you can.  See [1].

We pre-load app processes, so you probably want to attach GDB to the first child process you see.  When you load your app, you'll see another child process start up, but that's probably not the process you want.

[1] https://wiki.mozilla.org/Electrolysis/Debugging#To_debug_child_processes_only
Or disable preload, which is a pref.
Let's file a separate bug for the mozSettings crash.

This bug should cover audio not playing back from content processes, which is a very different bit of work.
Gene, can you take this?
Assignee: nobody → clian
It would be better for someone on the media team to look.  I'll assign to myself for now.

Gene, when you file a bug for the mozSettings crash, if you want to investigate that'd be great!

Media guys, this is the root of the tree of bugs for "non-audio-data audio doesn't work in content processes on gonk", complementing the "video doesn't work" companion bug.
Assignee: clian → jones.chris.g
(In reply to Chris Jones [:cjones] [:warhammer] from comment #28)
> It would be better for someone on the media team to look.  I'll assign to
> myself for now.

Thanks for dispatching this one to the experts, who should be able to investigate the media bugs in a more efficient way.

> 
> Gene, when you file a bug for the mozSettings crash, if you want to
> investigate that'd be great!

Yes! I'm looking at this one. Fire another Bug 784903 assigned to myself. Sorry for the delay.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → DUPLICATE
I thought I had filed a bug for this, but the time display in the Clock when OOP used to be completely garbled, and the "ON" text in the alarm-enable checkbox didn't display.

Apparently both were fixed by bug 783184.  (The display was so different from last time I tested I had to manually |kill| the process to verify it was OOP.)
Gene, perhaps bug 785233 is what you were referring to in comment 22?
(In reply to Chris Jones [:cjones] [:warhammer] from comment #32)
> Gene, perhaps bug 785233 is what you were referring to in comment 22?

Exactly! I'll take that. :)
You need to log in before you can comment on or make changes to this bug.