This is where it was added: https://codereview.chromium.org/767443002

+piman,ericrk

I'm wondering if either of you can add some more context here about resizing and Windows. Right now with VizDisplayCompositor we are dropping ContextFactoryPrivate::ResizeDisplay() messages and resizing the viz::Display when a CompositorFrame with a new size is received in the GPU process. We are also dropping Compositor::DisableSwapUntilResize() calls as a result, since that is implemented by calling ResizeDisplay() with an empty gfx::Size.

Compositor::DisableSwapUntilResize() was added to help avoid a backbuffer scaling issue. It's not totally clear to me why it was a problem looking at https://crrev.com/767443002 and there is no bug.

I have been testing canary on a Windows machine with VizDisplayCompositor disabled/enabled. Resize looks identical to me despite ContextFactoryPrivate::ResizeDisplay() doing nothing when VizDisplayCompositor is enabled. Are there any cases I'm missing?

I can add an IPC for all ResizeDisplay() calls, or just for DisableSwapUntilResize(), but only if they are actually required.

My understanding of this is the following:

1. When a window is resized under Window, we get a WM_WINDOWPOSCHANGING notification. This notification lets us know that the Window size is going to change, but is fired before the size *actually* changes (apps can still suppress / prevent resizes here).
2. When we get WM_WINDOWPOSCHANGING, we end up calling (through various functions) to DisableSwapUntilResize, which calls Display::Resize with a 0x0 size.
3. This is the last chance we can produce a frame at the current size, so we call  scheduler_->ForceImmediateSwapIfPossible() and ContextGL()->ShallowFinishCHROMIUM(), which will synchronously produce the frame and swap it to the current (correct sized) backbuffer.
4. We then set size to 0x0, preventing further frames until we get a new size.
5. Later, we get WM_WINDOWPOSCHANGED, which means the window has been resized. This triggers a Display::Resize to the correct size, allowing us to swap new frames of the correct size.

My understanding is that, if we were to produce a frame between (4) and (5), we might swap a frame at the old size after the window has changed to the new size. In these cases, DWM may stretch the content to fill the window, resulting in ugly rendering.

The challenge when dealing with Viz is that, in order to prevent DWM from stretching our content, we need to stop VIZ to not swap a frame between (4) and (5), however, we need to do this synchronously, as once we return from the WM_WINDOWPOSCHANGING call Windows may resize the window at any time.

I haven't tested this yet, but that's my understanding of the situation.

One additional note: In order to maintain current guttering quality, we probably want our synchronous command to Viz in step (3/4) to synchronously swap a frame if one is ready. Note that even better would be to pump a nested message loop and try to produce a frame within a short deadline here (I had a WIP patch for this that never landed due to some other blockers. Those have now been addressed, so the concept should ~work: https://codereview.chromium.org/1513053002/).

Thanks for the explanation! I got a chance to play around with this a bit more. I don't get visible stretching but I do get an occasional black flash for window contents on resize on my test machine, so I guess this can manifest in different ways. Adding an IPC for DisableSwapUntilResize() fixes the black flash.

I guess we can't asynchronously tell Windows it's okay to resize? The sync IPC makes me a bit nervous. Pausing browser process execution frequently during resize since that might hurt resize performance.

The black flashing described in #4 ended up being a different bug. I wasn't able to reproduce this before but I was mostly testing on Windows 10. fsamuel found the scaling happens on Windows 7.

So to get this out of the way for finch trial, a sync IPC from browser to gpu shouldn't be too horrible. After that I can work on reviving Eric's CL.

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

commit 3e5eaae29d310061c0504f7d011f8bb0263e0078
Author: Saman Sami <samans@chromium.org>
Date: Fri Jun 01 16:46:34 2018

Make Compositor::DisableSwapUntilResize work with viz

Send a sync IPC to Viz process when size is changing in order
to prevent frame swaps of wrong size. If a wrong size frame is
swapped then Windows might stretch our content to fill the window.

Bug:  811945 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I4960e5a349902b75522c42a9f70b6eac531fdff6
Reviewed-on: https://chromium-review.googlesource.com/1076530
Reviewed-by: kylechar <kylechar@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563676}
[modify] https://crrev.com/3e5eaae29d310061c0504f7d011f8bb0263e0078/components/viz/service/frame_sinks/root_compositor_frame_sink_impl.cc
[modify] https://crrev.com/3e5eaae29d310061c0504f7d011f8bb0263e0078/components/viz/service/frame_sinks/root_compositor_frame_sink_impl.h
[modify] https://crrev.com/3e5eaae29d310061c0504f7d011f8bb0263e0078/content/browser/compositor/gpu_process_transport_factory.cc
[modify] https://crrev.com/3e5eaae29d310061c0504f7d011f8bb0263e0078/content/browser/compositor/gpu_process_transport_factory.h
[modify] https://crrev.com/3e5eaae29d310061c0504f7d011f8bb0263e0078/content/browser/compositor/test/test_image_transport_factory.h
[modify] https://crrev.com/3e5eaae29d310061c0504f7d011f8bb0263e0078/content/browser/compositor/viz_process_transport_factory.cc
[modify] https://crrev.com/3e5eaae29d310061c0504f7d011f8bb0263e0078/content/browser/compositor/viz_process_transport_factory.h
[modify] https://crrev.com/3e5eaae29d310061c0504f7d011f8bb0263e0078/mojo/public/cpp/bindings/sync_call_restrictions.h
[modify] https://crrev.com/3e5eaae29d310061c0504f7d011f8bb0263e0078/services/ui/ws/compositor_frame_sink_client_binding.cc
[modify] https://crrev.com/3e5eaae29d310061c0504f7d011f8bb0263e0078/services/ui/ws/compositor_frame_sink_client_binding.h
[modify] https://crrev.com/3e5eaae29d310061c0504f7d011f8bb0263e0078/services/ui/ws/frame_generator.cc
[modify] https://crrev.com/3e5eaae29d310061c0504f7d011f8bb0263e0078/services/ui/ws/frame_generator.h
[modify] https://crrev.com/3e5eaae29d310061c0504f7d011f8bb0263e0078/services/ui/ws/frame_generator_unittest.cc
[modify] https://crrev.com/3e5eaae29d310061c0504f7d011f8bb0263e0078/services/ui/ws/platform_display_default.cc
[modify] https://crrev.com/3e5eaae29d310061c0504f7d011f8bb0263e0078/services/ui/ws/platform_display_mirror.cc
[modify] https://crrev.com/3e5eaae29d310061c0504f7d011f8bb0263e0078/services/viz/privileged/interfaces/compositing/display_private.mojom
[modify] https://crrev.com/3e5eaae29d310061c0504f7d011f8bb0263e0078/ui/compositor/compositor.cc
[modify] https://crrev.com/3e5eaae29d310061c0504f7d011f8bb0263e0078/ui/compositor/compositor.h
[modify] https://crrev.com/3e5eaae29d310061c0504f7d011f8bb0263e0078/ui/compositor/test/in_process_context_factory.cc
[modify] https://crrev.com/3e5eaae29d310061c0504f7d011f8bb0263e0078/ui/compositor/test/in_process_context_factory.h

