I can type at the login screen in --mus and --mash for (link, R63-9972.0.0) but --mash seems to crash on login. Trying to figure out why.

It seems as though chrome is exiting, not crashing. It may be related to 771801.

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

commit 5aff5c5eb953293d64d1f217ee384cc709661279
Author: Scott Violet <sky@chromium.org>
Date: Thu Oct 05 22:41:48 2017

chromeos: changes AuraInit to conditionally init MaterialDesignController

Doing this means we can have the utility process load the common
resources, which means we don't have to worry about copying resources
from the services to devices.
Additionally adds the service name to the command line so that it's
easier to tell what service a process is running.

BUG= 771980 
TEST=none

Change-Id: Ib9219471e4d758157b515d0b27743e996f9fa6e2
Reviewed-on: https://chromium-review.googlesource.com/703736
Reviewed-by: Elliot Glaysher <erg@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506892}
[modify] https://crrev.com/5aff5c5eb953293d64d1f217ee384cc709661279/chrome/app/chrome_main_delegate.cc
[modify] https://crrev.com/5aff5c5eb953293d64d1f217ee384cc709661279/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/5aff5c5eb953293d64d1f217ee384cc709661279/chrome/common/chrome_switches.cc
[modify] https://crrev.com/5aff5c5eb953293d64d1f217ee384cc709661279/chrome/common/chrome_switches.h
[modify] https://crrev.com/5aff5c5eb953293d64d1f217ee384cc709661279/ui/views/mus/aura_init.cc

Peng, could you try again on tip of tree? I fixed the issues I was seeing, but it's entirely possible you are seeing something else given I'm using a pixel, which doesn't include android.

I just synced and tested.
--mus is fine. For --mash, the login screen works now, and the cursor is not a yellow box anymore. But after login, the chrome crashes.

The crash stack is:

[1:10:1006/094350.424549:ERROR:layer_tree_host_impl.cc(2471)] Forcing zero-copy tile initialization as worker context is missing
[6810:6810:1006/094350.392232:FATAL:arc_app_shelf_id.cc(27)] Check failed: crx_file::id_util::IdIsValid(app_id).
#0 0x615d857b898c base::debug::StackTrace::StackTrace()
#1 0x615d857d44ce logging::LogMessage::~LogMessage()
#2 0x615d88083ce5 arc::ArcAppShelfId::FromString()
#3 0x615d8807af94 arc::IsArcItem()
#4 0x615d87f45c0c ChromeLauncherController::ShelfItemAdded()
#5 0x615d87d50c53 ash::ShelfModel::AddAt()
#6 0x615d87f44fe0 ChromeLauncherController::OnShelfItemAdded()
#7 0x615d8413cb21 ash::mojom::ShelfObserverStubDispatch::Accept()
#8 0x615d86604674 mojo::InterfaceEndpointClient::HandleValidatedMessage()
#9 0x615d86604246 mojo::FilterChain::Accept()
#10 0x615d86605b22 mojo::InterfaceEndpointClient::HandleIncomingMessage()
#11 0x615d8660f284 mojo::internal::MultiplexRouter::ProcessIncomingMessage()
#12 0x615d8660e759 mojo::internal::MultiplexRouter::Accept()
#13 0x615d86604246 mojo::FilterChain::Accept()
#14 0x615d8660333d mojo::Connector::ReadSingleMessage()
#15 0x615d86603dd4 mojo::Connector::ReadAllAvailableMessages()
#16 0x615d86603c36 mojo::Connector::OnHandleReadyInternal()
#17 0x615d837c2912 mojo::SimpleWatcher::DiscardReadyState()
#18 0x615d865fd23a mojo::SimpleWatcher::OnHandleReady()
#19 0x615d865fd740 _ZN4base8internal7InvokerINS0_9BindStateIMN4mojo13SimpleWatcherEFvijRKNS3_18HandleSignalsStateEEJNS_7WeakPtrIS4_EEijS5_EEEFvvEE7RunImplIRKS9_RKSt5tupleIJSB_ijS5_EEJLm0ELm1ELm2ELm3EEEEvOT_OT0_St16integer_sequenceImJXspT1_EEE
#20 0x615d857b9189 base::debug::TaskAnnotator::RunTask()
#21 0x615d85877d26 base::internal::IncomingTaskQueue::RunTask()
#22 0x615d857dcb4e base::MessageLoop::RunTask()
#23 0x615d857dd2d9 base::MessageLoop::DoWork()
#24 0x615d857dfc59 base::MessagePumpLibevent::Run()
#25 0x615d857dc59e base::MessageLoop::Run()
#26 0x615d85809e4c base::RunLoop::Run()
#27 0x615d853fbfa7 ChromeBrowserMainParts::MainMessageLoopRun()
#28 0x615d8381b5c4 content::BrowserMainLoop::RunMainMessageLoopParts()
#29 0x615d8381e283 content::BrowserMainRunnerImpl::Run()
#30 0x615d838168dc content::BrowserMain()
#31 0x615d853e395f content::RunNamedProcessTypeMain()
#32 0x615d853e433e content::ContentMainRunnerImpl::Run()
#33 0x615d853f04fd service_manager::Main()
#34 0x615d853e3001 content::ContentMain()
#35 0x615d82ed7b48 ChromeMain
#36 0x7917804d9736 __libc_start_main
#37 0x615d82ed7929 _start

