New issue
Advanced search Search tips

Issue 652421 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Aug 15
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

mash: mus crash clicking shelf item to activate

Project Member Reported by msw@chromium.org, Oct 3 2016

Issue description

mash: mus crash clicking shelf item to activate

(1) Run chrome --mash on linux desktop (the Chrome window should be active/foreground)
(2) Click the "QuickLaunch" shelf item (or between that and "Chromium - New Tab").
(3) Crash in_flight_change.cc: changed failed, type=15 (ui::ChangeType::REORDER)

[20706:20706:1003/130332:1725799198204:ERROR:in_flight_change.cc(65)] changed failed, type=15
[20706:20706:1003/130332:1725799198272:FATAL:in_flight_change.cc(66)] Check failed: false. 
#0 0x7fa99c66b9be base::debug::StackTrace::StackTrace()
#1 0x7fa99c6d23cc logging::LogMessage::~LogMessage()
#2 0x7fa99f1596f2 ui::CrashInFlightChange::ChangeFailed()
#3 0x7fa99f17f6a1 ui::WindowTreeClient::OnChangeCompleted()
#4 0x7fa99f0eb2f3 ui::mojom::WindowTreeClientStub::Accept()
#5 0x7fa99ce1f172 mojo::InterfaceEndpointClient::HandleValidatedMessage()
#6 0x7fa99ce1eb61 mojo::InterfaceEndpointClient::HandleIncomingMessageThunk::Accept()
#7 0x7fa99ce1c97a mojo::FilterChain::Accept()
#8 0x7fa99ce20a16 mojo::InterfaceEndpointClient::HandleIncomingMessage()
#9 0x7fa99ce315b9 mojo::internal::MultiplexRouter::ProcessIncomingMessage()
#10 0x7fa99ce30d74 mojo::internal::MultiplexRouter::Accept()
#11 0x7fa99ce1c97a mojo::FilterChain::Accept()
#12 0x7fa99ce120a5 mojo::Connector::ReadSingleMessage()
#13 0x7fa99ce12bc9 mojo::Connector::ReadAllAvailableMessages()
#14 0x7fa99ce12a51 mojo::Connector::OnHandleReadyInternal()
#15 0x7fa99ce1292b mojo::Connector::OnWatcherHandleReady()

In this case:
  ash::ShelfWindowWatcherItemDelegate::ItemSelected() calls 
  ash::WmWindowMus::Activate(), which calls:
  ui::Window::MoveToFront(), which calls:
  ui::Window::Reorder(), which calls:
  ui::WindowTreeClient::Reorder, which calls (via mojom):
  ui::ws::WindowTree::ReorderWindow, which returns failure because:
  ui::ws::WindowTree::CanReorderWindow returns false in this block:

bool WindowTree::CanReorderWindow(const ServerWindow* window,
                                  const ServerWindow* relative_window,
                                  mojom::OrderDirection direction) const {

... [passes earlier checks] ...

  const ServerWindow::Windows& children = window->parent()->children();
  const size_t child_i =
      std::find(children.begin(), children.end(), window) - children.begin();
  const size_t target_i =
      std::find(children.begin(), children.end(), relative_window) -
      children.begin();
  if ((direction == mojom::OrderDirection::ABOVE && child_i == target_i + 1) ||
      (direction == mojom::OrderDirection::BELOW && child_i + 1 == target_i)) {
    return false; << RETURNS FALSE HERE TO CAUSE ReorderWindow TO FAIL
  }

  return true;
}

Is this failing because the window is trying to order above itself?
Is there some timing/coordination issue between local/client orderings?
 

Comment 1 by sky@chromium.org, Oct 3 2016

Cc: -sky@chromium.org
Owner: sky@chromium.org
Status: Started (was: Available)
I'll poke at this.

Comment 2 by msw@chromium.org, Oct 3 2016

Thanks! It seems like WmWindowMus::Activate() can just call ui::Window::SetFocus(), and skip the following call to GetMusWindow(top_level)->MoveToFront(). If I comment that code out, the QuickLaunch window still comes to the foreground as expected.
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 4 2016

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

commit 3bddafc6834c9487e6223168120f5de7ecbf1d2b
Author: sky <sky@chromium.org>
Date: Tue Oct 04 01:11:50 2016

Makes mus not bring window to front on activation changes

The window manager should be doing it. To do otherwise means mus is
likely conflicting with the wm. Additionally changing the activation
as we were meant clients weren't getting notified and got out of sync
with mus.

BUG= 652421 
TEST=covered by tests
R=msw@chromium.org

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

[modify] https://crrev.com/3bddafc6834c9487e6223168120f5de7ecbf1d2b/services/ui/public/cpp/window.cc
[modify] https://crrev.com/3bddafc6834c9487e6223168120f5de7ecbf1d2b/services/ui/public/cpp/window.h
[modify] https://crrev.com/3bddafc6834c9487e6223168120f5de7ecbf1d2b/services/ui/ws/display.cc
[modify] https://crrev.com/3bddafc6834c9487e6223168120f5de7ecbf1d2b/services/ui/ws/window_manager_client_unittest.cc

Labels: Proj-Mustash
Components: Internals>MUS
Components: -MUS -Internals>MUS
Components: Internals>Services>Ash
Labels: -Proj-Mustash-Mash-WM
Status: WontFix (was: Started)
This bug is very dated. Many of the classes mentioned no longer exist. I'm closing out assuming it's no longer relevant.

Sign in to add a comment