717085 - Bad viewport appears after restart from forcequit

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P4)

Description

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

1. go to http://people.mozilla.com/~nhirata/html_tp/big_buck_bunny_480p.webm
2. set the video to play
3. open a new tab and go to about:home
4. force quit fennec so that these tabs open up at launch
5. wait for the about:home page to launch
6. go to the webm playing in the background

Expected: no errors in logcat in regards to viewport
Actual: 
01-10 15:30:45.183: E/GeckoSoftwareLayerClient(1242): Bad viewport description: {"x":null,"y":null,"width":1,"height":1,"offsetX":0,"offsetY":0,"pageWidth":null,"pageHeight":null,"zoom":null}
01-10 15:30:45.191: W/System.err(1242): java.lang.RuntimeException: org.json.JSONException: Value null at x of type org.json.JSONObject$1 cannot be converted to double
01-10 15:30:45.195: W/System.err(1242): 	at org.mozilla.gecko.gfx.GeckoSoftwareLayerClient.updateViewport(GeckoSoftwareLayerClient.java:212)
01-10 15:30:45.195: W/System.err(1242): 	at org.mozilla.gecko.gfx.GeckoSoftwareLayerClient.endDrawing(GeckoSoftwareLayerClient.java:223)
01-10 15:30:45.195: W/System.err(1242): 	at org.mozilla.gecko.GeckoAppShell.nativeRun(Native Method)
01-10 15:30:45.195: W/System.err(1242): 	at org.mozilla.gecko.GeckoAppShell.nativeRun(Native Method)
01-10 15:30:45.195: W/System.err(1242): 	at org.mozilla.gecko.GeckoAppShell.runGecko(GeckoAppShell.java:453)
01-10 15:30:45.195: W/System.err(1242): 	at org.mozilla.gecko.GeckoThread.run(GeckoThread.java:107)
01-10 15:30:45.195: W/System.err(1242): Caused by: org.json.JSONException: Value null at x of type org.json.JSONObject$1 cannot be converted to double
01-10 15:30:45.195: W/System.err(1242): 	at org.json.JSON.typeMismatch(JSON.java:96)
01-10 15:30:45.199: W/System.err(1242): 	at org.json.JSONObject.getDouble(JSONObject.java:412)
01-10 15:30:45.199: W/System.err(1242): 	at org.mozilla.gecko.gfx.ViewportMetrics.<init>(ViewportMetrics.java:95)
01-10 15:30:45.199: W/System.err(1242): 	at org.mozilla.gecko.gfx.GeckoSoftwareLayerClient.updateViewport(GeckoSoftwareLayerClient.java:191)
01-10 15:30:45.199: W/System.err(1242): 	... 5 more


Note : fennec 20120110, Nexus S, 2.3.1

Comment 1

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

Attachment: Ensure reasonable screen size is used for session-restoring tabs

I don't really like having to touch BrowserCLH.js, but this seemed like the best option to get gScreenWidth and gScreenHeight set to reasonable values before the session restore code created new tabs with 1x1 size, which causes the problem in the bug (although it manifested a little differently for me).

Comment 2

[Patrick Walton (:pcwalton)]

Comment on attachment 589888 [details] [diff] [review]
Ensure reasonable screen size is used for session-restoring tabs

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

Works for me. Less 1x1 = more happiness.

Comment 3

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/befdbbd1a136

Comment 4

[Ed Morley [:emorley]]

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

Comment 5

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

Comment on attachment 589888 [details] [diff] [review]
Ensure reasonable screen size is used for session-restoring tabs

[Approval Request Comment]
Regression caused by (bug #): none
User impact if declined: when opening fennec after it has been killed, the restored tabs may not paint or may be painted at odd zoom levels (see also bug 	708934 and bug 719745)
Testing completed (on m-c, etc.): on m-c for a day so far, may need to bake another couple of days to make sure
Risk to taking this patch (and alternatives if risky): fennec overestimates the visible area (because the display size is larger than the visible viewport size) and pages that shouldn't be scrollable are temporarily scrollable when first rendered.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 589888 [details] [diff] [review]
Ensure reasonable screen size is used for session-restoring tabs


>+        DisplayMetrics metrics = new DisplayMetrics();
>+        GeckoApp.mAppContext.getWindowManager().getDefaultDisplay().getMetrics(metrics);
>+        combinedArgs += " -width " + metrics.widthPixels + " -height " + metrics.heightPixels;

We probably talked about this before but, why is this different than screen.width/screen.height in JS? If it's the same, could we just init gScreenXxx using screen.xxx ?

Comment 7

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

(In reply to Mark Finkle (:mfinkle) from comment #6)
> We probably talked about this before but, why is this different than
> screen.width/screen.height in JS? If it's the same, could we just init
> gScreenXxx using screen.xxx ?

screen.xxx isn't initialized at that point; it only gets set up after the first screen-size-changed event is sent, which is after the session-restored tabs get created.

Comment 8

[Alex Keybl [:akeybl]]

Comment on attachment 589888 [details] [diff] [review]
Ensure reasonable screen size is used for session-restoring tabs

[Triage Comment]
Mobile only - approved for Aurora.

Comment 9

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/41c8a2291ea2