1112706 - Minimized and maximized statusbar can become out of sync

Status: RESOLVED FIXED

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

Description

[Michael Henretty [:mikehenrty][:mhenretty]]

I can't come up with a solid STR yet, but sometimes the minimized statusbar becomes "frozen" and won't update. See video:

https://www.youtube.com/watch?v=GDz_EEXAmTE

Comment 1

[Michael Henretty [:mikehenrty][:mhenretty]]

Guillaume, do you have an ideas on what might cause this?

Comment 2

[Guillaume Marty [:gmarty]]

Alex had this issue for a long time but has never been able to provide a STR. Also, Alex correct me if I'm wrong, but it seems to be more frequent on powerful devices like Nexus 4, so there may be a race condition somewhere.
I never had this issue on my Flame but I heard that it can happen.

Comment 3

[:gerard-majax]

100% possible, I still do see this as I said the other day on IRC, but I feel it's less frequent. Reproduced for sure on Xperia ZR (close to Nexus 4) and Nexus S (not fast, but often exposing interesting race conditions).

I can't come up with proper STRs, but I would say this started back in mid-october. It may be possible it gets reproduced when I leave an app as foreground and lock/unock the device to use it, e.g., with the Messages app.

Comment 5

[Guillaume Marty [:gmarty]]

After discussing this with :cwiiis, it may be caused by a platform bug in the implementation of -moz-element. We have a similar bug in the Wifi or network icons where there are not animated sometime.
I'll continue enquire in this direction.

Comment 6

[Michael Henretty [:mikehenrty][:mhenretty]]

I'm putting this on your plate for now Guillaume, since you are farther than anyone else. If you discover it's a platform bug feel free to re-assign.

Comment 7

[Guillaume Marty [:gmarty]]

Naoki, can you help me with STR here? According to the comments above, we don't have a way to repro it.

Comment 8

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

I wonder if this is related to bug 1117981 and the device sleeping?

Comment 9

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

also to note Bug 1119178.

Comment 10

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

Attachment: 2015-01-13-16-12-20.png

Nope, I don't think it has to do with either bugs now.  I figured out an easy way to reproduce this bug.

1. go to settings -> screenlock and turn off lockscreen
2. hit the power button twice.

The time could disappear. (see screenshot )
(happens in SMS, settings, ...)

Comment 11

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

Note: 
This was on today's build : 
Build ID               20150113010202
Gaia Revision          9946a490a9264b42e65385d703b28fa055ab2d42
Gaia Date              2015-01-12 20:31:05
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/3d846527576f
Gecko Version          38.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150113.043738
Firmware Date          Tue Jan 13 04:37:49 EST 2015
Bootloader             L1TC000118D0

Comment 12

[zhaoyang]

Hi,

  Our QA encountered a similar problem, Minimized statusbar update not normal.
  I can reproduce when monkey test.

  I find the value of '_paused' is not zero,when Minimized statusbar want to update.
  So Minimized statusbar update not mormal.
  
  _updateIconVisibility: function sb_updateIconVisibility() {
    if (this._paused !== 0) {
      return;
    }
    ...
  }
  
  When I limit value of '_paused',Minimized statusbar update normal.
  Please give me some advices.
  Thanks a lot.

ps:log and video

Comment 13

[zhaoyang]

sorry,log and video is too big to upload

Comment 14

[zhaoyang]

As comment 12，please give some guidence

Thanks a lot

Comment 15

[Guillaume Marty [:gmarty]]

I found a solid STR for this bug!

* Enable the lockscreen
* Open the utility tray
* Start to slide up as if you wanted to close the utility tray
* While keeping your finger on the screen, push the power button
* Release your finger on screen and push the power button again

The minimised status bar stops refreshing. As Zhaoyang pointed out, it's related to the value of _paused.

Comment 16

[Michael Henretty [:mikehenrty][:mhenretty]]

Zhaoyang, thank you for this information, it is very helpful! Guillaume is currently working on this bug, but if you want to help out please reach out to him. Thank you!!

Comment 17

[zhaoyang]

