1187092 - [Clock] Changing 'Sound' for an alarm will not preview the first track selected

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Oliver Nelson [:oliverthor]]

Description:
When the user is creating a new alarm, or editting an existing one, they may observe that when changing the 'Sound' via the selection-dialog, that the first selection will not preview it's sound to the user. Subsequent taps will begin to preview sounds or songs for their alarm sound/volume.

Repro Steps:
1) Update phone to 20150723010305
2) Open the Clock app
3) Add a new alarm
4) Tap the 'Sound' button to open dialog menu
5) Tap any other sound ** actual
6) Tap any other sound ** expected

Actual: 
First selected sound does not play

Expected:
Any selected sound will play


Environmental Variables:
---------------------------
Device: Aries 2.5
Build ID: 20150722124026
Gaia: b57aef5b7f52c40f88ee4c069ff722404e8e8521
Gecko: 954706e7611c
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 42.0a1 (2.5)
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0

Device: Flame 2.5
BuildID: 20150723010205
Gaia: f04fdbfa1943dddeab8ecd1299a76ab56e590d00
Gecko: 2ddec2dedced
Gonk: 41d3e221039d1c4486fc13ff26793a7a39226423
Version: 42.0a1 (2.5) 
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0
**********************

Issue DOES NOT REPRODUCE on 2.2 for flame devices
Results: Any selected sound will play

Device: Flame 2.2
BuildID: 20150723002503
Gaia: e1e6317f17a840b19af9dbb25f5a771d8d9fa161
Gecko: d8326043baec
Gonk: bd9cb3af2a0354577a6903917bc826489050b40d
Version: 37.0 (2.2) 
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
--------------------------


Repro Rate: 5/5
Attachments:
video- https://youtu.be/o_gRBdaqqRk
logcat

Comment 1

[Oliver Nelson [:oliverthor]]

Attachment: logcat_20150723_1522.txt

Comment 2

[Peter Bylenga [:PBylenga]]

[Blocking Requested - why for this release]:
Functional regression.

Requesting a window.

Comment 3

[Martin Shuman [:Marty]]

This bug appears to have a further problem, where if the song is not previewing in the select menu, it will also not ring when the alarm itself is triggered.

This particular functionality was previously masked by bug 1181982. Because this issue occurs with the default ringtone, and prevents the alarm from ringing, upgrading this to a smoketest blocker.

Comment 4

[Oliver Nelson [:oliverthor]]

Based on comment 3, escalating this to a qaurgent to find the window.

Comment 5

[Pi Wei Cheng [:piwei] (inactive)]

mozilla-inbound regression window:

Last Working
Device: Flame
BuildID: 20150710163851
Gaia: e4b63559eba364892867eb381c3002d6518e5d6a
Gecko: 07bcf36f5ab2
Gonk: 41d3e221039d1c4486fc13ff26793a7a39226423
Version: 42.0a1 (2.5 Master) 
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0

First Broken
Device: Flame
BuildID: 20150710170452
Gaia: e4b63559eba364892867eb381c3002d6518e5d6a
Gecko: 675ea719b91c
Gonk: 41d3e221039d1c4486fc13ff26793a7a39226423
Version: 42.0a1 (2.5 Master) 
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0

Gaia is the same so it's a Gecko issue.

Gecko pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=07bcf36f5ab2&tochange=675ea719b91c

Caused by changes made in Bug 1113086.

Comment 6

[KTucker [:KTucker][Inactive 3/4/2016]]

Andrea, here is another issue that looks to have been caused by the landing for bug 1113086.

Comment 7

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

This might be fixed via bug 1185051, is what I heard...  Please retest this once bug 1185051 is in the build.

Comment 8

[Andrea Marchesini [:baku]]

Moving the NI to Alastor. Do you confirm comment 7?

Comment 9

[Pi Wei Cheng [:piwei] (inactive)]

Flashing latest Gaia on Flame did not fix this. This needs a separate fix or a backout of bug 1113086.

Tested on:
Gaia commit 31ade93da3e68f5e62f79bdf509ad20001a1a3e4
Merge: 13e4ea7 3682754
Author: Zibi Braniecki <[e-mail]>
Date:   Fri Jul 24 16:19:33 2015 -0700

    Merge pull request #30491 from zbraniecki/1172699-migrate-timeicon-to-intl
    
    Bug 1172699 - Migrate timeIcon to Intl API. r=mhenretty

