New issue
Advanced search Search tips

Issue 760811 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocked on:
issue 890969



Sign in to add a comment

mash: Support tablet mode

Project Member Reported by jamescook@chromium.org, Aug 31 2017

Issue description

There's a bunch of code in chrome that uses ash::Shell::Get()->tablet_mode_controller(). This won't work with go/mustash.

Mostly it is checking to see if tablet mode is enabled or not. We ought to be able to store that in chrome somewhere, maybe with a mojom::TabletModeObserver that caches the state.

(The most recent example is a chrome --mash crash in ImmersiveModeControllerAsh when opening the settings window. But there are lots of places that check for tablet mode.)

Thoughts?

 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 31 2017

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

commit bd4a95ee10a03c861d3c4a7776a10deadd59e5bd
Author: James Cook <jamescook@chromium.org>
Date: Thu Aug 31 14:07:46 2017

cros: Fix crash when opening settings window under mash

The immersive mode window frame code was accessing ash::Shell to get the
tablet mode state.

Bug: 760811
Test: run chrome --mash, open settings via status tray menu
Change-Id: I663b5c3c9f524f478b7d033076b44ea72ab677e8
Reviewed-on: https://chromium-review.googlesource.com/644727
Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498855}
[modify] https://crrev.com/bd4a95ee10a03c861d3c4a7776a10deadd59e5bd/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc

Components: -Internals>MUS Internals>Services>WindowService
Components: -Internals>Services>WindowService Internals>Services>Ash
Labels: -Proj-Mustash-Mash
Owner: est...@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 2

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

commit d689b3355eccb93a97a70c00ea37abf24388951b
Author: Evan Stade <estade@chromium.org>
Date: Thu Aug 02 01:17:22 2018

OopAsh: make browser frame respond to changes in tablet mode.

Tested via --ash-debug-shortcuts and Ctrl+Alt+Shift+T

Bug:  854704 ,760811
Change-Id: Ia3aadae89fcd50d95a84fdf7d275e97bc2e9e2e9
Reviewed-on: https://chromium-review.googlesource.com/1152230
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580021}
[modify] https://crrev.com/d689b3355eccb93a97a70c00ea37abf24388951b/ash/frame/custom_frame_view_ash.cc
[modify] https://crrev.com/d689b3355eccb93a97a70c00ea37abf24388951b/ash/public/interfaces/shell_test_api.mojom
[modify] https://crrev.com/d689b3355eccb93a97a70c00ea37abf24388951b/ash/shell_test_api.cc
[modify] https://crrev.com/d689b3355eccb93a97a70c00ea37abf24388951b/ash/shell_test_api.h
[modify] https://crrev.com/d689b3355eccb93a97a70c00ea37abf24388951b/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
[modify] https://crrev.com/d689b3355eccb93a97a70c00ea37abf24388951b/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc
[modify] https://crrev.com/d689b3355eccb93a97a70c00ea37abf24388951b/chrome/browser/ui/views/frame/immersive_mode_controller_ash_browsertest.cc
[modify] https://crrev.com/d689b3355eccb93a97a70c00ea37abf24388951b/testing/buildbot/filters/mash.browser_tests.filter

Labels: Proj-Mustash
Labels: -Proj-Mustash Proj-Mash-SingleProcess
I'm tagging this with single-process mash mode. I suspect we don't necessarily need everything for single-process mash, but that needs to be evaluated.
I believe all non-test references are already fixed, and the test references are only in non-mash tests.

DisplayInfoProviderChromeosTest uses Shell::tablet_mode_controller, but has bigger issues in single process Mash (two InputDeviceManagers on the same thread).
Blockedon: 890969
Evan, Is there still work to do here for single-process-mash?
I think we're done for single process mash, and even for oopash I think we only need to update a test or two.
Labels: -Proj-Mash-SingleProcess Proj-Mash-MultiProcess
Excellent! I'm going to retarget for multi-process then.

Sign in to add a comment