[Refactor] Move the non-browser specific logic of ImmersiveModeControllerAsh into ash part 2

[Refactor] Move the non-browser specific logic of ImmersiveModeControllerAsh into ash part 2. The goal of the refactor is to allow CustomFrameViewAsh to use ash::ImmersiveFullscreenController so that v2 apps can be put into immersive fullscreen too.

BUG=307622
TEST=ImmersiveFullscreenControllerTest.*, ImmersiveModeControllerAshTest.*

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=233403

[pkotwicz, 2013-10-28 15:47:39]

James, PTAL
This is the last part of the refactor :)

[James Cook, 2013-10-28 18:32:51]

My god this is a big patch.

Overall looks good.  I'm having trouble telling if you are changing the behavior
with respect to the slide animations and CLOSED/REVEALED.

https://codereview.chromium.org/48963002/diff/40001/ash/wm/immersive_fullscre...
File ash/wm/immersive_fullscreen_controller.cc (left):

https://codereview.chromium.org/48963002/diff/40001/ash/wm/immersive_fullscre...
ash/wm/immersive_fullscreen_controller.cc:345: reveal_state_ = CLOSED;
You no longer want to snap to the closed state on disable?

https://codereview.chromium.org/48963002/diff/40001/ash/wm/immersive_fullscre...
File ash/wm/immersive_fullscreen_controller.h (right):

https://codereview.chromium.org/48963002/diff/40001/ash/wm/immersive_fullscre...
ash/wm/immersive_fullscreen_controller.h:62: virtual void SetVisibility(double
visible_fraction) = 0;
I initially thought this was either like SetVisible() or setting the alpha. I
would call it SetVisibleFraction() or similar.

https://codereview.chromium.org/48963002/diff/40001/ash/wm/immersive_fullscre...
ash/wm/immersive_fullscreen_controller.h:154: CLOSED,
Are the old comments no longer accurate?  What happens with y in the CLOSED and
REVEALED states?

https://codereview.chromium.org/48963002/diff/40001/ash/wm/immersive_fullscre...
ash/wm/immersive_fullscreen_controller.h:293: #endif  //
ASH_WM_IMMERSIVE_FULLSREEN_CONTROLLER_H_
FULLSREEN -> FULLSCREEN

https://codereview.chromium.org/48963002/diff/40001/ash/wm/immersive_revealed...
File ash/wm/immersive_revealed_lock.h (right):

https://codereview.chromium.org/48963002/diff/40001/ash/wm/immersive_revealed...
ash/wm/immersive_revealed_lock.h:35: explicit ImmersiveRevealedLock(const
base::WeakPtr<Delegate>& delegate,
no explicit

https://codereview.chromium.org/48963002/diff/40001/chrome/browser/ui/views/f...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/48963002/diff/40001/chrome/browser/ui/views/f...
chrome/browser/ui/views/frame/browser_view.cc:1989:
immersive_mode_controller_->Init(this);
Do you need the whole BrowserView, or can you just inject the pieces you need?

Edit: Damn, you need GetActiveWebContents. OK.

[pkotwicz, 2013-10-29 04:32:44]

James, can you please take another look?

https://codereview.chromium.org/48963002/diff/40001/ash/wm/immersive_fullscre...
File ash/wm/immersive_fullscreen_controller.cc (left):

https://codereview.chromium.org/48963002/diff/40001/ash/wm/immersive_fullscre...
ash/wm/immersive_fullscreen_controller.cc:345: reveal_state_ = CLOSED;
The change I was making was moving setting |reveal_state_| = CLOSED from when
SetEnabled(false) is called to when SetEnabled(true) is called.
However, this CL is large, so I should not add any "additional goodies"

https://codereview.chromium.org/48963002/diff/40001/ash/wm/immersive_fullscre...
File ash/wm/immersive_fullscreen_controller.cc (right):

https://codereview.chromium.org/48963002/diff/40001/ash/wm/immersive_fullscre...
ash/wm/immersive_fullscreen_controller.cc:786: RevealState new_reveal_state =
(animate == ANIMATE_NO) ?
Removed this change because it does not need to be in this CL either

https://codereview.chromium.org/48963002/diff/40001/ash/wm/immersive_fullscre...
File ash/wm/immersive_fullscreen_controller.h (right):

https://codereview.chromium.org/48963002/diff/40001/ash/wm/immersive_fullscre...
ash/wm/immersive_fullscreen_controller.h:62: virtual void SetVisibility(double
visible_fraction) = 0;
On 2013/10/28 18:32:51, James Cook wrote:
> I initially thought this was either like SetVisible() or setting the alpha. I
> would call it SetVisibleFraction() or similar.

Done.

https://codereview.chromium.org/48963002/diff/40001/ash/wm/immersive_fullscre...
ash/wm/immersive_fullscreen_controller.h:154: CLOSED,
The y position is actually an implementation detail of
ImmersiveModeControllerAsh not of ash::ImmersiveFullscreenController.
ash::ImmersiveFullscreenController knows nothing of the fact that we have tab
indicators that we want to show when CLOSED which forces y == 0 when CLOSED in
the case of ImmersiveModeControllerAsh

https://codereview.chromium.org/48963002/diff/40001/ash/wm/immersive_fullscre...
ash/wm/immersive_fullscreen_controller.h:293: #endif  //
ASH_WM_IMMERSIVE_FULLSREEN_CONTROLLER_H_
On 2013/10/28 18:32:51, James Cook wrote:
> FULLSREEN -> FULLSCREEN

Done.

https://codereview.chromium.org/48963002/diff/40001/ash/wm/immersive_revealed...
File ash/wm/immersive_revealed_lock.h (right):

https://codereview.chromium.org/48963002/diff/40001/ash/wm/immersive_revealed...
ash/wm/immersive_revealed_lock.h:35: explicit ImmersiveRevealedLock(const
base::WeakPtr<Delegate>& delegate,
On 2013/10/28 18:32:51, James Cook wrote:
> no explicit

Done.

[James Cook, 2013-10-29 17:02:19]

LGTM with nit

https://codereview.chromium.org/48963002/diff/410001/ash/wm/immersive_fullscr...
File ash/wm/immersive_fullscreen_controller.cc (right):

https://codereview.chromium.org/48963002/diff/410001/ash/wm/immersive_fullscr...
ash/wm/immersive_fullscreen_controller.cc:322: // Move the mouse off of the
top-of-window views so that it does no keep the
nit: "does no" -> "does not"

[commit-bot: I haz the power, 2013-11-04 20:28:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/48963002/630001

[commit-bot: I haz the power, 2013-11-04 22:44:41]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_x64_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-11-06 16:49:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/48963002/920001

[commit-bot: I haz the power, 2013-11-06 18:30:25]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_x64_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-11-06 19:17:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/48963002/1170001

[commit-bot: I haz the power, 2013-11-06 22:44:42]

Change committed as 233403