Gecko is cb8bdb8ffaef.

Comment 10

[Alastor Wu [:alwu]]

Keep NI for tracking, I will check it later.

Comment 11

[Martin Shuman [:Marty]]

Bug 1185051 has been verified fixed during smoketest, but this issue is still occurring.  It would appear that these are different issues.

Comment 12

[Alastor Wu [:alwu]]

This issue still can be reproduced, I'm debugging now.

Comment 13

[Alastor Wu [:alwu]]

It seems that we can't create the audio thread when we got the playing command from the system app.
Still finding more details.

Comment 14

[Alastor Wu [:alwu]]

The problem might be that we can't get data from the network, because we called suspend() too early before we have got the available data.

The normal audio playing flow is like that,
(1) Audio element play
(2) Be muted by AudioChannelService (muted by default in b2g)
(3) Unmuted by system app 

In step2, the suspend() would be triggered. The connection of the source data would be disconnected by this function. In step3, we would call resume() to re-connect the source. 

In the ideal situation, the step2 would be done after we really get data from the source. However, if the step2 was called before the ChannelMediaResource::OnDataAvailable(), we can't get any data.

Comment 15

[Alastor Wu [:alwu]]

The root cause is that we suspend the resource channel twice but only resume once.

When we want to playback the ringtone preview, the situation is like that, 
(1) muted by default in b2g
Because we mute the audio by default, and then wait for the system app to verify whether the audio can be playback, if so, we would open it later.
In this step, we would change the muted state in HTMLMediaElement.

(2) finish decoder setup
After finishing decoder setup, we would check whether we need to pause and suspend the HTMLMediaElement. 
We changed the muted state to "MUTED_BY_AUDIO_CHANNEL" in step1, so we called suspend(). This operation would suspend the media resource.

(3) Resource connection
In MediaElement::Load(), we need to establish the connection with the resource. 
The process of establishing connection is like that, "openChannel" -> "onStartRequest" -> "OnDataAvailable". 
After "OnDataAvailable" is called, we will have available data in media cache.

However, in "onStartRequest", we checked the counter of the suspend, and then suspended the resource again.

(4) Open audio command from system app
We would resume resource connection in this step. 
Since we suspended the resource twice (step2 & step3), and only call resume once that can't establish connection successfully.
That is why we can't play the audio.

----

Q : Why only the first sound can't be playback?
A : Because some reason (or bug, we might modify it later), the following sound would only be suspended once. The suspend would be called after the "onStartRequest".

Comment 16

[Alastor Wu [:alwu]]

Attachment: Don't suspend channel again if the suspend() has already been called

Hi, JW,
Could you help me review this patch?
Very appreciate :)

---

The root cause analysis is on the comment15.
I think that we should not call suspend() again if the mSuspendCount is larger than zero. That means we have already called the suspend().

---

Try-server result, it seems good.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cd5e3d4e7dbc

Comment 17

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8641596 [details] [diff] [review]
Don't suspend channel again if the suspend() has already been called

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

This is wrong because the channel in ChannelMediaResource::OnStartRequest() is a newly created one. We will need to suspend it if the suspend count is greater than 0 so the channel suspend status is consistent with the suspend count of ChannelMediaResource.

Comment 18

[Alastor Wu [:alwu]]

Attachment: Only suspend channel after the onStartRequest

Hi, JW,
Could you help me review this patch?
In this patch, we would ensure that the suspend() is always called after we start the request to channel. 
Very appreciate :)

---

Try-server result.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1d9985ebbf41

Comment 19

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8642257 [details] [diff] [review]
Only suspend channel after the onStartRequest

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

::: dom/media/MediaResource.cpp
@@ +1164,5 @@
>  ChannelMediaResource::PossiblySuspend()
>  {
>    bool isPending = false;
>    nsresult rv = mChannel->IsPending(&isPending);
> +  if (NS_SUCCEEDED(rv) && isPending && mIsChannelOnStart) {

When this happens before ChannelMediaResource::OnStartRequest() is called (i.e mIsChannelOnStart is false), mIgnoreResume is set to true. Later PossiblyResume() will not work as expected because mIgnoreResume is true.

Comment 20

[Alastor Wu [:alwu]]

Hi, Robert,
Sorry to bother you,
In MediaResource.cpp, we use the mSuspendCount [1] to compute the number we suspended. 
However, I think that the |mSuspendCount| and |mIgnoreResume| make the code more complex, could we replace the mSuspendCount with the bool variable? Would that bring any potential risks?
Very appreciate :)

[1] https://dxr.mozilla.org/mozilla-central/source/dom/media/MediaResource.h#781

Comment 21

[Robert O'Callahan (:roc) (email my personal email if necessary)]

There are multiple callers of Suspend that can independently resume, e.g. CacheClientSuspend and MediaDecoder::Suspend. If we replace the counter with a bool, and they both suspend, and then one resumes, that will actually resume loading the resource, which we don't want.

Comment 22

[Alastor Wu [:alwu]]

Attachment: Part 2 : Modify the suspend/resume process

Hi, JW,
Could you help me review this patch?
This patch is the part2, need to be reviewed with the previous one. 

Some things need to know in this patch,
(1) For channel operations (suspend/resume),
    The |mSuspendCount| and |mWaitToBeSuspended| would be involved in these process. 
    - Suspend
      If we suspend channel, and then increase the |mSuspendCount|.
      If we can't suspend channel successfully, set the |mWaitToBeSuspended| to true.
    - Resume
      If |mWaitToBeSuspended| is true, set it to false. Or decrease the |mSuspendCount|.

(2) For cache client operations, only the |mSuspendCount| would involved in.

(3) Every time we close the channel, we would resume it to make sure the channel internal status is correct.
    However, the channel needs to be suspended again when it is recreated. Use |mWaitToBeSuspended| to achieve that.

Very appreciate :)

Comment 23

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8643472 [details] [diff] [review]
Part 2 : Modify the suspend/resume process

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

I think you can create a new class to manage the channel suspend status and suspend count. Code is more maintainable when you put related piece together.

::: dom/media/MediaResource.cpp
@@ +845,5 @@
>        element->DownloadSuspended();
>      }
> +  } else {
> +    // for cache client.
> +    mSuspendCount++;

It is wrong not to increment mSuspendCount for the case where mChannel is not null and |aCloseImmediately && mCacheStream.IsTransportSeekable()| is true. The newly created channel will not be suspended properly.

Please use review board which provides a full context and makes it easier to review.

@@ +1144,5 @@
>  
> +bool
> +ChannelMediaResource::IsSuspendedOrHaveSuspendedRequest()
> +{
> +  MutexAutoLock lock(mLock);

Why lock is needed?

@@ +1190,5 @@
>  ChannelMediaResource::PossiblySuspend()
>  {
>    bool isPending = false;
>    nsresult rv = mChannel->IsPending(&isPending);
> +  bool isReadyToSuspend = (NS_SUCCEEDED(rv) && isPending && mIsChannelOnStart);

I think you should be able to do without mIsChannelOnStart since IsPending() could return true even before OnStartRequest() is called which means we can suspend the channel without any problem.

Comment 24

[Alastor Wu [:alwu]]

Attachment: Refactor the suspend/resume process

New patch, and try-server result.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=eca29d70b094

Comment 25

[Alastor Wu [:alwu]]

The purpose of this patch is to refactor the suspend/resume process, make the code more readable and modify the wrong suspend logic.

We use the new class, ChannelSuspendAgent, to manage the channel suspend status and suspend count. Every time we call the Suspend()/Resume(), the suspend count would be changed. However, the channel would only be suspended once, even if the suspend counter is larger than 1.

The Suspend()/Resume() can be called anytime. If we can't suspend the channel successfully, we would postpone this suspended request, and then done it after the channel can be suspended.

Comment 27

[Alastor Wu [:alwu]]

Attachment: MozReview Request: Bug 1187092 - refactor the suspend process

Bug1187092 - refactor the suspend process

Comment 28

[Alastor Wu [:alwu]]

Comment on attachment 8645538 [details]
MozReview Request: Bug 1187092 - refactor the suspend process

Hi, JW,
Could you give me some suggestions about this patch?
The detail of this patch is in the comment25.
Very appreciate :)

Comment 29

[JW Wang [:jwwang] [:jw_wang]]

https://reviewboard.mozilla.org/r/15531/#review13899

::: dom/media/MediaResource.h:613
(Diff revision 1)
> +  nsCOMPtr<nsIChannel> mChannel;

Since ChannelSuspendAgent will not live longer than the owning ChannelMediaResource, it is safe to hold a raw pointer in order not to change the lifetime of the channel.

::: dom/media/MediaResource.h:825
(Diff revision 1)
> +  bool mPendingSuspend;

mPendingSuspend should be handled internally by ChannelSuspendAgent without bothering ChannelMediaResource.

::: dom/media/MediaResource.h:827
(Diff revision 1)
> +  nsAutoPtr<ChannelSuspendAgent> mSuspendAgent;

Just say |ChannelSuspendAgent mSuspendAgent|.

::: dom/media/MediaResource.cpp:323
(Diff revision 1)
> -  if (mSuspendCount > 0) {
> +  if (mPendingSuspend) {

You can have ChannelSuspendAgent::NotifyChannelStatusChanged() to tell the agent about the possible change in the pending state of the channel and to try again if it wansn't able to suspend the channel properly. We want to have all logic about channel suspending in the agent without concerning ChannelMediaResource too much.

::: dom/media/MediaResource.cpp:703
(Diff revision 1)
> +  CancelChannel();

I fail to see the reason to extract the code below into a new function when it is only called from one place.

::: dom/media/MediaResource.h:610
(Diff revision 1)
> +  void NotifyOpenedChannel(nsIChannel* aChannel);

NofityChannel{Opened,Closed}.

::: dom/media/MediaResource.cpp:712
(Diff revision 1)
> -      PossiblyResume();
> +    if (mSuspendAgent->IsSuspended()) {

Just call mSuspendAgent->NotifyClosedChannel() here and let the agent handle all the details about suspend and force resume.

::: dom/media/MediaResource.cpp
(Diff revision 1)
> -    } else if (mSuspendCount == 0) {

It is wrong to remove |mChannelStatistics->Stop()| and |element->DownloadSuspended()|.

::: dom/media/MediaResource.cpp:834
(Diff revision 1)
> -  ++mSuspendCount;
> +  if (!mSuspendAgent->Suspend()) {

Also, you should just call mSuspendAgent->Suspend() and let the agent handle all the details.

Comment 30

[Alastor Wu [:alwu]]

Comment on attachment 8645538 [details]
MozReview Request: Bug 1187092 - refactor the suspend process

Bug1187092 - refactor the suspend process

Comment 31

[Alastor Wu [:alwu]]

Try-server result.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6112b61bf200

Comment 32

[Alastor Wu [:alwu]]

Comment on attachment 8645538 [details]
MozReview Request: Bug 1187092 - refactor the suspend process

Hi, JW,
Could you help me review this patch?
Very appreciate :)

Comment 33

[Alastor Wu [:alwu]]

Comment on attachment 8645538 [details]
MozReview Request: Bug 1187092 - refactor the suspend process

Bug1187092 - refactor the suspend process

Comment 34

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8645538 [details]
MozReview Request: Bug 1187092 - refactor the suspend process

https://reviewboard.mozilla.org/r/15531/#review13985

::: dom/media/MediaResource.cpp:1171
(Diff revision 3)
> +  --mSuspendCount;

Assert |mSuspendCount > 0| before decrementing it to avoid underflow.

::: dom/media/MediaResource.cpp:1160
(Diff revision 3)
> -    mChannel->Suspend();
> +      mChannel->Suspend();

Don't do it if mIsChannelSuspended is already true.

::: dom/media/MediaResource.cpp:1018
(Diff revision 3)
> +    MOZ_ASSERT(mSuspendAgent.IsSuspended(), "Resume without suspend!");

This assertion should go into the suspend agent which can check the suspend count upon Suspend/Resume.

::: dom/media/MediaResource.cpp
(Diff revision 3)
> -  return mSuspendCount > 0;

You can't drop the lock since mSuspendCount is written on the main thread and read by the MDSM on the state machine thread. It will be a data race which is bad. A quick fix is make it an Atomic. However, I think this issue deserves its own bug. Please file a new bug to deal with it.

::: dom/media/MediaResource.h:608
(Diff revision 3)
> +  bool IsSuspended() const;

Please document each function. It is unclear what the return values of Suspend/Resume mean.

Pretty close. Still some cirtical issues to fix.

Comment 35

[Alastor Wu [:alwu]]

Comment on attachment 8645538 [details]
MozReview Request: Bug 1187092 - refactor the suspend process

Bug1187092 - refactor the suspend process

Comment 36

[Alastor Wu [:alwu]]

Hi, JW,
Could you help me review my patch?
I have filed another bug to handle the Atomic (bug1193245).
Very appreciate :)

Comment 37

[JW Wang [:jwwang] [:jw_wang]]

No. This bug should depend on bug 1193245 which needs to be fixed first.

Comment 38

[Alastor Wu [:alwu]]

Comment on attachment 8645538 [details]
MozReview Request: Bug 1187092 - refactor the suspend process

Bug 1187092 - refactor the suspend process

Comment 39

[Ken Chang[:kenkai]]

regression bug.

Comment 40

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8645538 [details]
MozReview Request: Bug 1187092 - refactor the suspend process

https://reviewboard.mozilla.org/r/15531/#review14409

::: dom/media/MediaResource.h:598
(Diff revision 5)
> + * This class is responsible for manageing the suspend count and report suspend

s/manageing/managing/

::: dom/media/MediaResource.h:612
(Diff revision 5)
> +  // Return true when we suspend the channel successfully.

Come to think about it again, I think it should return true when the suspend count goes from 0 to 1 which means the channel is "logically" suspended. This is symmetric to Resume() which only returns true when the count reaches 0.

::: dom/media/MediaResource.h:617
(Diff revision 5)
> +  bool Resume();

It should return true only when the suspend count reaches zero. You will never say "it is resumed" when the suspended is still larger than 0.

::: dom/media/MediaResource.h:620
(Diff revision 5)
> +  // needs to be suspend.

s/suspend/suspended/

::: dom/media/MediaResource.cpp:818
(Diff revision 5)
> +  if (mSuspendAgent.Suspend()) {

This is why I changed mind about the return value of ChannelSuspendAgent::Suspend(). If Suspened() returns true only when the underlying channel is "physically" suspended, the media element won't be notified about "DownloadSuspended" if we are unable to physically suspend the channel here.

So if we change ChannelSuspendAgent::Suspend() to return true when the suspend count goes from 0 to 1, the element will be notifed when the channel is "logically" suspended which also preserves the original behavior.

Comment 41

[Alastor Wu [:alwu]]

Comment on attachment 8645538 [details]
MozReview Request: Bug 1187092 - refactor the suspend process

Bug 1187092 - refactor the suspend process

Comment 42

[Alastor Wu [:alwu]]

Comment on attachment 8645538 [details]
MozReview Request: Bug 1187092 - refactor the suspend process

Bug 1187092 - refactor the suspend process

Comment 43

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8645538 [details]
MozReview Request: Bug 1187092 - refactor the suspend process

https://reviewboard.mozilla.org/r/15531/#review14481

::: dom/media/MediaResource.h:558
(Diff revision 7)
> +  bool Suspend();

// Return true when the channel is logically suspended, i.e. the suspend count goes from 0 to 1.

::: dom/media/MediaResource.cpp:795
(Diff revision 7)
> +  if (mSuspendAgent.Suspend()) {

It should be:

if (aCloseImmediately && mCacheStream.IsTransportSeekable()) {
} else if (mSuspendAgent.Suspend()) {
}

Otherwise, |element->DownloadSuspended()| could be called twice.

::: dom/media/MediaResource.cpp:1127
(Diff revision 7)
> +  return (mSuspendCount == 1) ? true : isChannelSuspended;

Only return true when count goes from 0 to 1, otherwise |element->DownloadSuspended()| at #800 will be called multiple times.

Comment 44

[Alastor Wu [:alwu]]

Comment on attachment 8645538 [details]
MozReview Request: Bug 1187092 - refactor the suspend process

Bug 1187092 - refactor the suspend process

Comment 45

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8645538 [details]
MozReview Request: Bug 1187092 - refactor the suspend process

https://reviewboard.mozilla.org/r/15531/#review14485

::: dom/media/MediaResource.cpp:791
(Diff revisions 7 - 8)
> -    }
> +  } else if (mSuspendAgent.Suspend()) {

The logic is still wrong. If mChannel is not null, |mSuspendAgent.Suspend()| won't be called which means the suspend count is not increased.

Comment 46

[Alastor Wu [:alwu]]

Comment on attachment 8645538 [details]
MozReview Request: Bug 1187092 - refactor the suspend process

Bug 1187092 - refactor the suspend process

Comment 47

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8645538 [details]
MozReview Request: Bug 1187092 - refactor the suspend process

https://reviewboard.mozilla.org/r/15531/#review14491

::: dom/media/MediaResource.cpp:792
(Diff revision 9)
> +  } else if (mSuspendAgent.Suspend()) {

I am afraid that it is still wrong. You code will call |element->DownloadSuspended()| when mChannel is null and mSuspendAgent.Suspend() returns true. However, it should only be called when mChannel is non-null.

Comment 48

[Alastor Wu [:alwu]]

Comment on attachment 8645538 [details]
MozReview Request: Bug 1187092 - refactor the suspend process

Bug 1187092 - refactor the suspend process

Comment 50

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8645538 [details]
MozReview Request: Bug 1187092 - refactor the suspend process

https://reviewboard.mozilla.org/r/15531/#review14495

Ship It!

Comment 51

[Alastor Wu [:alwu]]

Thanks JW!

Try-server result is in the comment31.

Comment 52

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ee800c0e234f

Comment 53

[Ryan VanderMeulen [:RyanVM]]

Backed out for asserts/crashes.
https://treeherder.mozilla.org/logviewer.html#?job_id=12991684&repo=mozilla-inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/1c0e8950b4eb

Comment 54

[Alastor Wu [:alwu]]

The crash happens when we close channel before the onStartRequest(), because we doesn't set the ChannelSuspendAgent::mChannel yet. [1]

Therefore, every time we create the ChannelMediaResource::mChannel, we should also set it for the ChannelSuspendAgent::mChannel. [2]

[1] I added the log and push to try, we can see the code flow.
https://treeherder.mozilla.org/logviewer.html#?job_id=10535223&repo=try

[2] The try-server result of the modified patch, it seems good.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=67aac9731ad8

Comment 55

[Alastor Wu [:alwu]]

Comment on attachment 8645538 [details]
MozReview Request: Bug 1187092 - refactor the suspend process

Bug 1187092 - refactor the suspend process

Comment 56

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8645538 [details]
MozReview Request: Bug 1187092 - refactor the suspend process

https://reviewboard.mozilla.org/r/15531/#review14757

Ship It!

Comment 57

[Alastor Wu [:alwu]]

Hi, Ryan,
Could we land this patch again? The test fail have been solved.
Thanks!

Comment 58

[Ryan VanderMeulen [:RyanVM]]

I've been traveling all week. Please contact the sheriff on duty on IRC.

Comment 59

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ec26fe37ae8d

Comment 60

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ec26fe37ae8d

Comment 61

[Cola, OOO till Oct. 8th]

Attachment: Aries v2.5.3gp

This bug has been verified as "pass" on latest build of Flame master and Aries master by the STR in Comment 0.

Actual results:Any selected sound is played.

See attachment: Aries v2.5.3gp

Reproduce rate: 0/5

Device: Aries KK 2.5(Pass)
Build ID               20150823221817
Gaia Revision          cddb9f610cbe03d0ca39d81bbdce46a0fca841ab
Gaia Date              2015-08-23 03:34:38
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/4ccdd06e51d7209ba429196df7cab97bf66962db
Gecko Version          43.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20150823.214038
Firmware Date          Sun Aug 23 21:40:46 UTC 2015
Bootloader             s1

Device: Flame KK 2.5(Pass)
Build ID               20150823150207
Gaia Revision          cddb9f610cbe03d0ca39d81bbdce46a0fca841ab
Gaia Date              2015-08-23 03:34:38
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/4ccdd06e51d7209ba429196df7cab97bf66962db
Gecko Version          43.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150823.184539
Firmware Date          Sun Aug 23 18:45:51 EDT 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0

Comment 62

[Johan Lorenzo [:jlorenzo]]

Moving the bug to the component where the regression came from.