582950 - When there is only App tabs in tabstrip, browser UI freezes when I narrow window width

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Alice0775 White]

User-Agent:       Mozilla/5.0 (Windows; Windows NT 6.1; WOW64; rv:2.0b3pre) Gecko/20100728 Minefield/4.0b3pre ID:20100728041048
Build Identifier: Mozilla/5.0 (Windows; Windows NT 6.1; WOW64; rv:2.0b3pre) Gecko/20100728 Minefield/4.0b3pre ID:20100728041048

When there is only App tabs in tabstrip, browser UI freezes when I narrow window width.

Reproducible: Always

Steps to Reproduce:
1. Start Minefield with new profile
2. Open too many tabs(more than 3) and change all of them to App tab. 
3. Narrow window width to the limit.

Actual Results:
  browser UI freezes

Expected Results:
 never freeze

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

You will have to open enough App tabs so when you reach the minimum width of the window, the remaining App tabs at the right side are hidden. The freeze will happen immediately and cause 100% cpu load for my VM. No way for me to shutdown because the VM is not responsible anymore.

Comment 2

[Dietrich Ayala (:dietrich)]

Blocking+ for confirmed hang.

Comment 3

[:Felipe Gomes (needinfo for replies!)]

Attachment: 2min sampling

I think this is the same as bug 575515 (at least the result is the same: browser freezes, can't move mouse into taskbar, alt+tab unfreezes).

Following the STR here I used xperf to sample the browser activity for 2min, and here are the results (everything relevant is on the UI thread).

It seems that something is constantly triggering a reflow. Hard to tell why having only app tabs would trigger this situation. It looks like this is Windows only, so I think it could be a bug in the view system or the view/widget interactions. I added a printf to nsView::ResetWidgetBounds and it is constantly called during the hang.

Comment 4

[Jim Mathies [:jimm]]

WFM, vista, Mozilla/5.0 (Windows NT 6.0; rv:2.0b7pre) Gecko/20100914 Firefox/4.0b7pre

I have five app tabs, and I clip the last two when I move the window all the way in. Tabs on top, fx button enabled, bookmarks bar enabled.

Comment 5

[Timothy Nikkel (:tnikkel)]

If we keep hitting nsView::ResetWidgetBounds, what are the bounds each time? Do they change, do they oscillate, or they the same? What is the stack to the calls?

Comment 6

[Alice0775 White]

(In reply to comment #4)
> WFM, vista, Mozilla/5.0 (Windows NT 6.0; rv:2.0b7pre) Gecko/20100914
> Firefox/4.0b7pre
> 
> I have five app tabs, and I clip the last two when I move the window all the
> way in. Tabs on top, fx button enabled, bookmarks bar enabled.
Now, Menu Bar is necessary to reproduce the problem.

Comment 7

[Jim Mathies [:jimm]]

(In reply to comment #6)
> (In reply to comment #4)
> > WFM, vista, Mozilla/5.0 (Windows NT 6.0; rv:2.0b7pre) Gecko/20100914
> > Firefox/4.0b7pre
> > 
> > I have five app tabs, and I clip the last two when I move the window all the
> > way in. Tabs on top, fx button enabled, bookmarks bar enabled.
> Now, Menu Bar is necessary to reproduce the problem.

Hmm, same result. No hang.

Comment 8

[Jim Mathies [:jimm]]

(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #4)
> > > WFM, vista, Mozilla/5.0 (Windows NT 6.0; rv:2.0b7pre) Gecko/20100914
> > > Firefox/4.0b7pre
> > > 
> > > I have five app tabs, and I clip the last two when I move the window all the
> > > way in. Tabs on top, fx button enabled, bookmarks bar enabled.
> > Now, Menu Bar is necessary to reproduce the problem.
> 
> Hmm, same result. No hang.

 Note that's on vista, so it may be a win7 only bug.

Comment 9

[Alice0775 White]

Sorry , it also trigger with Firefox Button if number of AppTabs is 10 or more.

And it Happens on Windows 7 Classic/Aero and Windows XP Classic/Luna

Comment 10

[:Felipe Gomes (needinfo for replies!)]

Attachment: stack

(In reply to comment #5)
> If we keep hitting nsView::ResetWidgetBounds, what are the bounds each time? Do
> they change, do they oscillate, or they the same? What is the stack to the
> calls?

These are two different stacks that I got (maybe there are other ones, or slight variations)

The bounds do oscillate. It never gets to the place where it calculates newBounds though (i.e. it always returns on http://mxr.mozilla.org/mozilla-central/source/view/src/nsView.cpp#453, which is very odd)

Here is a sequence:
nsView::DoResetWidgetBounds curBounds: [188 59 0 0]
nsView::DoResetWidgetBounds curBounds: [0 0 0 0]
nsView::DoResetWidgetBounds curBounds: [227 51 0 0]
nsView::DoResetWidgetBounds curBounds: [188 59 0 0]
nsView::DoResetWidgetBounds curBounds: [0 0 0 0]
nsView::DoResetWidgetBounds curBounds: [227 51 0 0]
...

It repeats 4 times, then there's a series of nsView::ResetWidgetBounds that don't call DoResetWidgetBounds, and then the sequence repeats again.

Also, one time I saw the tabbar layout oscillating: the dropdown "List all tabs" button would appear/disappear and the tabcandy icon kept moving into the place of the button.

Comment 11

[Timothy Nikkel (:tnikkel)]

The DoResetWidgetBounds thing was a dead end, sorry for sending you down it. Those DoResetWidgetBounds calls were for hidden popups (like the awesome bar dropdown). We don't call DoResetWidgetBounds for the main window on Windows, it is handled in a different place.

Comment 12

[Timothy Nikkel (:tnikkel)]

So what seems to be happening here is that we get underflow/overflow events here http://mxr.mozilla.org/mozilla-central/source/browser/base/content/tabbrowser.xml#2520 and we call _positionPinnedTabs here http://mxr.mozilla.org/mozilla-central/source/browser/base/content/tabbrowser.xml#2734 where fetches scrollWidth for some elements. This getter flushes, and that flush causes us to get another overflow/underflow (whatever we didn't just recieve). The underflow/overflow event is dispatched from an event we run async. I guess we process those events before user input still, and we are in a modal resize loop (only Windows has this), so that kills us.

Comment 13

[:Felipe Gomes (needinfo for replies!)]

I'm looking into this

Comment 14

[:Felipe Gomes (needinfo for replies!)]

Attachment: Patch 1

Ok, so I've got two different patches here.
Just a brief explanation to the bug (as I've only talked on IRC), the problem is that the tabs container doesn't deal well when there's only app tabs. What happens is that the app tabs aren't inside the container (as they're position:fixed), and when an overflow happens the scrollboxes appear, but as there isn't any regular tab inside it it underflows, the scrollboxes disappear, and the cycle repeats.

The freeze here is just that this STR triggers the hang from bug 575515 (Win hang caused by window resizing).


This first approach is just a patch that avoids triggering the problem. The layout still oscillates, but it doesn't freeze the browser window. It's straightforward, but likely it's not necessary to take this patch when bug 575515 is fixed.

Comment 15

[:Felipe Gomes (needinfo for replies!)]

Attachment: Patch 2

This other patch fixes the behavior of only app tabs overflowing (and also fixes the freeze, of course).

When there's only app tabs, there's no need to take them out of the container and fix their position. So this is what this patch does, and the app tabs will then scroll inside the container. screenshot: http://grab.by/6GCR

I think we could take this bug regardless of waiting for bug 575515.

Comment 16

[Johnathan Nightingale [:johnath]]

Felipe - are these review ready?

Comment 17

[:Felipe Gomes (needinfo for replies!)]

Yes. I was gonna request feedback to decide which patch to pick (I'm leaning towards patch 2), but any of them can be reviewed.

Comment 18

[Dão Gottwald [:dao]]

Comment on attachment 480567 [details] [diff] [review]
Patch 2

>+          if (onlyAppTabs) {
>+            this.tabbrowser.tabContainer.setAttribute("dontpin", "true");
>+          } else {
>+            this.tabbrowser.tabContainer.removeAttribute("dontpin");
>+          }

nit: remove braces.

Can you rename onlyAppTabs to pinnedOnly and dontpin to pinnedonly?

r=me with that

Comment 19

[:Felipe Gomes (needinfo for replies!)]

Removed braces and renamed vars.

http://hg.mozilla.org/mozilla-central/rev/19850237259f

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b8pre) Gecko/20101111 Firefox/4.0b8pre