Closed Bug 1112706 Opened 7 years ago Closed 7 years ago

Minimized and maximized statusbar can become out of sync

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:2.1+, b2g-v2.1 fixed, b2g-v2.1S fixed, b2g-v2.2 verified, b2g-master verified)

RESOLVED FIXED
2.2 S5 (6feb)
blocking-b2g 2.1+
Tracking Status
b2g-v2.1 --- fixed
b2g-v2.1S --- fixed
b2g-v2.2 --- verified
b2g-master --- verified

People

(Reporter: mikehenrty, Assigned: gmarty)

References

()

Details

(Whiteboard: [systemsfe])

Attachments

(5 files)

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
Guillaume, do you have an ideas on what might cause this?
Flags: needinfo?(gmarty)
See Also: → 1107170
Flags: needinfo?(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.
Flags: needinfo?(lissyx+mozillians)
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.
Flags: needinfo?(lissyx+mozillians)
blocking-b2g: 2.2? → 2.2+
Duplicate of this bug: 1107170
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.
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.
Assignee: nobody → gmarty
Naoki, can you help me with STR here? According to the comments above, we don't have a way to repro it.
Flags: needinfo?(nhirata.bugzilla)
I wonder if this is related to bug 1117981 and the device sleeping?
Flags: needinfo?(nhirata.bugzilla)
Flags: needinfo?(nhirata.bugzilla)
Attached image 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, ...)
Flags: needinfo?(nhirata.bugzilla) → needinfo?(gmarty)
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
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
Flags: needinfo?(mhenretty)
sorry,log and video is too big to upload
As comment 12,please give some guidence

Thanks a lot
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.
Flags: needinfo?(gmarty)
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!!
Flags: needinfo?(mhenretty)
(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
Or the value of ‘_paused‘  is true/false can solve this problem?
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();
   },
Flags: needinfo?(gmarty)
Attached file 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?
Flags: needinfo?(gmarty)
Attachment #8550292 - Flags: review?(etienne)
(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).
(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
[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
blocking-b2g: 2.2+ → 2.1?
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)
Attachment #8550292 - Flags: review?(etienne)
blocking-b2g: 2.1? → 2.1+
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?
Attachment #8550292 - Flags: review?(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!
Attachment #8550292 - Flags: review?(etienne) → feedback+
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?
Attachment #8550292 - Flags: review?(etienne)
Comment on attachment 8550292 [details] [review]
Github PR

r=me with a couple of |testEventThatResume| to cover the statusbar change.
Attachment #8550292 - Flags: review?(etienne) → review+
Landed in https://github.com/mozilla-b2g/gaia/commit/41fe4d8ff55e03f777b4f06c313a500c9a089441
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Guillaume, don't forget approval for 2.1 and 2.2 :)
(because this is really painful :p)
Flags: needinfo?(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
Flags: needinfo?(gmarty)
Attachment #8550292 - Flags: approval-gaia-v2.2?(bbajaj)
Attachment #8550292 - Flags: approval-gaia-v2.1?(bbajaj)
Keywords: verifyme
Attachment #8550292 - Flags: approval-gaia-v2.2?(bbajaj)
Attachment #8550292 - Flags: approval-gaia-v2.2+
Attachment #8550292 - Flags: approval-gaia-v2.1?(bbajaj)
Attachment #8550292 - Flags: approval-gaia-v2.1+
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
Flags: needinfo?(vliu)
ni? gmarty for a branch patch
Flags: needinfo?(gmarty)
(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.
Flags: needinfo?(vliu)
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
Flags: needinfo?(whsu)
QA Whiteboard: [MGSEI-Triage+]
Attached video 151.MP4
(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.
Flags: needinfo?(whsu) → needinfo?(liuke)
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.
(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.
See Also: → 1129835
According to comment 36 and 41, this bug has been fixed, I have upload new bug 1129835, please check it, thanks.
Flags: needinfo?(liuke)
(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?
Flags: needinfo?(ryanvm)
ni? by myself to keep tracking.
Flags: needinfo?(vliu)
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
Flags: needinfo?(ryanvm)
Ryan, do you still need a branch patch from me or is Vincent going to take care of it?
Flags: needinfo?(gmarty) → needinfo?(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.
Flags: needinfo?(ryanvm) → needinfo?(gmarty)
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.
Flags: needinfo?(mhenretty)
Hey Lance, I believe this issue happens whatever the memory is.
Flags: needinfo?(mhenretty)
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
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?
(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.
(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.
Flags: needinfo?(gmarty)
Flags: needinfo?(gmarty)
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.
Flags: needinfo?(mhenretty)
Flags: needinfo?(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
Flags: needinfo?(gmarty)
(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.
> > 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.
(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?
Flags: needinfo?(mhenretty)
Depends on: 1144126
You need to log in before you can comment on or make changes to this bug.