mash: Reusing same WindowTreeHost for different a display causes a crash |
||||
Issue descriptionWhen the primary display is removed the corresponding WindowTreeHost isn't immediately deleted. If a new display is added before the WTH is deleted then WindowTreeHostManager reuses the old WTH for the new display. This happens here: https://cs.chromium.org/chromium/src/ash/display/window_tree_host_manager.cc?sq=package:chromium&dr=CSs&l=605 In mus/mash mode the WS never finds out about this. The WS still thinks the now removed display id still exists and doesn't know about the new display id. WindowTree::ProcessSetDisplayRoot() can't find a display for that id here: https://cs.chromium.org/chromium/src/services/ui/ws/window_tree.cc?sq=package:chromium&dr=CSs&l=286 Ash then crashes when an in flight change fails after that fails. This happens when switching from headless (0 displays) to 1 display since there is a dummy display created by ash for headless.
,
Jul 11 2017
If you replace the line in ScreenManagerForwarding that sends the initial list of DisplaySnapshots: https://cs.chromium.org/chromium/src/services/ui/display/screen_manager_forwarding.cc?sq=package:chromium&dr=CSs&l=90 with something like this: callback.Run(std::vector<std::unique_ptr<DisplaySnapshotMojo>>()); Ash will start in headless mode and then when it gets the list of DisplaySnapshots later make the switch (and crash).
,
Jul 12 2017
,
Jul 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/82ea8468f6a220584679985e78af7c8d5f912832 commit 82ea8468f6a220584679985e78af7c8d5f912832 Author: Scott Violet <sky@chromium.org> Date: Thu Jul 13 22:39:56 2017 chromeos: make WindowWatcher a ShellObserver vs DisplayObserver Ash may recycle a WindowTreeHost for a different display. This means overriding OnDisplayAdded() and looking up a root window may result in getting a root already seen. Fix this by converting to ShellObserver::OnRootWindowAdded() which doesn't suffer from this problem. BUG= 740589 TEST=none (this code is only used in non-production code) Change-Id: Ic4accf920cedfe0f6c9c3b2ce2ed27c7bf2a9f37 Reviewed-on: https://chromium-review.googlesource.com/570777 Reviewed-by: Michael Wasserman <msw@chromium.org> Commit-Queue: Scott Violet <sky@chromium.org> Cr-Commit-Position: refs/heads/master@{#486497} [modify] https://crrev.com/82ea8468f6a220584679985e78af7c8d5f912832/ash/mus/standalone/ash_standalone_main.cc [modify] https://crrev.com/82ea8468f6a220584679985e78af7c8d5f912832/ash/shell.h [modify] https://crrev.com/82ea8468f6a220584679985e78af7c8d5f912832/ash/shell/content/client/shell_browser_main_parts.cc [modify] https://crrev.com/82ea8468f6a220584679985e78af7c8d5f912832/ash/shell/content/client/shell_browser_main_parts.h [modify] https://crrev.com/82ea8468f6a220584679985e78af7c8d5f912832/ash/shell/window_watcher.cc [modify] https://crrev.com/82ea8468f6a220584679985e78af7c8d5f912832/ash/shell/window_watcher.h
,
Jul 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/49b77ab73d86d6c6ef797f3305f8b7685b5ccafc commit 49b77ab73d86d6c6ef797f3305f8b7685b5ccafc Author: Scott Violet <sky@chromium.org> Date: Fri Jul 14 19:30:14 2017 chromeos: gets reusing of WindowTreeHosts working for mus WindowTreeHostManager may reuse a WindowTreeHost for a different display. This happens if all displays are removed, and then a new display is added. Ash now handles this by way of calling through to SetDisplayRoot() and SetDisplayRoot() on mus has been updated to allow this to happen. In addition this changes ash's DisplaySynchronizer to only notify mus of the current display layout after all changes have been processed. This is necessary as DisplayManager notifies observers while internally the list of displays contains both added and removed entries. This means when FocusSynchronizer could notify of state display state, which mus didn't like and caused ash to crash (hit a CHECK). BUG= 740589 TEST=covered by tests Change-Id: Ie0a2a05a1b592645669355f39997f9e3a5e77c3f Reviewed-on: https://chromium-review.googlesource.com/571399 Reviewed-by: kylechar <kylechar@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org> Commit-Queue: Scott Violet <sky@chromium.org> Cr-Commit-Position: refs/heads/master@{#486845} [modify] https://crrev.com/49b77ab73d86d6c6ef797f3305f8b7685b5ccafc/ash/display/display_manager_unittest.cc [modify] https://crrev.com/49b77ab73d86d6c6ef797f3305f8b7685b5ccafc/ash/display/window_tree_host_manager.cc [modify] https://crrev.com/49b77ab73d86d6c6ef797f3305f8b7685b5ccafc/ash/display/window_tree_host_manager.h [modify] https://crrev.com/49b77ab73d86d6c6ef797f3305f8b7685b5ccafc/ash/mus/DEPS [modify] https://crrev.com/49b77ab73d86d6c6ef797f3305f8b7685b5ccafc/ash/mus/display_synchronizer.cc [modify] https://crrev.com/49b77ab73d86d6c6ef797f3305f8b7685b5ccafc/ash/mus/display_synchronizer.h [modify] https://crrev.com/49b77ab73d86d6c6ef797f3305f8b7685b5ccafc/services/ui/public/interfaces/window_manager.mojom [modify] https://crrev.com/49b77ab73d86d6c6ef797f3305f8b7685b5ccafc/services/ui/ws/window_tree.cc [modify] https://crrev.com/49b77ab73d86d6c6ef797f3305f8b7685b5ccafc/services/ui/ws/window_tree_unittest.cc [modify] https://crrev.com/49b77ab73d86d6c6ef797f3305f8b7685b5ccafc/ui/aura/mus/window_manager_delegate.h [modify] https://crrev.com/49b77ab73d86d6c6ef797f3305f8b7685b5ccafc/ui/aura/mus/window_tree_client.cc [modify] https://crrev.com/49b77ab73d86d6c6ef797f3305f8b7685b5ccafc/ui/aura/mus/window_tree_client.h [modify] https://crrev.com/49b77ab73d86d6c6ef797f3305f8b7685b5ccafc/ui/display/display_observer.cc [modify] https://crrev.com/49b77ab73d86d6c6ef797f3305f8b7685b5ccafc/ui/display/display_observer.h [modify] https://crrev.com/49b77ab73d86d6c6ef797f3305f8b7685b5ccafc/ui/display/manager/display_manager.cc [modify] https://crrev.com/49b77ab73d86d6c6ef797f3305f8b7685b5ccafc/ui/display/manager/display_manager.h
,
Jul 14 2017
,
Jan 22 2018
|
||||
►
Sign in to add a comment |
||||
Comment 1 by sky@chromium.org
, Jul 11 2017Status: Assigned (was: Available)