Closed Bug 1043762 Opened 5 years ago Closed 5 years ago

Volume buttons don't do anything in Settings and previewing new ringtones

Categories

(Firefox OS Graveyard :: Gaia::Ringtones, defect, P1)

x86
macOS
defect

Tracking

(blocking-b2g:2.0+, b2g-v1.4 wontfix, b2g-v2.0 verified, b2g-v2.0M verified, b2g-v2.1 verified, b2g-v2.2 verified)

RESOLVED FIXED
2.1 S8 (7Nov)
blocking-b2g 2.0+
Tracking Status
b2g-v1.4 --- wontfix
b2g-v2.0 --- verified
b2g-v2.0M --- verified
b2g-v2.1 --- verified
b2g-v2.2 --- verified

People

(Reporter: pdehaan, Assigned: salva)

References

Details

(Whiteboard: [caf priority: p2][CR 744388][2.0-319MB-bug-bash] [LibGLA,TD93605,QE1,B][TAM-triage?] )

Attachments

(3 files, 1 obsolete file)

Found in 512MB Flame w/ B2G 2.0 and 2014-07-24 User build.

### Steps to reproduce:
1. Launch Settings > Sound.
2. Select Ringer > Change.
3. Preview various different ringtones.
4. Holy crap that's loud and I'm in an office, use the volume buttons on the phone to try and make the preview quieter.


### Actual results:
The volume buttons on the phone have no effect. Your only choice is to drop the volume to 0 and thus mute it altogether, or awkwardly try and take the cover off the phone and do a battery pull.
Whiteboard: [2.0-319MB-bug-bash]
I checked today's 1.4 on Buri: same thing.
Hi Alive

May be you would elaborate more on it.

thanks!
Flags: needinfo?(alive)
Sounds a bug
Flags: needinfo?(alive) → needinfo?(dkuo)
Just now I have a quick test and found while previewing the ringtones, we are adjusting the Notifications & Ringer volume(this is correct) but the real volume was bind to the Media, it should be a bug and need to check how the Ringtones app set the mozAudioChannelType, or maybe a platform issue again...
Flags: needinfo?(dkuo)
Whiteboard: [2.0-319MB-bug-bash] → [2.0-319MB-bug-bash] [LibGLA,TD93605,QE1,B]
need your feedback on this..
Flags: needinfo?(squibblyflabbetydoo)
Shawn,

