New issue
Advanced search Search tips

Issue 911838 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

Crash in window_tree_client.cc:OnChangeCompleted : ChangeFailed

Project Member Reported by steve...@chromium.org, Dec 4

Issue description

From a recent Chrome ToT build @ #613608

Running on kevin with --enable-features=SingleProcessMash

1. Attach an external monitor to a device (in this case, kevin)
2. Enable a11y in the system menu (Settings > Advanced > Accessibility > Always show a11y options...)
3. In the system menu, enable Accessibility > On-screen keyboard
4. Disable the on-screen keyboard (you may need to enable / disable a second time).

Crash:

#0  ChangeFailed () at ../../ui/aura/mus/in_flight_change.cc:148
#1  0x0f13df10 in OnChangeCompleted () at ../../ui/aura/mus/window_tree_client.cc:1486
#2  0x0bad0bfc in Accept () at gen/services/ws/public/mojom/window_tree.mojom.cc:9137
#3  0x0e14f9ae in HandleValidatedMessage () at ../../mojo/public/cpp/bindings/lib/interface_endpoint_client.cc:423
#4  0x0e15be0c in Accept () at ../../mojo/public/cpp/bindings/lib/filter_chain.cc:40
#5  0x0e15081a in HandleIncomingMessage () at ../../mojo/public/cpp/bindings/lib/interface_endpoint_client.cc:306
#6  0x0e154a2e in ProcessIncomingMessage () at ../../mojo/public/cpp/bindings/lib/multiplex_router.cc:869
#7  0x0e1542d0 in Accept () at ../../mojo/public/cpp/bindings/lib/multiplex_router.cc:590
#8  0x0e15be0c in Accept () at ../../mojo/public/cpp/bindings/lib/filter_chain.cc:40
#9  0x0e14dc12 in ReadSingleMessage () at ../../mojo/public/cpp/bindings/lib/connector.cc:476
#10 0x0e14e32c in ReadAllAvailableMessages () at ../../mojo/public/cpp/bindings/lib/connector.cc:505
#11 0x0e14e1ea in OnHandleReadyInternal () at ../../mojo/public/cpp/bindings/lib/connector.cc:387
#12 0x0b9a8fa4 in Run () at ../../base/callback.h:129
#13 DiscardReadyState () at ../../mojo/public/cpp/system/simple_watcher.h:194
#14 0x0e1648e0 in Run () at ../../base/callback.h:129
#15 OnHandleReady () at ../../mojo/public/cpp/system/simple_watcher.cc:273
#16 0x0e164c68 in Invoke<void (mojo::SimpleWatcher::*)(int, unsigned int, mojo::HandleSignalsState const&), base::WeakPtr<mojo::SimpleWatcher> const&, int const&, unsigned int const&, mojo::HandleSignalsState const&> () at ../../base/bind_internal.h:516
#17 MakeItSo<void (mojo::SimpleWatcher::* const&)(int, unsigned int, mojo::HandleSignalsState const&), base::WeakPtr<mojo::SimpleWatcher> const&, int const&, unsigned int const&, mojo::HandleSignalsState const&> () at ../../base/bind_internal.h:636
#18 RunImpl<void (mojo::SimpleWatcher::* const&)(int, unsigned int, mojo::HandleSignalsState const&), std::__1::tuple<base::WeakPtr<mojo::SimpleWatcher>, int, unsigned int, mojo::HandleSignalsState> const&, 0, 1, 2, 3> () at ../../base/bind_internal.h:689
#19 0x0e10ddc4 in Run () at ../../base/callback.h:99
#20 RunTask () at ../../base/debug/task_annotator.cc:99
#21 0x0e0870b2 in RunTask () at ../../base/message_loop/message_loop_impl.cc:374
#22 0x0e0874f6 in DeferOrRunPendingTask () at ../../base/message_loop/message_loop_impl.cc:385
#23 DoWork () at ../../base/message_loop/message_loop_impl.cc:473
#24 0x0e10b418 in Run () at ../../base/message_loop/message_pump_libevent.cc:210
#25 0x0e086d30 in Run () at ../../base/message_loop/message_loop_impl.cc:326
#26 0x0e0a3b62 in Run () at ../../base/run_loop.cc:102
#27 0x0dcf08d0 in MainMessageLoopRun () at ../../chrome/browser/chrome_browser_main.cc:1865

Changing the DVLOG to VLOG and enabling it I get:

VERBOSE1:window_tree_client.cc(1484)] Change failed id=331 type=DELETE_WINDOW

I suspect the Ash keyboard window or the embedded keyboard window may be the deleted window, but it's not clear to me what failed or why this is triggering a crash.

 
The delete change should be create in WindowTreeClient::OnWindowMusDestroyed [1]. On WS side, it calls into WindowTree::DeleteWindowImpl [2] and somehow it returns false. Can you see which "return false" is hit?

I kinda suspect that this maybe a race that ash and the browser saide attempts to delete the same window (on-screen keyboard content window?).

[1] https://cs.chromium.org/chromium/src/ui/aura/mus/window_tree_client.cc?rcl=9dba130c11a2b689aab97e55212b0a6c13c2510e&l=739

[2] https://cs.chromium.org/chromium/src/services/ws/window_tree.cc?rcl=3a6cc7004b9bfe1bf8b23dfda95f846c949eaef4&l=771
Owner: sky@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 5

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

commit ce56c20ccae2edfcfc4a72f17cead8c8d9729ad9
Author: Scott Violet <sky@chromium.org>
Date: Wed Dec 05 20:52:09 2018

chromeos: return true on deleting window and no window is found

This avoids a race where both the client and server try to delete the window
at the same time. Another option is to put this logic in the client (to not
crash), but I'm worried that some shutdown paths in ash may trigger deletion
of other windows.

BUG= 911838 
TEST=none

Change-Id: I41c9e8a1a89e981deca715000cdc4f1974d53b1e
Reviewed-on: https://chromium-review.googlesource.com/c/1363851
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614092}
[modify] https://crrev.com/ce56c20ccae2edfcfc4a72f17cead8c8d9729ad9/services/ws/window_tree.cc
[modify] https://crrev.com/ce56c20ccae2edfcfc4a72f17cead8c8d9729ad9/services/ws/window_tree_client_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment