Last Comment Bug 717085 - Bad viewport appears after restart from forcequit
: Bad viewport appears after restart from forcequit
Status: RESOLVED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: 12 Branch
: ARM Android
P4 normal (vote)
: Firefox 12
Assigned To: Kartikaya Gupta (email:kats@mozilla.com)
:
: Sebastian Kaspari (:sebastian)
Mentors:
http://people.mozilla.com/~nhirata/ht...
: 708934 711945 719033 (view as bug list)
Depends on:
Blocks: 708934 719745
  Show dependency treegraph
 
Reported: 2012-01-10 15:39 PST by Naoki Hirata :nhirata (please use needinfo instead of cc)
Modified: 2012-02-03 17:11 PST (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed
11+


Attachments
Ensure reasonable screen size is used for session-restoring tabs (5.19 KB, patch)
2012-01-19 09:47 PST, Kartikaya Gupta (email:kats@mozilla.com)
pwalton: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description User image Naoki Hirata :nhirata (please use needinfo instead of cc) 2012-01-10 15:39:49 PST
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 User image Kartikaya Gupta (email:kats@mozilla.com) 2012-01-19 09:47:20 PST
Created attachment 589888 [details] [diff] [review]
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 User image Patrick Walton (:pcwalton) 2012-01-19 14:52:16 PST
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 User image Kartikaya Gupta (email:kats@mozilla.com) 2012-01-20 09:14:24 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/befdbbd1a136
Comment 4 User image Ed Morley [:emorley] 2012-01-21 06:01:17 PST
https://hg.mozilla.org/mozilla-central/rev/befdbbd1a136
Comment 5 User image Kartikaya Gupta (email:kats@mozilla.com) 2012-01-21 10:32:44 PST
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 User image Mark Finkle (:mfinkle) (use needinfo?) 2012-01-22 02:41:45 PST
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 User image Kartikaya Gupta (email:kats@mozilla.com) 2012-01-22 08:38:54 PST
(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 User image Alex Keybl [:akeybl] 2012-01-23 09:20:44 PST
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 User image Matt Brubeck (:mbrubeck) 2012-01-26 17:36:38 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/41c8a2291ea2
Comment 10 User image Matt Brubeck (:mbrubeck) 2012-01-27 11:00:02 PST
*** Bug 708934 has been marked as a duplicate of this bug. ***
Comment 11 User image Kartikaya Gupta (email:kats@mozilla.com) 2012-02-02 11:49:05 PST
*** Bug 719033 has been marked as a duplicate of this bug. ***
Comment 12 User image Kartikaya Gupta (email:kats@mozilla.com) 2012-02-03 17:11:44 PST
*** Bug 711945 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.