921930 - [B2G][Helix][Setting][Sound][Bug][shijinde]The vibrate state is error by Setting the volume mute and unmute in sleep menu function

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System, defect, P2)

Description

[lecky]

User Agent: Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 6.1; Trident/4.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; aff-kingsoft-ciba; .NET4.0C; .NET4.0E; Zune 4.7; Tablet PC 2.0)

Steps to reproduce:

1、set the vibrate state off by manual in setting->sound screen.
2、press power key long time, and select "silence incoming calls"
3、go to setting->sound, the vibrate state is off
4、press power key long time, and select "ring incoming calls"
5、go to setting->sound, the vibrate state is on
6、press volume down or up, the vibrate icon state is off in volume status bar


Actual results:

1、the vibrate state is on
2、the vibrate icon state is off in volume status bar


Expected results:

1、the vibrate state is on
2、the vibrate icon state is off in volume status bar

Comment 1

[lecky]

Hi, I had added the pictures and you can follow the steps to reproduce this bug.

Thanks

Comment 2

[lecky]

Attachment: 1 set vibrate state off.png

Comment 3

[lecky]

Attachment: 2 select silence incoming calls.png

Comment 4

[lecky]

Attachment: 3 sound screen.png

Comment 5

[lecky]

Attachment: 4 vibrate state on.png

Comment 6

[lecky]

Attachment: 5 the vibarte state is off in volume status bar.png

Comment 7

[lecky]

Hi, I had found this problem in sound_manager.js.

you need to record initial state about vibrate when you received the "mute" event.

I modified the code:

+ var oldstate = false;
  window.addEventListener('mute', function() {
    // Turn off vibration for really silence.
+   oldstate = vibrationEnabled;
    setVibrationEnabled(false);
    enterSilentMode('notification');
  });

  /**
   * The unmute event is dispatched from sleep menu.
   * But if we have a mute hardware button or virtual button,
   * we could make the button handler to fire this event, too.
   */
  window.addEventListener('unmute', function() {
    // Turn on vibration.
+   if(oldstate === true)
+   {
+     setVibrationEnabled(true);
+   }
    leaveSilentMode('notification');
    leaveSilentMode('content');
  });


I had test the code and the function is OK.

Thanks

Comment 8

[Wayne Chang [:wchang]]

Lecky,

Your expected result and actual result are the same, I think you copy and pasted incorrectly?

Evelyn,
Someone can take a look at sound manager? I think there is some status indicator out-of-sync issue?

Comment 9

[Evelyn Hung]

Tzu-Lin, can you help?

Comment 10

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Root cause: asynchronous issue
There is a 'vibration.enabled' observer in sound_manager.js (https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/sound_manager.js#L385).
When 'vibration.enabled' has changed, it will update local variable 'vibrationEnabled' inside sound_manager.js (https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/sound_manager.js#L400)

However, there is a semaphore 'pendingRequestCount' to protect modification on vibrationEnabled. When there are ongoing events related to volumn processing, 'pendingRequestCount' would be increase. And 'pendingRequestcount' would be decrease when processing is done.

The problem is that when either 'mute' or 'unmute' happens, sound_manager will increase 'pendingRequestCount' and update vibration (thru setVibrationEnabled()) thus trigger observer to react.
And 'pendingRequestCount' will not go back to zero until we got callback from SettingsListener. This results in every time sound_manager.js observer 'vibration.enabled' changed, 'pendingRequestCount' will always larger then zero and block the updating of local variable 'vibrationEnabled' of sound_manager.js

example sequence of problem: (assuming vibration is off)
1. The user hold power button and select 'ringing incoming calls'. 'unmute' event is triggered. https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/sound_manager.js#L45
2. call setVibrationEnabled() and set to true (this will not happen immediately)
3. call leaveSilentMode(), increase pendingRequestCount to one
4. sound_manager.js observed 'vibration.enabled' changed as result of 2, but 'pendingRequest' is now larger than zero. The function returns without modifying local variable 'vibrationEnabled'(https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/sound_manager.js#L393).
5. 'success' event of SettingsListener, 'pendingRequest' go back down to zero again.

Comment 11

[Tzu-Lin Huang [:dwi2][:tzhuang]]

accidentally clear the flag, reflag again.

Comment 12

[(inactive after 6/18) Alive Kuo [:alive]]

Sound manager bug.

Comment 13

[Tzu-Lin Huang [:dwi2][:tzhuang]]

proposed solution:
Implement an asynchronous semaphore. And make sure the code block by semaphore will get executed once semaphore drop back to zero.

Comment 14

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Attachment: pullrequest.html

Comment 15

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 814811 [details]
pullrequest.html

r+ for the semaphore idea, it's wonderful.
However I would like to have that defined in another file named async_semaphore.js
And please see github comments.

Once you have that module please also write an unit test for it(not sound manager).

Comment 16

[(inactive after 6/18) Alive Kuo [:alive]]

Please feedback?=me if you need feedback once the patch is modified.

Comment 17

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 814811 [details]
pullrequest.html

f+ instead.

Comment 18

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Hi Alive,

I've pull out async_semaphore.js and add unit tests for it as discussion on github.
Please help to give me some feedback.

Comment 19

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 814811 [details]
pullrequest.html

r=me!

Comment 20

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Hi Alive, 
This pull request is green light in Travis now. 
Please kindly help to land this patch since I don't have the privilege to do this.
Thanks

Comment 21

[(inactive after 6/18) Alive Kuo [:alive]]

Please say "what you done" in commit message instead of copy/paste bug summary next time.
https://github.com/mozilla-b2g/gaia/commit/5cfebf44f988ccde09a33ac58745a3ca70b4040f

Comment 22

[Tzu-Lin Huang [:dwi2][:tzhuang]]

got it, thanks

Comment 23

[lecky]

Hi, do you will merge to 1.1HD version?

Thanks.

Comment 24

[Wayne Chang [:wchang]]

Not a regression, similar on other v1.1 devices. Low user impact - not blocking for v1.1/hd.