Closed Bug 1029979 Opened 6 years ago Closed 6 years ago

JavascriptException: TypeError: Argument 1 of AudioContext.constructor 'null'

Categories

(Firefox OS Graveyard :: AudioChannel, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:2.0+, b2g-v2.0 fixed, b2g-v2.1 fixed)

RESOLVED FIXED
2.0 S6 (18july)
blocking-b2g 2.0+
Tracking Status
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: anshulj, Assigned: etienne)

References

Details

Attachments

(2 files, 2 obsolete files)

We are often seeing an exception "JavascriptException: TypeError: Argument 1 of AudioContext.constructor 'null' is not a valid value for enumeration AudioChannel" while running voice call related marionette tests.

I don't have a definitive STRs but in general below is what a test script is doing.

1. Place an outgoing call
2. Place an incoming call
3. Answer the incoming call
4. Hangup all the calls
blocking-b2g: 2.0? → 2.0+
Assign to Star, he'll back in office in 6/30
Assignee: nobody → scheng
If the Argument of AudioContext() is null, it will be set 'normal' audio channel. It's checked in Gecko layer. So it doesn't cause system damage. So I don't think this bug should be set as blocked.
Flags: needinfo?(hochang)
(In reply to Anshul from comment #0)
> We are often seeing an exception "JavascriptException: TypeError: Argument 1
> of AudioContext.constructor 'null' is not a valid value for enumeration
> AudioChannel" while running voice call related marionette tests.
> 
> I don't have a definitive STRs but in general below is what a test script is
> doing.
> 
> 1. Place an outgoing call
> 2. Place an incoming call
> 3. Answer the incoming call
> 4. Hangup all the calls


Hi, 

Could you help to try to figure out the failure marionette tests. It is easy to check the Argument of AudioContext().
Flags: needinfo?(anshulj)
ni Ivan for triage Comment 2, thanks!
Flags: needinfo?(hochang) → needinfo?(itsay)
(In reply to Star Cheng [:scheng] from comment #3)
> (In reply to Anshul from comment #0)
> > We are often seeing an exception "JavascriptException: TypeError: Argument 1
> > of AudioContext.constructor 'null' is not a valid value for enumeration
> > AudioChannel" while running voice call related marionette tests.
> > 
> > I don't have a definitive STRs but in general below is what a test script is
> > doing.
> > 
> > 1. Place an outgoing call
> > 2. Place an incoming call
> > 3. Answer the incoming call
> > 4. Hangup all the calls
> 
> 
> Hi, 
> 
> Could you help to try to figure out the failure marionette tests. It is easy
> to check the Argument of AudioContext().

Could you please be more specific on what you want me to do?
Flags: needinfo?(anshulj)
(In reply to Anshul from comment #5)
> (In reply to Star Cheng [:scheng] from comment #3)
> > (In reply to Anshul from comment #0)
> > > We are often seeing an exception "JavascriptException: TypeError: Argument 1
> > > of AudioContext.constructor 'null' is not a valid value for enumeration
> > > AudioChannel" while running voice call related marionette tests.
> > > 
> > > I don't have a definitive STRs but in general below is what a test script is
> > > doing.
> > > 
> > > 1. Place an outgoing call
> > > 2. Place an incoming call
> > > 3. Answer the incoming call
> > > 4. Hangup all the calls
> > 
> > 
> > Hi, 
> > 
> > Could you help to try to figure out the failure marionette tests. It is easy
> > to check the Argument of AudioContext().
> 
> Could you please be more specific on what you want me to do?

Hi, Hsin-Yi

Do you know any marionette test about the STR?
Flags: needinfo?(htsai)
(In reply to Star Cheng [:scheng] from comment #6)
> (In reply to Anshul from comment #5)
> > (In reply to Star Cheng [:scheng] from comment #3)
> > > (In reply to Anshul from comment #0)
> > > > We are often seeing an exception "JavascriptException: TypeError: Argument 1
> > > > of AudioContext.constructor 'null' is not a valid value for enumeration
> > > > AudioChannel" while running voice call related marionette tests.
> > > > 
> > > > I don't have a definitive STRs but in general below is what a test script is
> > > > doing.
> > > > 
> > > > 1. Place an outgoing call
> > > > 2. Place an incoming call
> > > > 3. Answer the incoming call
> > > > 4. Hangup all the calls
> > > 
> > > 
> > > Hi, 
> > > 
> > > Could you help to try to figure out the failure marionette tests. It is easy
> > > to check the Argument of AudioContext().
> > 
> > Could you please be more specific on what you want me to do?
> 
> Hi, Hsin-Yi
> 
> Do you know any marionette test about the STR?

Sorry I have no idea about Star's comment.

I also looked at log captured during gecko telephony marionette tests (not sure if they are what Anshul talked about) but didn't see this error.
Flags: needinfo?(htsai)
I also don't think this is the blocker unless the exception causes the crash of the phone. It looks to me that the exception is handled in gecko as the response in comment 2 by Star. For further analysis, we also need a clear STR or the log for analysis. Backlog it for now.

Also ni? Star to clarify what he needs from our partner.
blocking-b2g: 2.0+ → backlog
Flags: needinfo?(itsay) → needinfo?(scheng)
(In reply to Ivan Tsay (:ITsay) from comment #8)
> I also don't think this is the blocker unless the exception causes the crash
> of the phone. It looks to me that the exception is handled in gecko as the
> response in comment 2 by Star. For further analysis, we also need a clear
> STR or the log for analysis. Backlog it for now.
> 
> Also ni? Star to clarify what he needs from our partner.

I am trying to find the marionette test which probably cause "JavascriptException: TypeError" in marionette folder. But I can't find it. If there is the marionette test which cause the "TypeError", it should be easy to analysis.
Flags: needinfo?(scheng)
Hi, Etienne

Could you help me to clarify the following question: 

In tone_player.js file, if we use .trashAudio() and then use .ensureAudio() in certain code flow. Does it cause to new AudioContext() constructor as "null"? Or this code flow is impossible.
Flags: needinfo?(etienne)
(In reply to Ivan Tsay (:ITsay) from comment #8)
> I also don't think this is the blocker unless the exception causes the crash
> of the phone. It looks to me that the exception is handled in gecko as the
> response in comment 2 by Star. For further analysis, we also need a clear
> STR or the log for analysis. Backlog it for now.
STRs are clearly mentioned in comment #0. This crash doesn't happen every time so running it once or twice might not be enough. We are seeing it 1 out of 10 times in our nightly run. Please find the logs attached. 


> Also ni? Star to clarify what he needs from our partner.
Attached file audio context exception (obsolete) —
Attachment #8449168 - Attachment is obsolete: true
(In reply to Star Cheng [:scheng] from comment #10)
> Hi, Etienne
> 
> Could you help me to clarify the following question: 
> 
> In tone_player.js file, if we use .trashAudio() and then use .ensureAudio()
> in certain code flow. Does it cause to new AudioContext() constructor as
> "null"? Or this code flow is impossible.

I think it's possible :/
I'll wip up a patch so we can confirm this.
Flags: needinfo?(etienne)
Here's a wip, with no tests yet.
But can we confirm this fixes the issue? (without causing new ones ;))
Attachment #8450095 - Flags: feedback?(scheng)
(In reply to Etienne Segonzac (:etienne) from comment #15)
> Created attachment 8450095 [details] [diff] [review]
> 0001-Bug-1029979-WIP-fix-bad-audio-context-constructor.patch
> 
> Here's a wip, with no tests yet.
> But can we confirm this fixes the issue? (without causing new ones ;))

Look good to me. 
Maybe we can push the patch to TPBL and re-trigger it more than 10 times to verify. If there is no marionette error, it should get better.
Comment on attachment 8450095 [details] [diff] [review]
0001-Bug-1029979-WIP-fix-bad-audio-context-constructor.patch


It looks good to me.
Attachment #8450095 - Flags: feedback?(scheng) → feedback+
(In reply to Star Cheng [:scheng] from comment #16)
> Look good to me. 
> Maybe we can push the patch to TPBL and re-trigger it more than 10 times to
> verify. If there is no marionette error, it should get better.

Yes we definitely should!
Which suite should we run?
see question in Comment 18
Flags: needinfo?(anshulj)
Sure, I will try the patch on Monday and get back to you guys.
Flags: needinfo?(anshulj)
Changing the blocking flag, given this is crash is possible to hit after trying the STR and something we should fix if we can. Looks like Etienne has a patch for anshul already.
blocking-b2g: backlog → 2.0+
Comment on attachment 8450095 [details] [diff] [review]
0001-Bug-1029979-WIP-fix-bad-audio-context-constructor.patch

Review of attachment 8450095 [details] [diff] [review]:
-----------------------------------------------------------------

This patch doesn't seem to fix the issue. I am still seeing the exact same issue. I did confirm that the fix was properly pushed to the device.
Attachment #8450095 - Flags: feedback+ → feedback-
(In reply to Etienne Segonzac (:etienne) from comment #18)
> (In reply to Star Cheng [:scheng] from comment #16)
> > Look good to me. 
> > Maybe we can push the patch to TPBL and re-trigger it more than 10 times to
> > verify. If there is no marionette error, it should get better.
> 
> Yes we definitely should!
> Which suite should we run?

Maybe we can use this try chooser option for marionette test. "try: -b o -p emulator -u marionette-webapi -t none" and re-trigger 10 times to check if there is any error.
Hi Etienne,

According to comment 22, the patch didn't seem to work. Could you investigate more to see if there is any other possible causes? Star can help with answering Gecko questions so please feel free to ask.

Thanks.
Flags: needinfo?(etienne)
Testing a new patch, it's on try
https://tbpl.mozilla.org/?tree=Try&rev=3e75098f3c7b
Flags: needinfo?(etienne)
The first run went well, I've triggered 15 more.
The try looks good, looks like the only errors are known intermittents.
I'll send a patch for review today,
(In reply to Etienne Segonzac (:etienne) from comment #27)
> The try looks good, looks like the only errors are known intermittents.
> I'll send a patch for review today,

Thanks , Etienne!
Attached file Gaia PR
I know you love this code :)
Assignee: scheng → etienne
Attachment #8450095 - Attachment is obsolete: true
Attachment #8455369 - Flags: review?(gsvelto)
Comment on attachment 8455369 [details] [review]
Gaia PR

Awww, it actually took me a while to figure out what was going on. This LGTM but I'd like a better commit comment to explain why this change was made. Something like "prevent successive trash/ensureAudio() calls from creating an AudioContext with a null channel".
Attachment #8455369 - Flags: review?(gsvelto) → review+
https://github.com/mozilla-b2g/gaia/commit/f31f95ec18882decc43c0e2f02df581580a32dc8
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Flags: in-moztrap?(ychung)
The bug involves a Marionette JavaScript test. Unable to create test case
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ychung)
Flags: needinfo?(ychung) → needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Flags: in-moztrap?(ychung)
Flags: in-moztrap-
You need to log in before you can comment on or make changes to this bug.