1096076 - [E10s] When resizing a window, all the tabs are resized

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

In non-e10s builds we resize only the foremost tab. In e10s we seem to resize all the tabs and that pretty much hangs the child process in case one has lots of tabs or some
heavy pages, like http://www.whatwg.org/specs/web-apps/current-work/

Not sure who to CC to this bug.

(This is probably _the_ blocker for me to switch to use e10s in my main profile. I'd need to be very careful to not resize the window, or not load any large pages.)

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

We probably want to also limit the number of UpdateDimensions messages from parent to child, so that 
there isn't huge number messages waiting for processing and new ones coming from the parent all the time. I guess child should sends a message back to parent once the UpdateDimensions is processed,
and before that parent should just cache the most recent to-be-sent values.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Need to perhaps make http://mxr.mozilla.org/mozilla-central/source/view/nsViewManager.cpp#197 to work in e10s too.

Comment 3

[Jim Mathies [:jimm]]

related to bug 1076820 maybe.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Could be, if we end up doing lots of resizes. Just building a debug build with some logging to see how
many UpdateDimensions messages are sent.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Hmm, I'm not at all familiar with compress in IPDL.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

I have a wip patch for this. Will upload after testing in an opt build.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Attachment: v1

This should make us resize only foreground tab.
Hopefully Bug 1076820 helps with sending less resize messages.

https://tbpl.mozilla.org/?tree=Try&rev=2bd0e4d83496

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Attachment: v2

https://tbpl.mozilla.org/?tree=Try&rev=776e8870473e

Comment 9

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

IIRC we had a bug on file for this in B2G but i can't find it now and don't remember if it was ever resolved. But please make sure that this patch doesn't break the case where you have two apps open and rotate the phone and then switch apps (i.e. app in background during resize should update properly).

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

What you mean with "update properly". I'd expect it to be notified what the new viewport looks like, and when the app is brought to foreground, do the resize reflow. (or that resize reflow might happen lazily while the app is in background.)

Comment 11

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

(In reply to Olli Pettay [:smaug] from comment #10)
> What you mean with "update properly".

I just mean that the app does somehow get notified and redraw itself at the new size. As opposed to redrawing at the wrong size.

> I'd expect it to be notified what the
> new viewport looks like, and when the app is brought to foreground, do the
> resize reflow. (or that resize reflow might happen lazily while the app is
> in background.)

I don't really care which of these happens (or even if it happens by some other mechanism) as long as it happens. It's just that the CSS viewport updating code in TabChild is brittle and has been impacted by changes like this before.

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

The toplevel presshell is notified about its bounds when it is set active, and in e10s/b2g setup that
ends up calling show(true) on widget. So it all should just work, but trying to get someone to verify,
or figuring out if any of my b2g devices are still supported for testing.
(not Flame, since that is for dogfooding.)

Comment 13

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

I built with this patch on latest master and didn't see any problems.

Comment 14

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

Comment on attachment 8519628 [details] [diff] [review]
v2

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

I don't think I can officially review this, I don't know the intricacies of nsPresShell visibility code. I can just f+ based on my testing on B2G to say it didn't obviously break anything there.

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8519628 [details] [diff] [review]
v2

Trying roc then, given that this relies Views having certain setup.

Comment 16

[Olli Pettay [:smaug][[e-mail]]]

Attachment: v2 + commit message

Comment 17

[Olli Pettay [:smaug][[e-mail]]]

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

Comment 18

[Ryan VanderMeulen [:RyanVM]]

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