473647 - Improve resize story

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[(dormant account)]

Continuing from bug 473424.
On startup we currently have two resizes. First the 1x1 window gets resized to
the size requested in
http://mxr.mozilla.org/mobile-browser/source/chrome/content/browser.xul#54

Then maemo forces the window to be sized down to 420 vertical pixels. For some
reason horizontal number in the code on startup is always 720, even if it's
specified as 800.

Changing the height to 420 in browser.xul avoids the extra event.

Comment 1

[(dormant account)]

Attachment: make repeat resizes almost free

I did not feel satisfied with small improvements in bug 473424, this patch is a lot more satisfying. Turns out that querying widget sizes causes a hefty perf hit(40ms), I'm guessing that it's triggering some sort of reflows. However we can actually avoid all of those queries in the typical case resulting in 10ms resize event handling, instead of a current 60ish ms.

So this patch on it's own saves 50ms on startup and on every subsequent resize. 

If we change browser.xul default height, then this patch will only save 10ms of startup, but subsequent savings of 50+ms per resize event will still apply.

Comment 2

[(dormant account)]

After further investigation, I am not sure that it'll be possible to move the initial window size into prefs.
I tried taking out the initial size out of browser.xul, setting w/h to 1/1, etc but that only caused 2 bonus resize events with bogus dimensions instead of 1 extra one now.

Comment 3

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

Tara - perhaps doing <window ... persist="width height">  might make a difference

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 357049 [details] [diff] [review]
make repeat resizes almost free

>diff --git a/chrome/content/WidgetStack.js b/chrome/content/WidgetStack.js

>+  updateSize: function updateSize(width, height) {
>     // XXX assumes we can only be resized from the bottom left/bottom right
>-    let rect = this._el.getBoundingClientRect();
>-    this._viewingRect.width = rect.width;
>-    this._viewingRect.height = rect.height;
>-
>+    this._viewingRect.width = width;
>+    this._viewingRect.height = height;

Callers won't necessarily have access to the width/height, so we probably should keep the old code around:

if (width == undefined || height == undefined) {
  let rect = this._el.getBoundingClientRect();
  width = rect.width;
  height = rect.height;
}

>diff --git a/chrome/content/browser-ui.js b/chrome/content/browser-ui.js

>-    let windowW = window.innerWidth;
>+    let windowW = window.innerWidth

Don't push this change :)

Comment 5

[(dormant account)]

pushed http://hg.mozilla.org/mobile-browser/rev/906f97a8819b with Mark's suggestions

Comment 6

[(dormant account)]

Attachment: change default sizes to match device

We don't need to modify width(it seems to get ignored on n810 for some reason), only height. I modified both for consistency.

Comment 7

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

Comment on attachment 357440 [details] [diff] [review]
change default sizes to match device

I guess as long as we the sizes in the XUL they should be optimized.

Comment 8

[(dormant account)]

pushed http://hg.mozilla.org/mobile-browser/rev/07bbae5dca0e

Comment 9

[Joel Maher ( :jmaher ) (UTC -8)]

we have cleaned up the resize logic a few months ago.  Now we switch easily between different sizes on all platforms.