Can anyone from taipei check this?
Flags: needinfo?(sku)
(In reply to Dominic Kuo [:dkuo] from comment #4)
> Just now I have a quick test and found while previewing the ringtones, we
> are adjusting the Notifications & Ringer volume(this is correct) but the
> real volume was bind to the Media, it should be a bug and need to check how
> the Ringtones app set the mozAudioChannelType, or maybe a platform issue
> again...

Here's the relevant code in the ringtones app: https://github.com/mozilla-b2g/gaia/blob/master/apps/ringtones/js/tone_player.js#L126

No idea why things don't work as expected...
Flags: needinfo?(squibblyflabbetydoo)
Hi Viral:
 Please help check if key code do pass to Gecko/Gaia.

Hi Wayne:
 Please help check if audio part works normally (Loop Star if needed).

Thanks!!
Shawn
Flags: needinfo?(waychen)
Flags: needinfo?(vwang)
key event looks normal.
Also we can mute the ringtone by several times volume down.
Looks like we can't control the volume now.
Flags: needinfo?(vwang)
- Output 2 dump:
 Sampling rate: 44100
 Format: 1
 Channels: 00000003
 Latency: 96
 Flags 00000002
 Devices 00000002
 Stream volume refCount muteCount
 00     1.000     00       00       -> Telephony
 01     0.290     01       00       -> Normal
 02     0.078     00       00       -> Ringer
 03     0.425     00       00
 04     1.000     00       00
 05     0.078     00       00
 06     -1.000     00       00
 07     0.501     00       00
 08     0.000     00       00
 09     0.000     00       00
 10     1.000     00       00


To check the stream type reference count. The "Normal" audio channel is used instead of "Ringer".
Flags: needinfo?(waychen)
ni? Star for tracking issue.
Flags: needinfo?(scheng)
Hi, Paul

I add some log to check the stream type for previewing ringtone in settings.

I/Gecko - MSG( 1201): [star] mAudioChannel 0 stream_type 1  @GraphicDriver.cpp
I/Cubeb_OpenSL( 1201): [star] stream_type 1                 @Cubeb_opensl.c

I saw the audio channel type is 0 (Normal channel type) and the stream is 1 (system stream type).

Can we set the AudioChannelType (@GraphicDriver.cpp) for webAudio node (AudioContext) which mentioned in comment 7? If we can't, do you have any suggestion for set the correct AudioChannelType for it. 

Thanks
Flags: needinfo?(scheng) → needinfo?(paul)
Why is this code using an AudioContext _at all_ ?!

It should just be using an <audio> element.
Flags: needinfo?(paul)
AudioContext is used here because we don't want it to be mixed with some other tones.
Let's say music app is running in background, so then as soon as we enter ringtone app we will stop any background app, once we are done with ringtone app then the background sound is restored..
Well, this does not work anymore since the MSG refactoring landed, and was a bad hack that killed battery anyways.

Can't you just loop the ringtone to keep the channel busy ? Or keep playing a muted <audio> ? At least this won't make a super-high priority thread do crazy stuff. Those are still hacks, but AudioContext are really expensive.
Jim, would be the best person to answer this. You may put a needinfo to him.
Jim, any thoughts on comment 15 ?
Flags: needinfo?(squibblyflabbetydoo)
Since ppl are checking/discussing already, clear ni? for myself.
Flags: needinfo?(sku)
No, we shouldn't loop the ringtone. Then it'd be hard to tell when the ringtone actually ends (especially important for notifications, which generally don't loop when they're fired for real).

I don't think it particularly matters that AudioContexts are expensive; it's not like the ringtones app is doing a lot of other performance-intensive stuff. While I'm sure it does drain battery life faster, the ringtones app won't even be open for very long (maybe a minute or so, tops), so it doesn't matter too much in the long run.

The real problem is just that it's broken when it comes to the volume buttons, which should be fixed regardless of whether the ringtone app uses an AudioContext. In the long run, we'll probably be able to fix this with the new audio competition policies in Gaia, but I don't think there's been much progress there.

(Also, we'll probably need to start looking at how to make AudioContext less expensive in the future, since the music app will want to use it for things like EQ, ReplayGain, gapless playback, etc.)
Flags: needinfo?(squibblyflabbetydoo)
Yes, that's planned, at a spec level.
If it's impossible to fix AudioContext (or the audio competition policies) in the necessary timeframe, but we need to fix this bug quickly, I'd probably rather we just get rid of the AudioContext stuff and let background audio intrude when you're not previewing a ringtone. It's moderately annoying, but I really don't think it'll happen that often.
Hi Wayne

Greetings!

Please look into this & assign to someone.
Also mark this as backlog.

Thanks

Ankit
Flags: needinfo?(wchang)
[Blocking Requested - why for this release]:

It sounds like there is no way for a user to preview a ringtone at suitable volume levels with this bug. We should have that corrected or users will always have to hide in a room when changing ringtones.
blocking-b2g: --- → 2.0?
Flags: needinfo?(wchang)
Hi Jim -

Per Comment#23, we are nominating this one as 2.0 blocker, do you think you are the one that can help to move this bug forward?

Thanks

Vance
Flags: needinfo?(squibblyflabbetydoo)
No, not unless people who know the audio channel code are willing to help walk me through it a bit. (This might be a good idea though, since I think we could use more people who know about it.)
Flags: needinfo?(squibblyflabbetydoo)
Hi Wayne, we looked at this bug in triage and are not sure how this meets the blocking criteria, especially for 2.0. Can it ride the trains like a normal bug fix? We've shipped every release to date with this problem.

It should definitely be fixed, but every landing on older branches is risk of new problems, and more delays.
Flags: needinfo?(wchang)
Whiteboard: [2.0-319MB-bug-bash] [LibGLA,TD93605,QE1,B] → [2.0-319MB-bug-bash] [LibGLA,TD93605,QE1,B][TAM-triage?]
Hi Dietrich,

I nominated from a user perspective since we'll be launching some high end devices in to new markets, user expectation may be higher. But happy to leave this to triage decision.

Will discuss/work with partner on possible solution if they need something sooner.
Flags: needinfo?(wchang)
However we should have this fixed as soon as possible, 2.1 maybe?
Moving to right component. Hoping to get some help from Eric Chou
Component: Gaia::Ringtones → Video/Audio
Product: Firefox OS → Core
Adding qawanted for branch checks.
Keywords: qawanted
This bug repro's on Flame KK builds: Flame 2.2, Flame 2.1, Flame 2.0, Flame 2.0 Base, OpenC 2.2

Actual Results: Unable to lower the ringer preview volume when the audio is playing. Only mute is possible.

Repro Rate: 5/5

Environmental Variables:
Device: Flame Master
BuildID: 20140917212258
Gaia: d37950eb09e28aa18d0e01df9ff90574bd4337e0
Gecko: 426497473505
Version: 35.0a1 (Master) 
Firmware Version: L1TC10011800
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0
-----------------------------------------------------------------
Environmental Variables:
Device: Flame 2.1 KK
BuildID: 20140918091821
Gaia: b0b0ab8e0de5d1150feaf76f63f437edf695f15f
Gecko: 904483d05623
Version: 34.0a2
Firmware Version: L1TC10011800
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
-----------------------------------------------------------------
Environmental Variables:
Device: Flame 2.0
BuildID: 20140918082321
Gaia: 31434a3949556171f3565ca47ac2b44e810e95e6
Gecko: 5cf783171d5c
Version: 32.0 (2.0) 
Firmware Version: L1TC10011800
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
-----------------------------------------------------------------
Environmental Variables:
Device: Flame 2.0 KK Base
BuildID: 20140904160718
Gaia: 506da297098326c671523707caae6eaba7e718da
Gecko: 
Version: 32.0 (2.0)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
-----------------------------------------------------------------
Environmental Variables:
Device: Open_C 2.2
BuildID: 20140917212258
Gaia: d37950eb09e28aa18d0e01df9ff90574bd4337e0
Gecko: 426497473505
Version: 35.0a1 (2.2)
Firmware: P821A10v1.0.0B06_LOG_DL
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Keywords: qawanted
QA Contact: croesch
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
Triage result: 2.0+

Hi Start, could you keep checking this one?

Thanks
blocking-b2g: 2.0? → 2.0+
Flags: needinfo?(scheng)
(In reply to Vance Chen [:vchen][vchen@mozilla.com] from comment #32)
> Triage result: 2.0+
> 
> Hi Start, could you keep checking this one?
> 
> Thanks

Hi, Vance

According to above discussion, the conclusion is to use Audio element instead of AudioContext for Settings app. I am afraid I can't provide any suggestion and help for that in audio playback.
Flags: needinfo?(scheng)
Hi Keven, based on the triage yesterday, we reach consensus to set it to 2.0+.

However based on comment33, it seemed we need gaia team to help us analyze the issue about settings.
Could you please find the right person for this ?
Thank you very much !
Flags: needinfo?(kkuo)
Hi! Dominic,

According to comment 33, it looks like a regression of bug 958470.
Could you help on this case? Thanks

--
Keven
Flags: needinfo?(kkuo) → needinfo?(dkuo)
Attached patch Mozv0.patchSplinter Review
Hello All

I did this & it worked. Seems like some issue with Audiocontext.

Thanks
Flags: needinfo?(squibblyflabbetydoo)
I found if we are receiving a call and then I lower the volume, the ringer is totally muted. Is this intended?

ankit, your solution is weird because is precisely that what it's done now when setting the exclusive mode. I think we have deeper causes. I'm digging in.
Assignee: nobody → salva
(In reply to Salvador de la Puente González [:salva] from comment #37)
> I found if we are receiving a call and then I lower the volume, the ringer
> is totally muted. Is this intended?

Yep. Thanks to Beatriz for the offline clarification. It's intended as specified in bug 890434 comment 7.
Attachment #8495976 - Flags: review?(dflanagan)
I took the solution from comment 36 as a workaround. Before merging, I would :baku or :mchen to take a look into the back-end because mozAudioChannelType seems to have some problem. If I replace [1] by:

> this._context = new AudioContext();
> this._context.mozAudioChannelType = 'ringer';

Then it does not work neither. :baku, can you tell me what is the difference between explicitly passing the channel to the constructor against setting just after instantiating the object?

[1] https://github.com/mozilla-b2g/gaia/pull/24476/files#diff-ab8292963acfb37c1ca5512bf1c1c657R18
Flags: needinfo?(amarchesini)
Ok... unfortunately, debugging this issue I found 2 problems:

1. var a = new AudioContext('ringer') and
   var a = new AudioContext(); a.setMozAudioChannelType('ringer');

are not the same as they should. The reason is because,
in the  constructor of AudioContext we create a MediaStreamGraph with an AudioChannel ('normal' in the former case, 'ringer' in the latter). From there we create a MediaStream object. At this point we initialize the backend system (cube on gonk... correct?)

When we set MozAudioChannelType, we change the audiochannel in stream object, but this doesn't change the AudioChannel in the graph, and in the backend system (cube).

2. https://mxr.mozilla.org/mozilla-central/source/content/media/MediaStreamGraph.cpp#2781
When we create an AudioContext, this creates an AudioDestinationNode, and here we create a MediaStreamGraph using MediaStreamGraph::GetInstance().
GetInstance() wants an AudioChannel and it creates a 'gGraph' object, just the first time. This means that if we do:

new AudioContext('ringer'); new AudioContext('normal'); probably the second AudioContext uses the same gGraph, and the same backend, as 'ringer'.

I have to investigate both issues, but I think I'm right for what I read so far.

... but about this particular NI, yes, the workaround is ok.

I need to know who can I ping for asking info about Cube backend... can you give me a name? Thanks!
Flags: needinfo?(amarchesini)
Depends on: 1073615
Comment on attachment 8495976 [details] [review]
Passing 'ringer' as channel for the context.

Passing the review to Jim who owns the ringtones module and who has been following this bug more than I have.

One comment, though: is it possible to resolve this bug using something with lower priority than 'ringer'?   If we use ringer for the ringtone preview, what happens if a call comes in while the user is previewing ringtones?
Attachment #8495976 - Flags: review?(dflanagan) → review?(squibblyflabbetydoo)
(In reply to Keven Kuo [:kkuo] from comment #35)
> Hi! Dominic,
> 
> According to comment 33, it looks like a regression of bug 958470.
> Could you help on this case? Thanks
> 
> --
> Keven

We used audio context to replace audio element intentionally at that time to fix the audio competing issue, and it works fine at that time so probably it's not a regression.
Flags: needinfo?(dkuo)
Priority: -- → P1
Comment on attachment 8495976 [details] [review]
Passing 'ringer' as channel for the context.

Sorry, but in light of bug 1074757, this isn't sufficient, and in any case, it only fixes some of the issues. For instance, playing two ringtones in a row causes the AudioContext to get confused and let the music app start playing again.
Attachment #8495976 - Flags: review?(squibblyflabbetydoo) → review-
Flags: needinfo?(squibblyflabbetydoo)
I think the right way to fix this would be to create a new AudioContext and hook it up when we start playing the first ringtone, and then delete the AudioContext when the page is hidden to let other apps start playing music again.
Hi Ankit, 

Per comment42, 44 and 45, would you mind providing the revised patch ? Thanks!
Flags: needinfo?(ankit93040)
Trying solution in comment 45...
(In reply to Jim Porter (:squib) from comment #45)
> I think the right way to fix this would be to create a new AudioContext and
> hook it up when we start playing the first ringtone, and then delete the
> AudioContext when the page is hidden to let other apps start playing music
> again.

Sorry but I'm not able to reproduce this behavior. If I'm playing music and play two ringtones in a row. When playing the first, the music is stopped and when playing the second the music keeps muted.
Attachment #8495976 - Attachment is obsolete: true
Hi Rachelle

I will give it a try.

Thanks
Flags: needinfo?(ankit93040)
(In reply to ankit93040 from comment #50)
> Hi Rachelle
> 
> I will give it a try.
> 
> Thanks

Before starting, could you review my patch. It's providing what you request and I already performed part of the research with Baku.
Attachment #8499487 - Flags: feedback?(ankit93040)
Hi Jim , Could you please help us review the patch in comment 49 ?
Thank you very much.
Flags: needinfo?(squibblyflabbetydoo)
Comment on attachment 8499487 [details] [review]
Now destroying AudioContext when hidding the app and regenerating when showing again

I already reviewed it on Github. r-'ing here for now.
Attachment #8499487 - Flags: review?(squibblyflabbetydoo) → review-
Flags: needinfo?(squibblyflabbetydoo)
Duplicate of this bug: 993293
Comment on attachment 8499487 [details] [review]
Now destroying AudioContext when hidding the app and regenerating when showing again

Hi, I addressed your comments on GitHub. Could you review again?
Attachment #8499487 - Flags: review- → review?(squibblyflabbetydoo)
Comment on attachment 8499487 [details] [review]
Now destroying AudioContext when hidding the app and regenerating when showing again

See my comments on GitHub.
Attachment #8499487 - Flags: review?(squibblyflabbetydoo) → review-
Comment on attachment 8499487 [details] [review]
Now destroying AudioContext when hidding the app and regenerating when showing again

I found why that line was removed. I thought I was messed something up but not: it is because comment 41 where it's said `new AudioContext()` is returning a context with the same graph.

Anyway I changed the implementation to hide this side effect.
Attachment #8499487 - Flags: review- → review?(squibblyflabbetydoo)
Comment on attachment 8499487 [details] [review]
Now destroying AudioContext when hidding the app and regenerating when showing again

Ok, things look pretty good now. r=me with the appropriate changes made (see my comments on GitHub).
Attachment #8499487 - Flags: review?(squibblyflabbetydoo) → review+
Hi Salva

Sorry for delay. Actually I don't permission for feedback.
Anyways Jim is there for us.

Thanks
I've to rebase the patch so waiting for test to end before merging.
master: 349c19fc7ad8c358e9a0d53eb4012fdce8b0044c
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Please request Gaia v2.0 and v2.1 approval on this when you get a chance.
Component: Video/Audio → Gaia::Ringtones
Flags: needinfo?(salva)
Product: Core → Firefox OS
Target Milestone: --- → 2.1 S8 (7Nov)
Comment on attachment 8499487 [details] [review]
Now destroying AudioContext when hidding the app and regenerating when showing again

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): bug 1043762 comment 41
[User impact] if declined: medium (can not lower volume while testing ringtones which could be very annoying).
[Testing completed]: on device.
[Risk to taking this patch] (and alternatives if risky): low
[String changes made]: none
Flags: needinfo?(salva)
Attachment #8499487 - Flags: approval-gaia-v2.0?
Comment on attachment 8499487 [details] [review]
Now destroying AudioContext when hidding the app and regenerating when showing again

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): bug 1043762 comment 41
[User impact] if declined: medium (can not lower volume while testing ringtones which could be very annoying).
[Testing completed]: on device.
[Risk to taking this patch] (and alternatives if risky): low
[String changes made]: none
Attachment #8499487 - Flags: approval-gaia-v2.1?
Depends on: 1091137
FYI, this patch causes test failures and will need bug 1091137 to land with it (and that probably means bug 1048673 needs uplifted to 2.0 too).
(In reply to Jim Porter (:squib) from comment #65)
> FYI, this patch causes test failures and will need bug 1091137 to land with
> it (and that probably means bug 1048673 needs uplifted to 2.0 too).

Can you double check this? See the results of the tests [1] and you'll see there is nothing related with ringtones failing. Moreover, I don't get to reproduce and I can not access to your logs.

I think, for the sake of traceability, if the patch introduces a regression we should back it out and fix it properly.

Furthermore, the follow-up bug introduces a lot of changes and remove the comment pointing to this bug, comment 41 which evidences the real problem. If we back this out we can simply correct everything and not to risk the uplift but introducing a unnecessary dependency.

[1] https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=cb6484954d63
Flags: needinfo?(squibblyflabbetydoo)
I got to reproduce it hiding the settings before playing any sound. It could have been solved by simply checking for `this._source` nullity before disconnecting and not requiring further bugs.
(In reply to Salvador de la Puente González [:salva] from comment #64)
> Comment on attachment 8499487 [details] [review]
> Now destroying AudioContext when hidding the app and regenerating when
> showing again
> 
> [Approval Request Comment]
> [Bug caused by] (feature/regressing bug #): bug 1043762 comment 41
> [User impact] if declined: medium (can not lower volume while testing
> ringtones which could be very annoying).
> [Testing completed]: on device.
> [Risk to taking this patch] (and alternatives if risky): low
> [String changes made]: none

Bhavana, we need your help to uplift this. it will also fix bug 1087200.
Flags: needinfo?(bbajaj)
(In reply to Salvador de la Puente González [:salva] from comment #66)
> (In reply to Jim Porter (:squib) from comment #65)
> > FYI, this patch causes test failures and will need bug 1091137 to land with
> > it (and that probably means bug 1048673 needs uplifted to 2.0 too).
> 
> Can you double check this? See the results of the tests [1] and you'll see
> there is nothing related with ringtones failing.

That's because the test harness failed. In cases like that, if you haven't done a full test run locally (at least of the app you changed), please retrigger gaia-try, e.g. by rebasing.

> Moreover, I don't get to reproduce and I can not access to your logs.

The logs expired, but it's quite easy to reproduce: check out the revision you landed for this bug and run marionette on the ringtones app:

  JavaScriptError: (17) TypeError: this._source is undefined
  Remote Stack:
  @app://ringtones.gaiamobile.org/js/tone_player.js, line 154
      at Error.MarionetteError (/home/jim/src/gaia/node_modules/marionette-client/lib/marionette/error.js:67:13)
      at Object.Client._handleCallback (/home/jim/src/gaia/node_modules/marionette-client/lib/marionette/client.js:476:19)
      at /home/jim/src/gaia/node_modules/marionette-client/lib/marionette/client.js:510:21
      at TcpSync.send (/home/jim/src/gaia/node_modules/marionette-client/lib/marionette/drivers/tcp-sync.js:153:10)
      at Object.send (/home/jim/src/gaia/node_modules/marionette-client/lib/marionette/client.js:457:36)
      at Object.Client._sendCommand (/home/jim/src/gaia/node_modules/marionette-client/lib/marionette/client.js:503:19)
      at Object._executeScript (/home/jim/src/gaia/node_modules/marionette-client/lib/marionette/client.js:1468:19)
      at Object.executeAsyncScript (/home/jim/src/gaia/node_modules/marionette-client/lib/marionette/client.js:1249:19)
      at Object.MarionetteHelper.wait (/home/jim/src/gaia/node_modules/marionette-helper/index.js:68:11)
      at switchToAppFixed (/home/jim/src/gaia/apps/ringtones/test/marionette/lib/ringtones.js:385:17)
      at Object.Ringtones.inManager (/home/jim/src/gaia/apps/ringtones/test/marionette/lib/ringtones.js:436:5)
      at Context.<anonymous> (/home/jim/src/gaia/apps/ringtones/test/marionette/manage_test.js:87:11)
      at callFn (/home/jim/src/gaia/node_modules/mocha/lib/runnable.js:223:21)
      at Test.Runnable.run (/home/jim/src/gaia/node_modules/mocha/lib/runnable.js:216:7)
      at Runner.runTest (/home/jim/src/gaia/node_modules/mocha/lib/runner.js:373:10)
      at /home/jim/src/gaia/node_modules/mocha/lib/runner.js:451:12
      at next (/home/jim/src/gaia/node_modules/mocha/lib/runner.js:298:14)
      at /home/jim/src/gaia/node_modules/mocha/lib/runner.js:308:7
      at next (/home/jim/src/gaia/node_modules/mocha/lib/runner.js:246:23)
      at Object._onImmediate (/home/jim/src/gaia/node_modules/mocha/lib/runner.js:275:5)
      at processImmediate [as _immediateCallback] (timers.js:330:15)

> I think, for the sake of traceability, if the patch introduces a regression
> we should back it out and fix it properly.

That's usually more work than just filing a followup and using Bugzilla to track deps. Generally, Gaia policy is to avoid backing things out if we can quickly fix the issue, which I did.

> Furthermore, the follow-up bug introduces a lot of changes and remove the
> comment pointing to this bug, comment 41 which evidences the real problem.
> If we back this out we can simply correct everything and not to risk the
> uplift but introducing a unnecessary dependency.

I added a small extra change that needs uplifted anyway (since a similar patch is on 1.3T), but there's nothing actually stopping us from making a 2.0-specific version of the patch.

I removed the comment because it's not really necessary; the behavior of the function is explained in the functions documentation above. (The comment is also not quite accurate anyway, since the implementation details of AudioContext aren't the primary reason why we only create one AudioContext.)
Flags: needinfo?(squibblyflabbetydoo)
Attachment #8499487 - Flags: approval-gaia-v2.1? → approval-gaia-v2.1+
Flags: needinfo?(bbajaj)
Attachment #8499487 - Flags: approval-gaia-v2.0? → approval-gaia-v2.0+
Blocks: 1087200
Duplicate of this bug: 1087200
Whiteboard: [2.0-319MB-bug-bash] [LibGLA,TD93605,QE1,B][TAM-triage?] → [CR 744388][2.0-319MB-bug-bash] [LibGLA,TD93605,QE1,B][TAM-triage?]
Whiteboard: [CR 744388][2.0-319MB-bug-bash] [LibGLA,TD93605,QE1,B][TAM-triage?] → [caf priority: p2][CR 744388][2.0-319MB-bug-bash] [LibGLA,TD93605,QE1,B][TAM-triage?]
QA Contact: croesch
Verify passed, this issue can't be repro on  Woodduck 2.0; Flame2.0&2.1&2.2.
Attachment:Verify_Woodduck_Ring.MP4
Reproducing rate: 0/5

Woodduck 2.0 build:
Gaia-Rev        60146ec47cd38a8be8ed22e0116902eceb9ac067
Gecko-Rev       cdfbe9866cf0b71b33454926638ce0cd8bb1fb00
Build-ID        20141117050313
Version         32.0
Flame 2.0 build:
Gaia-Rev        086a668942292168f312b3bb53e275fa0886fab1
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/a57b299c5cf2
Build-ID        20141116000206
Version         32.0

Flame2.1 build:
Gaia-Rev        81160ad79e5b4c21967418dd63f1a1d08d77924e
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/3572aa3e6766
Build-ID        20141116001201
Version         34.0

Flame2.2 build:
Gaia-Rev        1dd8ad8f96988afebc9691e1b818fa37aa32c790
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/a52bf59965a0
Build-ID        20141116040209
Status: VERIFIED → RESOLVED
Closed: 5 years ago5 years ago
Attachment #8499487 - Flags: feedback?(ankit93040)
You need to log in before you can comment on or make changes to this bug.