987771 - [B2G] Taking the phone in and out of standby with the keyboard up will hide messages

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Julien Wajsberg [:julienw]]

+++ This bug was initially created as a clone of Bug #980041 +++

Description:
The messages in the Message app will be cut off after unlocking the phone.

Repro Steps:
1) Updated Peak 1.3 to images-peak-v1.3-2014-03-25.Gecko-72f48b2.Gaia-b789780.zip
2) Select the Message app.
3) Select a message thread with multiple messages.
4) Tap the text field to open up the keyboard.
5) Press the power button to put the phone to sleep.
6) Press the power button again and unlock the phone.

Actual:
Some of the messages in the app are cut off and cannot be seen.

Expected:
No messages are cut off.

Environmental Variables:
Device: Peak 1.3 Geeksphone
BuildID: 20140325
Gaia: b789780
Gecko: 72f48b2
Version: 28

Notes:
The Gaia and Gecko hashes are the Git hashes.

Repro frequency: 100%

With a build from after the fix from bug 980041 (Gecko f677c14), I also got Bug 968960 a lot (didn't reproduce on the new build yet, as it's more intermittent).

QAWanted: can you try on Buri 1.3?
Requesting 1.3? already so that this bug is acted on earlier, even if it's not confirmed on Buri yet.

Comment 1

[Matthew Vaughan]

This issue does reproduce on the 03/24/14 1.3 build on a Buri.

Device: Buri v1.3 MOZ RIL
BuildID: 20140324004001
Gaia: f7742fb4929cc57c9f72955ce5cebb8279745ac0
Gecko: e42b778a010f
Version: 28.0
Firmware Version: V1.2-device.cfg

Comment 2

[Kartikaya Gupta (email:[e-mail])]

QAwanted to get a video showing this on 1.3. Also I would like to know if this is reproducible on 1.4 or master.

Comment 3

[Matthew Vaughan]

Attachment: Video of issue on 1.3

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #2)
> QAwanted to get a video showing this on 1.3. Also I would like to know if
> this is reproducible on 1.4 or master.

Attached is the requested video for 1.3 (sorry about the quality, but it should be okay). 

On 1.4 and master (using the Buri), the issue occurs for less than a second and then fixes itself.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Thanks!

Comment 5

[Milan Sreckovic [:milan] (needinfo for best results)]

Wasn't clear - is this a regression from bug 980041 or some things haven't been backported by that bug?

Comment 6

[Kartikaya Gupta (email:[e-mail])]

I think the symptoms have multiple causes. I was never able to reproduce bug 980041 myself [1], I was only able to reproduce bug 973980 and so that's what I fixed. It's possible there's some other issues causing the same symptoms on 1.3.

Personally I'd like to close this bug as WFM and wontfix it for 1.3.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=980041#c24

Comment 7