+msw for shelf related crash

It'd be nice to know what id is failing, but I'll try to make a speculative fix.
(wfh without physical access to my cros device setup for debugging)

I have a speculative fix out for review, could anyone help with testing it?
https://chromium-review.googlesource.com/#/c/chromium/src/+/705264

I bet it's ash::ShelfWindowWatcher non-crx_file ids for non-Chrome items:
https://cs.chromium.org/chromium/src/ash/shelf/shelf_window_watcher.cc?l=50
(mash's QuickLaunch shelf item is synced from ash to chrome, w/o crx_file use)
Maybe ash should use components/crx_file for non-Chrome app windows?

I'm landing the arc_app_utils.cc workaround for now, can followup as needed.

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

commit f4bb53188b2c773c46397064f9f767a9ade132ab
Author: Mike Wasserman <msw@chromium.org>
Date: Fri Oct 06 19:03:19 2017

mash: Speculative fix for IsArcItem crash.

Check if the Id is a valid crx id before creating an ArcAppShelfId.
(the ArcAppShelfId ctor DCHECKs that the id is a valid crx id)

Bug:  771980 
Test: No crash logging in to a device with --mus and --mash.
Change-Id: Ifd006fccf88349f5b13d9b8bae174293422d36c7
Reviewed-on: https://chromium-review.googlesource.com/705264
Commit-Queue: Michael Wasserman <msw@chromium.org>
Reviewed-by: Yury Khmel <khmel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507134}
[modify] https://crrev.com/f4bb53188b2c773c46397064f9f767a9ade132ab/chrome/browser/ui/app_list/arc/arc_app_utils.cc

Should be fixed? Please help verify, thanks.

Can we follow up on this?

It looks the workaround gets reverted:

https://chromium-review.googlesource.com/c/chromium/src/+/822768

And I am getting this error when trying to add browser_test

[188295:188295:0226/134130.894374:ERROR:arc_app_shelf_id.cc(27)] ShelfWindowWatcher0
[188295:188295:0226/134130.894462:FATAL:arc_app_shelf_id.cc(28)] Check failed: crx_file::id_util::IdIsValid(app_id). 

What if we change:

const ash::ShelfID shelf_id("ShelfWindowWatcher" + std::to_string(id++));
to
const ash::ShelfID shelf_id(crx_file::id_util::GenerateId("ShelfWindowWatcher" + std::to_string(id++)))
?

Why is it important that shelf ids be valid crx ids?
Is there precedent for making this a fatal check elsewhere?

ShelfId easy come to several services, including arc code, app list sync services and others. This part can potentially passed to server via sync. This is standard that extension id, arc app id follows this structure. That is why we have several DCHECK. I remember we already fixed few issues from unit test last summer (I remember you was reviewer).
You also may grep for "crx_file::id_util::GenerateId(" you would see this is common pattern.

I recall fixing test dchecks with const pre-generated ids, but this is different.
I'm not sure that src/ash can (or should) take a dependency on components/crx_file.
Also, we shouldn't need to persist ShelfWindowWatcher's placeholder ids at all.

