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: (Back on May31) Kartikaya Gupta (email:kats@mozilla.com)
:
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, (Back on May31) Kartikaya Gupta (email:kats@mozilla.com)
pwalton: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Review

Description 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 (Back on May31) 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 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 (Back on May31) Kartikaya Gupta (email:kats@mozilla.com) 2012-01-20 09:14:24 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/befdbbd1a136
Comment 4 Ed Morley [:emorley] 2012-01-21 06:01:17 PST
https://hg.mozilla.org/mozilla-central/rev/befdbbd1a136
Comment 5 (Back on May31) 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 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 (Back on May31) 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 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 Matt Brubeck (:mbrubeck) 2012-01-26 17:36:38 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/41c8a2291ea2
Comment 10 Matt Brubeck (:mbrubeck) 2012-01-27 11:00:02 PST
*** Bug 708934 has been marked as a duplicate of this bug. ***
Comment 11 (Back on May31) 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 (Back on May31) 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.