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

RESOLVED FIXED in mozilla36

Status

()

Core
Layout
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: smaug, Assigned: smaug)

Tracking

(Blocks: 1 bug)

36 Branch
mozilla36
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10s+)

Details

Attachments

(2 attachments, 1 obsolete attachment)

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.)
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.
Need to perhaps make http://mxr.mozilla.org/mozilla-central/source/view/nsViewManager.cpp#197 to work in e10s too.

Comment 3

3 years ago
related to bug 1076820 maybe.
tracking-e10s: --- → ?
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.
Hmm, I'm not at all familiar with compress in IPDL.
I have a wip patch for this. Will upload after testing in an opt build.
Assignee: nobody → bugs
Created attachment 8519612 [details] [diff] [review]
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
Depends on: 1076820
Created attachment 8519628 [details] [diff] [review]
v2

https://tbpl.mozilla.org/?tree=Try&rev=776e8870473e
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).
Attachment #8519612 - Attachment is obsolete: true
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.)
(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.
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.)
I built with this patch on latest master and didn't see any problems.
Attachment #8519628 - Flags: review?(bugmail.mozilla)
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.
Attachment #8519628 - Flags: review?(bugmail.mozilla) → feedback+
Comment on attachment 8519628 [details] [diff] [review]
v2

Trying roc then, given that this relies Views having certain setup.
Attachment #8519628 - Flags: review?(roc)
Attachment #8519628 - Flags: review?(roc) → review+
Created attachment 8520232 [details] [diff] [review]
v2 + commit message
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/dae3fdd2f791
Keywords: checkin-needed
Blocks: 1063169
tracking-e10s: ? → +
https://hg.mozilla.org/mozilla-central/rev/dae3fdd2f791
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.