1042959 - [B2G][Browser][Youtube]Viewing a video in portrait fullscreen and returning to homescreen causes youtube to display inappropriately

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Ross Kuhlman ([e-mail])]

Attachment: YoutubeLog.txt

Youtube.com will not display correctly if the after user presses home button to return to homescreen while watching a video in fullscreen in portrait orientation, 

Youtube will display incorrectly in one of two ways:
1) Youtube content is is shrunken down and large sections of empty whitespace are displayed
2) The video still plays in fullscreen dimensions, resulting in only part of the screen being visible

Repro Steps:
1) Update a Flame to BuildID: 20140723040201
2) Launch Browser and navigate to www.youtube.com
3) Begin watching a video
4) Set the video to fullscreen, and hold device in portrait orientation
5) Press the home button twice to return to homescreen
6) Tap on browser app.

Actual:
Youtube page is displayed inappropriately. 

Expected:
Youtube page displays as expected

Master M/C Environmental Variables:
Device: Flame Master M/C MOZ
BuildID: 20140723040201
Gaia: 01d86c8cc615658694b114ca5711763efc4ef205
Gecko: d0f6259e8446
Version: 34.0a1
Firmware Version: v122


Notes:

Repro frequency: 75% 
See attached: logcat, screenshot

Comment 1

[Ross Kuhlman ([e-mail])]

Attachment: YoutubeDisplayIssue.jpg

Comment 2

[Peter Bylenga [:PBylenga]]

QAWanted for branch checks.

Comment 3

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

After working with the reporter I found that this issue requires a 273 mb configuration to reproduce on Flame.

This issue does occur on Flame 2.1 (273 mb), 2.0 Flame (273 mb), and 2.1 Buri.

Environmental Variables:
Device: Flame Master (273 mb)
BuildID: 20140723121905
Gaia: 15c84c943e41ad834640a45e1e1c2ac804168af7
Gecko: 30907d52c4c2
Version: 34.0a1 (Master) 
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Environmental Variables:
Device: Flame 2.0
BuildID: 20140723062208
Gaia: ed998af8528c8b1bbd3aee6975ee9a1bdec1d429
Gecko: c74478151c2f
Version: 32.0 (2.0) 
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Environmental Variables:
Device: Buri Master
BuildID: 20140723121905
Gaia: 15c84c943e41ad834640a45e1e1c2ac804168af7
Gecko: 30907d52c4c2
Version: 34.0a1 (Master) 
Firmware Version: v1.2device.cfg
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0



Issue does not occur on 2.1 Flame (512 mb), 2.0 Flame (512 mb), 1.4 Flame (512 mb), 1.4 Flame (273mb)

Environmental Variables:
Device: Flame Master (512 mb)
BuildID: 20140723121905
Gaia: 15c84c943e41ad834640a45e1e1c2ac804168af7
Gecko: 30907d52c4c2
Version: 34.0a1 (Master) 
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Environmental Variables:
Device: Flame 2.0 (512 mb)
BuildID: 20140723062208
Gaia: ed998af8528c8b1bbd3aee6975ee9a1bdec1d429
Gecko: c74478151c2f
Version: 32.0 (2.0) 
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Environmental Variables:
Device: Flame 1.4
BuildID: 20140723075714
Gaia: 6e4eaa5befce9c1812e07bcc78b2138645bdbe7a
Gecko: 2d16ba0b45bc
Version: 30.0 (1.4) 
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0

Environmental Variables:
Device: Flame 1.4
BuildID: 20140723075714
Gaia: 6e4eaa5befce9c1812e07bcc78b2138645bdbe7a
Gecko: 2d16ba0b45bc
Version: 30.0 (1.4) 
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0

Comment 4

[Peter Bylenga [:PBylenga]]

[Blocking Requested - why for this release]:
Smoketest blocker, nominating for 2.0 as it's a 1.4 to 2.0 regression.

Comment 5

[Jason Smith [:jsmith]]

Josh - I think this needs to be checked on 319 MB & needs a window. Can you fix this?

Comment 6

[Joshua Mitchell (Inactive)]

QA-wanted to test the affected builds in 319mb (first)

Comment 7

[Pi Wei Cheng [:piwei] (inactive)]

Original STR's step 4 should be to put the phone in Landscape, not portrait. And then at Step 6 go back to youtube in Portrait.

Issue still reproducible on 2.1 Flame master, 2.0 Flame, and 2.1 Buri. Comment 3 is still true. The repro rate overall is about 1/3 to 1/5.

Device: Flame (319MB mem)
Build ID: 20140725034810
Gaia: 3a06aa58245eaf848242d6d1497c1af536fffabd
Gecko: 6c0971104909
Version: 34.0a1 (Master)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Device: Flame (319MB mem)
Build ID: 20140724190006
Gaia: 9b6d7357031f2412b18a2fb140d5c974842d4393
Gecko: fbb3b8be8f6c
Version: 32.0 (2.0)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Device: Buri
Build ID: 20140725034810
Gaia: 3a06aa58245eaf848242d6d1497c1af536fffabd
Gecko: 6c0971104909
Version: 34.0a1 (Master)
Firmware Version: v1.2device.cfg
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 8

