Don't delay menu item fixup when the number of tabs change....

Don't delay menu item fixup when the number of tabs change.
BUG=24878
TEST=cmd-w/cmd-shift-w are always correct for close-tab/close-window when window layering changes.

[pink (ping after 24hrs), 2009-10-16 19:11:10]

Mark: this was bugging him enough to file a bug.
TVL: was reviewer on the workaround that caused this issue.

Review from either is fine.

[TVL, 2009-10-16 19:18:41]

menu fixup: lg

http://codereview.chromium.org/283017/diff/1/3
File chrome/browser/cocoa/tab_controller.mm (right):

http://codereview.chromium.org/283017/diff/1/3#newcode216
Line 216: [titleView_ setHidden:hideTitle];
this seems unrelated?  what's magic about 10?

[Mark Mentovai, 2009-10-16 19:19:55]

LGTM

http://codereview.chromium.org/283017/diff/1/2
File chrome/browser/app_controller_mac.mm (right):

http://codereview.chromium.org/283017/diff/1/2#newcode274
Line 274: // We don't need to do this on a delay, the window layering isn't
changing.
The comment makes sense as a checkin comment, but reading the code in isolation,
without the context of what this used to do, this doesn't make any sense.

// It's not illegal to do this in Canada, so don't check for Eskimos

huh?

http://codereview.chromium.org/283017/diff/1/3
File chrome/browser/cocoa/tab_controller.mm (right):

http://codereview.chromium.org/283017/diff/1/3#newcode214
Line 214: // Somtimes the view is just too small to show any text.
This slip in by mistake?

[pink (ping after 24hrs), 2009-10-16 19:22:07]

http://codereview.chromium.org/283017/diff/1/2
File chrome/browser/app_controller_mac.mm (right):

http://codereview.chromium.org/283017/diff/1/2#newcode274
Line 274: // We don't need to do this on a delay, the window layering isn't
changing.
I disagree, the method above fixes up the close menu on a delay, so this
specifies why it's different. Suggestions?

http://codereview.chromium.org/283017/diff/1/3
File chrome/browser/cocoa/tab_controller.mm (right):

http://codereview.chromium.org/283017/diff/1/3#newcode214
Line 214: // Somtimes the view is just too small to show any text.
On 2009/10/16 19:19:55, Mark Mentovai wrote:
> This slip in by mistake?

i hate how the new mode is to include all changes in your client. removed.

[pink (ping after 24hrs), 2009-10-16 19:29:05]

comment updated to be more understandable in function context, though still no
mention about that pesky neighbor to the north.

[Mark Mentovai, 2009-10-16 19:31:04]

pinkerton@chromium.org wrote:
> http://codereview.chromium.org/283017/diff/1/2#newcode274
> Line 274: // We don't need to do this on a delay, the window layering
> isn't changing.
> I disagree, the method above fixes up the close menu on a delay, so this
> specifies why it's different. Suggestions?

I guess was thinking function context, but you're thinking file context.

Clarify the ambiguity by saying "We don't need to do this on a delay as above?"

I really just wanted to make up a preposterous comment about Canada.

> i hate how the new mode is to include all changes in your client.

Me too.

LGTM

[Mark Mentovai, 2009-10-16 19:33:20]

lgtm

http://codereview.chromium.org/283017/diff/6002/7001
File chrome/browser/app_controller_mac.mm (right):

http://codereview.chromium.org/283017/diff/6002/7001#newcode277
Line 277: // we hacked around above by clearing the key equivalents.
Ever notice how everyone from Canada sounds like a walrus?