New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 793839 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

mash: Crash in OnPointerEventObserved from click in system tray on second monitor

Project Member Reported by jamescook@chromium.org, Dec 11 2017

Issue description

ToT chromeos-linux-simulator

out/Default/chrome --ash-host-window-bounds="1200x800,1250+0-1200x800" --user-data-dir=/w/udd --ash-dev-shortcuts --ash-debug-shortcuts --mash

* Open system tray on second monitor
* Click on ethernet item

Received signal 11 SEGV_MAPERR 0000000000a8
#0 0x7fb1fe75e47d base::debug::StackTrace::StackTrace()
#1 0x7fb1fe75c9fc base::debug::StackTrace::StackTrace()
#2 0x7fb1fe75de77 base::debug::(anonymous namespace)::StackDumpSignalHandler()
#3 0x7fb1fed45330 <unknown>
#4 0x7fb1f34e84fc aura::Window::IsRootWindow()
#5 0x7fb1f3570db9 aura::Window::GetRootWindow()
#6 0x7fb1f3570e35 aura::Window::GetHost()
#7 0x7fb1f356efc5 aura::Window::GetHost()
#8 0x7fb1ef05f1a5 ash::WindowManager::OnPointerEventObserved()
#9 0x7fb1f354422f aura::WindowTreeClient::OnPointerEventObserved()
#10 0x7fb1f3638186 ui::mojom::WindowTreeClientStubDispatch::Accept()
#11 0x7fb1f3557c03 ui::mojom::WindowTreeClientStub<>::Accept()
#12 0x7fb1fce6c15b mojo::InterfaceEndpointClient::HandleValidatedMessage()
#13 0x7fb1fce6acc1 mojo::InterfaceEndpointClient::HandleIncomingMessageThunk::Accept()
#14 0x7fb1fce69362 mojo::FilterChain::Accept()
#15 0x7fb1fce6eaef mojo::InterfaceEndpointClient::HandleIncomingMessage()
#16 0x7fb1fce84e0b mojo::internal::MultiplexRouter::ProcessIncomingMessage()

Relevant source:
  // We have received a pointer event on |target|. However, we need to fixup
  // |target| first. WindowManager's WindowTree is the entire root tree and
  // we must adjust |target| from being the root to the aura::Window which
  // would handle |event|.
  std::unique_ptr<ui::Event> event_copy = ui::Event::Clone(event);
  ui::EventTarget* e_target =
      target->GetHost()
          ->dispatcher()
          ->GetDefaultEventTargeter()
          ->FindTargetForEvent(target, event_copy.get());

  pointer_watcher_event_router_->OnPointerEventObserved(
      event, static_cast<aura::Window*>(e_target));

Maybe |target| is null sometimes?

erg, can you take a look? I think you added this code recently. Feel free to bounce back to me if it's not related.

 

Comment 1 by e...@chromium.org, Dec 11 2017

My patch is probably wrong; https://chromium-review.googlesource.com/c/chromium/src/+/820290 is the revert in progress.

Sorry about this.
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 11 2017

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

commit d26fedcf5784cd7e087a3d367e6578da7ab02413
Author: Elliot Glaysher <erg@chromium.org>
Date: Mon Dec 11 19:16:37 2017

Revert "Start fixing ImmersiveFullscreenController tests."

This reverts commit 3738ee850a6bdca56b1e8967760f7cf718130f7d.

Reason for revert: Patch is wrong, and is probably causing a crash on device. When running ash, target is actually set correctly. It's not set correctly in the unit tests though.

Original change's description:
> Start fixing ImmersiveFullscreenController tests.
> 
> The OnPointerEventObserved() interface acts differently between mash and
> mus/classic. In mus/classic, the target window is taken from the
> LocatedEvent's target(), relying on previous event dispatch metadata. On
> mash, this event came from a remote service which hasn't been targeted,
> target is therefore the top of the window tree. In the --mash case only,
> target the event on the window and use that as the window we pass to the
> rest of the system.
> 
> Bug: 698085
> Change-Id: I8a1859055d9427b269cd5256c7ff114a1e97574d
> Reviewed-on: https://chromium-review.googlesource.com/801976
> Reviewed-by: James Cook <jamescook@chromium.org>
> Commit-Queue: Elliot Glaysher <erg@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#521090}

TBR=jamescook@chromium.org,erg@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 698085,  793839 
Change-Id: I5d9325b72853c8793f5ce1f3e66fe74461eea9be
Reviewed-on: https://chromium-review.googlesource.com/820290
Reviewed-by: Elliot Glaysher <erg@chromium.org>
Commit-Queue: Elliot Glaysher <erg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523163}
[modify] https://crrev.com/d26fedcf5784cd7e087a3d367e6578da7ab02413/ash/window_manager.cc
[modify] https://crrev.com/d26fedcf5784cd7e087a3d367e6578da7ab02413/testing/buildbot/filters/ash_unittests_mash.filter

Comment 3 by e...@chromium.org, Dec 11 2017

Status: Fixed (was: Assigned)
Components: -Internals>MUS Internals>Services>WindowService

Sign in to add a comment