984673 - [B2G][Marketplace]Attempted creation of New Account demonstrates unexpected white area section to display within Latch App

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Montoya Clemmons (MClemmons)]

When user taps Need an Account? Register for free! within Latch app, there is an option for the user to create a new account with name, email, password and confirm password. When these fields are filled and the user checks the box confirming reading and agreeing to User agreement, a white area appears and remains beneath the Read agreement button. This white area remains in place until user interacts with that area to have the Register button appear.

Prerequisites: 
Tap Marketplace, download, install and tap Latch

Repro Steps:
1) Updated Buri to BuildID: 20140317040204
2) Tap Need an account? Register for free!
3) Insert name, email, password and confirm password in appropriate fields
4) Tap the box confirming reading and agreeing to User Agreement

Actual:
Unexpected white space appears at bottom of device screen beneath the Read agreement button. It remains until user interacts with device in that area for the Register button to appear.

Expected:
No white space appears at bottom of device screen beneath the Read agreement button. The Register button appears beneath the Read agreement button as expected. 

Environmental Variables:
Device: Buri 1.4 MOZ
BuildID: 20140317040204
Gaia: 8f802237927c7d5e024fb7dca054dd5efef6b2e6
Gecko: 25cfa01ba054
Version: 30.0a1
Base image: v1.2-device.cfg

Notes:
Repro frequency: (5/5, 100%)
Link to failed test case: none found
Video Attached = http://www.youtube.com/watch?v=aX7WYJb2Dxs

Comment 1

[Montoya Clemmons (MClemmons)]

This issue does not reproduce on 1.3 Buri. Following the STR in Comment 0, user witnesses the Register button appearing beneath the Read agreement button naturally without a white area displaying. 

Environmental Variables:
Device: Buri 1.3 MOZ
BuildID: 20140317004001
Gaia: 0ab8a9cbcef5f23cec904a3d7f7675e44de29951
Gecko: f824e9d91a2d
Version: 28.0
Base image: v1.2-device.cfg

Comment 2

[Ivan Tsay (:ITsay)]

Triage: Plus this regression bug for v1.4

Rudy,

Mind if you can take a look this regression bug on keyboard.

Comment 3

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

I don't think this is a keyboard issue, since the keyboard would be dismissed correctly.
And the underlying area needs to be re-painted, but somehow it was not.

I tried to disable APZ and found it could not be reproduced after disabling APZ.
So, change the component to get more eyes on this.

Thanks.

Comment 4

[Jason Smith [:jsmith]]

Does this reproduce with tiling disabled on 1.4?

Comment 5

[J Zimbrick]

