Repro video: https://storage.cloud.google.com/chromiumos-test-logs/bugfiles/cr/871582/IMG_0254.MOV?_ga=2.229285396.-967102564.1532641052

Hi sdantuluri@

Could you please try again by turning off the flag: "Keyboard Shortcut Viewer mojo app" in chrome://flags.

Thanks.

Issue not reproduced on turning off "Keyboard Shortcut Viewer mojo app" flag

+jamescook@

Hi msw@, could you please take a look.

+dxie FYI

We're punting the KSV app to M-70 once we ship a single M-69 Beta with the app.

Fady or Sadrul, do you know offhand what's wrong to trigger this DCHECK? Advice/help appreciated, thanks!

Here's linux-chromeos repro instructions:
(1) Run "chrome --ash-debug-shortcuts --use-first-display-as-internal"
    (ash-debug-shortcuts so you can enter tablet mode)
    (use-first-display-as-internal so the ui scale accelerator works)
(2) Change the UI scale with Ctrl-Shift-+ or Ctrl-Shift-- (+ to the max seems to be most consistent)
(3) Enter tablet mode with Ctrl-Alt-Shift-T
(4) Open the KSV app with Ctrl-Alt-/
Expected: No crash, app window shows.
Actual: KSV app [shortcut] flashes open and immediately crashes on a compositor.cc DCHECK:

[shortcut:253421:253421:0816/090012.063593:FATAL:compositor.cc(370)] Check failed: local_surface_id != host_->local_surface_id_from_parent() (LocalSurfaceId(3, 1, 4800...) vs. LocalSurfaceId(3, 1, 4800...))
#0 0x7fada014756c base::debug::StackTrace::StackTrace()
#1 0x7fada007b70b logging::LogMessage::~LogMessage()
#2 0x7fad9b6c9799 ui::Compositor::SetScaleAndSize()
#3 0x7fad9b793368 aura::WindowTreeHost::OnHostResizedInPixels()
#4 0x7fad9b7949b5 aura::WindowTreeHostPlatform::OnBoundsChanged()
#5 0x7fad9925a106 views::DesktopWindowTreeHostMus::SetBoundsInPixels()
#6 0x7fad9b77f9fe aura::WindowTreeHostMus::SetBoundsFromServerInPixels()
#7 0x7fad9b7772bf aura::WindowTreeClient::SetWindowBoundsFromServer()
#8 0x7fad9b779d2b aura::WindowTreeClient::OnWindowBoundsChanged()
#9 0x7fad9b7a4c48 ui::mojom::WindowTreeClientStubDispatch::Accept()

A new LocalSurfaceId must be allocated when a resize happens. That's not happening right now.

Something here is wrong:

https://cs.chromium.org/chromium/src/ui/aura/window_tree_host.cc?type=cs&q=aura::WindowTreeHost::OnHostResizedInPixels&sq=package:chromium&g=0&l=393

Is WindowTreeClient::OnWindowBoundsChanged receiving a valid LocalSurfaceId?

The id is valid, afaict. I think the defect is related to dip->pixel conversion and coordination between ws and client.
(1) The client centers the window in dips during init, and sends those bounds to the ws, ie. (55,0 800x464)
(2) The client sets bounds locally in pixels, gfx::ConvertRectToPixel w/scale=1.5 yields an enclosing rect: (82,0 1201x696) (null surface id)
(3) The client gets OnChangeCompleted with an OBO dip location (54,0 800x464), ConvertRectToPixel yields an OBO px size (81,0 1200x696) (valid id)
(4) The client gets OnWindowBoundsChanged with the old dip location (55,0 800x464), ConvertRectToPixel yields the old px size (82,0 1201x696) (same id)

So, one defect is that the client and service disagree on the dip location for centering the window.
Another defect is that the same dip bounds can yield different px sizes but re-use the old surface id.

I have a possible fix that uses ConvertPointToPixel and ConverSizeToPixel separately to avoid px size disagreements:
https://chromium-review.googlesource.com/c/chromium/src/+/1180265
(I think that ConvertRectToPixel is meant more for invalidating paint rects, not setting window bounds)

I noticed that my WIP CL also fixes a similar defect running Mash:
(1) Run chrome --enable-features=Mash --login-manager --ash-debug-shortcuts --use-first-display-as-internal
(2) Change ui scale with Ctrl-Shift-[+/-] (not sure what setting repros most)
(3) Right click the desktop and select "Set wallpaper"
(4) Drag the wallpaper picker window around
Expected: No issues moving the wallpaper picker window
Actual: DCHECK crash with same callstack as above:

[164520:164520:0821/110819.949919:FATAL:compositor.cc(370)] Check failed: local_surface_id != host_->local_surface_id_from_parent() (LocalSurfaceId(4, 1, 436A...) vs. LocalSurfaceId(4, 1, 436A...))
#0 0x7ff32f89f55c base::debug::StackTrace::StackTrace()
#1 0x7ff32f7d2e5b logging::LogMessage::~LogMessage()
#2 0x7ff32ae24799 ui::Compositor::SetScaleAndSize()
#3 0x7ff32aeee548 aura::WindowTreeHost::OnHostResizedInPixels()
#4 0x7ff32aeefb95 aura::WindowTreeHostPlatform::OnBoundsChanged()
#5 0x7ff3289a8286 views::DesktopWindowTreeHostMus::SetBoundsInPixels()
#6 0x7ff32aedab8e aura::WindowTreeHostMus::SetBoundsFromServerInPixels()
#7 0x7ff32aed244f aura::WindowTreeClient::SetWindowBoundsFromServer()
#8 0x7ff32aed4ebb aura::WindowTreeClient::OnWindowBoundsChanged()
#9 0x7ff32aeffe28 ui::mojom::WindowTreeClientStubDispatch::Accept()

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

commit fd7064c6183ed4b00e69a1a10a01f17a009085ad
Author: Mike Wasserman <msw@chromium.org>
Date: Tue Aug 21 19:01:06 2018

ws: Convert dip->px rects piecewise to avoid enclosing rect issues.

Using ConvertRectToPixel yields unexpected enclosing rect bounds.
(changes in dip locations can alter pixel bounds unintentionally)
Instead, convert the rect origin and size pixel values separately.

Bug:  871582 
Test: No KSV (Ctrl-Alt-/) DCHECKs with non-1 ui/dsf scale [tablet mode].
Change-Id: Ia22ea75bb34382e323753bb173ce81b2c4b143b9
Reviewed-on: https://chromium-review.googlesource.com/1180265
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584870}
[modify] https://crrev.com/fd7064c6183ed4b00e69a1a10a01f17a009085ad/ui/aura/mus/window_tree_client.cc
[modify] https://crrev.com/fd7064c6183ed4b00e69a1a10a01f17a009085ad/ui/views/mus/desktop_window_tree_host_mus.cc