estade@ please triage

I believe this regressed in 29a551c095f3df8e42a5c95994150b226a1d36f8 because when I revert the changes to 
chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc from that CL, it fixes this bug.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7768d47c0d737c0b9bd8a41166b9277f703cdf1e

commit 7768d47c0d737c0b9bd8a41166b9277f703cdf1e
Author: Evan Stade <estade@chromium.org>
Date: Thu Oct 25 22:40:17 2018

Fix AcceleratorCommandsFullscreenBrowserTest for Mash.

The test doesn't actually pass in Mash because of  crbug.com/897260 
but the test should pass once
ImmersiveModeControllerAsh::OnWindowPropertyChanged is fixed.

Bug:  897260 
Change-Id: Ic5914a4c1b17ba49a899f3775be5b21b2f6e1cff
Reviewed-on: https://chromium-review.googlesource.com/c/1297731
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602904}
[modify] https://crrev.com/7768d47c0d737c0b9bd8a41166b9277f703cdf1e/ash/public/interfaces/shell_test_api.mojom
[modify] https://crrev.com/7768d47c0d737c0b9bd8a41166b9277f703cdf1e/ash/shell_test_api.cc
[modify] https://crrev.com/7768d47c0d737c0b9bd8a41166b9277f703cdf1e/ash/shell_test_api.h
[modify] https://crrev.com/7768d47c0d737c0b9bd8a41166b9277f703cdf1e/chrome/browser/ui/ash/accelerator_commands_browsertest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9cfb85d4c5c1850b11aa132ea9ee539a96e307d8

commit 9cfb85d4c5c1850b11aa132ea9ee539a96e307d8
Author: Ivan Sandrk <isandrk@google.com>
Date: Fri Oct 26 07:16:54 2018

[Immersive mode] Make the controller track the right window property keys

The controller currently tracks ash::kWindowStateTypeKey which doesn't capture
the state transitions properly, instead use aura::client::kShowStateKey for
regular stuff and ash::kWindowPinTypeKey for pinned modes (locked fullscreen).

Bug:  897260 
Change-Id: I9e0160f4cbf4951f147954f222c52c11670fc466
Reviewed-on: https://chromium-review.googlesource.com/c/1299238
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Ivan Šandrk <isandrk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603019}
[modify] https://crrev.com/9cfb85d4c5c1850b11aa132ea9ee539a96e307d8/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc

Should be good now.

I think we need to merge this to M70.

Are you sure?  crrev.com/c/1234334 under "Included in" doesn't mention 70.

We do need to merge it to M71, and thanks for catching that.

This bug requires manual review: M71 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

On a second look, I did backmerge it to M70...  Thanks a lot for catching this Evan.

kuscher@, immersive mode is kinda broken on M70 (see OP), should we merge the fix (comment #5) to M70?  I'm guessing that would require a stable re-spin.

Hi release TPMs, tentatively marking this as merge-request-70 as this might need to be backmerged.

Curious:  Is this a common workflow?  Really a P1?

Also, prior to either merge:  Hi, has this been verified?  What's the risk if we merge?  Not seeing that on the bug.. Thanks.

I am not convinced this is a core flow to be honest. If the risk is high, I would wait for M71

Not sure how common it is, and I cannot comment on the priority (kuscher might be a better person for that).  Though there's a simple work-around that fixes the issue - just clicking the fullscreen button twice again fixes the problem.

I have verified the fix, and so has estade@ (and he provided a second pair of eyes to look at the code changes).  I'd be fairly comfortable merging this to M71, but I'm a bit wary of merging it directly to M70 considering my merge broke it on M70 in the first place.  I burned myself and I learned a lesson there - be much more careful about backmerges.

So thinking about it out loud, I'd probably want it merged to M71, but not to M70.

Immersive full-screen is the mode of full-screen reached by the full-screen button on the ChromeOS keyboard, so having it broken is very noticable.  I personally used that mode a lot because my Chromebook is one of the ones with a smaller screen, so getting the tabstrip and taskbar out of the way is important.

Okay, I'm approving for merge for M71 Chrome OS since it has been verified.. geohsu@ will have to review for M70.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8da5b29b5229f2bc64f6c84f0ae557799c7ddd48

commit 8da5b29b5229f2bc64f6c84f0ae557799c7ddd48
Author: Ivan Sandrk <isandrk@google.com>
Date: Tue Oct 30 16:50:12 2018

[M70 Merge][Immersive mode] Make the controller track the right window property keys

The controller currently tracks ash::kWindowStateTypeKey which doesn't capture
the state transitions properly, instead use aura::client::kShowStateKey for
regular stuff and ash::kWindowPinTypeKey for pinned modes (locked fullscreen).

TBR=isandrk@chromium.org

(cherry picked from commit 9cfb85d4c5c1850b11aa132ea9ee539a96e307d8)

Bug:  897260 
Change-Id: I9e0160f4cbf4951f147954f222c52c11670fc466
Reviewed-on: https://chromium-review.googlesource.com/c/1299238
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Ivan Šandrk <isandrk@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#603019}
Reviewed-on: https://chromium-review.googlesource.com/c/1307412
Reviewed-by: Ivan Šandrk <isandrk@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#400}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/8da5b29b5229f2bc64f6c84f0ae557799c7ddd48/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc

I won't be doing a back-merge to M70 for this, just too risky.

The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/8da5b29b5229f2bc64f6c84f0ae557799c7ddd48

Commit: 8da5b29b5229f2bc64f6c84f0ae557799c7ddd48
Author: isandrk@google.com
Commiter: isandrk@chromium.org
Date: 2018-10-30 16:50:12 +0000 UTC

[M70 Merge][Immersive mode] Make the controller track the right window property keys

The controller currently tracks ash::kWindowStateTypeKey which doesn't capture
the state transitions properly, instead use aura::client::kShowStateKey for
regular stuff and ash::kWindowPinTypeKey for pinned modes (locked fullscreen).

TBR=isandrk@chromium.org

(cherry picked from commit 9cfb85d4c5c1850b11aa132ea9ee539a96e307d8)

Bug:  897260 
Change-Id: I9e0160f4cbf4951f147954f222c52c11670fc466
Reviewed-on: https://chromium-review.googlesource.com/c/1299238
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Ivan Šandrk <isandrk@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#603019}
Reviewed-on: https://chromium-review.googlesource.com/c/1307412
Reviewed-by: Ivan Šandrk <isandrk@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#400}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

It might be worth testing whether OS fullscreen (i.e. the hardware fullscreen button, not F11) works as expected for triggering immersive mode in M70. That imo would warrant a merge.

Re #20: The OS fullscreen button _does_ trigger Immersive, but if you minimize an Immersive window then when you restore it, the tab-strip and taskbar no longer auto-show.

wez, what happens if after doing a immersive/minimize/restore cycle you press the OS fullscreen button again twice?

Re #22: That gets me back to a working Immersive mode, with auto-showing tab-strip and taskbar.

I'm afraid that will have to do for now, merging anything to M70 is just too risky at this point since the ship already sailed.  Sorry for the trouble, I'll be more careful about merging stuff in the future.