[Joshua Mitchell (Inactive)]

Not requesting regression-window based on extremely low repro rate

Comment 9

[Joshua Mitchell (Inactive)]

striking smoketest keyword based on new (lower) repro rate (below 50%)

Comment 10

[Jason Smith [:jsmith]]

(In reply to Joshua Mitchell [:Joshua_M] from comment #8)
> Not requesting regression-window based on extremely low repro rate

How long does it take to reproduce the bug on average?

Comment 11

[Joshua Mitchell (Inactive)]

(In reply to Jason Smith [:jsmith] - At Work Week, Slow to Respond from comment #10)
> (In reply to Joshua Mitchell [:Joshua_M] from comment #8)
> > Not requesting regression-window based on extremely low repro rate
> 
> How long does it take to reproduce the bug on average?

3-5 minutes depending on how many times you have to retry to hit the 25% repro rate.

Comment 12

[Jason Smith [:jsmith]]

Triage thinks this will likely block YT certification, so +

Comment 13

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

So, no chance of getting a regression window?  Also, given that it appears to be memory specific, can we request that this gets tested on QRD with 8x10 and 8926?

Comment 14

[Jason Smith [:jsmith]]

(In reply to Milan Sreckovic [:milan] from comment #13)
> So, no chance of getting a regression window?  Also, given that it appears
> to be memory specific, can we request that this gets tested on QRD with 8x10
> and 8926?

I don't think we can do that. QA does not have builds & QRD devices. We're also supposed to support the minimum Flame spec as well (319 MB Flame), so this bug does fall under the supported device configurations we need to support.

I'll flag regression window wanted here since it seems we're going to have to do this even if the repro rate is low.

Comment 15

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

I meant "ask QC to test on QRD".  But, yes, thanks for trying for the regression window, that would be a lot of help.

Comment 16

[Pi Wei Cheng [:piwei] (inactive)]

Mozilla inbound regression window:

Last Working Environmental Variables:
Device: Buri
Build ID: 20140708144432
Gaia: 740faa5d0060fb218b407cf224330654ddf833a5
Gecko: 99c726eaa286
Version: 33.0a1 (Master)
Firmware Version: v1.2device.cfg
User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0

First Broken Environmental Variables:
Device: Buri
Build ID: 20140708150218
Gaia: 740faa5d0060fb218b407cf224330654ddf833a5
Gecko: a50ea7808d85
Version: 33.0a1 (Master)
Firmware Version: v1.2device.cfg
User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0

Gaia is the same so it's a Gecko issue.

Gecko pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=99c726eaa286&tochange=a50ea7808d85

There is only one bug in the pushlog, Bug 1027593.

Comment 17

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

(In reply to Pi Wei Cheng [:piwei] from comment #16)
> Mozilla inbound regression window:

Awesome!  Thanks for a very quick turnaround on this.

Comment 18

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

Attachment: Patch

I was able to reproduce this intermittently so I added some logging to figure out what was going on. It turns out that HandlePossibleViewportChange pulls the "old screen size" from mLastRootMetrics.mCompositionBounds when computing how much to adjust the zoom by on rotation. While this mostly works (because the mCompositionBounds is updated with the new screen size), it can potentially not work if we get a RecvUpdateFrame call with different composition bounds in the middle of a rotation. This is what was happening. The log indicated that:

- HandlePossibleViewportChanged was called with mInnerSize = 854x293
- This set mLastRootMetrics.mCompositionBounds to 854x293
- An "old" RecvUpdateFrame came in which set mLastRootMetrics.mCompositionBounds to 854x480
- HandlePossibleViewportChanged was called again with mInnerSize = 854x293
- On this second call, the oldScreenSize was taken as 854x480, and so the zoom was adjusted when it shouldn't have been.

The fix stops using mLastRootMetrics.mCompositionBounds as the source of the old screen size, because it is unreliable. Instead, it just has the callers pass in the old screen size. At the one callsite that the screen size actually changes, the caller saves the old screen size and passes it in. This fixes the problem as far as I can tell.

Comment 19

[Botond Ballo [:botond]]

Comment on attachment 8468532 [details] [diff] [review]
Patch

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

::: dom/ipc/TabChild.cpp
@@ +89,5 @@
>  #define BROWSER_ELEMENT_CHILD_SCRIPT \
>      NS_LITERAL_STRING("chrome://global/content/BrowserElementChild.js")
>  
> +#define TC_LOG(...)
> +// #define TC_LOG(...) printf_stderr("TC: " __VA_ARGS__)

Is "TC" sufficiently greppable? :)

Comment 20

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

It seemed to be in my case, but I changed it to "TABC" just to be safe.

Landed on b2g-inbound: https://hg.mozilla.org/integration/b2g-inbound/rev/a21244068b7f

Comment 21

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

https://hg.mozilla.org/mozilla-central/rev/a21244068b7f

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/6eb27b5f82f0