262815 - Closing tab while bookmarks view is shown can lead to crash [@ objc_msgSend]

Status: RESOLVED FIXED

(Camino Graveyard :: Tabbed Browsing, defect)

Description

[Wevah (Nate Weaver)]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a5) Gecko/20040930 Camino/0.8+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a5) Gecko/20040930 Camino/0.8+

Closing a tab while the bookmarks view is open will crash Camino when the mouse
cursor is moved over the rect where the tab used to be.

Reproducible: Always
Steps to Reproduce:
1. Open 3 or more tabs (so the tab bar isn't hidden when one is closed).
2. Show the bookmarks view.
3. Press Command-W to close a tab (no visual change yet, because the bookmarks
view is open).
4. Hide the bookmarks view; note that the tab is gone as could be expected.
5. Move the mouse cursor over the rect where the tab used to be.

Actual Results:  
Camino crashed.

Expected Results:  
Not crashed.

Comment 1

[Wevah (Nate Weaver)]

Attachment: Crash log.

Comment 2

[Geoff "temporarily off bugmail" Beier]

Confirmed.

Comment 3

[Kurt Weinschenker]

FWIW, confirming this bug under Mac OS 10.3.5, Camino nightly build 2004100308.

Comment 4

[Geoff "temporarily off bugmail" Beier]

Attachment: fixes the crash

This patch makes tab bar visibility explicit, as opposed to just looking at the
tab bar's height, which makes the code much easier to understand and gives a
mechanism for unregistering the tracking rects when the bookmark manager is
shown. The cause of the crash was that closing tabs while the tab bar was not
visible was failing to unregister the tracking rects (because the
BrowserTabView is out of the hierarchy then).

If we don't get to putting the bm manager in a tab soon, we may want to
re-evaluate the behavior of cmd-w when it's open.

Comment 5

[Josh Aas]

Comment on attachment 166068 [details] [diff] [review]
fixes the crash

introduces a drawing problem and still crashes under the right circumstances. I
already explained it to Geoff and he can reproduce, and since its odd and sort
of complicated I don't want to write it out here.

Comment 6

[Geoff "temporarily off bugmail" Beier]

Attachment: fixes the problem Josh found

The problem Josh found was that the tab bar was not being rebuilt under certain
circumstances when the BrowserTabView had changed while it was hidden. This
version rebuilds the tab bar whenever it is shown after having been hidden,
which fixes that problem.

Comment 7

[Bruce Davidson]

Attachment: Alternate stack trace

Having applied the first patch it still crashes with the same set of steps to
reproduce. However the crash log is different (but the same as from an
unpatched nightly). This crash log attached.

Comment 8

[Geoff "temporarily off bugmail" Beier]

Attachment: v3

This should fix the crash Josh and Bruce saw.

Comment 9

[Josh Aas]

Geoff - do you remember the comments made on IRC the other day? I should have
copied them into the bug. Waiting for a patch that addresses those...

Comment 10

[Geoff "temporarily off bugmail" Beier]

Attachment: address comments from IRC

Sorry... didn't forget. A combination of tree bustage and the holiday are
making me a little slow. This should address the comments from IRC.

Removed some extra brackets, added comments requested by Mike and Josh.

Comment 11

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 167139 [details] [diff] [review]
address comments from IRC

+  } else if (!show && mVisible) { // being hi

i assume this should be "being hidden"

sr=pink with that change.

Comment 12

[Josh Aas]

landed