Hide the tab indicators and the shelf when in immersive + tab fullscreen

Hide the tab indicators and the shelf when in immersive + tab fullscreen.

BUG=229213, 233271
Test=ImmersiveModeControllerAshTest.TabAndBrowserFullscreen,
     FullscreenChromeOSTest.TransientParentIsFullscreen

[pkotwicz, 2013-04-20 22:15:38]

James, can you please take a look?

This CL happens to fix
https://code.google.com/p/chromium/issues/detail?id=180107
In particular, WorkspaceManager::GetWindowState() checks the target visibilty &
opacity instead of the actual visibility of the window.

This CL also makes IsFullScreenMode() include transient windows. I kept the
existing behavior for Windows Ash because I am not sure what to do there and I
wasn't able to find a place where Chrome uses dialogs in fullscreen in Metro
mode.

[James Cook, 2013-04-22 18:14:16]

Nice patch. A couple comments.

https://codereview.chromium.org/14156005/diff/2001/ash/root_window_controller...
File ash/root_window_controller_unittest.cc (right):

https://codereview.chromium.org/14156005/diff/2001/ash/root_window_controller...
ash/root_window_controller_unittest.cc:373: }
Can you add a test for the GetFullscreenWindow() function you added?

https://codereview.chromium.org/14156005/diff/2001/chrome/browser/ui/views/fr...
File chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc (right):

https://codereview.chromium.org/14156005/diff/2001/chrome/browser/ui/views/fr...
chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc:320: bool
should_enable = window->GetProperty(kImmersiveModeKey);
Do we still need kImmersiveModeKey?  I don't think we do - I think it was only
being used by RootWindowController to detect immersive mode, in spite of the
comment.

https://codereview.chromium.org/14156005/diff/2001/chrome/browser/ui/views/fr...
File chrome/browser/ui/views/frame/immersive_mode_controller_ash.h (right):

https://codereview.chromium.org/14156005/diff/2001/chrome/browser/ui/views/fr...
chrome/browser/ui/views/frame/immersive_mode_controller_ash.h:135: void
UpdateUseMinimalChrome(bool skip_layout);
Optional: Consider re-introducing the LAYOUT_YES, LAYOUT_NO enum.

https://codereview.chromium.org/14156005/diff/2001/chrome/browser/ui/views/fr...
File chrome/browser/ui/views/frame/immersive_mode_controller_ash_browsertest.cc
(right):

https://codereview.chromium.org/14156005/diff/2001/chrome/browser/ui/views/fr...
chrome/browser/ui/views/frame/immersive_mode_controller_ash_browsertest.cc:662:
// 3) Test that exiting tab fullscreen and immersive fullscreen
Nice case to test.

[sky, 2013-04-23 22:34:08]

https://codereview.chromium.org/14156005/diff/2001/chrome/browser/fullscreen_...
File chrome/browser/fullscreen_chromeos.cc (right):

https://codereview.chromium.org/14156005/diff/2001/chrome/browser/fullscreen_...
chrome/browser/fullscreen_chromeos.cc:10: bool IsFullScreenMode() {
Don't we want this code for windows when in ash?

https://codereview.chromium.org/14156005/diff/2001/chrome/browser/fullscreen_...
File chrome/browser/fullscreen_chromeos_unittest.cc (right):

https://codereview.chromium.org/14156005/diff/2001/chrome/browser/fullscreen_...
chrome/browser/fullscreen_chromeos_unittest.cc:1: #include
"ash/test/ash_test_base.h"
You're missing the copright header here.

https://codereview.chromium.org/14156005/diff/2001/chrome/browser/fullscreen_...
chrome/browser/fullscreen_chromeos_unittest.cc:37:
EXPECT_TRUE(IsFullScreenMode());
How about an expect for !InFullScreenMode above?

[pkotwicz, 2013-04-23 22:50:17]

Scott, I actually migrated this CL to https://codereview.chromium.org/14340007/.
I actually ported the test in fullscreen_chromeos_unittest.cc
to root_window_controller_unittest.cc in a followup patch in
https://codereview.chromium.org/14340007/.

[sky, 2013-04-24 00:40:39]

So, is this cl dead then?

On Tue, Apr 23, 2013 at 3:50 PM,  <pkotwicz@chromium.org> wrote:
> Scott, I actually migrated this CL to
> https://codereview.chromium.org/14340007/.
> I actually ported the test in fullscreen_chromeos_unittest.cc
> to root_window_controller_unittest.cc in a followup patch in
> https://codereview.chromium.org/14340007/.
>
> https://codereview.chromium.org/14156005/

[pkotwicz, 2013-04-24 01:20:06]

Yes, its dead. I am keeping it "alive" for context.