Move full-screen detection logic from PanelManager to DisplaySettingsProvider.

Move full-screen detection logic from PanelManager to DisplaySettingsProvider.

Also fixed a bug that pnels remain visible with full screen apps on Snow Leopard.
The fix is to also call orderBack/orderFront when full-screen is detected. This is because the full-sreen window is still in normal level and our panel windows stay in the top of z-order when its level is changed to normal.

BUG=123058
TEST=Manual test by launching panels and make an app full-screen


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

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

[Dmitry Titov, 2012-04-12 00:08:51]

There are relevant compile errors on bots as well...

http://codereview.chromium.org/10051020/diff/1/chrome/browser/ui/panels/displ...
File chrome/browser/ui/panels/display_settings_provider.h (right):

http://codereview.chromium.org/10051020/diff/1/chrome/browser/ui/panels/displ...
chrome/browser/ui/panels/display_settings_provider.h:19: enum ObserverType {
I think it would be simpler to have no base type for 3 observers, and no enum.
Just declare 3 different observer types and 3 different pairs of methods to
add/remove them.
This also will remove switch statements in implementation.  
Internally, there are 3 separate lists anyways...

http://codereview.chromium.org/10051020/diff/1/chrome/browser/ui/panels/panel...
File chrome/browser/ui/panels/panel_manager.cc (right):

http://codereview.chromium.org/10051020/diff/1/chrome/browser/ui/panels/panel...
chrome/browser/ui/panels/panel_manager.cc:142:
static_cast<DisplaySettingsProvider::FullScreenObserver*>(this));
These casts will also go away if observers are not derived from the same base
class and there are 3 separate AddXXXObserver() methods with specific types.

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

http://codereview.chromium.org/10051020/diff/1/chrome/browser/ui/panels/panel...
chrome/browser/ui/panels/panel_window_controller_cocoa.mm:1022: if
(isFullScreen)
Lets add a comment on why this is necessary, it's not very obvious from the
code...

[dcheng, 2012-04-12 00:18:28]

http://codereview.chromium.org/10051020/diff/1/chrome/browser/ui/panels/displ...
File chrome/browser/ui/panels/display_settings_provider.h (right):

http://codereview.chromium.org/10051020/diff/1/chrome/browser/ui/panels/displ...
chrome/browser/ui/panels/display_settings_provider.h:152:
base::RepeatingTimer<DisplaySettingsProvider> full_screen_mode_timer_;
Minor nit: DISALLOW_COPY_AND_ASSIGN should be used for all the classes defined
in here.

[jianli, 2012-04-12 00:45:17]

http://codereview.chromium.org/10051020/diff/1/chrome/browser/ui/panels/displ...
File chrome/browser/ui/panels/display_settings_provider.h (right):

http://codereview.chromium.org/10051020/diff/1/chrome/browser/ui/panels/displ...
chrome/browser/ui/panels/display_settings_provider.h:19: enum ObserverType {
On 2012/04/12 00:08:51, Dmitry Titov wrote:
> I think it would be simpler to have no base type for 3 observers, and no enum.
> Just declare 3 different observer types and 3 different pairs of methods to
> add/remove them.
> This also will remove switch statements in implementation.  
> Internally, there are 3 separate lists anyways...

Done.

http://codereview.chromium.org/10051020/diff/1/chrome/browser/ui/panels/displ...
chrome/browser/ui/panels/display_settings_provider.h:152:
base::RepeatingTimer<DisplaySettingsProvider> full_screen_mode_timer_;
On 2012/04/12 00:18:28, dcheng wrote:
> Minor nit: DISALLOW_COPY_AND_ASSIGN should be used for all the classes defined
> in here.

Done.

http://codereview.chromium.org/10051020/diff/1/chrome/browser/ui/panels/panel...
File chrome/browser/ui/panels/panel_manager.cc (right):

http://codereview.chromium.org/10051020/diff/1/chrome/browser/ui/panels/panel...
chrome/browser/ui/panels/panel_manager.cc:142:
static_cast<DisplaySettingsProvider::FullScreenObserver*>(this));
On 2012/04/12 00:08:51, Dmitry Titov wrote:
> These casts will also go away if observers are not derived from the same base
> class and there are 3 separate AddXXXObserver() methods with specific types.

Done.

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

http://codereview.chromium.org/10051020/diff/1/chrome/browser/ui/panels/panel...
chrome/browser/ui/panels/panel_window_controller_cocoa.mm:1022: if
(isFullScreen)
On 2012/04/12 00:08:51, Dmitry Titov wrote:
> Lets add a comment on why this is necessary, it's not very obvious from the
> code...

Done.

[Dmitry Titov, 2012-04-12 01:18:59]

LGTM with one comment:

http://codereview.chromium.org/10051020/diff/2002/chrome/browser/ui/panels/ba...
File chrome/browser/ui/panels/base_panel_browser_test.cc (right):

http://codereview.chromium.org/10051020/diff/2002/chrome/browser/ui/panels/ba...
chrome/browser/ui/panels/base_panel_browser_test.cc:96: 
Does it make sense to add 
DCHECK(!display_area_observers_iter.GetNext());
since you are assuming there is only one observer registered at this time?
Same for below.

[commit-bot: I haz the power, 2012-04-12 22:49:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jianli@chromium.org/10051020/13001

[commit-bot: I haz the power, 2012-04-12 22:49:32]

Can't apply patch for file chrome/browser/ui/panels/panel_manager.cc.
While running patch -p1 --forward --force;
patching file chrome/browser/ui/panels/panel_manager.cc
Hunk #1 FAILED at 8.
Hunk #2 FAILED at 36.
Hunk #3 succeeded at 86 (offset 5 lines).
Hunk #4 succeeded at 100 (offset 5 lines).
Hunk #5 succeeded at 119 (offset 5 lines).
Hunk #6 succeeded at 145 (offset 5 lines).
2 out of 6 hunks FAILED -- saving rejects to file
chrome/browser/ui/panels/panel_manager.cc.rej

[commit-bot: I haz the power, 2012-04-12 23:07:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jianli@chromium.org/10051020/14012

[commit-bot: I haz the power, 2012-04-13 02:14:17]

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

[commit-bot: I haz the power, 2012-04-13 03:23:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jianli@chromium.org/10051020/14012

[commit-bot: I haz the power, 2012-04-13 04:50:52]

Change committed as 132144

[commit-bot: I haz the power, 2012-04-16 21:50:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jianli@chromium.org/10051020/21001

[commit-bot: I haz the power, 2012-04-16 23:31:19]

Change committed as 132487