Hack to fix Panel focus issues on Mac.

Hack to fix Panel focus issues on Mac.

This is just a temporary workaround for dogfood.

BUG=97831
TEST=test app on crbug.com/101413


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=107634

[Dmitry Titov, 2011-10-26 01:47:15]

lgtm with a nit (please coordinate with jennb):

http://codereview.chromium.org/8393014/diff/1/chrome/browser/ui/panels/panel_...
File chrome/browser/ui/panels/panel_window_controller_cocoa.mm (right):

http://codereview.chromium.org/8393014/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_window_controller_cocoa.mm:193:
DCHECK(tab_contents);
I would replace this DCHECK with if(). It can happen (and happens) in tests that
we don't have tab content at all. Jennb knows more about it.

[jennb, 2011-10-26 17:48:09]

http://codereview.chromium.org/8393014/diff/1/chrome/browser/ui/panels/panel_...
File chrome/browser/ui/panels/panel_window_controller_cocoa.mm (right):

http://codereview.chromium.org/8393014/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_window_controller_cocoa.mm:191: TabContents*
tab_contents =
Could you also add a comment explaining why we need to set RWHV !active upon
reveal?

http://codereview.chromium.org/8393014/diff/1/chrome/browser/ui/panels/panel_...
chrome/browser/ui/panels/panel_window_controller_cocoa.mm:193:
DCHECK(tab_contents);
On 2011/10/26 01:47:15, Dmitry Titov wrote:
> I would replace this DCHECK with if(). It can happen (and happens) in tests
that
> we don't have tab content at all. Jennb knows more about it.

Yes, please replace with if() before committing. Thanks.

[dcheng, 2011-10-26 21:32:02]

Updated with a comment. Let me know what you think.

[dcheng, 2011-10-26 22:17:03]

PTAL. Thanks =)

[Dmitry Titov, 2011-10-26 22:26:16]

LGTM with a note

[Dmitry Titov, 2011-10-26 22:26:31]

Grr. here is the note :-)

http://codereview.chromium.org/8393014/diff/7001/chrome/browser/ui/panels/pan...
File chrome/browser/ui/panels/panel_window_controller_cocoa.mm (right):

http://codereview.chromium.org/8393014/diff/7001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_window_controller_cocoa.mm:194: // we always
deactivate the controls here. If we're created as an active
Not clear... Window gets orderFrontRegardless -> who become the first responder?
-> who will have focus? -> why  we deactivate when we have focus? 
It feels like not enough context for understanding.

Maybe have a good description in a bug and only rever from here to the bug link?

http://codereview.chromium.org/8393014/diff/7001/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_window_controller_cocoa.mm:195: // panel, we'll
get a NSWindowDidBecomeKeynotification and reactivate the web
NSWindowDidBecomeKeynotification - need space

[commit-bot: I haz the power, 2011-10-26 23:01:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dcheng@chromium.org/8393014/11001

[commit-bot: I haz the power, 2011-10-27 00:43:23]

Try job failure for 8393014-11001 (retry) on mac_rel for step "unit_tests".
It's a second try, previously, step "unit_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[jennb, 2011-10-27 19:04:51]

LGTM on RWHV check.

[commit-bot: I haz the power, 2011-10-27 19:05:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dcheng@chromium.org/8393014/12001

[commit-bot: I haz the power, 2011-10-27 21:47:45]

Change committed as 107634