New issue
Advanced search Search tips
Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 17
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Chrome crashes when running in mash on device

Project Member Reported by xiy...@chromium.org, Apr 16

Issue description

Chrome crashes when running in mash on device.
Looks like caused by 
https://chromium-review.googlesource.com/882191

Basically, the following line returns null on mash:
  aura::client::GetCursorClient(ash::Shell::GetPrimaryRootWindow())
and exo::Pointer code crashes when trying to add an observer.

e.g.

desktopui_MashLogin crashes:

Build: gandof-release/R68-10588.0.0.

Reason:
Unhandled BrowserGoneException: Connection is already closed..
build artifacts:
https://storage.cloud.google.com/?arg=chromeos-image-archive/gandof-release/R68-10588.0.0.
results log: http://ubercautotest.corp.google.com/tko/retrieve_logs.cgi?job=/results/192443834-chromeos-test/chromeos4-row5-rack6-host13/debug/.
 
Status: Assigned (was: Untriaged)
Can the mash checks be added back to that code? It looks like there were some checks that were removed.

It would be good to get the lab green so we can watch for new regressions.

Cc: derat@chromium.org bmgordon@google.com osh...@chromium.org
 Issue 796284  has been merged into this issue.
xiyuan, are you sure that's the right CL? The CL landed in January.

Re #3: Yes, pretty sure that is the root cause.

Think it is revealed now because sky's CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1005567

That turns on wayland server on mash (i.e. --ash-enable-wayland-server is gone and --enable-wayland-server would turn on wayland server for mash).
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 16

Labels: merge-merged-release-R67-10575.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/1c3c85a039e8345ed05e54078bba49f2a071f336

commit 1c3c85a039e8345ed05e54078bba49f2a071f336
Author: James Cook <jamescook@chromium.org>
Date: Mon Apr 16 23:27:21 2018

autotest: Disable desktopui_MashLogin on R67 branch

The test fails due to a known issue that has been fixed on
tip-of-trunk, so just disable the test on the branch to
reduce email spam from the lab.

BUG= chromium:833456 
TEST=desktopui_MashLogin
TBR=xiyuan@chromium.org

Change-Id: I0984b0e44ed6be31cf4ef27f78be9c1a6a6f02c3
Reviewed-on: https://chromium-review.googlesource.com/1014622
Reviewed-by: James Cook <jamescook@chromium.org>
Tested-by: James Cook <jamescook@chromium.org>

[modify] https://crrev.com/1c3c85a039e8345ed05e54078bba49f2a071f336/client/site_tests/desktopui_MashLogin/control

What's the right way to access cursor client/manager in mash?
Cc: sky@chromium.org
Components: Internals>Services>Ash
+sky, I don't think we have a good cursor client/manager solution for mash. erg@ worked on cursors before he left. I think it was an intentional design decision that ash::Shell::cursor_manager() was null, because there was no "global" cursor manager. I suspect cursor management is per-top-level-window, but I'm not sure what the intended replacement was.

Maybe just skip the code for now and leave a TODO for mash?

I have a question:  the CL in comment 5 said "The test fails due to a known issue that has been fixed on tip-of-trunk"

Which CL has fixed that null pointer access issue? I checked history of pointer.cc it seems the latest commit is still 3 days aog:

https://chromium.googlesource.com/chromium/src.git/+log/master/components/exo/pointer.cc
Ok, let me do that or now, since other places does seem to do the same.
Status: Started (was: Assigned)
Project Member

Comment 11 by bugdroid1@chromium.org, Apr 17

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

commit bf470281c992fae0012930e6e38121d6884a65cd
Author: Mitsuru Oshima <oshima@chromium.org>
Date: Tue Apr 17 09:00:57 2018

Skip cursor client operation in mash.

BUG= 833456 
TEST=none

Change-Id: I9d01f94670a406ea7b30b4c232775dc76ad4a48c
Reviewed-on: https://chromium-review.googlesource.com/1013272
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551282}
[modify] https://crrev.com/bf470281c992fae0012930e6e38121d6884a65cd/components/exo/pointer.cc

@lepton - The commit comment in comment 5 was wrong, sorry. I copy/pasted it from the last time I disabled this test on a branch. It looks like oshima just fixed the crash.
Status: Fixed (was: Started)
Cc: jdufault@chromium.org lepton@chromium.org
 Issue 834321  has been merged into this issue.

Sign in to add a comment