The reason I wasn't able to reproduce this on Windows 10 originally was because direct composition was enabled. With direct composition there is a child HWND in the GPU process to draw into. There is no stretching of the child HWND contents when the parent HWND is resized, so DisableSwapUntilResize() isn't necessary. Adding a child HWND for the non-direct composition path might be best long term solution assuming it works correctly?

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

commit 554a4b6a90144952665ec035d62fd60faade7412
Author: Saman Sami <samans@chromium.org>
Date: Mon Jun 04 23:06:45 2018

Revert "Make Compositor::DisableSwapUntilResize work with viz"

This reverts commit 3e5eaae29d310061c0504f7d011f8bb0263e0078.

Reason for revert: causes multiple bugs on Windows and breaks viz_content_browsertests on Mac FYI bots

Original change's description:
> Make Compositor::DisableSwapUntilResize work with viz
> 
> Send a sync IPC to Viz process when size is changing in order
> to prevent frame swaps of wrong size. If a wrong size frame is
> swapped then Windows might stretch our content to fill the window.
> 
> Bug:  811945 
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
> Change-Id: I4960e5a349902b75522c42a9f70b6eac531fdff6
> Reviewed-on: https://chromium-review.googlesource.com/1076530
> Reviewed-by: kylechar <kylechar@chromium.org>
> Reviewed-by: Ken Rockot <rockot@chromium.org>
> Reviewed-by: danakj <danakj@chromium.org>
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Commit-Queue: Saman Sami <samans@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#563676}

TBR=danakj@chromium.org,rockot@chromium.org,tsepez@chromium.org,kylechar@chromium.org,samans@chromium.org

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

Bug:  811945 ,  849283 ,  849169 ,  849155 
Change-Id: Ifada5eb26caf9fb7be5a242c0fb4b97070f4df57
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/1085971
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564284}
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/components/viz/service/frame_sinks/root_compositor_frame_sink_impl.cc
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/components/viz/service/frame_sinks/root_compositor_frame_sink_impl.h
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/content/browser/compositor/gpu_process_transport_factory.cc
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/content/browser/compositor/gpu_process_transport_factory.h
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/content/browser/compositor/test/test_image_transport_factory.h
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/content/browser/compositor/viz_process_transport_factory.cc
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/content/browser/compositor/viz_process_transport_factory.h
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/mojo/public/cpp/bindings/sync_call_restrictions.h
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/services/ui/ws/compositor_frame_sink_client_binding.cc
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/services/ui/ws/compositor_frame_sink_client_binding.h
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/services/ui/ws/frame_generator.cc
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/services/ui/ws/frame_generator.h
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/services/ui/ws/frame_generator_unittest.cc
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/services/ui/ws/platform_display_default.cc
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/services/ui/ws/platform_display_mirror.cc
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/services/viz/privileged/interfaces/compositing/display_private.mojom
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/ui/compositor/compositor.cc
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/ui/compositor/compositor.h
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/ui/compositor/test/in_process_context_factory.cc
[modify] https://crrev.com/554a4b6a90144952665ec035d62fd60faade7412/ui/compositor/test/in_process_context_factory.h

My CL relanded here:

https://chromium-review.googlesource.com/c/chromium/src/+/1087040

Marking as fixed.