New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 634566 link

Starred by 5 users

Issue metadata

Status: Verified
Owner:
Closed: Aug 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

[ARC++] Unable to show shelf in immersive fullscreen.

Project Member Reported by osh...@chromium.org, Aug 5 2016

Issue description

Repro step:
1) open arc app
2) go to immersive fullscreen (F5)
3) move the mouse to the bottom (or edge depending on your shelf layout) of the screen

Expected: shelf shows up.
Actual: no shelf

This could be due to the way exo steals the mouse event.

-> mitsuji@ for triage

Please feel free to assign me back.
 
I have the same behavior when I make a youtube video fullscreen but maybe that's expected to be different?
That's true fullscreen (html/pepper fullscreen), which is different from immersive fullscreen. 
I've noticed that web apps use a different fullscreen mode. I'm not sure this is that we want for arc++ apps such as games and video. One downside with the chrome apps fullscreen mode (the mode that doesn't cover the whole screen) is that it doesn't allow us to use hw overlays as easily. The true fullscreen case that we use for arc++ apps today is great as it makes it easy to skip compositing and improve performance and power usage significantly. 
It'll be shown only when a user move the mouse to the edge, so that shouldn't be a problem.

In the long run, android should have these two, because fullscreening youtube app, and clicking fullscreen icon in the video should have different meaning.

If it's only when the user moves the mouse to the edge then that's different and is not going to prevent hw overlay support. However, this is not how chrome apps seem to work today. They currently leave a few pixels at the bottom of the screen for the shelf.
We can keep the current layout and still make the shelf available (that's what I was going to do), unless UX wants different behavior.

By the way, can't we detect such apps (that uses fullscreen video, fullscreen 3d/2d context)? If so (and if it makes sense), we can implement different behavior depending on that condition.
Currently layout but still making shelf available sgtm.
Cc: vadimt@chromium.org
Labels: ReleaseBlock-Stable M-53
Owner: osh...@chromium.org
Status: Assigned (was: Untriaged)
Status: Started (was: Assigned)
oshima@ how soon can we land these changes so we get enough bake time?
Project Member

Comment 12 by bugdroid1@chromium.org, Aug 16 2016

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

commit 9913eed0106818160f3828ae7c2fd90dbf985700
Author: oshima <oshima@chromium.org>
Date: Tue Aug 16 19:48:47 2016

Use MD-ash's auto hide behavior for arc++ windows.

This add a runtime flag to enable MD-ash's auto hide behavior , which makes the shelf invisible, for arc++ windows. I'll remove this mode once MD ash is launched.

BUG= 634566 
TEST=covered by unit test. Manually tested on minnie.

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

[modify] https://crrev.com/9913eed0106818160f3828ae7c2fd90dbf985700/ash/common/wm/window_state.cc
[modify] https://crrev.com/9913eed0106818160f3828ae7c2fd90dbf985700/ash/common/wm/window_state.h
[modify] https://crrev.com/9913eed0106818160f3828ae7c2fd90dbf985700/ash/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/9913eed0106818160f3828ae7c2fd90dbf985700/ash/shelf/shelf_layout_manager.h
[modify] https://crrev.com/9913eed0106818160f3828ae7c2fd90dbf985700/ash/shelf/shelf_layout_manager_unittest.cc
[modify] https://crrev.com/9913eed0106818160f3828ae7c2fd90dbf985700/ash/system/web_notification/web_notification_tray_unittest.cc
[modify] https://crrev.com/9913eed0106818160f3828ae7c2fd90dbf985700/ash/wm/immersive_fullscreen_controller.cc
[modify] https://crrev.com/9913eed0106818160f3828ae7c2fd90dbf985700/chrome/browser/chromeos/login/lock/screen_locker.cc
[modify] https://crrev.com/9913eed0106818160f3828ae7c2fd90dbf985700/chrome/browser/chromeos/login/lock/screen_locker_browsertest.cc
[modify] https://crrev.com/9913eed0106818160f3828ae7c2fd90dbf985700/chrome/browser/notifications/fullscreen_notification_blocker.cc
[modify] https://crrev.com/9913eed0106818160f3828ae7c2fd90dbf985700/chrome/browser/ui/ash/accelerator_commands_browsertest.cc
[modify] https://crrev.com/9913eed0106818160f3828ae7c2fd90dbf985700/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc
[modify] https://crrev.com/9913eed0106818160f3828ae7c2fd90dbf985700/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc
[modify] https://crrev.com/9913eed0106818160f3828ae7c2fd90dbf985700/components/exo/shell_surface.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Aug 16 2016

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

commit 299ee4033eba6c42845db57d106dc3eb8deeda7e
Author: avi <avi@chromium.org>
Date: Tue Aug 16 20:16:11 2016

Revert of Use MD-ash's auto hide behavior for arc++ windows. (patchset #6 id:100001 of https://codereview.chromium.org/2248773002/ )

Reason for revert:
Breaks every Linux ChromiumOS bot:

FAILED: obj/components/exo/exo/shell_surface.o
/b/c/cipd/goma/gomacc ../../third_party/llvm-build/Release+Asserts/bin/clang++ -MMD -MF obj/components/exo/exo/shell_surface.o.d -DV8_DEPRECATION_WARNINGS -DENABLE_MDNS=1 -DENABLE_NOTIFICATIONS -DENABLE_PEPPER_CDMS -DENABLE_PLUGINS=1 -DENABLE_PDF=1 -DENABLE_PRINTING=1 -DENABLE_BASIC_PRINTING=1 -DENABLE_PRINT_PREVIEW=1 -DENABLE_SPELLCHECK=1 -DUSE_UDEV -DUI_COMPOSITOR_IMAGE_TRANSPORT -DUSE_ASH=1 -DUSE_AURA=1 -DUSE_DEFAULT_RENDER_THEME=1 -DUSE_NSS_CERTS=1 -DUSE_OZONE=1 -DENABLE_WEBRTC=1 -DENABLE_EXTENSIONS=1 -DENABLE_TASK_MANAGER=1 -DENABLE_THEMES=1 -DENABLE_CAPTIVE_PORTAL_DETECTION=1 -DENABLE_SESSION_SERVICE=1 -DENABLE_APP_LIST=1 -DENABLE_SUPERVISED_USERS=1 -DENABLE_SERVICE_DISCOVERY=1 -DIMAGE_LOADER_EXTENSION=1 -DENABLE_WAYLAND_SERVER=1 -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL -DCHROMIUM_BUILD -DENABLE_MEDIA_ROUTER=1 -DFIELDTRIAL_TESTING_ENABLED -DCR_CLANG_REVISION=277962-1 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -DOS_CHROMEOS -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DUSE_EGL -DTOOLKIT_VIEWS=1 -DSK_IGNORE_DW_GRAY_FIX -DSK_IGNORE_LINEONLY_AA_CONVEX_PATH_OPTS -DSK_SUPPORT_GPU=1 -DENABLE_IPC_FUZZER -DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0 -DU_NOEXCEPT= -DU_STATIC_IMPLEMENTATION -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -DMESA_EGL_NO_X11_HEADERS -I../.. -Igen -I../../third_party/khronos -I../../gpu -Igen/ash/common/strings -Igen/ash/resources -I../../skia/config -I../../skia/ext -I../../third_party/skia/include/c -I../../third_party/skia/include/config -I../../third_party/skia/include/core -I../../third_party/skia/include/effects -I../../third_party/skia/include/images -I../../third_party/skia/include/lazy -I../../third_party/skia/include/pathops -I../../third_party/skia/include/pdf -I../../third_party/skia/include/pipe -I../../third_party/skia/include/ports -I../../third_party/skia/include/utils -I../../third_party/skia/include/gpu -I../../third_party/skia/src/gpu -I../../third_party/icu/source/common -I../../third_party/icu/source/i18n -I../../third_party/mesa/src/include -Igen -Igen/ui/views/resources -fno-strict-aliasing -funwind-tables -fPIC -pipe -B../../third_party/binutils/Linux_x64/Release/bin -fcolor-diagnostics -fdebug-prefix-map=/mnt/data/b/c/b/linux_chromium_gn_chromeos/src=. -pthread -m64 -march=x86-64 -Wall -Werror -Wextra -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default -Wno-deprecated-register -Wno-unneeded-internal-declaration -Wno-inconsistent-missing-override -Wno-shift-negative-value -Wno-undefined-var-template -Wno-nonportable-include-path -O2 -fno-ident -fdata-sections -ffunction-sections -g0 -fvisibility=hidden -Xclang -load -Xclang ../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.so -Xclang -add-plugin -Xclang find-bad-constructs -Xclang -plugin-arg-find-bad-constructs -Xclang check-templates -Xclang -plugin-arg-find-bad-constructs -Xclang follow-macro-expansion -Xclang -plugin-arg-find-bad-constructs -Xclang check-implicit-copy-ctors -Xclang -plugin-arg-find-bad-constructs -Xclang check-ipc -Wheader-hygiene -Wstring-conversion -fno-threadsafe-statics -fvisibility-inlines-hidden -std=gnu++11 -Wno-reserved-user-defined-literal -fno-rtti -fno-exceptions -c ../../components/exo/shell_surface.cc -o obj/components/exo/exo/shell_surface.o
../../components/exo/shell_surface.cc:57:8: error: no member named 'Shell' in namespace 'ash'
  ash::Shell::GetInstance()->UpdateShelfVisibility();
  ~~~~~^
1 error generated.

Original issue's description:
> Use MD-ash's auto hide behavior for arc++ windows.
>
> This add a runtime flag to enable MD-ash's auto hide behavior , which makes the shelf invisible, for arc++ windows. I'll remove this mode once MD ash is launched.
>
> BUG= 634566 
> TEST=covered by unit test. Manually tested on minnie.
>
> Committed: https://crrev.com/9913eed0106818160f3828ae7c2fd90dbf985700
> Cr-Commit-Position: refs/heads/master@{#412309}

TBR=reveman@chromium.org,tdanderson@chromium.org,sky@chromium.org,oshima@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 634566 

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

[modify] https://crrev.com/299ee4033eba6c42845db57d106dc3eb8deeda7e/ash/common/wm/window_state.cc
[modify] https://crrev.com/299ee4033eba6c42845db57d106dc3eb8deeda7e/ash/common/wm/window_state.h
[modify] https://crrev.com/299ee4033eba6c42845db57d106dc3eb8deeda7e/ash/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/299ee4033eba6c42845db57d106dc3eb8deeda7e/ash/shelf/shelf_layout_manager.h
[modify] https://crrev.com/299ee4033eba6c42845db57d106dc3eb8deeda7e/ash/shelf/shelf_layout_manager_unittest.cc
[modify] https://crrev.com/299ee4033eba6c42845db57d106dc3eb8deeda7e/ash/system/web_notification/web_notification_tray_unittest.cc
[modify] https://crrev.com/299ee4033eba6c42845db57d106dc3eb8deeda7e/ash/wm/immersive_fullscreen_controller.cc
[modify] https://crrev.com/299ee4033eba6c42845db57d106dc3eb8deeda7e/chrome/browser/chromeos/login/lock/screen_locker.cc
[modify] https://crrev.com/299ee4033eba6c42845db57d106dc3eb8deeda7e/chrome/browser/chromeos/login/lock/screen_locker_browsertest.cc
[modify] https://crrev.com/299ee4033eba6c42845db57d106dc3eb8deeda7e/chrome/browser/notifications/fullscreen_notification_blocker.cc
[modify] https://crrev.com/299ee4033eba6c42845db57d106dc3eb8deeda7e/chrome/browser/ui/ash/accelerator_commands_browsertest.cc
[modify] https://crrev.com/299ee4033eba6c42845db57d106dc3eb8deeda7e/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc
[modify] https://crrev.com/299ee4033eba6c42845db57d106dc3eb8deeda7e/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc
[modify] https://crrev.com/299ee4033eba6c42845db57d106dc3eb8deeda7e/components/exo/shell_surface.cc

Project Member

Comment 14 by bugdroid1@chromium.org, Aug 16 2016

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

commit 41fe96e27f8964dc1c6d9b4e7ffe3ad741b357ef
Author: oshima <oshima@chromium.org>
Date: Tue Aug 16 22:09:13 2016

Use MD-ash's auto hide behavior for arc++ windows.

This add a runtime flag to enable MD-ash's auto hide behavior , which makes the shelf invisible, for arc++ windows. I'll remove this mode once MD ash is launched.

BUG= 634566 
TEST=covered by unit test. Manually tested on minnie.

Committed: https://crrev.com/9913eed0106818160f3828ae7c2fd90dbf985700
Review-Url: https://codereview.chromium.org/2248773002
Cr-Original-Commit-Position: refs/heads/master@{#412309}
Cr-Commit-Position: refs/heads/master@{#412352}

[modify] https://crrev.com/41fe96e27f8964dc1c6d9b4e7ffe3ad741b357ef/ash/common/wm/window_state.cc
[modify] https://crrev.com/41fe96e27f8964dc1c6d9b4e7ffe3ad741b357ef/ash/common/wm/window_state.h
[modify] https://crrev.com/41fe96e27f8964dc1c6d9b4e7ffe3ad741b357ef/ash/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/41fe96e27f8964dc1c6d9b4e7ffe3ad741b357ef/ash/shelf/shelf_layout_manager.h
[modify] https://crrev.com/41fe96e27f8964dc1c6d9b4e7ffe3ad741b357ef/ash/shelf/shelf_layout_manager_unittest.cc
[modify] https://crrev.com/41fe96e27f8964dc1c6d9b4e7ffe3ad741b357ef/ash/system/web_notification/web_notification_tray_unittest.cc
[modify] https://crrev.com/41fe96e27f8964dc1c6d9b4e7ffe3ad741b357ef/ash/wm/immersive_fullscreen_controller.cc
[modify] https://crrev.com/41fe96e27f8964dc1c6d9b4e7ffe3ad741b357ef/chrome/browser/chromeos/login/lock/screen_locker.cc
[modify] https://crrev.com/41fe96e27f8964dc1c6d9b4e7ffe3ad741b357ef/chrome/browser/chromeos/login/lock/screen_locker_browsertest.cc
[modify] https://crrev.com/41fe96e27f8964dc1c6d9b4e7ffe3ad741b357ef/chrome/browser/notifications/fullscreen_notification_blocker.cc
[modify] https://crrev.com/41fe96e27f8964dc1c6d9b4e7ffe3ad741b357ef/chrome/browser/ui/ash/accelerator_commands_browsertest.cc
[modify] https://crrev.com/41fe96e27f8964dc1c6d9b4e7ffe3ad741b357ef/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc
[modify] https://crrev.com/41fe96e27f8964dc1c6d9b4e7ffe3ad741b357ef/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc
[modify] https://crrev.com/41fe96e27f8964dc1c6d9b4e7ffe3ad741b357ef/components/exo/shell_surface.cc

Summary: [ARC++] Unable to show shelf in immersive fullscreen. (was: ARC++: Unable to show shelf in immersive fullscreen.)
oshima@ let me know if we are ready to merge to M53 cros.
Yes, it's ready to merge. Tested on ToT.
Labels: Merge-Request-53

Comment 19 by dimu@chromium.org, Aug 18 2016

Labels: -Merge-Request-53 Merge-Approved-53 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M53 (branch: 2785)
Project Member

Comment 20 by bugdroid1@chromium.org, Aug 19 2016

Labels: -merge-approved-53 merge-merged-2785
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/09b361d92cba6a2e1b78998f74ccda1f0485de39

commit 09b361d92cba6a2e1b78998f74ccda1f0485de39
Author: Mitsuru Oshima <oshima@chromium.org>
Date: Fri Aug 19 17:37:47 2016

Use MD-ash's auto hide behavior for arc++ windows.

This add a runtime flag to enable MD-ash's auto hide behavior , which makes the shelf invisible, for arc++ windows. I'll remove this mode once MD ash is launched.

BUG= 634566 
TEST=covered by unit test. Manually tested on minnie.

Committed: https://crrev.com/9913eed0106818160f3828ae7c2fd90dbf985700
Review-Url: https://codereview.chromium.org/2248773002
Cr-Original-Commit-Position: refs/heads/master@{#412309}
Cr-Commit-Position: refs/heads/master@{#412352}
(cherry picked from commit 41fe96e27f8964dc1c6d9b4e7ffe3ad741b357ef)

Register the window in MaximizeModeWindowManager when the window is shown.

This is necessary for arc app to set the "don't manage me" flag.

BUG= 635899 

Review-Url: https://codereview.chromium.org/2250963006
Cr-Commit-Position: refs/heads/master@{#412882}
(cherry picked from commit 5ef3cfef16fad285e84d8655e592d522401b6ff8)

Review URL: https://codereview.chromium.org/2255313005 .

Cr-Commit-Position: refs/branch-heads/2785@{#680}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/09b361d92cba6a2e1b78998f74ccda1f0485de39/ash/common/wm/window_state.cc
[modify] https://crrev.com/09b361d92cba6a2e1b78998f74ccda1f0485de39/ash/common/wm/window_state.h
[modify] https://crrev.com/09b361d92cba6a2e1b78998f74ccda1f0485de39/ash/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/09b361d92cba6a2e1b78998f74ccda1f0485de39/ash/shelf/shelf_layout_manager.h
[modify] https://crrev.com/09b361d92cba6a2e1b78998f74ccda1f0485de39/ash/shelf/shelf_layout_manager_unittest.cc
[modify] https://crrev.com/09b361d92cba6a2e1b78998f74ccda1f0485de39/ash/system/web_notification/web_notification_tray_unittest.cc
[modify] https://crrev.com/09b361d92cba6a2e1b78998f74ccda1f0485de39/ash/wm/gestures/shelf_gesture_handler.cc
[modify] https://crrev.com/09b361d92cba6a2e1b78998f74ccda1f0485de39/ash/wm/immersive_fullscreen_controller.cc
[modify] https://crrev.com/09b361d92cba6a2e1b78998f74ccda1f0485de39/chrome/browser/chromeos/login/lock/screen_locker.cc
[modify] https://crrev.com/09b361d92cba6a2e1b78998f74ccda1f0485de39/chrome/browser/chromeos/login/lock/screen_locker_browsertest.cc
[modify] https://crrev.com/09b361d92cba6a2e1b78998f74ccda1f0485de39/chrome/browser/notifications/fullscreen_notification_blocker.cc
[modify] https://crrev.com/09b361d92cba6a2e1b78998f74ccda1f0485de39/chrome/browser/ui/ash/accelerator_commands_browsertest.cc
[modify] https://crrev.com/09b361d92cba6a2e1b78998f74ccda1f0485de39/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc
[modify] https://crrev.com/09b361d92cba6a2e1b78998f74ccda1f0485de39/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc
[modify] https://crrev.com/09b361d92cba6a2e1b78998f74ccda1f0485de39/components/exo/shell_surface.cc

Status: Fixed (was: Started)
Project Member

Comment 22 by bugdroid1@chromium.org, Aug 19 2016

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

commit 6002f379e9052cda4a3573391d79179c95d3c97e
Author: Mitsuru Oshima <oshima@chromium.org>
Date: Fri Aug 19 17:47:49 2016

Use MD-ash's auto hide behavior for arc++ windows.

This add a runtime flag to enable MD-ash's auto hide behavior , which makes the shelf invisible, for arc++ windows. I'll remove this mode once MD ash is launched.

BUG= 634566 
TEST=covered by unit test. Manually tested on minnie.

Committed: https://crrev.com/9913eed0106818160f3828ae7c2fd90dbf985700
Review-Url: https://codereview.chromium.org/2248773002
Cr-Original-Commit-Position: refs/heads/master@{#412309}
Cr-Commit-Position: refs/heads/master@{#412352}
(cherry picked from commit 41fe96e27f8964dc1c6d9b4e7ffe3ad741b357ef)

Register the window in MaximizeModeWindowManager when the window is shown.

This is necessary for arc app to set the "don't manage me" flag.

BUG= 635899 

Review-Url: https://codereview.chromium.org/2250963006
Cr-Commit-Position: refs/heads/master@{#412882}
(cherry picked from commit 5ef3cfef16fad285e84d8655e592d522401b6ff8)

Review URL: https://codereview.chromium.org/2255313005 .

Committed: https://chromium.googlesource.com/chromium/src/+/09b361d92cba6a2e1b78998f74ccda1f0485de39
Cr-Commit-Position: refs/branch-heads/2785@{#681}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/6002f379e9052cda4a3573391d79179c95d3c97e/components/exo/shell_surface.cc
[modify] https://crrev.com/6002f379e9052cda4a3573391d79179c95d3c97e/components/exo/shell_surface_unittest.cc

Status: Verified (was: Fixed)
Chrome OS version 53.0.2785.70/8530.62.0 beta cyan

Sign in to add a comment