355714 - Optimize tab close button hover tracking

Status: RESOLVED FIXED

(Camino Graveyard :: Tabbed Browsing, enhancement)

Description

[Stuart Morgan]

In order to pay for the tracking rects I added for tab tooltips, I looked into optimizing the rects we already had. Since we are already tracking when the mouse is in a tab for highlighting, and there's no way to mouse over a tab close button without first moving the mouse into a tab, we don't really need to track mouseover of all the close buttons; we just need to track the close button for a given tab while the mouse is in that tab.  That gives us one less rect per visible tab, bringing us back to pre-tooltip levels.

Comment 1

[Stuart Morgan]

Attachment: trunk version

Trunk version. Also does a bit of name cleanup, and adds logic to prevent resetting the tracking rect three times per frame change.

Comment 2

[Stuart Morgan]

Attachment: branch-esque version

I don't have a branch handy at the moment, so this is actually a pre-tab-scrolling trunk patch, which I'm hoping will still apply on the branch.

Comment 3

[Stuart Morgan]

Comment on attachment 241447 [details] [diff] [review]
trunk version

Simon, would you mind reviewing this?  It is a present for you ;)

Comment 4

[Stuart Morgan]

Comment on attachment 241449 [details] [diff] [review]
branch-esque version

(Clearing review flags for branch; it's the same approach.)

Comment 5

[Simon Fraser [no longer active]]

Comment on attachment 241447 [details] [diff] [review]
trunk version

To avoid having to override setFrame:, setFrameOrigin: and setFrameSize: can't you override resizeSubviewsWithOldSize: and do the tracking rect update there? Or you could register for frame changed notifications and use those.

Other than that, the patch seems ok.

Comment 6

[Stuart Morgan]

(In reply to comment #5)
> (From update of attachment 241447 [details] [diff] [review] [edit])
> To avoid having to override setFrame:, setFrameOrigin: and setFrameSize: can't
> you override resizeSubviewsWithOldSize: and do the tracking rect update there?

Unfortunately resizeSubviewsWithOldSize: is only called for bounds (frame size) changes, not all frame changes, which isn't enough for the tracking rects.

> Or you could register for frame changed notifications and use those.

Wouldn't posting a notification per tab per frame change in something that gets resized as much as the tabs be a lot of overhead?

Given that, I'd like to leave that for later consideration and get the part we know is a perf win now.  Frame-setter overrides are one of the suggested ways of doing tracking rect updates in the docs, so it's not like it's especially hackish.

Comment 7

[Stuart Morgan]

Attachment: unbitrotted trunk version

Branch version coming shortly.

Comment 8

[Stuart Morgan]

(In reply to comment #7)
> Branch version coming shortly.

Nevermind; branch version still applies cleanly.

Comment 9

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 243749 [details] [diff] [review]
unbitrotted trunk version

sr=pink

Comment 10

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Will we need to apply an additional part of the trunk patch to the branch when tab scrolling lands on the branch?

Comment 11

[Stuart Morgan]

Comment on attachment 243749 [details] [diff] [review]
unbitrotted trunk version

Crap, this version is missing the BrowserTabBarView changes.  I'll post the complete version tonight.

(In reply to comment #10)
> Will we need to apply an additional part of the trunk patch to the branch when
> tab scrolling lands on the branch?

No, the code is the same between branch and trunk; they just have different context.

Comment 12

[Stuart Morgan]

Attachment: correct trunk version

This has the changes that were missing from the trunk patch.

Comment 13

[froodian (Ian Leue)]

Checked in on 1.8branch and trunk.