So, is it really necessary to use crx ids for shelf app ids, or just customary?
Can we just bail earlier in arc code (without a dcheck) if the id isn't valid crx?
What really fails if we remove the arc_app_shelf_id.cc dcheck?

I don't expect if something fails if we change DCHECK to if (). However not 100% sure.

I started investigating, but this seems to be blocked by another DCHECK crash,  issue 817624 .
I'll try to work around that to investigate here.

Here's a repro on Chrome OS debug ToT@#539689 (66.0.3350.3) (plus commenting out WeakReference::Flag::IsValid's DCHECK) on cyan:

(1) Run a debug build of chrome on a device with Arc++ ability (eg. cyan)
(2) Modify chrome_dev.conf to include --enable-features=Mash
(3a) Press the "mash" button in the shelf's status area (launches the QuickLaunch mojo app)
(3b) Open the Task manager (search+esc or 3-dot menu -> more tools -> task manager)

[21138:21138:0228/164352.107010:ERROR:arc_app_shelf_id.cc(28)] MSW INVALID CRX ID... ShelfWindowWatcher0
#0 0x6174bde28a68 base::debug::StackTrace::StackTrace()
#1 0x6174bde26f8c base::debug::StackTrace::StackTrace()
#2 0x6174c6857441 arc::ArcAppShelfId::ArcAppShelfId()
#3 0x6174c6857a20 arc::ArcAppShelfId::FromString()
#4 0x6174c6836d4f arc::IsArcItem()
#5 0x6174c67fae04 ArcAppIconLoader::CanLoadImageForApp()
#6 0x6174c63e72bf ChromeLauncherController::GetAppIconLoaderForApp()
#7 0x6174c63f32cd ChromeLauncherController::ShelfItemAdded()
#8 0x6174c5bdea27 ash::ShelfModel::AddAt()
#9 0x6174c63f195b ChromeLauncherController::OnShelfItemAdded()
#10 0x6174ba041917 ash::mojom::ShelfObserverStubDispatch::Accept()

That's the only callstack I see having an issue with the non-crx app id.
I think we should generally try to support non-crx shelf app ids.
I'm inclined to pursue a solution of checking for a valid crx id in IsArcItem before attempting to create an ArcAppShelfId.
I'm unsure whether or not it makes sense to keep the ArcAppShelfId DCHECK, we might just hit this issue again later if we do.

The exact check I would add existed previously, but it was removed here:
  https://chromium-review.googlesource.com/c/chromium/src/+/822768/3/chrome/browser/ui/app_list/arc/arc_app_utils.cc#b399
Yury, can you clarify your comment there about "complex shelf app id"? Is there a reason we shouldn't just restore that check?

#27 - It can be in complex form: https://cs.chromium.org/chromium/src/chrome/browser/ui/ash/launcher/arc_app_shelf_id.cc?sq=package:chromium&l=35

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

commit 32f92ecad974162be03b26db9e0ff708133f669e
Author: Mike Wasserman <msw@chromium.org>
Date: Sat Mar 03 06:59:23 2018

Actually check for valid ARC app shelf ids before DCHECKing validity.

Bug:  771980 
Change-Id: I600e523b71c8c4261c47b12a477cc2d394a222bf
Reviewed-on: https://chromium-review.googlesource.com/944715
Commit-Queue: Michael Wasserman <msw@chromium.org>
Reviewed-by: Yury Khmel <khmel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540742}
[modify] https://crrev.com/32f92ecad974162be03b26db9e0ff708133f669e/chrome/browser/ui/app_list/arc/arc_app_utils.cc
[modify] https://crrev.com/32f92ecad974162be03b26db9e0ff708133f669e/chrome/browser/ui/app_list/arc/arc_app_utils_unittest.cc
[modify] https://crrev.com/32f92ecad974162be03b26db9e0ff708133f669e/chrome/browser/ui/ash/launcher/arc_app_shelf_id.cc
[modify] https://crrev.com/32f92ecad974162be03b26db9e0ff708133f669e/chrome/browser/ui/ash/launcher/arc_app_shelf_id.h
[modify] https://crrev.com/32f92ecad974162be03b26db9e0ff708133f669e/chrome/browser/ui/ash/launcher/arc_app_shelf_id_unittest.cc