(In reply to Jason Smith [:jsmith] from comment #4)
> Does this reproduce with tiling disabled on 1.4?

With tiling disabled, you can still occasionally see the white detailed in this bug, but it does not stick as it does in the attached youtube video, instead it disappears after about a second without any user interaction.

With APZ disabled, the white does not seem to appear in any way when following the repro steps.

Environmental Variables:
Device: Buri
BuildID: 20140319000200
Gaia: c03a6af9028c4b74a84b5a98085bbb0c07261175
Gecko: b07ecc057abf
Version: 30.0a2
Base Image: V1.2-device.cfg

Comment 6

[Jason Smith [:jsmith]]

comment 5 makes me think this is likely a tiling problem.

Comment 7

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

(In reply to Jason Smith [:jsmith] from comment #6)
> comment 5 makes me think this is likely a tiling problem.

Hmm, I was going with the part of comment 5 that says "With tiling disabled, you can still occasionally see the white..." and "With APZ disabled, the white does not seem to appear..." and considered it APZC related, rather than tiling related, but you never know...

Comment 8

[Jason Smith [:jsmith]]

(In reply to Milan Sreckovic [:milan] from comment #7)
> (In reply to Jason Smith [:jsmith] from comment #6)
> > comment 5 makes me think this is likely a tiling problem.
> 
> Hmm, I was going with the part of comment 5 that says "With tiling disabled,
> you can still occasionally see the white..." and "With APZ disabled, the
> white does not seem to appear..." and considered it APZC related, rather
> than tiling related, but you never know...

Oh - the way I interpreted this was:

* APZC + Tiling: Persistent White
* APZC + No Tiling: 1 second white shown
* No APZC + No Tiling: No White

Comment 9

[J Zimbrick]

(In reply to Jason Smith [:jsmith] from comment #8)
> (In reply to Milan Sreckovic [:milan] from comment #7)
> > (In reply to Jason Smith [:jsmith] from comment #6)
> > > comment 5 makes me think this is likely a tiling problem.
> > 
> > Hmm, I was going with the part of comment 5 that says "With tiling disabled,
> > you can still occasionally see the white..." and "With APZ disabled, the
> > white does not seem to appear..." and considered it APZC related, rather
> > than tiling related, but you never know...
> 
> Oh - the way I interpreted this was:
> 
> * APZC + Tiling: Persistent White
> * APZC + No Tiling: 1 second white shown
> * No APZC + No Tiling: No White

This is correct.

Comment 10

[Botond Ballo [:botond]]

I tested this with Nexus 4 running trunk. 

I saw the brief flash of white (not persistent) both with tiling enabled and disabled. However, note that the Latch app is not scrollable on Nexus 4 (because the screen is large enough to fit the whole contents) and tiling is only enabled for scrollable layers, so I wasn't really testing it with tiling even when it was enabled. I believe this is further evidence that the "persistent white" is an issue caused by tiling.

I believe the brief flash of white that we see without tiling is normal - it takes this short period of time for the app to repaint the area under the keyboard that is newly made visible. You can see this elsewhere, too, for example on the "Add Contact" page of the Contacts app if you tap on an input field (to activate the keyboard) and then tap away from any input fields (to hide the keyboard).

Comment 11

[Botond Ballo [:botond]]

> I believe the brief flash of white that we see without tiling is normal - it
> takes this short period of time for the app to repaint the area under the
> keyboard that is newly made visible.

I misspoke here. If it were merely the case that the region of the app under the keyboard wasn't visible, it might still be reasonable to expect that that region is part of the displayport and therefore already painted. In this case, however, the app actually resizes to be shorter than the screen when the keyboard is activated, so even though the displayport is the whole app, it does not include all of the area occluded by the keyboard. When the keyboard hides, the app resizes again to fill the screen's height, and the newly added area must be repainted. The conclusion - that this behaviour is normal - stands.

Comment 12

[Botond Ballo [:botond]]

I tested with Buri and reproduced the "persistent white" issue with tiling on. At the time the white area is shown, the page's scrollable rect is large enough to fill the screen, but the displayport isn't, suggesting that this is in fact an APZ issue. I'm not sure what the interaction with tiling is - possibly something to do with aligning the displayport to tile boundaries. I'm continuing to investigate.

Comment 13

[Botond Ballo [:botond]]

Note that I can only reproduce the "persistent white" issue intermittently.

Comment 14

[Botond Ballo [:botond]]

I have tried some more to reproduce the "persistent white" issue but my repro rate is so low that I can't do any useful debugging. 

Milan suggested that perhaps this may be related to bug 984618. My understanding is that Chris is working on a patch for that, once that is posted I'll try to see if it fixes the problem.

Comment 15

[Botond Ballo [:botond]]

Some more assorted pieces of information:

  - Even when tiling is enabled, I sometimes see the page in question use tiles,
    and sometimes not. (I turn on 'draw tile borders' to tell. I've confirmed
    with the debugger that this is accurate, i.e. when I see tile borders I see
    ClientTiledThebesLayer::RenderLayer() being called but not ClientThebesLayer::
    RenderLayer(), and vice versa when I do not see tile borders). The "permament
    white" problem reproduces, very intermittently, _independently_ of whether
    tiles are actually being used or not.

  - Adding a 50 ms sleep in APZCTreeManager::UpdatePanZoomControllTree causes
    the bug to repro marginally more often.

  - Scrolling the page while the keyboard is active, and then quickly tapping to
    make the keyboard disappear causes the bug to repro significantly more often.

Comment 16

[Botond Ballo [:botond]]

Attachment: Do not compress the PBrowser::UpdateFrame message

So it turns out the problem is that the 'UpdateFrame' IPC message that TabParent sends to TabChild when requesting a content repaint, was being dropped. It is upon receiving this message that TabChild sets the displayport requested by APZ. Since the message is dropped, the displayport is not updated to reflect the larger size of the app after the keyboard disappears.

The reason the message is dropped is because it is declared as having a 'compress' attribute [1], which basically means that if multiple messages of the same type get sent to the same destination in quick succession, the protocol is allowed to drop all but the most recent one. This may have been appropriate for PBrowser::UpdateFrame before we had subframe scrolling, but it isn't now that we have subframe scrolling because UpdateFrame can potentially be called for each scrollable frame, and we want each of those calls to be processed.

The fix is simply to remove the 'compress' attribute.

[1] http://dxr.mozilla.org/mozilla-central/source/dom/ipc/PBrowser.ipdl?from=PBrowser.ipdl#357
[2] http://dxr.mozilla.org/mozilla-central/source/ipc/glue/MessageChannel.cpp#468

Comment 17

[Botond Ballo [:botond]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9ecc9b16ef53

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9ecc9b16ef53

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ad93689d9dc8

Comment 20

[Srinivas Vemula]

Any One is facing issue of Image rotation with this patch ?
Steps to produce the issue:
1) Open the image from gallery
2) Rotate the image landscape to portrait (Portrait to Landscape) 10-12 times(max).The image is getting enlarged(starts rotation with portrait to landscape)or shrinking(Starts rotation with landscape to portrait).
This issue is not reproducing when compress is used.

Comment 21

[jongsoo.oh]

Comment 20 issue is detected to side effect of bug984673 patch.
The issue(bug 1022462) was created.

Comment 22

[Botond Ballo [:botond]]

My understanding is that 'compress' on UpdateFrame is an optimization that was appropriate when we only had APZ for the root frame, and became inappropriate when we introduced APZ for subframes.

If the behaviour of the gallery changed as a result of removing 'compress', and what we want is the old behaviour, it's possible that something else is going wrong, and we were getting the behaviour we wanted with 'compress' by accident.

Let's take any follow-up discussion to bug 1022462.