The following seems to be what is happening:

RenderWidgetHostImpl::DidUpdateVisualProperties with a new size and advanced child sequence number
RenderWidgetHostViewAura::InternalSetBounds starts but with LocalSurfaceId allocation surpressed, as its from a child update
RenderWidgetHostViewAura::SynchronizeVisualProperties sends message of newly updated VisualProperties, but with the old LocalSurfaceID
RenderWidgetHostViewAura::OnDidUpdateVisualPropertiesComplete triggers and update to the new child sequence number

So we end up processing a new size from the child, sending the sync message with and old LSId and then updating the internal state.

samans@ do you know why we update from child at the end of this sequence? And if sending SyncVisualProperties with the newly advance child sequence number would cause any issues in the render?


I wouldn't be surprised is a similar path to the above is that is happening on Mac in issue 900948

This issue has an owner, a component and a priority, but is still listed as untriaged or unconfirmed. By definition, this bug is triaged. Changing status to "assigned". Please reach out to me if you disagree with how I've done this.

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

commit 9746a213f859bf3ce39f73c20b57e3bd24f8c130
Author: jonross <jonross@chromium.org>
Date: Fri Jan 18 19:08:41 2019

Enforce Surface Invariants in RenderWidgetHostImpl

Currently, when the expectations around Surface Synchronization
are broken, we do not find out until after Compositor Frames are
submitted. With the Viz Service verifying, and then rejecting the
submission.

With one additional check in cc:LayerTreeHost::SetViewportSizeAndScale,
which was only running on Mac. Which occurs in the Renderer process.

This makes violations tough to debug. Especially when the Browser
process is often allocating the viz::LocalSurfaceIds incorrectly.

The Mac check has been failing on webkit_layout_tests, but is flaky.
The check is also noted as to not be valid on Aura or Mus due to other
bugs.

This change updated the cc::LayerTreeHost::SetViewportSizeAndScale check
to be on all platforms. While updating Aura and Mus to fix violations.

This change also adds a Browser side check in RenderWidgetHostImpl::SynchronizeVisualProperties
to ensure that if sizes are changing, that viz::LocalSurfaceIds have
advanced. This check will let us catch the cause of errors, such as
the flaking Mac webkit_layout_tests, so that we can fix them.

This new check however caused lots of tests to fail. This is because
we were incorrectly handling ChildLocalSurfaceId updates. We were
previously doing:
     - RenderWidgetHostImpl::DidUpdateVisualProperties
         - updated child sequence number and new size
     - RenderWidgstHostImpl::SyncrhonizeVisualProperties
         - messages Renderer with new size, but old LocalSurfaceIds
     - viz::ScopedSurfaceIdAllocator callbacks run
         - most of which call ParentLocalSurfaceId::UpdateFromChild
     - RenderWidgstHostImpl::SyncrhonizeVisualProperties
         - called with newly merged Id
         - if there has been no ack or changes, then no message sent to Renderer

This leads to the Browser and Renderer having different viz::LocalSurfaceIds.
And with the Renderer re-using the same id for different sizes, which is
a violation.

This change updates the logic to not send messages until the ids have merged:
    - RenderWidgetHostImpl::DidUpdateVisualProperties
         - updated child sequence number and new size
     - RenderWidgstHostImpl::SyncrhonizeVisualProperties
         - exits early, not messaging Renderer
     - viz::ScopedSurfaceIdAllocator callbacks run
         - most of which call ParentLocalSurfaceId::UpdateFromChild
     - RenderWidgstHostImpl::SyncrhonizeVisualProperties
         - message Renderer with new size, and new merged LocalSurfaceId.

TBR=sadrul@chromium.org

Bug:  915915 , 900948
Change-Id: I72e2b60c8e1adbdcfd276e2b18c2d83427475fcc
Reviewed-on: https://chromium-review.googlesource.com/c/1379972
Commit-Queue: Jonathan Ross <jonross@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624227}
[modify] https://crrev.com/9746a213f859bf3ce39f73c20b57e3bd24f8c130/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/9746a213f859bf3ce39f73c20b57e3bd24f8c130/components/viz/common/surfaces/parent_local_surface_id_allocator.cc
[modify] https://crrev.com/9746a213f859bf3ce39f73c20b57e3bd24f8c130/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/9746a213f859bf3ce39f73c20b57e3bd24f8c130/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/9746a213f859bf3ce39f73c20b57e3bd24f8c130/ui/compositor/layer_unittest.cc