New issue
Advanced search Search tips

Issue 640365 link

Starred by 7 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 7
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug


Sign in to add a comment

Get immersive mode working for chrome in mash

Project Member Reported by sky@chromium.org, Aug 23 2016

Issue description

When chrome is in immersive mode it wants to define it's own top components, as well as define when reveal should be triggered. Because of this chrome really needs to control immersive mode and not use it from the window manager.
 

Comment 1 by sky@chromium.org, Aug 23 2016

Blocking: 548435

Comment 2 by sky@chromium.org, Aug 23 2016

Blockedon: 640371

Comment 3 by sky@chromium.org, Aug 23 2016

Blockedon: 640374

Comment 4 by sky@chromium.org, Aug 23 2016

Blockedon: 640378

Comment 5 by sky@chromium.org, Aug 23 2016

Blockedon: 640381

Comment 6 by sky@chromium.org, Aug 23 2016

Blockedon: 640384

Comment 7 by sky@chromium.org, Aug 23 2016

Blockedon: 640390
Project Member

Comment 8 by bugdroid1@chromium.org, Aug 26 2016

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

commit 2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf
Author: sky <sky@chromium.org>
Date: Fri Aug 26 05:27:35 2016

Wires up immersive mode for chrome and mash

There is still a slew of things to do, but this is a good start (and
rather lengthy).

There are two distinct ways for immersive mode to work in mash:

1. Mash takes care of it all. In this mode a separate ui::Window is
   created for the reveal of the title area. HeaderView is used to
   render the title area of the reveal in the separate window. The
   client does nothing special here.
2. The client takes control of it all (as happens in chrome). Chrome
   too creates a separate window for the reveal. This window is a
   child of the chrome frame. Mash knows to paint window decorations
   by way of a special property set on the window. Chrome runs all the
   immersive logic, including positioning of the reveal window.

To get 2 to work I had to relax a constraint of mus, in particular
this allows the window manager to set the underlay of any
window. Previously only the owner could, but as 2 requires mash to set
the underlay I had to relax the constraint.

I also changed it so that events in the underlay always go to the
window manager, not the owner as previously.

BUG= 634972 ,  640365 
TEST=none
R=jamescook@chromium.org, tsepez@chromium.org