[Jason Smith [:jsmith]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #6)
> I think the symptoms have multiple causes. I was never able to reproduce bug
> 980041 myself [1], I was only able to reproduce bug 973980 and so that's
> what I fixed. It's possible there's some other issues causing the same
> symptoms on 1.3.
> 
> Personally I'd like to close this bug as WFM and wontfix it for 1.3.
> 
> [1] https://bugzilla.mozilla.org/show_bug.cgi?id=980041#c24

That's not happening. TCL is blocking on this right now because they can reproduce this. QA can reproduce this as well, I'm not sure why you can't reproduce this. You need to figure out why can't reproduce right now, cause no one has had a problem right now reproducing this.

Comment 8

[Jason Smith [:jsmith]]

(In reply to Milan Sreckovic [:milan] from comment #5)
> Wasn't clear - is this a regression from bug 980041 or some things haven't
> been backported by that bug?

The window for this bug is here - https://bugzilla.mozilla.org/show_bug.cgi?id=980041#c18.

To my understanding, the patch in bug 980041 never actually fixed the problem in the bug.

Comment 9

[Jason Smith [:jsmith]]

(In reply to Jason Smith [:jsmith] from comment #7)
> (In reply to Kartikaya Gupta (email:[e-mail]) from comment #6)
> > I think the symptoms have multiple causes. I was never able to reproduce bug
> > 980041 myself [1], I was only able to reproduce bug 973980 and so that's
> > what I fixed. It's possible there's some other issues causing the same
> > symptoms on 1.3.
> > 
> > Personally I'd like to close this bug as WFM and wontfix it for 1.3.
> > 
> > [1] https://bugzilla.mozilla.org/show_bug.cgi?id=980041#c24
> 
> That's not happening. TCL is blocking on this right now because they can
> reproduce this. QA can reproduce this as well, I'm not sure why you can't
> reproduce this. You need to figure out why can't reproduce right now, cause
> no one has had a problem right now reproducing this.

One idea to try here is to see if you can reproduce with a pvtbuild, rather than a local build. There have been cases where certain graphics bugs have only reproduced with a pvtbuild and not a local build.

Comment 10

[Julien Wajsberg [:julienw]]

Kats, Milan, if this is any useful, I can reproduce this very easily and every time on a 1.3 build from geeksphone. If necessary I can show this to Nical tomorrow, who is in the same office.

During 1 week I used this phone only, and during that 1 week, I got this a lot, and I also got Bug 968960 a lot. 

Here are my personal feelings:
* this bug makes the phone look broken, but is not _that_ bad. I mean that it's quite easy to recover from it.
* Bug 968960 is a lot more awkward, and it's not obvious how to recover.

However, we don't have clear STR for Bug 968960, whereas this bug is 100% reproducible for me. My hope is that the fix for this bug will also fix Bug 968960.

Hope this helps.

Comment 11

[Milan Sreckovic [:milan] (needinfo for best results)]

Perfect, thanks. I think we have to assume that the problem you're seeing on Keon is the same bug as what's seen on Buri, so having a higher percentage of reproduction would help.

Comment 12

[Kartikaya Gupta (email:[e-mail])]

I tried reproducing this on the 2014-03-28-00-40-02 Hamachi 1.3 pvtbuild with a medium reference workload. I used a bunch of the different SMS threads but still was unable to reproduce.

Comment 13

[Jason Smith [:jsmith]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #12)
> I tried reproducing this on the 2014-03-28-00-40-02 Hamachi 1.3 pvtbuild
> with a medium reference workload. I used a bunch of the different SMS
> threads but still was unable to reproduce.

No-Jun - Can you try to reproduce this? If you can reproduce this, then can you show this to Botond directly, since I think he's in the same office as you?

Comment 14

[Kartikaya Gupta (email:[e-mail])]

Note that Botond is PTO today and will be at a work week next week so he won't be back in the office until the week after.

Comment 15

[Jason Smith [:jsmith]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #14)
> Note that Botond is PTO today and will be at a work week next week so he
> won't be back in the office until the week after.

Well I guess that idea failed. I'll go talk to Milan about this to see what ideas he has.

Comment 16

[Nicolas Silva [:nical]]

So I have Julien's phone with me and here are some observations about the bug:

Once the bug reproduces, Layer borders show that the layer containing the messages stops where the keyboard used to be, so the content that is cut off isn't there because the layer doesn't extend far enough.

If I go back to the home screen with the home button and get back to the sms app, paint flashing seems to indicate that the content is repainted, even in the area that is cut off, but the layer is still smaller than it should be.

If I go to the sms list page by pressing the top left back button, and go to another discussion, the layer containing the messages will also be cut vertically at the same place, even if it has a different width.

As long as I don't scroll in a discussion the layers containing the messages will be cut, even if things get repainted.

I don't know if it is the presence of the keyboard on to of the app or the app being resized that influences how we shape our layers, but it is still affecting the layerization (of the layer containing the messages specifically), after the keyboard disappears.

I am hesitant to upload videos illustrating this because it's in the middle of Julien's personal conversations...

Anyway I think we can rule out compositor-side bugs. I think this is happening in layout in the app's process.

Comment 17

[Kartikaya Gupta (email:[e-mail])]

(In reply to Nicolas Silva [:nical] from comment #16)
> Anyway I think we can rule out compositor-side bugs. I think this is
> happening in layout in the app's process.

Do you mean a bug in the layout code, or a bug in the app in that it's not dealing with the resize properly? We should able to confirm the latter by using the App Manager and checking the size of the appropriate container elements to see if they are correct.

Comment 18

[Julien Wajsberg [:julienw]]

We detect the resize event, and we set the height of the container using inline styles (for various reasons) (see [1]).

We checked with the app manager that it behaves like this accordingly, both using style.height and .offsetHeight properties ("document.getElementById('messages-container')" finds you the correct element).

We have a different value when the keyboard is here and when it's hidden.
We have the same value with the keyboard hidden when the bug happens and when it doesn't.

[1] https://github.com/mozilla-b2g/gaia/blob/v1.3/apps/sms/js/thread_ui.js#L1005

Comment 19

[Kartikaya Gupta (email:[e-mail])]

So if the app is resizing things correctly, but the visible region of the layer is wrong in the compositor (that is what is painted as the layer border [1]), then yeah, the problem would be in layout somewhere.

[1] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/Compositor.cpp?rev=b2fc3f9509b0#87

Comment 20

[Jason Smith [:jsmith]]

Milan - Can you get someone assigned to fix this?

Comment 21

[Julien Wajsberg [:julienw]]

Hey kats, can you suggest an owner for this bug?

Comment 22

[Kartikaya Gupta (email:[e-mail])]

Since you can reproduce it most easily, would you be willing to enabling various bits of logging and send it to me? You'd have to build gecko locally with some changes.

Comment 23

[Julien Wajsberg [:julienw]]

Yes of course, just tell me what and how to enable and I'll gladly do it.

Comment 24

[Kartikaya Gupta (email:[e-mail])]

Attachment: Logging patch

Here's a patch which enables some APZ and layers logging. Please run with this and grab |adb logcat -v time| while reproducing the problem. If the log is really big it would also be good to indicate roughly where in the log (maybe using the timestamp) the problem occurred. Alternatively just stop logging right after it happens and I can look at the end of the log. Thanks!

Comment 25

[Julien Wajsberg [:julienw]]

Attachment: apzc.log

Here is the log, I expunged it from wpa_supplicant lines.

Around 11:32:35 I pressed the "power" button to put the phone on sleep
I think 11:32:39.179 is where I pressed it back to ake up the phone
Then (probably 11:32:44.609 ?) I scrolled up/down which made the invisible area to finally show up.

Here is the output of b2g-ps so that you have the pids:

APPLICATION      USER     PID   PPID  VSIZE  RSS     WCHAN    PC         NAME
b2g              root      126   1     216476 74708 ffffffff 400be580 S /system/b2g/b2g
(Nuwa)           root      335   126   52268  19336 ffffffff 400cf580 S /system/b2g/plugin-container
Usage            app_355   355   335   64316  24408 ffffffff 400cf580 S /system/b2g/plugin-container
Homescreen       app_387   387   335   67752  28284 ffffffff 400cf580 S /system/b2g/plugin-container
Messages         app_435   435   335   95804  40932 ffffffff 400cf580 S /system/b2g/plugin-container
(Preallocated a  root      482   335   60452  19736 ffffffff 400cf580 S /system/b2g/plugin-container

Comment 26

[Julien Wajsberg [:julienw]]

Also remember it's on a 1.3 Gecko (I resolved the easy conflicts and applied your patch on my 1.3 tree).

Comment 27

[Kartikaya Gupta (email:[e-mail])]

Thanks! Based on the log I was able to create a test page that reproduces the problem. I will see if I can put together a low-risk fix.

Comment 28

[Kartikaya Gupta (email:[e-mail])]

So on 1.3 and 1.4 both the APZC instance is destroyed when going to the lockscreen and a new one is created when coming back. The difference is that on 1.3 when we come back the first NLU call already has the updated composition bounds and viewport, and so we never take the codepath that sets needsContentRepaint to true. On 1.4 when we come back the NLU still has the old composition bounds and viewport. These get updated in another call to NLU and therefore exercise the needsContentRepaint=true codepath. The new repaint sends a new displayport which is correct and so prevents the bug from happening.

The easy fix is to just stick in a needsContentRepaint=true in the isDefault path for NLU. However I'll have to do some code archaeology first because I'm pretty sure that used to be there and it was taken out for some reason.

Comment 29

[Kartikaya Gupta (email:[e-mail])]

Yeah it was taken out in bug 916379. I'll see if I can come up with a reasonable fix that doesn't regress that one.

Comment 30

[Kartikaya Gupta (email:[e-mail])]

Attachment: Possible fix for 1.3

This seems to work for me on my test page. Julien, can you check if it fixes the problem for you?

Comment 31

[Julien Wajsberg [:julienw]]

I needed to clone gecko on this new computer first, and it was very slow; hence the delay. But I'll get you a feedback before the end of the day.

Comment 32

[Preeti Raghunath(:Preeti)]

Vance

Does TCL care about this bug?

Comment 33

[Julien Wajsberg [:julienw]]

Comment on attachment 8402904 [details] [diff] [review]
Possible fix for 1.3

I see the same behavior than on master now :)

Good work !!

That said I don't completely understand why there is a delay to repaint (shouldn't we try to keep the already painted surface so that it's easily repainted?), but I guess this is for another bug, because this delay is on master too.

Comment 34

[Kartikaya Gupta (email:[e-mail])]

Waiting for needinfo in comment 32 before requesting review here, because if TCL doesn't care about this bug for 1.3 then it's WFM and review time will be wasted.

Comment 35

[Julien Wajsberg [:julienw]]

About the STR in comment 0, you need a thread that has enough messages to scroll.

Kats, it would be nice if you could share your simpler testcase, so that we can see if the issue reproduces on other devices. I reproduce on the Peak for sure, but I think we'll need to know if this reproduce on any shipped device to keep the 1.3+ flag.

Comment 36

[Jason Smith [:jsmith]]

(In reply to Julien Wajsberg [:julienw] from comment #35)
> About the STR in comment 0, you need a thread that has enough messages to
> scroll.
> 
> Kats, it would be nice if you could share your simpler testcase, so that we
> can see if the issue reproduces on other devices. I reproduce on the Peak
> for sure, but I think we'll need to know if this reproduce on any shipped
> device to keep the 1.3+ flag.

Roland already indicated on the other bug that this reproduces on a Buri device.

Comment 37

[Kartikaya Gupta (email:[e-mail])]

(In reply to Julien Wajsberg [:julienw] from comment #35)
> Kats, it would be nice if you could share your simpler testcase, so that we
> can see if the issue reproduces on other devices.

I added the test case to the bug URL in comment 27. There's a 10-second timeout hardcoded in the test, so you have to make sure you scroll the subframe to bottom and lock the screen before the 10-second timeout expires.

Comment 38

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8402904 [details] [diff] [review]
Possible fix for 1.3

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

Right, I forgot about Roland's comment in the other bug. Hopefully this patch fixes what he was seeing as well. r? to botond - note that this patch is for 1.3 only. This issue doesn't occur in 1.4+ (one possible reason is the layer margins change) so I don't think this patch needs to land there yet. It's possible we'll need it on 1.4+ in the future but until we run into a use case that requires it I'd rather not.

Comment 39

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8402904 [details] [diff] [review]
Possible fix for 1.3

NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): APZC
User impact if declined: in some cases when a scrollable container is resized while not visible it will end up not painting properly until the user triggers a repaint by touching it
Testing completed: locally, julien tested it as well. will need wider testing once it's in a 1.3 build
Risk to taking this patch (and alternatives if risky): fairly low risk. since this affects 1.3 only i made the patch very specific so that the new codepath only triggers in specific circumstances.
String or UUID changes made by this patch: none

Comment 40

[bhavana bajaj [:bajaj]]

Comment on attachment 8402904 [details] [diff] [review]
Possible fix for 1.3

Approving and flagging it for verification for QA help here.

Comment 41

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/596bec445506

Comment 42

[Roland Kunkel [:RolandK]]

Verified fixed v1.3T. I am unable to reproduce this issue on the latest v1.3T Tarako build:

v1.3T Environmental Variables:
Device: Tarako v1.3T MOZ RIL
BuildID: 20140530014002
Gaia: e68858693b71d917c9c5ee7e215f7ceea04635f7
Gecko: 1945abae19ff
Version: 28.1
Firmware Version: SP6821a-Gonk-4.0-5-12

-

1) Leaving verifyme, v1.3 branch still needs to be verified.