(In reply to Guillaume Marty [:gmarty] from comment #15)
> I found a solid STR for this bug!
> 
> * Enable the lockscreen
> * Open the utility tray
> * Start to slide up as if you wanted to close the utility tray
> * While keeping your finger on the screen, push the power button
> * Release your finger on screen and push the power button again
> 
> The minimised status bar stops refreshing. As Zhaoyang pointed out, it's
> related to the value of _paused.

Hi Guillaume,

  If don't use variable '_paused', it will bring the risk ? looks like 'pause' and 'resume' is not to appear in pairs

Comment 18

[zhaoyang]

Or the value of ‘_paused‘  is true/false can solve this problem？

Comment 19

[zhaoyang]

Hi Guillaume，
  
 It will bring the risk？
  
 pauseUpdate: function sb_pauseUpdate() {
-    this._paused++;
+    this._paused = 1;
   },
 
   resumeUpdate: function sb_resumeUpdate() {
-    this._paused--;
+    this._paused = 0;
     this._updateIconVisibility();
   },

Comment 20

[Guillaume Marty [:gmarty]]

Attachment: Github PR

This patch fixes the issue when the screen is locked while the utility tray is being active. Hopefully that's what cause the issue here.
Etienne can you have a look?

Comment 21

[Guillaume Marty [:gmarty]]

(In reply to zhaoyang from comment #19)
>  It will bring the risk？
>   
>  pauseUpdate: function sb_pauseUpdate() {
> -    this._paused++;
> +    this._paused = 1;
>    },
>  
>    resumeUpdate: function sb_resumeUpdate() {
> -    this._paused--;
> +    this._paused = 0;
>      this._updateIconVisibility();
>    },

We cannot really use a boolean here because there are many different events interacting with pauseUpdate() and resumeUpdate() and we really want to refresh the status bar when these events have been fired symmetrically (i.e. pauseUpdate() and resumeUpdate() must be called the same number of times).

Comment 22

[zhaoyang]

(In reply to Guillaume Marty [:gmarty] from comment #21)
> (In reply to zhaoyang from comment #19)
> >  It will bring the risk？
> >   
> >  pauseUpdate: function sb_pauseUpdate() {
> > -    this._paused++;
> > +    this._paused = 1;
> >    },
> >  
> >    resumeUpdate: function sb_resumeUpdate() {
> > -    this._paused--;
> > +    this._paused = 0;
> >      this._updateIconVisibility();
> >    },
> 
> We cannot really use a boolean here because there are many different events
> interacting with pauseUpdate() and resumeUpdate() and we really want to
> refresh the status bar when these events have been fired symmetrically (i.e.
> pauseUpdate() and resumeUpdate() must be called the same number of times).

Forgive me，if call ‘resumeUpdate()’,we want to update status bar,why not is 'this._paused = 0'?
If because some accident，‘resumeUpdate()’ and ‘pauseUpdate()‘ cannot be called symmetrically？
Sorry，this is just my doubt，because I encountered unexpected value of '_paused'（even negative）,when monkey test

Comment 23

[Johan Lorenzo [:jlorenzo]]

[Blocking Requested - why for this release]: :julienw and I hit this issue multiple times on our dogfooding device on 2.1[1] and I repro'd it with the solid STRs described in comment 15.

[1] Gaia-Rev        230a9c5303e2113f7dbcaf431441ae2138853141
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/e43e97795f5d
Build-ID        20150116001340
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141120.194707
FW-Date         Thu Nov 20 19:47:17 EST 2014
Bootloader      L1TC00011880

Comment 24

[Etienne Segonzac (:etienne)]

Comment on attachment 8550292 [details] [review]
Github PR

Instead of preventing this call to `.hide` I'd like to change the `hide` method to make sure that we prevent unwanted events from being dispatched. Sounds safer for the future :)

And the unit test we'll be clearer (ie. `should not dispatch XXX if already hidden`),

What do you think? (if it's too much of a hassle we'll keep this approach and try to add an integration test and better comments)

Comment 25

[Guillaume Marty [:gmarty]]

Comment on attachment 8550292 [details] [review]
Github PR

I've updated the patch with the following:
* Reorganise events to be more meaningful
* Introduce new events 'utility-tray-abortopen' and 'utility-tray-abortclose'

I'm very tempted to go further and merge similar events together ('utility-tray-overlayopened' and 'utilitytrayshow' ; same for closing the utility tray). Or maybe even use 'utilitytray-activated' only.

Etienne, can you tell me what you think of that?

Comment 26

[Etienne Segonzac (:etienne)]

Comment on attachment 8550292 [details] [review]
Github PR

The current approach makes for a very readable patch :)

I think we should keep it that way and just add more unit testing (I'd like to assert that when a abortopen is dispatched, close is not) + cover the statusbar change.

Thanks!

Comment 27

[Guillaume Marty [:gmarty]]

Comment on attachment 8550292 [details] [review]
Github PR

I wrote unit tests for this patch (and for all events triggered by hide/show).
Is there anything missing here?

Comment 28

[Etienne Segonzac (:etienne)]

Comment on attachment 8550292 [details] [review]
Github PR

r=me with a couple of |testEventThatResume| to cover the statusbar change.

Comment 29

[Guillaume Marty [:gmarty]]

Landed in https://github.com/mozilla-b2g/gaia/commit/41fe4d8ff55e03f777b4f06c313a500c9a089441

Comment 30

[Julien Wajsberg [:julienw]]

Guillaume, don't forget approval for 2.1 and 2.2 :)
(because this is really painful :p)

Comment 31

[Guillaume Marty [:gmarty]]

Comment on attachment 8550292 [details] [review]
Github PR

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Status bar
[User impact] if declined: Under some circumstances, the minimised status bar freezes and the icons are not updated (time, networks status...). The only fix is then to reboot the device.
[Testing completed]: This patch is fully unit tested, but manual testing is required too.
[Risk to taking this patch] (and alternatives if risky): Very low risk as it's completely tested.
[String changes made]: None

Comment 32

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/46d31f52446f17511d41f8a02908ec92a96e9284
v2.1: https://github.com/mozilla-b2g/gaia/commit/bca70e96979fbd714012dc442a92b9fa156f63f7

Comment 33

[Ryan VanderMeulen [:RyanVM]]

Reverted from v2.1 for Gaia unit test failures. Alas, my attempts at rebasing fell short :(
v2.1: https://github.com/mozilla-b2g/gaia/commit/17bf14f12e43043654498330d610d469b8b55e64

https://treeherder.mozilla.org/logviewer.html#?job_id=78071&repo=mozilla-b2g34_v2_1

Vincent, you should really hold off on merging v2.1 to v2.1s until Treeherder is showing green on the uplifts, FWIW.
https://treeherder.mozilla.org/#/jobs?repo=mozilla-b2g34_v2_1

Comment 34

[Ryan VanderMeulen [:RyanVM]]

ni? gmarty for a branch patch

Comment 35

[Vincent Liu[:vliu]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #33)
> Reverted from v2.1 for Gaia unit test failures. Alas, my attempts at
> rebasing fell short :(
> v2.1:
> https://github.com/mozilla-b2g/gaia/commit/
> 17bf14f12e43043654498330d610d469b8b55e64
> 
> https://treeherder.mozilla.org/logviewer.html#?job_id=78071&repo=mozilla-
> b2g34_v2_1
> 
> Vincent, you should really hold off on merging v2.1 to v2.1s until
> Treeherder is showing green on the uplifts, FWIW.
> https://treeherder.mozilla.org/#/jobs?repo=mozilla-b2g34_v2_1

ok.

Comment 36

[Lance（leave from Mozilla）]

This bug has been verified to fail on Flame 2.2 and 3.0.

Steps:
1. Enable the lockscreen.
2. Open the utility tray.
3. Start to slide up as if you wanted to close the utility tray.
4. While keeping your finger on the screen, push the power button.
5. Release your finger on screen and push the power button again.
6. Unlock device, wait for 1 minute, view the time at top-right.

Actual result:
6. The time will skip 2 minutes later(ex:1:50 -> 1:52).

Expect result:
6. The time should update immediately.

Found time:1:51
See attachment(Flame 2.2):151.mp4 and logcat_151.txt
Fail rate:3/5

Flame 2.2 version:
Build ID               20150203002504
Gaia Revision          cd62ff9fe199fb43920ba27bd5fdbc5c311016fc
Gaia Date              2015-02-03 00:56:43
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/11d93135c678
Gecko Version          37.0a2
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150203.041704
Firmware Date          Tue Feb  3 04:17:15 EST 2015
Bootloader             L1TC000118D0

Flame 3.0 version:
Build ID               20150203055658
Gaia Revision          ae5a1580da948c3b9f93528146b007fc4f6a712b
Gaia Date              2015-02-02 19:50:21
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/ae5d04409cd9
Gecko Version          38.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150203.093120
Firmware Date          Tue Feb  3 09:31:32 EST 2015
Bootloader             L1TC000118D0

Comment 37

[Lance（leave from Mozilla）]

Attachment: logcat_151.txt

Comment 38

[Lance（leave from Mozilla）]

Attachment: 151.MP4

Comment 39

[William Hsu [:whsu]]

(In reply to Lance from comment #36)
> This bug has been verified to fail on Flame 2.2 and 3.0.
> 
> Steps:
> 1. Enable the lockscreen.
> 2. Open the utility tray.
> 3. Start to slide up as if you wanted to close the utility tray.
> 4. While keeping your finger on the screen, push the power button.
> 5. Release your finger on screen and push the power button again.
> 6. Unlock device, wait for 1 minute, view the time at top-right.
> 
> Actual result:
> 6. The time will skip 2 minutes later(ex:1:50 -> 1:52).
> 
> Expect result:
> 6. The time should update immediately.
> 

Hi, Lance,

I am not sure if it is a side effect of this patch.
But, I suggest to file a new bug to trace it.

Could you help on this?
Many thanks.

Comment 40

[Julien Wajsberg [:julienw]]

I'm quite sure I saw what Lance sees in previous versions of Firefox OS. So I agree it's likely not related to this.

Comment 41

[William Hsu [:whsu]]

(In reply to Julien Wajsberg [:julienw] from comment #40)
> I'm quite sure I saw what Lance sees in previous versions of Firefox OS. So
> I agree it's likely not related to this.

Cool! Thanks Julien! :D

--- -- - --- -- - --- -- - --- -- -
Hi, Lance,

Just a kind reminder.
Please don't clone this bug since it's likely another problem.

Comment 42

[Lance（leave from Mozilla）]

According to comment 36 and 41, this bug has been fixed, I have upload new bug 1129835, please check it, thanks.

Comment 43

[Vincent Liu[:vliu]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #33)
> Reverted from v2.1 for Gaia unit test failures. Alas, my attempts at
> rebasing fell short :(
> v2.1:
> https://github.com/mozilla-b2g/gaia/commit/
> 17bf14f12e43043654498330d610d469b8b55e64
> 
> https://treeherder.mozilla.org/logviewer.html#?job_id=78071&repo=mozilla-
> b2g34_v2_1
> 
> Vincent, you should really hold off on merging v2.1 to v2.1s until
> Treeherder is showing green on the uplifts, FWIW.
> https://treeherder.mozilla.org/#/jobs?repo=mozilla-b2g34_v2_1

Hi Ryan,

Should I still hold off merging?

Comment 44

[Vincent Liu[:vliu]]

ni? by myself to keep tracking.

Comment 45

[Ryan VanderMeulen [:RyanVM]]

Yes, you should definitely merge the backout to 2.1s! My point is that you shouldn't have merged 2.1 to 2.1s in the first place without double-checking Treeherder to make sure that tests were passing on b2g34 so you don't end up merging bad patches to your branch as well.
https://treeherder.mozilla.org/#/jobs?repo=mozilla-b2g34_v2_1

Comment 46

[Guillaume Marty [:gmarty]]

Ryan, do you still need a branch patch from me or is Vincent going to take care of it?

Comment 47

[Ryan VanderMeulen [:RyanVM]]

Yes, we still need a branch patch from you that can land on v2.1. Vincent is merging v2.1 to v2.1s, so he's depending on you as well.

Comment 48

[GitHub Autolander Bot]

Attachment: [PullReq] gmarty:Bug-1112706-Minimized-and-maximized-statusbar-can-become-out-of-sync-v2.1 to mozilla-b2g:v2.1

Comment 49

[Lance（leave from Mozilla）]

Hi Reporter,

    Could you please tell me what is your device MEM when you found this bug on 2.1S ?  512M or 256M？  You can get the "MemTotal" using below command: " adb shell cat /proc/meminfo ".

Thank you very much.

Comment 50

[Julien Wajsberg [:julienw]]

Hey Lance, I believe this issue happens whatever the memory is.

Comment 51

[Lance（leave from Mozilla）]

Thanks for Julien's help.

The problem is verified not happen on latest 2.1s(256m and 512m) build.

Steps:
1. Enable the lockscreen.
2. Open the utility tray.
3. Start to slide up as if you wanted to close the utility tray.
4. While keeping your finger on the screen, push the power button.
5. Release your finger on screen and push the power button again.
6. Unlock device, wait for 1 minute, view the time at top-right.

Actual result:
6. The time should update immediately.

Fail rate:0/5
See attachment:1649.mp4

2.1s(256m) version:
Build ID               20150208161201
Gaia Revision          bca70e96979fbd714012dc442a92b9fa156f63f7
Gaia Date              2015-02-03 00:37:47
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/afac5ac46ff6
Gecko Version          34.0
Device Name            scx15_sp7715ga
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150208.194618
Firmware Date          Sun Feb  8 19:46:30 EST 2015

2.1s(512m) version:
Gaia-Rev        bca70e96979fbd714012dc442a92b9fa156f63f7
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/afac5ac46ff6
Build-ID        20150208161201
Version         34.0
Device-Name     scx15_sp7715ea
FW-Release      4.4.2
FW-Incremental  93
FW-Date         Thu Jan 22 15:21:20 CST 2015

Comment 52

[Julien Wajsberg [:julienw]]

Hey Vincent,

I'm confused... is the patch still in v2.1S? It was backed out from v2.1 in comment 33...
If it was backed out from v2.1s as well, how could the issue be fixed?

Comment 53

[Vincent Liu[:vliu]]

(In reply to Julien Wajsberg [:julienw] from comment #52)
> Hey Vincent,
> 
> I'm confused... is the patch still in v2.1S? It was backed out from v2.1 in
> comment 33...
> If it was backed out from v2.1s as well, how could the issue be fixed?

I'd checked the revert has be done in v2.1. I will try to merge this revert to v2.1s.

Comment 54

[Michael Henretty [:mikehenrty][:mhenretty]]

(In reply to Autolander from comment #48)
> Created attachment 8561190 [details] [review]
> [PullReq]
> gmarty:Bug-1112706-Minimized-and-maximized-statusbar-can-become-out-of-sync-
> v2.1 to mozilla-b2g:v2.1

Guillaume, did you verify your patch passed the failing tests like system/test/unit/utility_tray_test.js? If so, please either land your PR or flag for checkin-needed.

Comment 55

[Ryan VanderMeulen [:RyanVM]]

v2.1: https://github.com/mozilla-b2g/gaia/commit/4540eed68c929d39c596c24c2af2ed0413774f7c

Comment 56

[:gerard-majax]

Sorry, but I am still experiencing this issue very often on Z3 Compact running a master build from yesterday. As a matter of fact, it looks to have been reproducing much more often since I have restored an old profile on this device.

Comment 57

[Guillaume Marty [:gmarty]]

Alex, are there any recurrent patterns that trigger this bug?
Next time it happens to you, can you connect to the WebIDE and give me the value of StatusBar._paused?
Also, can you deactivate screen lock and see if it has an impact over the reproducibility rate?k

Comment 58

[:gerard-majax]

(In reply to Guillaume Marty [:gmarty] from comment #57)
> Alex, are there any recurrent patterns that trigger this bug?

Sadly, no.

> Next time it happens to you, can you connect to the WebIDE and give me the
> value of StatusBar._paused?

Does it helps even several minutes after ? It's often happening when I don't have a way to plug WebIDE.

> Also, can you deactivate screen lock and see if it has an impact over the
> reproducibility rate?k

That's something we could try.

Comment 59

[:gerard-majax]

> > Next time it happens to you, can you connect to the WebIDE and give me the
> > value of StatusBar._paused?
> 
> Does it helps even several minutes after ? It's often happening when I don't
> have a way to plug WebIDE.

It's reproducing right now, and I have -4.

Comment 60

[Michael Henretty [:mikehenrty][:mhenretty]]

(In reply to Alexandre LISSY :gerard-majax from comment #59)
> > > Next time it happens to you, can you connect to the WebIDE and give me the
> > > value of StatusBar._paused?
> > 
> > Does it helps even several minutes after ? It's often happening when I don't
> > have a way to plug WebIDE.
> 
> It's reproducing right now, and I have -4.

Yup, that sounds like a bug. Alex, could you file a new bug and cc Guillaume and myself?