Review-Url: https://codereview.chromium.org/2277563002
Cr-Commit-Position: refs/heads/master@{#414655}

[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/ash/common/frame/custom_frame_view_ash.cc
[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/ash/common/frame/custom_frame_view_ash.h
[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/ash/common/frame/default_header_painter.cc
[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/ash/common/frame/header_view.cc
[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/ash/common/frame/header_view.h
[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/ash/mus/BUILD.gn
[add] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/ash/mus/bridge/immersive_handler_factory_mus.cc
[add] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/ash/mus/bridge/immersive_handler_factory_mus.h
[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/ash/mus/bridge/wm_shell_mus.cc
[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/ash/mus/bridge/wm_shell_mus.h
[add] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/ash/mus/frame/README.md
[add] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/ash/mus/frame/custom_frame_view_mus.cc
[add] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/ash/mus/frame/custom_frame_view_mus.h
[add] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/ash/mus/frame/detached_title_area_renderer.cc
[add] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/ash/mus/frame/detached_title_area_renderer.h
[add] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/ash/mus/frame/detached_title_area_renderer_host.h
[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/ash/mus/non_client_frame_controller.cc
[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/ash/mus/non_client_frame_controller.h
[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/ash/mus/property_util.cc
[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/ash/mus/property_util.h
[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/ash/mus/root_window_controller.cc
[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/ash/wm/resize_shadow_and_cursor_unittest.cc
[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/chrome/browser/fullscreen_chromeos.cc
[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/chrome/browser/ui/views/ash/chrome_browser_main_extra_parts_ash.cc
[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/chrome/browser/ui/views/ash/chrome_browser_main_extra_parts_ash.h
[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/chrome/browser/ui/views/frame/browser_frame_mus.cc
[add] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/chrome/browser/ui/views/frame/immersive_context_mus.cc
[add] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/chrome/browser/ui/views/frame/immersive_context_mus.h
[add] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/chrome/browser/ui/views/frame/immersive_handler_factory_mus.cc
[add] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/chrome/browser/ui/views/frame/immersive_handler_factory_mus.h
[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc
[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/chrome/browser/ui/views/frame/immersive_mode_controller_ash.h
[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/chrome/browser/ui/views/frame/immersive_mode_controller_factory_views.cc
[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/chrome/chrome_browser_ui.gypi
[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/services/ui/public/interfaces/window_manager.mojom
[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/services/ui/ws/window_finder.cc
[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/services/ui/ws/window_manager_access_policy.cc
[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/services/ui/ws/window_manager_state.cc
[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/services/ui/ws/window_tree.cc
[modify] https://crrev.com/2d6a8e0fcb704248b64e7b65f4e76ace658a4bdf/ui/views/mus/native_widget_mus.cc

Components: MUS
Labels: Proj-Mustash
Components: Internals>MUS
Blockedon: 665179
Blocking: 665179
Blockedon: -665179

Comment 15 by sky@chromium.org, Jan 11 2017

Blockedon: 679919
Project Member

Comment 16 by bugdroid1@chromium.org, Jan 25 2017

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

commit 0ce28d085257f28db8594dda526f33dc20597fd0
Author: sky <sky@chromium.org>
Date: Wed Jan 25 19:31:22 2017

Wires up ShouldDescendIntoChildForEventHandling() for DesktopNativeWidgetAura

I promoted the implementation from NativeWidgetAura to Widget and both
call it. As Widget is used by mac I had to pass in gfx::NativeView and
the layers. I could have ifdef'd the function, but hopefully mac will
eventually be aura based and will want to use the Widget
implementation too.

BUG= 640365 
TEST=none
R=erg@chromium.org

Review-Url: https://codereview.chromium.org/2651753003
Cr-Commit-Position: refs/heads/master@{#446085}

[modify] https://crrev.com/0ce28d085257f28db8594dda526f33dc20597fd0/ui/views/view.cc
[modify] https://crrev.com/0ce28d085257f28db8594dda526f33dc20597fd0/ui/views/view.h
[modify] https://crrev.com/0ce28d085257f28db8594dda526f33dc20597fd0/ui/views/view_unittest_aura.cc
[modify] https://crrev.com/0ce28d085257f28db8594dda526f33dc20597fd0/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc
[modify] https://crrev.com/0ce28d085257f28db8594dda526f33dc20597fd0/ui/views/widget/native_widget_aura.cc
[modify] https://crrev.com/0ce28d085257f28db8594dda526f33dc20597fd0/ui/views/widget/native_widget_aura_unittest.cc
[modify] https://crrev.com/0ce28d085257f28db8594dda526f33dc20597fd0/ui/views/widget/native_widget_delegate.h
[modify] https://crrev.com/0ce28d085257f28db8594dda526f33dc20597fd0/ui/views/widget/widget.cc
[modify] https://crrev.com/0ce28d085257f28db8594dda526f33dc20597fd0/ui/views/widget/widget.h

Comment 17 by sky@chromium.org, Feb 8 2017

Blockedon: 640392

Comment 18 by sky@chromium.org, Feb 8 2017

Blocking: -548435

Comment 19 by sky@chromium.org, Feb 8 2017

Issue 548435 has been merged into this issue.

Comment 20 by e...@chromium.org, Sep 26 2017

Currently, when entering immersive mode in mash, we crash in aura::WindowPortMus::SetFrameSinkIdFromServer() because the window we're trying to set the frame sink id on is a WindowMusType::LOCAL one.
Components: -Internals>MUS Internals>Services>WindowService
Components: -MUS
Blocking: 791148

Comment 24 Deleted

Blockedon: 844748
Cc: est...@chromium.org sky@chromium.org mustash-bugs@google.com
 Issue 844748  has been merged into this issue.
Project Member

Comment 27 by bugdroid1@chromium.org, Jul 20

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

commit b78ad2575f1449648fa7f3f64d0657a15bc83fd7
Author: Evan Stade <estade@chromium.org>
Date: Fri Jul 20 16:38:07 2018

OopAsh: leave fullscreen w/restore button on immersive reveal widget

Observe aura::client::kShowStateKey instead of ash::kWindowStateTypeKey.

Bug:  844748 , 640365 
Change-Id: I06306988084cc7b6cafd0553447b7491301cbab2
Reviewed-on: https://chromium-review.googlesource.com/1141994
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Ivan Å andrk <isandrk@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576899}
[modify] https://crrev.com/b78ad2575f1449648fa7f3f64d0657a15bc83fd7/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc

Owner: est...@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 29 by bugdroid1@chromium.org, Jul 26

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

commit 4480bf47dbf3708f96dc79b8314a07105b534588
Author: Evan Stade <estade@chromium.org>
Date: Thu Jul 26 17:32:23 2018

OopAsh: improve immersive fullscreen for app windows

Immersive fullscreen for an app window almost works aside from the fact
that the normal frame remains when the app window is fullscreened. This
is a result of not updating the client area inset when the window show
state changes (e.g. from restored to fullscreen). The client area is
updated when the window bounds change, as they would when entering
fullscreen, but the bounds change comes through just before the state
change. Thus, when ClientSideNonClientFrameView checks fullscreen state
during layout, it still finds the widget is restored and calculates the
wrong bounds. The solution is to explicitly kick off a layout when
entering or exiting fullscreen.

      after launching with --enable-features=Mash --ash-dev-shortcuts

Test: enter and exit fullscreen on an app window with ctrl+shift+F
Bug:  640365 
Change-Id: I499e29427ed072f3428de1f195ede1dc9892e71f
Reviewed-on: https://chromium-review.googlesource.com/1150618
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578353}
[modify] https://crrev.com/4480bf47dbf3708f96dc79b8314a07105b534588/ui/views/mus/desktop_window_tree_host_mus.cc
[modify] https://crrev.com/4480bf47dbf3708f96dc79b8314a07105b534588/ui/views/mus/desktop_window_tree_host_mus.h
[modify] https://crrev.com/4480bf47dbf3708f96dc79b8314a07105b534588/ui/views/mus/desktop_window_tree_host_mus_unittest.cc

Project Member

Comment 30 by bugdroid1@chromium.org, Jul 26

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

commit 48d9716bcc72f5899be15244c35297c363ab41e0
Author: Stephen McGruer <smcgruer@chromium.org>
Date: Thu Jul 26 18:32:59 2018

Revert "OopAsh: improve immersive fullscreen for app windows"

This reverts commit 4480bf47dbf3708f96dc79b8314a07105b534588.

Reason for revert: Strong suspect for causing crashes in WidgetTest.FullscreenFrameLayout on linux-chromeos-rel

Bug:  868004 

Original change's description:
> OopAsh: improve immersive fullscreen for app windows
> 
> Immersive fullscreen for an app window almost works aside from the fact
> that the normal frame remains when the app window is fullscreened. This
> is a result of not updating the client area inset when the window show
> state changes (e.g. from restored to fullscreen). The client area is
> updated when the window bounds change, as they would when entering
> fullscreen, but the bounds change comes through just before the state
> change. Thus, when ClientSideNonClientFrameView checks fullscreen state
> during layout, it still finds the widget is restored and calculates the
> wrong bounds. The solution is to explicitly kick off a layout when
> entering or exiting fullscreen.
> 
>       after launching with --enable-features=Mash --ash-dev-shortcuts
> 
> Test: enter and exit fullscreen on an app window with ctrl+shift+F
> Bug:  640365 
> Change-Id: I499e29427ed072f3428de1f195ede1dc9892e71f
> Reviewed-on: https://chromium-review.googlesource.com/1150618
> Reviewed-by: Scott Violet <sky@chromium.org>
> Commit-Queue: Evan Stade <estade@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#578353}

TBR=sky@chromium.org,estade@chromium.org

Change-Id: I2effa307fa2da94610130e6b084b6979fc4c070e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  640365 
Reviewed-on: https://chromium-review.googlesource.com/1151908
Reviewed-by: Stephen McGruer <smcgruer@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578380}
[modify] https://crrev.com/48d9716bcc72f5899be15244c35297c363ab41e0/ui/views/mus/desktop_window_tree_host_mus.cc
[modify] https://crrev.com/48d9716bcc72f5899be15244c35297c363ab41e0/ui/views/mus/desktop_window_tree_host_mus.h
[modify] https://crrev.com/48d9716bcc72f5899be15244c35297c363ab41e0/ui/views/mus/desktop_window_tree_host_mus_unittest.cc

Project Member

Comment 31 by bugdroid1@chromium.org, Jul 27

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

commit f1c4d0ae689f146144480f0a3f37f6473fc330f2
Author: Evan Stade <estade@chromium.org>
Date: Fri Jul 27 22:10:02 2018

Reland "OopAsh: improve immersive fullscreen for app windows"

This relands commit 4480bf47dbf3708f96dc79b8314a07105b534588.

Difference to original: stop observing view on its destruction.

Bug:  640365 

Original change's description:
> OopAsh: improve immersive fullscreen for app windows
>
> Immersive fullscreen for an app window almost works aside from the fact
> that the normal frame remains when the app window is fullscreened. This
> is a result of not updating the client area inset when the window show
> state changes (e.g. from restored to fullscreen). The client area is
> updated when the window bounds change, as they would when entering
> fullscreen, but the bounds change comes through just before the state
> change. Thus, when ClientSideNonClientFrameView checks fullscreen state
> during layout, it still finds the widget is restored and calculates the
> wrong bounds. The solution is to explicitly kick off a layout when
> entering or exiting fullscreen.
>
>       after launching with --enable-features=Mash --ash-dev-shortcuts
>
> Test: enter and exit fullscreen on an app window with ctrl+shift+F
> Bug:  640365 
> Change-Id: I499e29427ed072f3428de1f195ede1dc9892e71f
> Reviewed-on: https://chromium-review.googlesource.com/1150618
> Reviewed-by: Scott Violet <sky@chromium.org>
> Commit-Queue: Evan Stade <estade@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#578353}

Change-Id: I46b213a595e15d277f66cd77d1323dcf454ba8e1
Reviewed-on: https://chromium-review.googlesource.com/1153397
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578825}
[modify] https://crrev.com/f1c4d0ae689f146144480f0a3f37f6473fc330f2/ui/views/mus/desktop_window_tree_host_mus.cc
[modify] https://crrev.com/f1c4d0ae689f146144480f0a3f37f6473fc330f2/ui/views/mus/desktop_window_tree_host_mus.h
[modify] https://crrev.com/f1c4d0ae689f146144480f0a3f37f6473fc330f2/ui/views/mus/desktop_window_tree_host_mus_unittest.cc

Project Member

Comment 32 by bugdroid1@chromium.org, Aug 1

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

commit 30b0191768abc5ab9fdc52a59fe09d9080da1b34
Author: Evan Stade <estade@chromium.org>
Date: Wed Aug 01 23:58:21 2018

OopAsh: respond to WM-initiated changes in fullscreen state.

This means Chrome will enter immersive when the user presses
the fullscreen key (F4 on a normal keyboard --- although F4
on a normal keyboard doesn't actually work right now because
event rewriters aren't functioning in OopAsh).

The way this works in classic Ash is via
BrowserWindowStateDelegate, which isn't used in OopAsh.

Bug:  640365 
Change-Id: I3b7e7a8911c5d8cba48f2d770e74104dba61e9e2
Reviewed-on: https://chromium-review.googlesource.com/1156995
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579998}
[modify] https://crrev.com/30b0191768abc5ab9fdc52a59fe09d9080da1b34/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc
[modify] https://crrev.com/30b0191768abc5ab9fdc52a59fe09d9080da1b34/chrome/browser/ui/views/frame/immersive_mode_controller_ash.h

Project Member

Comment 33 by bugdroid1@chromium.org, Aug 9

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

commit 5621e83a75398f783c081a19f84f40077f6f34d0
Author: Evan Stade <estade@chromium.org>
Date: Thu Aug 09 02:06:42 2018

OopAsh: fix rendering of Mash reveal widget

The Ash-side DetachedTitleAreaRenderer was drawing a default header
(like for normal windows) instead of a custom header (like for browser
windows).

Bug:  640365 
Change-Id: I219d9e2146ee379ff63c988951c19049a99b38f3
Reviewed-on: https://chromium-review.googlesource.com/1157438
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581753}
[modify] https://crrev.com/5621e83a75398f783c081a19f84f40077f6f34d0/ash/frame/detached_title_area_renderer.cc

 Issue 548433  has been merged into this issue.
 Issue 612629  has been merged into this issue.
Labels: -Pri-3 -Proj-Mustash Proj-Mash-SingleProcess Pri-2
Project Member

Comment 37 by bugdroid1@chromium.org, Aug 23

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

commit b22709147259a45d9c005d03336fd14f7f873eea
Author: Evan Stade <estade@chromium.org>
Date: Thu Aug 23 23:16:27 2018

OopAsh: improve immersive mode reveal widget.

1. Make sure located events go to the top container when it's revealed.
This is accomplished with an EventRewriter which clears the target
window, which had been provided by the window service, from the event.
Without this, clicks on the top container are targeted to the
web contents window because the Window Service doesn't know about the
top container's window. In the long term, we may be able to remove this
EventRewriter with a more general solution (e.g. improving how the window
service targets the event in the first place).

2. Instead of mirroring the top container's layers, make the widget
completely transparent except for the ash-provided window controls.
Mirroring doesn't update the reveal widget in response to changes in
the top container's layers, such as when a button is hovered and an ink
drop is added.

3. Allow the reveal widget to accept events so that it's possible to
interact with the window controls. Resize the reveal widget to be just
big enough for the window controls so that clicks anywhere else will
go straight through to the underlying top container view.

4. Make the browser-side non client frame view paint the frame bg. This
is necessary because the reveal widget (i.e. window controls) is now on
top of the container view and has no client-side rendering (what the
mirroring view used to provide).

Future issues to address:
- the cursor is not updated (e.g. when hovering the omnibox) presumably
  because the cursor is coming from the window that the WS thinks is
  underneath the pointer, i.e. the web contents.

Alternatives considered:
- update the mirrored layers every time the layer tree changes. This
  doesn't seem to be easy to accomplish or likely to be very performant.
- move the top container into the reveal widget. This somewhat works
  with a few tweaks here and there to code that assumes the top container
  is always in the browser widget, but ultimately it seems that making
  the reveal widget focusable causes difficulties with window activation
  and focus traversal.

Bug:  640365 
Change-Id: I03956c07354e20dadc2919f9a0402ce1f7b4a672
Reviewed-on: https://chromium-review.googlesource.com/1171547
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585649}
[modify] https://crrev.com/b22709147259a45d9c005d03336fd14f7f873eea/ash/frame/custom_frame_header.cc
[modify] https://crrev.com/b22709147259a45d9c005d03336fd14f7f873eea/ash/frame/detached_title_area_renderer.h
[modify] https://crrev.com/b22709147259a45d9c005d03336fd14f7f873eea/ash/frame/non_client_frame_view_ash.cc
[modify] https://crrev.com/b22709147259a45d9c005d03336fd14f7f873eea/ash/public/cpp/BUILD.gn
[delete] https://crrev.com/c2ed26d45c79c204138878a63d73daa4509ada40/ash/public/cpp/frame_border_hit_test.cc
[delete] https://crrev.com/c2ed26d45c79c204138878a63d73daa4509ada40/ash/public/cpp/frame_border_hit_test.h
[add] https://crrev.com/b22709147259a45d9c005d03336fd14f7f873eea/ash/public/cpp/frame_utils.cc
[add] https://crrev.com/b22709147259a45d9c005d03336fd14f7f873eea/ash/public/cpp/frame_utils.h
[modify] https://crrev.com/b22709147259a45d9c005d03336fd14f7f873eea/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
[modify] https://crrev.com/b22709147259a45d9c005d03336fd14f7f873eea/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc
[modify] https://crrev.com/b22709147259a45d9c005d03336fd14f7f873eea/chrome/browser/ui/views/frame/immersive_mode_controller_ash.h

Project Member

Comment 38 by bugdroid1@chromium.org, Aug 24

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

commit 82f3ef43188f8056f536d500945dbba8608a7461
Author: Evan Stade <estade@chromium.org>
Date: Fri Aug 24 16:53:56 2018

Fix DCHECK in LocatedEventRetargeter.

The wrong version of this DCHECK slipped by in b22709147259a45d9c005

Bug:  640365 
Change-Id: I9e1922c537851be269f483e98878bb2d200e9260
Reviewed-on: https://chromium-review.googlesource.com/1187712
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585877}
[modify] https://crrev.com/82f3ef43188f8056f536d500945dbba8608a7461/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc

Project Member

Comment 39 by bugdroid1@chromium.org, Aug 27

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

commit f23670494ae6bf4750ed51e994fa2f664a5df6c9
Author: Evan Stade <estade@chromium.org>
Date: Mon Aug 27 18:23:32 2018

Create ImmersiveFocusWatcher for use in Chrome in Mash.

This keeps the immersive reveal going when bubbles anchored to the top
container (like the bookmark bubble) are open, when focus in in the top
container, or when transient children (like the bookmark editor) are
open.

ImmersiveFocusWatcherMus is largely copied from
ImmersiveFocusWatcherClassic, but
1. uses aura::client::TransientWindowClient instead of
   ::wm::TransientWindowManager
2. uses WindowTreeClient::FocusSynchronizer instead of
   ::wm::GetActivationClient
3. Appends |->GetRootWindow()| in a couple places.

Bug:  640365 
Change-Id: I46cd0c3d66808d726fea484ff13ceea53dcabd5c
Reviewed-on: https://chromium-review.googlesource.com/1187708
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586323}
[modify] https://crrev.com/f23670494ae6bf4750ed51e994fa2f664a5df6c9/chrome/browser/ui/BUILD.gn
[add] https://crrev.com/f23670494ae6bf4750ed51e994fa2f664a5df6c9/chrome/browser/ui/views/frame/immersive_focus_watcher_mus.cc
[add] https://crrev.com/f23670494ae6bf4750ed51e994fa2f664a5df6c9/chrome/browser/ui/views/frame/immersive_focus_watcher_mus.h
[modify] https://crrev.com/f23670494ae6bf4750ed51e994fa2f664a5df6c9/chrome/browser/ui/views/frame/immersive_handler_factory_mus.cc
[modify] https://crrev.com/f23670494ae6bf4750ed51e994fa2f664a5df6c9/ui/aura/client/transient_window_client.h
[modify] https://crrev.com/f23670494ae6bf4750ed51e994fa2f664a5df6c9/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc
[modify] https://crrev.com/f23670494ae6bf4750ed51e994fa2f664a5df6c9/ui/wm/core/transient_window_controller.cc
[modify] https://crrev.com/f23670494ae6bf4750ed51e994fa2f664a5df6c9/ui/wm/core/transient_window_controller.h

Project Member

Comment 40 by bugdroid1@chromium.org, Sep 5

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

commit 0b22a42414ea3bf94d1ff160a86f7ad5d24f7d0e
Author: Evan Stade <estade@chromium.org>
Date: Wed Sep 05 17:03:01 2018

Mash: Support Chrome App immersive mode

The client-side window frame/client layout code uses a per-window
property (the pre-existing kTopViewInset) to layout the contents,
rather than hardcoded values plus logic related to window state. This
is necessary so that apps will be laid out appropriately in immersive
mode when not fullscreened (i.e. maximized in tablet mode).

Also, propagate some window properties across processes.

Bug:  640365 
Change-Id: I485920c242e1c98450d0868e078f534489525f7c
Reviewed-on: https://chromium-review.googlesource.com/1195886
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588894}
[modify] https://crrev.com/0b22a42414ea3bf94d1ff160a86f7ad5d24f7d0e/ash/frame/header_view.cc
[modify] https://crrev.com/0b22a42414ea3bf94d1ff160a86f7ad5d24f7d0e/ash/frame/header_view.h
[modify] https://crrev.com/0b22a42414ea3bf94d1ff160a86f7ad5d24f7d0e/ash/frame/non_client_frame_view_ash.cc
[modify] https://crrev.com/0b22a42414ea3bf94d1ff160a86f7ad5d24f7d0e/ash/frame/non_client_frame_view_ash.h
[modify] https://crrev.com/0b22a42414ea3bf94d1ff160a86f7ad5d24f7d0e/ash/frame/non_client_frame_view_ash_unittest.cc
[modify] https://crrev.com/0b22a42414ea3bf94d1ff160a86f7ad5d24f7d0e/ash/public/cpp/mus_property_mirror_ash.cc
[modify] https://crrev.com/0b22a42414ea3bf94d1ff160a86f7ad5d24f7d0e/ash/public/cpp/window_properties.cc
[modify] https://crrev.com/0b22a42414ea3bf94d1ff160a86f7ad5d24f7d0e/ash/public/interfaces/window_properties.mojom
[modify] https://crrev.com/0b22a42414ea3bf94d1ff160a86f7ad5d24f7d0e/ash/wm/non_client_frame_controller.cc
[modify] https://crrev.com/0b22a42414ea3bf94d1ff160a86f7ad5d24f7d0e/ash/wm/non_client_frame_controller.h
[modify] https://crrev.com/0b22a42414ea3bf94d1ff160a86f7ad5d24f7d0e/chrome/browser/ui/views/apps/DEPS
[modify] https://crrev.com/0b22a42414ea3bf94d1ff160a86f7ad5d24f7d0e/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc
[modify] https://crrev.com/0b22a42414ea3bf94d1ff160a86f7ad5d24f7d0e/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.h
[modify] https://crrev.com/0b22a42414ea3bf94d1ff160a86f7ad5d24f7d0e/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash_browsertest.cc
[modify] https://crrev.com/0b22a42414ea3bf94d1ff160a86f7ad5d24f7d0e/chrome/browser/ui/views/frame/browser_frame_mash.cc
[modify] https://crrev.com/0b22a42414ea3bf94d1ff160a86f7ad5d24f7d0e/testing/buildbot/filters/chromeos.mash.fyi.browser_tests.filter
[modify] https://crrev.com/0b22a42414ea3bf94d1ff160a86f7ad5d24f7d0e/ui/views/mus/desktop_window_tree_host_mus.cc
[modify] https://crrev.com/0b22a42414ea3bf94d1ff160a86f7ad5d24f7d0e/ui/views/mus/desktop_window_tree_host_mus_unittest.cc

Project Member

Comment 41 by bugdroid1@chromium.org, Oct 11

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

commit 1467204e63f6036f9ae8d719da3efd2a5eeed8e1
Author: Evan Stade <estade@chromium.org>
Date: Thu Oct 11 02:03:50 2018

Fix immersive mode for SingleProcessMash.

In single process Mash, the browser needs to use ImmersiveContextMus,
whereas Ash needs to go on using ImmersiveContextAsh, so remove the
ImmersiveContext singleton in favor of passing the context instance
to ImmersiveFullscreenController at construction time.

Bug:  640365 
Change-Id: I3cd91563d58b40cdf8720208acf7088cb191621e
Reviewed-on: https://chromium-review.googlesource.com/c/1272091
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598632}
[modify] https://crrev.com/1467204e63f6036f9ae8d719da3efd2a5eeed8e1/ash/frame/non_client_frame_view_ash.cc
[modify] https://crrev.com/1467204e63f6036f9ae8d719da3efd2a5eeed8e1/ash/frame/non_client_frame_view_ash_unittest.cc
[modify] https://crrev.com/1467204e63f6036f9ae8d719da3efd2a5eeed8e1/ash/public/cpp/BUILD.gn
[delete] https://crrev.com/83e0cbb508d756326ef59edddd4644afc7861839/ash/public/cpp/immersive/immersive_context.cc
[modify] https://crrev.com/1467204e63f6036f9ae8d719da3efd2a5eeed8e1/ash/public/cpp/immersive/immersive_context.h
[modify] https://crrev.com/1467204e63f6036f9ae8d719da3efd2a5eeed8e1/ash/public/cpp/immersive/immersive_fullscreen_controller.cc
[modify] https://crrev.com/1467204e63f6036f9ae8d719da3efd2a5eeed8e1/ash/public/cpp/immersive/immersive_fullscreen_controller.h
[modify] https://crrev.com/1467204e63f6036f9ae8d719da3efd2a5eeed8e1/ash/shell.h
[modify] https://crrev.com/1467204e63f6036f9ae8d719da3efd2a5eeed8e1/ash/wm/immersive_fullscreen_controller_unittest.cc
[modify] https://crrev.com/1467204e63f6036f9ae8d719da3efd2a5eeed8e1/chrome/browser/ui/ash/chrome_browser_main_extra_parts_ash.cc
[modify] https://crrev.com/1467204e63f6036f9ae8d719da3efd2a5eeed8e1/chrome/browser/ui/views/frame/DEPS
[modify] https://crrev.com/1467204e63f6036f9ae8d719da3efd2a5eeed8e1/chrome/browser/ui/views/frame/browser_frame_mash.cc
[modify] https://crrev.com/1467204e63f6036f9ae8d719da3efd2a5eeed8e1/chrome/browser/ui/views/frame/immersive_context_mus.cc
[modify] https://crrev.com/1467204e63f6036f9ae8d719da3efd2a5eeed8e1/chrome/browser/ui/views/frame/immersive_context_mus.h
[modify] https://crrev.com/1467204e63f6036f9ae8d719da3efd2a5eeed8e1/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc
[modify] https://crrev.com/1467204e63f6036f9ae8d719da3efd2a5eeed8e1/components/exo/client_controlled_shell_surface.cc
[modify] https://crrev.com/1467204e63f6036f9ae8d719da3efd2a5eeed8e1/testing/buildbot/filters/chromeos.mash.fyi.browser_tests.filter
[modify] https://crrev.com/1467204e63f6036f9ae8d719da3efd2a5eeed8e1/testing/buildbot/filters/chromeos.single_process_mash.browser_tests.filter

Project Member

Comment 42 by bugdroid1@chromium.org, Oct 16

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

commit 27871aa8893c123f9081cf5288901a96f357f319
Author: Evan Stade <estade@chromium.org>
Date: Tue Oct 16 21:06:49 2018

Combine ImmersiveFocusWatcher{Mus,Classic}.

Use of TransientWindowClient and ActivationClient allow the same
implementation to work in both classic and Mash.

Bug:  640365 
Change-Id: I10324dce04bb014c09d04f45c0be6e691017a648
Reviewed-on: https://chromium-review.googlesource.com/c/1271403
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600119}
[modify] https://crrev.com/27871aa8893c123f9081cf5288901a96f357f319/ash/BUILD.gn
[modify] https://crrev.com/27871aa8893c123f9081cf5288901a96f357f319/ash/public/cpp/BUILD.gn
[rename] https://crrev.com/27871aa8893c123f9081cf5288901a96f357f319/ash/public/cpp/immersive/immersive_focus_watcher.cc
[modify] https://crrev.com/27871aa8893c123f9081cf5288901a96f357f319/ash/public/cpp/immersive/immersive_focus_watcher.h
[modify] https://crrev.com/27871aa8893c123f9081cf5288901a96f357f319/ash/public/cpp/immersive/immersive_fullscreen_controller.cc
[modify] https://crrev.com/27871aa8893c123f9081cf5288901a96f357f319/ash/public/cpp/immersive/immersive_handler_factory.h
[delete] https://crrev.com/de56eaf9dcccb87f8df9b08525cd777d471880cf/ash/wm/immersive_focus_watcher_classic.cc
[delete] https://crrev.com/de56eaf9dcccb87f8df9b08525cd777d471880cf/ash/wm/immersive_focus_watcher_classic.h
[modify] https://crrev.com/27871aa8893c123f9081cf5288901a96f357f319/ash/wm/immersive_handler_factory_ash.cc
[modify] https://crrev.com/27871aa8893c123f9081cf5288901a96f357f319/ash/wm/immersive_handler_factory_ash.h
[modify] https://crrev.com/27871aa8893c123f9081cf5288901a96f357f319/chrome/browser/ui/BUILD.gn
[delete] https://crrev.com/de56eaf9dcccb87f8df9b08525cd777d471880cf/chrome/browser/ui/views/frame/immersive_focus_watcher_mus.h
[modify] https://crrev.com/27871aa8893c123f9081cf5288901a96f357f319/chrome/browser/ui/views/frame/immersive_handler_factory_mus.cc
[modify] https://crrev.com/27871aa8893c123f9081cf5288901a96f357f319/chrome/browser/ui/views/frame/immersive_handler_factory_mus.h
[modify] https://crrev.com/27871aa8893c123f9081cf5288901a96f357f319/testing/buildbot/filters/chromeos.single_process_mash.browser_tests.filter

Status: Fixed (was: Started)
Status: Started (was: Fixed)
still need ImmersiveGestureHandlerMus
Blocking: 896947
Project Member

Comment 46 by bugdroid1@chromium.org, Oct 22

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

commit d2681cd3edf8640e266837df6df7621583979064
Author: Evan Stade <estade@chromium.org>
Date: Mon Oct 22 21:15:00 2018

Mash: fix and enable ImmersiveModeBrowserViewTest*

Bug:  640365 
Change-Id: I9683f06da8528cf50ce67943932bad09edac28db
Reviewed-on: https://chromium-review.googlesource.com/c/1292410
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601725}
[modify] https://crrev.com/d2681cd3edf8640e266837df6df7621583979064/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc
[modify] https://crrev.com/d2681cd3edf8640e266837df6df7621583979064/testing/buildbot/filters/chromeos.single_process_mash.browser_tests.filter

Blockedon: 897968
Project Member

Comment 48 by bugdroid1@chromium.org, Oct 29

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

commit ddbc630ab4b94abe3deb54c3ac46a66063904134
Author: Evan Stade <estade@chromium.org>
Date: Mon Oct 29 21:03:01 2018

Enable AcceleratorCommands browser tests on Mash.

Tests fixed in 7768d47c0d737c
Behavior fixed in 9cfb85d4c5c185

Bug:  640365 
Change-Id: I92d955c5c450d672f5d8c6fd263c44169051eb66
Reviewed-on: https://chromium-review.googlesource.com/c/1303235
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603622}
[modify] https://crrev.com/ddbc630ab4b94abe3deb54c3ac46a66063904134/testing/buildbot/filters/chromeos.mash.fyi.browser_tests.filter
[modify] https://crrev.com/ddbc630ab4b94abe3deb54c3ac46a66063904134/testing/buildbot/filters/chromeos.single_process_mash.browser_tests.filter

Project Member

Comment 49 by bugdroid1@chromium.org, Oct 30

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

commit b7e396b6be7c84d8d7eca6961218b20e4544d8d4
Author: Evan Stade <estade@chromium.org>
Date: Tue Oct 30 22:33:28 2018

Reland "Merge two immersive mode window properties."

This relands commit 1e9af0714f0e6164ff6d3444bee334c4f2d02778.

The original commit and subsequent test failure uncovered a real
bug which only impacted Mash. In classic mode, frameless windows
don't act as a WindowState observer so they don't inform the app
of OS-triggered fullscreen changes and therefore
ShouldEnableImmersive() doesn't return true, allowing the test to
pass. It seems like a bug that the app doesn't know it's in
fullscreen just because it's frameless, but I will err on the side
of preserving existing behavior there.

In contrast, in Mash, frameless windows do update the app object
wrt changes in window state, therefore the implementation of
ShouldEnableImmersive must be enhanced slightly to explicitly
rule out frameless windows from being placed in immersive (as the
briefly failing test verifies).

The test passed before 1e9af0714f0e6164ff6d3444bee simply because
the two different immersive mode properties lagged in respect to
one another.

TBR=tsepez@chromium.org
Bug:  640365 

Original change's description:
> Merge two immersive mode window properties.
>
> aura::client::kImmersiveFullscreenKey is folded into
> ash::kImmersiveIsActive
>
> No bug here currently, but it's confusing to have two different
> properties that are both meant to indicate immersive mode is active.
>
> Also removed some unnecessary ash:: prefixes from //ash.
>
> Change-Id: I0c43873fa8f128c3799d32f2c479a1f2caa9a7cf
> Reviewed-on: https://chromium-review.googlesource.com/c/1298493
> Reviewed-by: Scott Violet <sky@chromium.org>
> 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@{#603778}

Change-Id: Id4a498cf7d6f721d6459bd75392564abbfda60b5
Reviewed-on: https://chromium-review.googlesource.com/c/1307842
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604038}
[modify] https://crrev.com/b7e396b6be7c84d8d7eca6961218b20e4544d8d4/ash/shelf/shelf_layout_manager_unittest.cc
[modify] https://crrev.com/b7e396b6be7c84d8d7eca6961218b20e4544d8d4/ash/wm/immersive_context_ash.cc
[modify] https://crrev.com/b7e396b6be7c84d8d7eca6961218b20e4544d8d4/ash/wm/splitview/split_view_controller_unittest.cc
[modify] https://crrev.com/b7e396b6be7c84d8d7eca6961218b20e4544d8d4/ash/wm/tablet_mode/tablet_mode_window_manager_unittest.cc
[modify] https://crrev.com/b7e396b6be7c84d8d7eca6961218b20e4544d8d4/ash/wm/window_state.cc
[modify] https://crrev.com/b7e396b6be7c84d8d7eca6961218b20e4544d8d4/ash/wm/window_state.h
[modify] https://crrev.com/b7e396b6be7c84d8d7eca6961218b20e4544d8d4/chrome/browser/ui/ash/accelerator_commands_browsertest.cc
[modify] https://crrev.com/b7e396b6be7c84d8d7eca6961218b20e4544d8d4/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc
[modify] https://crrev.com/b7e396b6be7c84d8d7eca6961218b20e4544d8d4/chrome/browser/ui/views/frame/immersive_context_mus.cc
[modify] https://crrev.com/b7e396b6be7c84d8d7eca6961218b20e4544d8d4/components/exo/shell_surface_base.cc
[modify] https://crrev.com/b7e396b6be7c84d8d7eca6961218b20e4544d8d4/services/ws/public/mojom/window_manager.mojom
[modify] https://crrev.com/b7e396b6be7c84d8d7eca6961218b20e4544d8d4/ui/aura/client/aura_constants.cc
[modify] https://crrev.com/b7e396b6be7c84d8d7eca6961218b20e4544d8d4/ui/aura/client/aura_constants.h
[modify] https://crrev.com/b7e396b6be7c84d8d7eca6961218b20e4544d8d4/ui/aura/mus/property_converter.cc

Status: Fixed (was: Started)

Sign in to add a comment