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

Implement surface synchronization system (out-of-browser-process/thread ResizeLock replacement)

Project Member Reported by fsam...@chromium.org, Dec 9 2016

Issue description

We need a way to ensure that multiple compositorframes are submitted and displayed on screen atomically.
 
Showing comments 278 - 377 of 377 Older
Project Member

Comment 278 by bugdroid1@chromium.org, May 17 2018

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

commit 7ebe05398167d54eaf20d07c6bfb5276c0e83cc4
Author: Fady Samuel <fsamuel@chromium.org>
Date: Thu May 17 22:48:11 2018

Surface Synchronization: Call SynchronizeVisualProperties if ID changes

If the child allocates a new LocalSurfaceId, then RenderFrameProxy or
BrowserPlugin should call SynchronizeVisualProperties to embed the new
ID. This is a non-issue today, because for auto-resize, we allocate a new
parent ID too when we resize the parent which then gets embedded, but is
necessary to synchronize text selection handles.

Bug:  672962 
Change-Id: If15638a0679a25b3d0a22eddc391b6079553d44a
Reviewed-on: https://chromium-review.googlesource.com/1064970
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559715}
[modify] https://crrev.com/7ebe05398167d54eaf20d07c6bfb5276c0e83cc4/content/renderer/browser_plugin/browser_plugin.cc
[modify] https://crrev.com/7ebe05398167d54eaf20d07c6bfb5276c0e83cc4/content/renderer/render_frame_proxy.cc

Project Member

Comment 279 by bugdroid1@chromium.org, May 21 2018

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

commit 014cb0fc75c9c26af2c1645ae49d13c8e52b7c44
Author: Fady Samuel <fsamuel@chromium.org>
Date: Mon May 21 15:51:13 2018

Surface synchronization: Prevent LatencyInfo from accumulating across surfaces

The LatencyInfo vector in CompositorFrameMetadata can accumulate without bound
when allocating new surfaces. In order to track latency, we carry LatencyInfo
objects across surfaces. The new surface grabs the LatencyInfo objects from
the previous surface.

We used to check if the new Surface has too many LatencyInfos and if not,
grab all LatencyInfos from the previous Surface and we don't check again.
The previous Surface might have a large number of LatencyInfo objects
accumulated, and will accumulate further with every new surface allocated
for a given FrameSink.

The fix is to verify the LatencyInfo vector AFTER taking the items from
the previous surface.

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I6814fd3657b7b0a9e9d275013e661b176095e740
Bug:  834421 ,  672962 
Reviewed-on: https://chromium-review.googlesource.com/1064810
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#559668}(cherry picked from commit d1564e4226387f17c349e4d8a20cbd3ff8a08dc5)
Reviewed-on: https://chromium-review.googlesource.com/1067056
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#659}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/014cb0fc75c9c26af2c1645ae49d13c8e52b7c44/components/viz/service/frame_sinks/surface_synchronization_unittest.cc
[modify] https://crrev.com/014cb0fc75c9c26af2c1645ae49d13c8e52b7c44/components/viz/service/surfaces/surface.cc
[modify] https://crrev.com/014cb0fc75c9c26af2c1645ae49d13c8e52b7c44/components/viz/test/compositor_frame_helpers.cc
[modify] https://crrev.com/014cb0fc75c9c26af2c1645ae49d13c8e52b7c44/components/viz/test/compositor_frame_helpers.h

Project Member

Comment 280 by bugdroid1@chromium.org, May 22 2018

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

commit 0d7d74584f957455326b334f3476be4720b70353
Author: Fady Samuel <fsamuel@chromium.org>
Date: Tue May 22 23:17:42 2018

Surface synchronization: Remove content_source_id in VisualProperties

This CL removes content_source_id in VisualProperties which is no longer
used. It also renames |local_surface_id_| to |local_surface_id_from_parent_|
in RenderWidget to clarify where the viz::LocalSurfaceId is coming from.

Bug:  672962 
Change-Id: I11763addeef9d84eab93be624bacedc40757b616
Reviewed-on: https://chromium-review.googlesource.com/1055470
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560829}
[modify] https://crrev.com/0d7d74584f957455326b334f3476be4720b70353/content/browser/renderer_host/browser_compositor_view_mac.h
[modify] https://crrev.com/0d7d74584f957455326b334f3476be4720b70353/content/browser/renderer_host/browser_compositor_view_mac.mm
[modify] https://crrev.com/0d7d74584f957455326b334f3476be4720b70353/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/0d7d74584f957455326b334f3476be4720b70353/content/browser/renderer_host/delegated_frame_host.h
[modify] https://crrev.com/0d7d74584f957455326b334f3476be4720b70353/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/0d7d74584f957455326b334f3476be4720b70353/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/0d7d74584f957455326b334f3476be4720b70353/content/browser/renderer_host/render_widget_host_unittest.cc
[modify] https://crrev.com/0d7d74584f957455326b334f3476be4720b70353/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/0d7d74584f957455326b334f3476be4720b70353/content/browser/renderer_host/render_widget_host_view_aura.h
[modify] https://crrev.com/0d7d74584f957455326b334f3476be4720b70353/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/0d7d74584f957455326b334f3476be4720b70353/content/browser/renderer_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/0d7d74584f957455326b334f3476be4720b70353/content/browser/web_contents/aura/overscroll_navigation_overlay_unittest.cc
[modify] https://crrev.com/0d7d74584f957455326b334f3476be4720b70353/content/common/view_messages.h
[modify] https://crrev.com/0d7d74584f957455326b334f3476be4720b70353/content/common/visual_properties.h
[modify] https://crrev.com/0d7d74584f957455326b334f3476be4720b70353/content/public/test/render_view_test.cc
[modify] https://crrev.com/0d7d74584f957455326b334f3476be4720b70353/content/renderer/render_frame_impl_browsertest.cc
[modify] https://crrev.com/0d7d74584f957455326b334f3476be4720b70353/content/renderer/render_view_browsertest.cc
[modify] https://crrev.com/0d7d74584f957455326b334f3476be4720b70353/content/renderer/render_view_impl.cc
[modify] https://crrev.com/0d7d74584f957455326b334f3476be4720b70353/content/renderer/render_widget.cc
[modify] https://crrev.com/0d7d74584f957455326b334f3476be4720b70353/content/renderer/render_widget.h
[modify] https://crrev.com/0d7d74584f957455326b334f3476be4720b70353/content/renderer/render_widget_browsertest.cc
[modify] https://crrev.com/0d7d74584f957455326b334f3476be4720b70353/content/renderer/render_widget_unittest.cc

Project Member

Comment 281 by bugdroid1@chromium.org, May 23 2018

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

commit 98749dbcac704b9a03810cdaee9758d54593be65
Author: Fady Samuel <fsamuel@chromium.org>
Date: Wed May 23 18:42:08 2018

Revert "Surface synchronization: Remove content_source_id in VisualProperties"

This reverts commit 0d7d74584f957455326b334f3476be4720b70353.

Reason for revert: This broke combobox popups and causes flashes on new tab page.

Original change's description:
> Surface synchronization: Remove content_source_id in VisualProperties
> 
> This CL removes content_source_id in VisualProperties which is no longer
> used. It also renames |local_surface_id_| to |local_surface_id_from_parent_|
> in RenderWidget to clarify where the viz::LocalSurfaceId is coming from.
> 
> Bug:  672962 
> Change-Id: I11763addeef9d84eab93be624bacedc40757b616
> Reviewed-on: https://chromium-review.googlesource.com/1055470
> Commit-Queue: Fady Samuel <fsamuel@chromium.org>
> Reviewed-by: Ken Buchanan <kenrb@chromium.org>
> Reviewed-by: Antoine Labour <piman@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#560829}

TBR=kenrb@chromium.org,fsamuel@chromium.org,piman@chromium.org,samans@chromium.org

Change-Id: Ic7ecea4b1a6211e3c5df60d849590ee097531bfd
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  672962 
Reviewed-on: https://chromium-review.googlesource.com/1070493
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561177}
[modify] https://crrev.com/98749dbcac704b9a03810cdaee9758d54593be65/content/browser/renderer_host/browser_compositor_view_mac.h
[modify] https://crrev.com/98749dbcac704b9a03810cdaee9758d54593be65/content/browser/renderer_host/browser_compositor_view_mac.mm
[modify] https://crrev.com/98749dbcac704b9a03810cdaee9758d54593be65/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/98749dbcac704b9a03810cdaee9758d54593be65/content/browser/renderer_host/delegated_frame_host.h
[modify] https://crrev.com/98749dbcac704b9a03810cdaee9758d54593be65/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/98749dbcac704b9a03810cdaee9758d54593be65/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/98749dbcac704b9a03810cdaee9758d54593be65/content/browser/renderer_host/render_widget_host_unittest.cc
[modify] https://crrev.com/98749dbcac704b9a03810cdaee9758d54593be65/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/98749dbcac704b9a03810cdaee9758d54593be65/content/browser/renderer_host/render_widget_host_view_aura.h
[modify] https://crrev.com/98749dbcac704b9a03810cdaee9758d54593be65/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/98749dbcac704b9a03810cdaee9758d54593be65/content/browser/renderer_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/98749dbcac704b9a03810cdaee9758d54593be65/content/browser/web_contents/aura/overscroll_navigation_overlay_unittest.cc
[modify] https://crrev.com/98749dbcac704b9a03810cdaee9758d54593be65/content/common/view_messages.h
[modify] https://crrev.com/98749dbcac704b9a03810cdaee9758d54593be65/content/common/visual_properties.h
[modify] https://crrev.com/98749dbcac704b9a03810cdaee9758d54593be65/content/public/test/render_view_test.cc
[modify] https://crrev.com/98749dbcac704b9a03810cdaee9758d54593be65/content/renderer/render_frame_impl_browsertest.cc
[modify] https://crrev.com/98749dbcac704b9a03810cdaee9758d54593be65/content/renderer/render_view_browsertest.cc
[modify] https://crrev.com/98749dbcac704b9a03810cdaee9758d54593be65/content/renderer/render_view_impl.cc
[modify] https://crrev.com/98749dbcac704b9a03810cdaee9758d54593be65/content/renderer/render_widget.cc
[modify] https://crrev.com/98749dbcac704b9a03810cdaee9758d54593be65/content/renderer/render_widget.h
[modify] https://crrev.com/98749dbcac704b9a03810cdaee9758d54593be65/content/renderer/render_widget_browsertest.cc
[modify] https://crrev.com/98749dbcac704b9a03810cdaee9758d54593be65/content/renderer/render_widget_unittest.cc

Project Member

Comment 282 by bugdroid1@chromium.org, May 23 2018

Labels: merge-merged-3438
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/07ced2a034575d54264af555279bc336fd1eabf5

commit 07ced2a034575d54264af555279bc336fd1eabf5
Author: Fady Samuel <fsamuel@chromium.org>
Date: Wed May 23 18:46:08 2018

Revert "Surface synchronization: Remove content_source_id in VisualProperties"

This reverts commit 0d7d74584f957455326b334f3476be4720b70353.

Reason for revert: This broke combobox popups and causes flashes on new tab page.

Original change's description:
> Surface synchronization: Remove content_source_id in VisualProperties
> 
> This CL removes content_source_id in VisualProperties which is no longer
> used. It also renames |local_surface_id_| to |local_surface_id_from_parent_|
> in RenderWidget to clarify where the viz::LocalSurfaceId is coming from.
> 
> Bug:  672962 
> Change-Id: I11763addeef9d84eab93be624bacedc40757b616
> Reviewed-on: https://chromium-review.googlesource.com/1055470
> Commit-Queue: Fady Samuel <fsamuel@chromium.org>
> Reviewed-by: Ken Buchanan <kenrb@chromium.org>
> Reviewed-by: Antoine Labour <piman@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#560829}

TBR=kenrb@chromium.org,fsamuel@chromium.org,piman@chromium.org,samans@chromium.org

Change-Id: Ic7ecea4b1a6211e3c5df60d849590ee097531bfd
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  672962 
Reviewed-on: https://chromium-review.googlesource.com/1070493
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#561177}(cherry picked from commit 98749dbcac704b9a03810cdaee9758d54593be65)
Reviewed-on: https://chromium-review.googlesource.com/1070495
Cr-Commit-Position: refs/branch-heads/3438@{#5}
Cr-Branched-From: 8da9d1449c15b828ec9479e2bf4a3a922d6eb2ac-refs/heads/master@{#560883}
[modify] https://crrev.com/07ced2a034575d54264af555279bc336fd1eabf5/content/browser/renderer_host/browser_compositor_view_mac.h
[modify] https://crrev.com/07ced2a034575d54264af555279bc336fd1eabf5/content/browser/renderer_host/browser_compositor_view_mac.mm
[modify] https://crrev.com/07ced2a034575d54264af555279bc336fd1eabf5/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/07ced2a034575d54264af555279bc336fd1eabf5/content/browser/renderer_host/delegated_frame_host.h
[modify] https://crrev.com/07ced2a034575d54264af555279bc336fd1eabf5/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/07ced2a034575d54264af555279bc336fd1eabf5/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/07ced2a034575d54264af555279bc336fd1eabf5/content/browser/renderer_host/render_widget_host_unittest.cc
[modify] https://crrev.com/07ced2a034575d54264af555279bc336fd1eabf5/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/07ced2a034575d54264af555279bc336fd1eabf5/content/browser/renderer_host/render_widget_host_view_aura.h
[modify] https://crrev.com/07ced2a034575d54264af555279bc336fd1eabf5/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/07ced2a034575d54264af555279bc336fd1eabf5/content/browser/renderer_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/07ced2a034575d54264af555279bc336fd1eabf5/content/browser/web_contents/aura/overscroll_navigation_overlay_unittest.cc
[modify] https://crrev.com/07ced2a034575d54264af555279bc336fd1eabf5/content/common/view_messages.h
[modify] https://crrev.com/07ced2a034575d54264af555279bc336fd1eabf5/content/common/visual_properties.h
[modify] https://crrev.com/07ced2a034575d54264af555279bc336fd1eabf5/content/public/test/render_view_test.cc
[modify] https://crrev.com/07ced2a034575d54264af555279bc336fd1eabf5/content/renderer/render_frame_impl_browsertest.cc
[modify] https://crrev.com/07ced2a034575d54264af555279bc336fd1eabf5/content/renderer/render_view_browsertest.cc
[modify] https://crrev.com/07ced2a034575d54264af555279bc336fd1eabf5/content/renderer/render_view_impl.cc
[modify] https://crrev.com/07ced2a034575d54264af555279bc336fd1eabf5/content/renderer/render_widget.cc
[modify] https://crrev.com/07ced2a034575d54264af555279bc336fd1eabf5/content/renderer/render_widget.h
[modify] https://crrev.com/07ced2a034575d54264af555279bc336fd1eabf5/content/renderer/render_widget_browsertest.cc
[modify] https://crrev.com/07ced2a034575d54264af555279bc336fd1eabf5/content/renderer/render_widget_unittest.cc

Project Member

Comment 283 by bugdroid1@chromium.org, May 24 2018

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

commit 4255c18dd8d9b3411e6cdbaa92782c4cf2a97581
Author: Fady Samuel <fsamuel@chromium.org>
Date: Thu May 24 20:42:36 2018

RELAND: Surface synchronization: Remove content_source_id in VisualProperties

This CL removes content_source_id in VisualProperties which is no longer
used. It also renames |local_surface_id_| to |local_surface_id_from_parent_|
in RenderWidget to clarify where the viz::LocalSurfaceId is coming from.

A previous version of this CL also

1. Did not embed renderer that have not navigated yet.
2. Always allocated a new LocalSurfaceId for navigation.

1. Broke combobox popups because they're renderers that never navigate
2. Caused new tab page flickering because the browser would submit a
   CompositorFrame that refers to no content until navigation which
   can cause SurfaceAggregator to flash the background color.

The fix to 1. and 2. is to embed LocalSurfaceIds even if they did not navigate,
and to not allocate a new LocalSurfaceId on the first navigation as before (so
we block on new tab page to show the new content).

This CL will likely cause an improvement to the timeToFirstPaint metric (and
other navigation to paint metrics) because it will not block frame submission on
allocating a new LocalSurfaceId anymore. However, the browser will still
allocate a new LocalSurfaceId on DidNavigate. There is a race here which might
cause Chrome to do a bit more work than it would otherwise (generate one more
frame than it would otherwise) but that should be pretty small.

Bug:  672962 
TBR: kenrb@chromium.org, piman@chromium.org
Change-Id: I9754ff7b9330c99a0a1106b8d02aef98a26b1318
Reviewed-on: https://chromium-review.googlesource.com/1070476
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561615}
[modify] https://crrev.com/4255c18dd8d9b3411e6cdbaa92782c4cf2a97581/content/browser/renderer_host/browser_compositor_view_mac.h
[modify] https://crrev.com/4255c18dd8d9b3411e6cdbaa92782c4cf2a97581/content/browser/renderer_host/browser_compositor_view_mac.mm
[modify] https://crrev.com/4255c18dd8d9b3411e6cdbaa92782c4cf2a97581/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/4255c18dd8d9b3411e6cdbaa92782c4cf2a97581/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/4255c18dd8d9b3411e6cdbaa92782c4cf2a97581/content/browser/renderer_host/render_widget_host_unittest.cc
[modify] https://crrev.com/4255c18dd8d9b3411e6cdbaa92782c4cf2a97581/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/4255c18dd8d9b3411e6cdbaa92782c4cf2a97581/content/browser/renderer_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/4255c18dd8d9b3411e6cdbaa92782c4cf2a97581/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/4255c18dd8d9b3411e6cdbaa92782c4cf2a97581/content/browser/web_contents/aura/overscroll_navigation_overlay_unittest.cc
[modify] https://crrev.com/4255c18dd8d9b3411e6cdbaa92782c4cf2a97581/content/common/view_messages.h
[modify] https://crrev.com/4255c18dd8d9b3411e6cdbaa92782c4cf2a97581/content/common/visual_properties.h
[modify] https://crrev.com/4255c18dd8d9b3411e6cdbaa92782c4cf2a97581/content/public/test/render_view_test.cc
[modify] https://crrev.com/4255c18dd8d9b3411e6cdbaa92782c4cf2a97581/content/renderer/render_frame_impl_browsertest.cc
[modify] https://crrev.com/4255c18dd8d9b3411e6cdbaa92782c4cf2a97581/content/renderer/render_view_browsertest.cc
[modify] https://crrev.com/4255c18dd8d9b3411e6cdbaa92782c4cf2a97581/content/renderer/render_view_impl.cc
[modify] https://crrev.com/4255c18dd8d9b3411e6cdbaa92782c4cf2a97581/content/renderer/render_widget.cc
[modify] https://crrev.com/4255c18dd8d9b3411e6cdbaa92782c4cf2a97581/content/renderer/render_widget.h
[modify] https://crrev.com/4255c18dd8d9b3411e6cdbaa92782c4cf2a97581/content/renderer/render_widget_browsertest.cc
[modify] https://crrev.com/4255c18dd8d9b3411e6cdbaa92782c4cf2a97581/content/renderer/render_widget_unittest.cc

Project Member

Comment 284 by bugdroid1@chromium.org, May 25 2018

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

commit 87443afeac98fd894103fd0e752c7c35ccb519d6
Author: Fady Samuel <fsamuel@chromium.org>
Date: Fri May 25 20:24:47 2018

Surface Synchronization: DelegatedFrameHost::{SynchronizeVisualProperties => EmbedSurface}

This CL renames DelegatedFrameHost::SynchronizeVisualProperties =>
DelegatedFrameHost::EmbedSurface. RenderWidgetHostImpl::SynchronizeVisualProperties
makes sense because it sends across a set of VisualProperties and a
viz::LocalSurfaceId to the child whereas the DelegatedFrameHost method actually
embeds that viz::LocalSurfaceId that's being sent to the renderer.

This CL renames that method to something more appropriate and less confusing.

Bug:  672962 
Change-Id: Icc9b5a1438c9efaab9ddb5f4f37f0ff62e1d5df1
Reviewed-on: https://chromium-review.googlesource.com/1073654
Reviewed-by: kylechar <kylechar@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561993}
[modify] https://crrev.com/87443afeac98fd894103fd0e752c7c35ccb519d6/content/browser/renderer_host/browser_compositor_view_mac.mm
[modify] https://crrev.com/87443afeac98fd894103fd0e752c7c35ccb519d6/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/87443afeac98fd894103fd0e752c7c35ccb519d6/content/browser/renderer_host/delegated_frame_host.h
[modify] https://crrev.com/87443afeac98fd894103fd0e752c7c35ccb519d6/content/browser/renderer_host/render_widget_host_view_aura.cc

Project Member

Comment 285 by bugdroid1@chromium.org, May 30 2018

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

commit dba483cee6a5f15e2e2d73df16968ab10b38a2bf
Author: Fady Samuel <fsamuel@chromium.org>
Date: Wed May 30 04:42:35 2018

Surface Sychronization: Clean-up DelegatedFrameHostAndroid

This CL does two things:

1. It moves ParentLocalSurfaceIdAllocator from DelegatedFrameHostAndroid
to RenderWidgetHostViewAndroid.

2. It makes DelegatedFrameHostAndroid::SynchronizeVisualProperties
   and DelegatedFrameHostAndroid::OnFirstSurfaceActivation better
   match their DelegatedFrameHost counterparts.

3. Creating and destroying SurfaceLayers is a relatively heavyweight
   operation. This CL avoids this when surface sync is on.

Change-Id: Ia265489cc95494eb0ae0cf6ab9d5a300a79a53e3
Bug:  672962 
Reviewed-on: https://chromium-review.googlesource.com/1067954
Reviewed-by: Khushal <khushalsagar@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562737}
[modify] https://crrev.com/dba483cee6a5f15e2e2d73df16968ab10b38a2bf/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/dba483cee6a5f15e2e2d73df16968ab10b38a2bf/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/dba483cee6a5f15e2e2d73df16968ab10b38a2bf/ui/android/BUILD.gn
[modify] https://crrev.com/dba483cee6a5f15e2e2d73df16968ab10b38a2bf/ui/android/DEPS
[modify] https://crrev.com/dba483cee6a5f15e2e2d73df16968ab10b38a2bf/ui/android/delegated_frame_host_android.cc
[modify] https://crrev.com/dba483cee6a5f15e2e2d73df16968ab10b38a2bf/ui/android/delegated_frame_host_android.h
[modify] https://crrev.com/dba483cee6a5f15e2e2d73df16968ab10b38a2bf/ui/android/delegated_frame_host_android_unittest.cc

Project Member

Comment 286 by bugdroid1@chromium.org, May 30 2018

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

commit 5d8e61ffb132e86594a0939334f5af635c39a637
Author: Fady Samuel <fsamuel@chromium.org>
Date: Wed May 30 16:58:54 2018

Surface synchronization: Remove DelegatedFrameHost::GetRequestedRendererSize

The requested renderer size is always the current window size which is the
GetViewBounds().size(). No need to plumb out that value from DelegatedFrameHost
since it's really coming from aura::Window and RenderWidgetHostViewAura.

Bug:  672962 
Change-Id: Idf169ae4a32c170e8035c41ffb2d03a0a9c0a5af
Reviewed-on: https://chromium-review.googlesource.com/1079039
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562873}
[modify] https://crrev.com/5d8e61ffb132e86594a0939334f5af635c39a637/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/5d8e61ffb132e86594a0939334f5af635c39a637/content/browser/renderer_host/delegated_frame_host.h
[modify] https://crrev.com/5d8e61ffb132e86594a0939334f5af635c39a637/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/5d8e61ffb132e86594a0939334f5af635c39a637/content/browser/renderer_host/render_widget_host_view_aura.h

Project Member

Comment 287 by bugdroid1@chromium.org, May 30 2018

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

commit 9bd58dc39b847052e1cb23bfc876a28eaad0aa55
Author: Fady Samuel <fsamuel@chromium.org>
Date: Wed May 30 18:25:27 2018

Surface synchronization: Delete some dead code

CompositorFrames will always have a non-zero size and so we don't need
the checks in RenderWidgetHostViewAndroid that verify that the CompositorFrame
is non-empty.

Bug:  672962 
Change-Id: I958f038c1a9f59e2c9bc8693f89f514e689bac1a
Reviewed-on: https://chromium-review.googlesource.com/1079189
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562920}
[modify] https://crrev.com/9bd58dc39b847052e1cb23bfc876a28eaad0aa55/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/9bd58dc39b847052e1cb23bfc876a28eaad0aa55/content/browser/renderer_host/render_widget_host_view_android.h

Project Member

Comment 288 by bugdroid1@chromium.org, Jun 1 2018

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

commit ac325ce7176fe23df953b34ee5f903f50d1fe4e4
Author: Fady Samuel <fsamuel@chromium.org>
Date: Fri Jun 01 15:23:27 2018

Surface Synchronization: Move FrameEvictor to DelegatedFrameHostAndroid

In an effort to transition DelegatedFrameHostAndroid to look more like
DelegatedFrameHost (and eventually merge the two) and to make surface
sync work well on Android, this CL moves FrameEvictor into
DelegatedFrameHostAndroid from RenderWidgetHostViewAndroid similar to
desktop platforms.

Webview only has a single renderer and has no browser compositor so it
doesn't need a frame evictor.

Bug:  672962 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I50679aff08675cbacba40017448de753c7a438eb
Reviewed-on: https://chromium-review.googlesource.com/1080848
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563637}
[modify] https://crrev.com/ac325ce7176fe23df953b34ee5f903f50d1fe4e4/components/viz/client/frame_evictor.h
[modify] https://crrev.com/ac325ce7176fe23df953b34ee5f903f50d1fe4e4/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/ac325ce7176fe23df953b34ee5f903f50d1fe4e4/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/ac325ce7176fe23df953b34ee5f903f50d1fe4e4/ui/android/BUILD.gn
[modify] https://crrev.com/ac325ce7176fe23df953b34ee5f903f50d1fe4e4/ui/android/DEPS
[modify] https://crrev.com/ac325ce7176fe23df953b34ee5f903f50d1fe4e4/ui/android/delegated_frame_host_android.cc
[modify] https://crrev.com/ac325ce7176fe23df953b34ee5f903f50d1fe4e4/ui/android/delegated_frame_host_android.h

Blocking: 837884

Comment 290 by kbr@chromium.org, Jun 2 2018

Cc: vikassoni@chromium.org cblume@chromium.org ericrk@chromium.org
 Issue 837884  has been merged into this issue.
Project Member

Comment 291 by bugdroid1@chromium.org, Jun 5 2018

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

commit 507ecab6119abfb0b9b7a1c27dd42b0185b64b05
Author: Fady Samuel <fsamuel@chromium.org>
Date: Tue Jun 05 16:16:36 2018

Surface synchronization: Synchronize Text Selection Handles

This CL refactors text selection code to be synchronized via surface
synchronization. Attributes needed to synchronize text selection
handles have been added to RenderFrameMetadata.

Bug:  672962 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I0e5110ffa00d4d7720d0577b544a72690c8376c1
Reviewed-on: https://chromium-review.googlesource.com/1081173
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564527}
[modify] https://crrev.com/507ecab6119abfb0b9b7a1c27dd42b0185b64b05/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/507ecab6119abfb0b9b7a1c27dd42b0185b64b05/cc/trees/render_frame_metadata.cc
[modify] https://crrev.com/507ecab6119abfb0b9b7a1c27dd42b0185b64b05/cc/trees/render_frame_metadata.h
[modify] https://crrev.com/507ecab6119abfb0b9b7a1c27dd42b0185b64b05/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/507ecab6119abfb0b9b7a1c27dd42b0185b64b05/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/507ecab6119abfb0b9b7a1c27dd42b0185b64b05/content/common/render_frame_metadata.mojom
[modify] https://crrev.com/507ecab6119abfb0b9b7a1c27dd42b0185b64b05/content/common/render_frame_metadata_struct_traits.cc
[modify] https://crrev.com/507ecab6119abfb0b9b7a1c27dd42b0185b64b05/content/common/render_frame_metadata_struct_traits.h

Project Member

Comment 292 by bugdroid1@chromium.org, Jun 12 2018

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

commit 17a31897605229278ff39159d804253f10c11567
Author: Fady Samuel <fsamuel@chromium.org>
Date: Tue Jun 12 01:38:59 2018

Surface synchronization: Fix OOPIF resize

Allocating LocalSurfaceIds is an asynchronous process for OOPIFs because
they have to happen in the parent renderer. The first VisualProperties
object shipped to the child renderer has an invalid LocalSurfaceId which
suspends commits when surface synchronization is enabled.

For OOPIFs, we previously forced a new SynchronizeVisualProperties
push for OOPIF navigation which set the |visual_properties_pending_ack_|
on the browser side. This forced push meant that if nothing is changing renderer-side
then and ACK will not be sent back. This could cause a hang of renderer-resizes/visual
properties updates.

The reason we forced a push is a workaround for another bug. At some point, we pushed
a SynchronizeVisualProperties and set the pending bit, but we never got back an ACK.
We should only set the pending bit if the size has changed AND we have a valid
LocalSurfaceId.

Change-Id: Icd2baee36d6ebef3f7108b207821ef414a8e8a88
Bug:  672962 ,  850203 
Reviewed-on: https://chromium-review.googlesource.com/1095417
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566230}
[modify] https://crrev.com/17a31897605229278ff39159d804253f10c11567/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/17a31897605229278ff39159d804253f10c11567/content/browser/renderer_host/render_widget_host_unittest.cc
[modify] https://crrev.com/17a31897605229278ff39159d804253f10c11567/content/browser/renderer_host/render_widget_host_view_child_frame.cc

Project Member

Comment 293 by bugdroid1@chromium.org, Jun 12 2018

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

commit eed1194297dbe8160533ae40eca57f0a16a40cdf
Author: Fady Samuel <fsamuel@chromium.org>
Date: Tue Jun 12 15:48:53 2018

Surface synchronization: Accept update from parent if embed token changed

The ChildLocalSurfaceIdAllocator should accept updates from the parent if
the embed token changed. This can happen if an iframe performs a cross-process
navigation.

Bug:  672962 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I1f78a39d787bc7cf43b13e2092f4b5974eace954
Reviewed-on: https://chromium-review.googlesource.com/1096104
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566436}
[modify] https://crrev.com/eed1194297dbe8160533ae40eca57f0a16a40cdf/components/viz/common/surfaces/child_local_surface_id_allocator.cc
[modify] https://crrev.com/eed1194297dbe8160533ae40eca57f0a16a40cdf/components/viz/common/surfaces/child_local_surface_id_allocator_unittest.cc

Project Member

Comment 294 by bugdroid1@chromium.org, Jun 13 2018

Labels: merge-merged-3440
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9281f4c88dd2e8c7c7f0d79366a90d3a0a9d4dc5

commit 9281f4c88dd2e8c7c7f0d79366a90d3a0a9d4dc5
Author: Fady Samuel <fsamuel@chromium.org>
Date: Wed Jun 13 17:55:41 2018

Surface synchronization: Fix OOPIF resize

Allocating LocalSurfaceIds is an asynchronous process for OOPIFs because
they have to happen in the parent renderer. The first VisualProperties
object shipped to the child renderer has an invalid LocalSurfaceId which
suspends commits when surface synchronization is enabled.

For OOPIFs, we previously forced a new SynchronizeVisualProperties
push for OOPIF navigation which set the |visual_properties_pending_ack_|
on the browser side. This forced push meant that if nothing is changing renderer-side
then and ACK will not be sent back. This could cause a hang of renderer-resizes/visual
properties updates.

The reason we forced a push is a workaround for another bug. At some point, we pushed
a SynchronizeVisualProperties and set the pending bit, but we never got back an ACK.
We should only set the pending bit if the size has changed AND we have a valid
LocalSurfaceId.

Change-Id: Icd2baee36d6ebef3f7108b207821ef414a8e8a88
Bug:  672962 ,  850203 
Reviewed-on: https://chromium-review.googlesource.com/1095417
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#566230}(cherry picked from commit 17a31897605229278ff39159d804253f10c11567)
Reviewed-on: https://chromium-review.googlesource.com/1099515
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#335}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/9281f4c88dd2e8c7c7f0d79366a90d3a0a9d4dc5/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/9281f4c88dd2e8c7c7f0d79366a90d3a0a9d4dc5/content/browser/renderer_host/render_widget_host_unittest.cc
[modify] https://crrev.com/9281f4c88dd2e8c7c7f0d79366a90d3a0a9d4dc5/content/browser/renderer_host/render_widget_host_view_child_frame.cc

Project Member

Comment 295 by bugdroid1@chromium.org, Jun 14 2018

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

commit 4576fad344baac5eb8821459489a9c0f792576e9
Author: Fady Samuel <fsamuel@chromium.org>
Date: Thu Jun 14 01:06:01 2018

Surface synchronization: Plumb more things through RenderFrameMetadata

This CL plumbs through additional data when surface synchronization is on on Android for:

1. IME
2. Accessibility
3. OverScrollController
4. view FrameInfo
5. GestureListenerManager

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I812daf08fa00cfba2d92cffaf988604593299c81
Bug:  672962 
TBR: yfriedman@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/1097937
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567074}
[modify] https://crrev.com/4576fad344baac5eb8821459489a9c0f792576e9/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/4576fad344baac5eb8821459489a9c0f792576e9/cc/trees/render_frame_metadata.cc
[modify] https://crrev.com/4576fad344baac5eb8821459489a9c0f792576e9/cc/trees/render_frame_metadata.h
[modify] https://crrev.com/4576fad344baac5eb8821459489a9c0f792576e9/content/browser/android/overscroll_controller_android.cc
[modify] https://crrev.com/4576fad344baac5eb8821459489a9c0f792576e9/content/browser/android/overscroll_controller_android.h
[modify] https://crrev.com/4576fad344baac5eb8821459489a9c0f792576e9/content/browser/renderer_host/frame_metadata_util.cc
[modify] https://crrev.com/4576fad344baac5eb8821459489a9c0f792576e9/content/browser/renderer_host/frame_metadata_util.h
[modify] https://crrev.com/4576fad344baac5eb8821459489a9c0f792576e9/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/4576fad344baac5eb8821459489a9c0f792576e9/content/common/render_frame_metadata.mojom
[modify] https://crrev.com/4576fad344baac5eb8821459489a9c0f792576e9/content/common/render_frame_metadata_struct_traits.cc
[modify] https://crrev.com/4576fad344baac5eb8821459489a9c0f792576e9/content/common/render_frame_metadata_struct_traits.h

Project Member

Comment 296 by bugdroid1@chromium.org, Jun 14 2018

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

commit d2a810ba3b6d344962623aeccc4917d80ddf6750
Author: Fady Samuel <fsamuel@chromium.org>
Date: Thu Jun 14 01:38:02 2018

Surface synchronization: Plumb has_transparent_background

This CL plumbs the has_transparent_background bit through RenderFrameMetadata
on android when surface synchronization is enabled. This enables the correct
background color to be displayed when surface sync is on on android.

Bug:  672962 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Id8ca61371cbe04a7ea58fbbf255bf80f6208e2b8
Reviewed-on: https://chromium-review.googlesource.com/1100065
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567080}
[modify] https://crrev.com/d2a810ba3b6d344962623aeccc4917d80ddf6750/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/d2a810ba3b6d344962623aeccc4917d80ddf6750/cc/trees/layer_tree_host_impl.h
[modify] https://crrev.com/d2a810ba3b6d344962623aeccc4917d80ddf6750/cc/trees/render_frame_metadata.cc
[modify] https://crrev.com/d2a810ba3b6d344962623aeccc4917d80ddf6750/cc/trees/render_frame_metadata.h
[modify] https://crrev.com/d2a810ba3b6d344962623aeccc4917d80ddf6750/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/d2a810ba3b6d344962623aeccc4917d80ddf6750/content/common/render_frame_metadata.mojom
[modify] https://crrev.com/d2a810ba3b6d344962623aeccc4917d80ddf6750/content/common/render_frame_metadata_struct_traits.cc
[modify] https://crrev.com/d2a810ba3b6d344962623aeccc4917d80ddf6750/content/common/render_frame_metadata_struct_traits.h

Project Member

Comment 297 by bugdroid1@chromium.org, Jun 14 2018

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

commit 4675b1de9bac4d6e049bb054f104b3d2f1b6a672
Author: Fady Samuel <fsamuel@chromium.org>
Date: Thu Jun 14 20:50:00 2018

Surface synchronization: Only plumb android specific properties on android

Prior to this CL, we plumbed a lot of properties through RenderFrameMetadata that
were irrelevant to non-Android platforms. This CL reduces overhead on other platforms
by using the EnableIf decorator on fields in render_frame_metadata.mojom and
if defines in the render_frame_metadata.cc/h.

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I7b4bb58983ff641aa61ccde3a33f86cc12ec1ebc
Bug:  672962 
Reviewed-on: https://chromium-review.googlesource.com/1101061
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567396}
[modify] https://crrev.com/4675b1de9bac4d6e049bb054f104b3d2f1b6a672/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/4675b1de9bac4d6e049bb054f104b3d2f1b6a672/cc/trees/render_frame_metadata.cc
[modify] https://crrev.com/4675b1de9bac4d6e049bb054f104b3d2f1b6a672/cc/trees/render_frame_metadata.h
[modify] https://crrev.com/4675b1de9bac4d6e049bb054f104b3d2f1b6a672/content/common/render_frame_metadata.mojom
[modify] https://crrev.com/4675b1de9bac4d6e049bb054f104b3d2f1b6a672/content/common/render_frame_metadata_struct_traits.cc
[modify] https://crrev.com/4675b1de9bac4d6e049bb054f104b3d2f1b6a672/content/common/render_frame_metadata_struct_traits.h

Project Member

Comment 298 by bugdroid1@chromium.org, Jun 14 2018

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

commit f3982a30501b728bacce4396c97d17557671edc0
Author: Fady Samuel <fsamuel@chromium.org>
Date: Thu Jun 14 20:52:13 2018

Surface synchronization: Simplify returned resources

This CL simplifies returned resources in RenderWidgetHostViewAndroid to
better match how resources are returned on other platforms. Android did
a lot of weird things that are incompatible with OOP-D and surface
synchronization that worked fine assuming synchronous operations.

Bug:  672962 
Change-Id: Ibc980a0639752e126cbea9a44cb9df5ecc6e589a
Reviewed-on: https://chromium-review.googlesource.com/1101409
Reviewed-by: Saman Sami <samans@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567399}
[modify] https://crrev.com/f3982a30501b728bacce4396c97d17557671edc0/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/f3982a30501b728bacce4396c97d17557671edc0/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/f3982a30501b728bacce4396c97d17557671edc0/ui/android/delegated_frame_host_android.cc
[modify] https://crrev.com/f3982a30501b728bacce4396c97d17557671edc0/ui/android/delegated_frame_host_android.h
[modify] https://crrev.com/f3982a30501b728bacce4396c97d17557671edc0/ui/android/delegated_frame_host_android_unittest.cc

Project Member

Comment 299 by bugdroid1@chromium.org, Jun 18 2018

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

commit 6535d45903d9c80f4c783f602b5b2cf628368372
Author: Fady Samuel <fsamuel@chromium.org>
Date: Mon Jun 18 18:45:01 2018

Surface synchronization: Check surface invariants on Android

We would like to start a Finch trial for surface sync on Android soon.
This CL re-enables Android-specific surface invariants violations checks
on Android by removing code in RenderWidgetHostImpl that bypasses those
checks. This CL also ensures that when the background transparency
changes a new LocalSurfaceId is allocated as this matches the behavior
pre-surface-sync.

Bug:  672962 ,  789259 ,  801350 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Iad2dc6c508770314ea33eb53f05944609f29b0b9
Reviewed-on: https://chromium-review.googlesource.com/1104315
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568091}
[modify] https://crrev.com/6535d45903d9c80f4c783f602b5b2cf628368372/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/6535d45903d9c80f4c783f602b5b2cf628368372/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/6535d45903d9c80f4c783f602b5b2cf628368372/content/browser/renderer_host/render_widget_host_impl.h

Project Member

Comment 300 by bugdroid1@chromium.org, Jun 19 2018

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

commit d26f7fbdc87e4f9e9ee5c54e2f00d985eed36a7b
Author: Fady Samuel <fsamuel@chromium.org>
Date: Tue Jun 19 04:13:37 2018

cc: Make clearing caches explicit

We eventually want to get rid of the notion of content source ID in favor
of surface synchronization. In order to do this, we need to introduce an
explicit API to clear caches in cc.

This CL introduces LayerTreeHost::ClearCachesOnNextCommit API.

Bug:  672962 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ib78a5e7dde17f335c1e591032f7e01c1fa9213f9
Reviewed-on: https://chromium-review.googlesource.com/1105343
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568337}
[modify] https://crrev.com/d26f7fbdc87e4f9e9ee5c54e2f00d985eed36a7b/cc/scheduler/compositor_timing_history.cc
[modify] https://crrev.com/d26f7fbdc87e4f9e9ee5c54e2f00d985eed36a7b/cc/scheduler/compositor_timing_history.h
[modify] https://crrev.com/d26f7fbdc87e4f9e9ee5c54e2f00d985eed36a7b/cc/scheduler/scheduler.cc
[modify] https://crrev.com/d26f7fbdc87e4f9e9ee5c54e2f00d985eed36a7b/cc/scheduler/scheduler.h
[modify] https://crrev.com/d26f7fbdc87e4f9e9ee5c54e2f00d985eed36a7b/cc/scheduler/scheduler_unittest.cc
[modify] https://crrev.com/d26f7fbdc87e4f9e9ee5c54e2f00d985eed36a7b/cc/test/fake_proxy.h
[modify] https://crrev.com/d26f7fbdc87e4f9e9ee5c54e2f00d985eed36a7b/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/d26f7fbdc87e4f9e9ee5c54e2f00d985eed36a7b/cc/trees/layer_tree_host.h
[modify] https://crrev.com/d26f7fbdc87e4f9e9ee5c54e2f00d985eed36a7b/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/d26f7fbdc87e4f9e9ee5c54e2f00d985eed36a7b/cc/trees/layer_tree_host_impl.h
[modify] https://crrev.com/d26f7fbdc87e4f9e9ee5c54e2f00d985eed36a7b/cc/trees/proxy.h
[modify] https://crrev.com/d26f7fbdc87e4f9e9ee5c54e2f00d985eed36a7b/cc/trees/proxy_impl.cc
[modify] https://crrev.com/d26f7fbdc87e4f9e9ee5c54e2f00d985eed36a7b/cc/trees/proxy_impl.h
[modify] https://crrev.com/d26f7fbdc87e4f9e9ee5c54e2f00d985eed36a7b/cc/trees/proxy_main.cc
[modify] https://crrev.com/d26f7fbdc87e4f9e9ee5c54e2f00d985eed36a7b/cc/trees/proxy_main.h
[modify] https://crrev.com/d26f7fbdc87e4f9e9ee5c54e2f00d985eed36a7b/cc/trees/single_thread_proxy.cc
[modify] https://crrev.com/d26f7fbdc87e4f9e9ee5c54e2f00d985eed36a7b/cc/trees/single_thread_proxy.h
[modify] https://crrev.com/d26f7fbdc87e4f9e9ee5c54e2f00d985eed36a7b/content/renderer/gpu/render_widget_compositor.cc
[modify] https://crrev.com/d26f7fbdc87e4f9e9ee5c54e2f00d985eed36a7b/content/renderer/gpu/render_widget_compositor.h
[modify] https://crrev.com/d26f7fbdc87e4f9e9ee5c54e2f00d985eed36a7b/content/renderer/render_widget.cc

Project Member

Comment 301 by bugdroid1@chromium.org, Jun 19 2018

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

commit 880f62b8d5fbaf17d6a0b4efc494d6a7279ec8a6
Author: Fady Samuel <fsamuel@chromium.org>
Date: Tue Jun 19 18:08:01 2018

Surface Synchronization: Simplify RenderWidgetHostViewAura

RenderWidgetHostViewAura had a SyncSurfaceProperties and
SynchronizeVisualProperties. The only difference is
SynchronizeVisualProperties took an optional LocalSurfaceId and
updated the ParentLocalSurfaceIdAllocator using that Id if specified.
If unspecified, it allocated a new ID.

This CL merges SyncSurfaceProperties into SynchronizeVisualProperties
thereby making the code a tiny bit easier to understand.

Bug:  672962 
Change-Id: I902f6d86b37a2a4d33bef00b4e30014d2c15f20c
Reviewed-on: https://chromium-review.googlesource.com/1106237
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568529}
[modify] https://crrev.com/880f62b8d5fbaf17d6a0b4efc494d6a7279ec8a6/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/880f62b8d5fbaf17d6a0b4efc494d6a7279ec8a6/content/browser/renderer_host/render_widget_host_view_aura.h

Blockedon: 854307
Project Member

Comment 303 by bugdroid1@chromium.org, Jun 21 2018

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

commit 6ca67282f7e406dd83b31a526d601423f4f026e7
Author: Fady Samuel <fsamuel@chromium.org>
Date: Thu Jun 21 23:00:41 2018

Surface Synchronization: Reset deadline on invalid SurfaceId

If a SurfaceLayer is given an invalid SurfaceId then it should not block
on that invalid SurfaceId and the specified deadline should play no part
in the deadline of the CompositorFrame.

Bug:  672962 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I71c31315ff0dbcd0ab9697e98c5f58aa3691bf67
Reviewed-on: https://chromium-review.googlesource.com/1110028
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569406}
[modify] https://crrev.com/6ca67282f7e406dd83b31a526d601423f4f026e7/cc/layers/surface_layer.cc
[modify] https://crrev.com/6ca67282f7e406dd83b31a526d601423f4f026e7/cc/layers/surface_layer_unittest.cc
[modify] https://crrev.com/6ca67282f7e406dd83b31a526d601423f4f026e7/ui/compositor/layer_unittest.cc

Project Member

Comment 304 by bugdroid1@chromium.org, Jun 22 2018

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

commit e59b611ddde7200d14b873183bc1ac48f5747a41
Author: Fady Samuel <fsamuel@chromium.org>
Date: Fri Jun 22 21:25:14 2018

Surface synchronization: Remove check in LayerTreeImpl

We still get the occasional surface invariants violations in the wild and
we don't necessarily want to crash the renderer in this case. We will already
log things appropriately.

Bug:  672962 , 828741
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I262850f2cf704cd59366da6dd4d154227aab5220
Reviewed-on: https://chromium-review.googlesource.com/1112491
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569781}
[modify] https://crrev.com/e59b611ddde7200d14b873183bc1ac48f5747a41/cc/trees/layer_tree_impl.cc

Project Member

Comment 305 by bugdroid1@chromium.org, Jun 24 2018

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

commit 365f2933e8f72ccaae5efa972141504f0caad015
Author: Fady Samuel <fsamuel@chromium.org>
Date: Sun Jun 24 14:14:50 2018

Surface Synchronization: Update RenderFrameMetadataProvider::Observer

This CL renames the method OnRenderFrameMetadataChanged to
OnRenderFrameMetadataChangedAfterActivation and introduces a new
method: OnRenderFrameMetadataChangedBeforeActivation.

OnRenderFrameMetadataChangedBeforeActivation is called as soon as the
RenderFrameMetadata arrives in the browser process.

OnRenderFrameMetadataChangedAfterActivation is called after the corresponding
frame token arrives from Viz. This frame token corresponds to a surface
activation.

The BeforeActivation method will be used by Surface Sync for Android to
update the overscroll controller which controls the glow effect when
scrolling.

This effect should be roughly synchronized with the renderer but does
not have to be exactly synchronized as that introduces a lot of overhead.

Bug:  672962 
Change-Id: I6c1ac68c6a65f0c074dfe519311ecdb83908d76b
Reviewed-on: https://chromium-review.googlesource.com/1112074
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569931}
[modify] https://crrev.com/365f2933e8f72ccaae5efa972141504f0caad015/content/browser/renderer_host/render_frame_metadata_provider_impl.cc
[modify] https://crrev.com/365f2933e8f72ccaae5efa972141504f0caad015/content/browser/renderer_host/render_frame_metadata_provider_impl.h
[modify] https://crrev.com/365f2933e8f72ccaae5efa972141504f0caad015/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/365f2933e8f72ccaae5efa972141504f0caad015/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/365f2933e8f72ccaae5efa972141504f0caad015/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/365f2933e8f72ccaae5efa972141504f0caad015/content/browser/renderer_host/render_widget_host_view_aura.h
[modify] https://crrev.com/365f2933e8f72ccaae5efa972141504f0caad015/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/365f2933e8f72ccaae5efa972141504f0caad015/content/browser/renderer_host/render_widget_host_view_base.cc
[modify] https://crrev.com/365f2933e8f72ccaae5efa972141504f0caad015/content/browser/renderer_host/render_widget_host_view_base.h
[modify] https://crrev.com/365f2933e8f72ccaae5efa972141504f0caad015/content/browser/renderer_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/365f2933e8f72ccaae5efa972141504f0caad015/content/browser/renderer_host/render_widget_host_view_child_frame.h
[modify] https://crrev.com/365f2933e8f72ccaae5efa972141504f0caad015/content/browser/renderer_host/render_widget_host_view_mac.h
[modify] https://crrev.com/365f2933e8f72ccaae5efa972141504f0caad015/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/365f2933e8f72ccaae5efa972141504f0caad015/content/public/browser/render_frame_metadata_provider.h
[modify] https://crrev.com/365f2933e8f72ccaae5efa972141504f0caad015/content/public/test/browser_test_utils.cc
[modify] https://crrev.com/365f2933e8f72ccaae5efa972141504f0caad015/content/public/test/browser_test_utils.h

Project Member

Comment 306 by bugdroid1@chromium.org, Jun 25 2018

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

commit c6bd1218e88d15c43a495a638c8cd65105c146e4
Author: akaba <akaba@google.com>
Date: Mon Jun 25 20:10:48 2018

Use Surface Synchronization for desktop zoom.

This CL rewires zoom to use surface synchronization instead of page messages which solves the problem of OOPIFs zooming faster than their embedding frame,
In other words this allows zoom to be executed atomically for a web page containing OOPIFs.

Design Doc (Draft): https://docs.google.com/document/d/1J7BTRsylGApm6KHaaTu-m6LLvSWJgf1B9CM-USKIp1k/edit?usp=sharing

Bug:  819611 , 672962 
Change-Id: Ifc61b47886fd8ebd99c98851442b9caf59905413
Reviewed-on: https://chromium-review.googlesource.com/1095557
Commit-Queue: Andre Kaba <akaba@google.com>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570143}
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/browser/browser_plugin/browser_plugin_guest.cc
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/browser/browser_plugin/browser_plugin_guest.h
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/browser/frame_host/cross_process_frame_connector.cc
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/browser/frame_host/cross_process_frame_connector.h
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/browser/host_zoom_map_impl.cc
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/browser/host_zoom_map_impl.h
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/browser/renderer_host/render_view_host_delegate.cc
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/browser/renderer_host/render_view_host_delegate.h
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/browser/renderer_host/render_view_host_impl.cc
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/browser/renderer_host/render_widget_host_delegate.cc
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/browser/renderer_host/render_widget_host_delegate.h
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/browser/renderer_host/render_widget_host_unittest.cc
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/browser/web_contents/web_contents_impl.h
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/common/BUILD.gn
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/common/frame_messages.h
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/common/frame_visual_properties.h
[delete] https://crrev.com/216b806d4d01e50a729039219f693184b1896ef6/content/common/page_message_enums.h
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/common/page_messages.h
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/common/renderer.mojom
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/common/view_messages.h
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/common/visual_properties.h
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/public/renderer/render_frame.h
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/public/renderer/render_view.h
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/renderer/browser_plugin/browser_plugin.cc
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/renderer/browser_plugin/browser_plugin.h
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/renderer/render_frame_impl.h
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/renderer/render_frame_proxy.cc
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/renderer/render_frame_proxy.h
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/renderer/render_view_browsertest.cc
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/renderer/render_view_impl.cc
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/renderer/render_view_impl.h
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/renderer/render_widget.cc
[modify] https://crrev.com/c6bd1218e88d15c43a495a638c8cd65105c146e4/content/renderer/render_widget.h

Project Member

Comment 307 by bugdroid1@chromium.org, Jun 25 2018

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

commit 0f108c08b8916d7915aaecca25c365d24f6438aa
Author: Fady Samuel <fsamuel@chromium.org>
Date: Mon Jun 25 20:19:50 2018

Surface Synchronization: Don't ship RenderFrameMetadata on change in scroll

We don't need to ship updated RenderFrameMetadata on change in root scroll
offset. Shipping RenderFrameMetadata is expensive and so we should minimize
shipping them to the browser process as it also involves synchronization
with Viz.

We initially decided to plumb this value via RenderFrameMetadata in order
to properly support the overscroll glow on Android but this turned out to
be too expensive and so we are now investigating ways to compute the
overscroll glow at least partially in the renderer.

Bug:  672962 ,  855473 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I139f773eaf673785fec04c9f989abb36e0941ad9
Reviewed-on: https://chromium-review.googlesource.com/1113799
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570153}
[modify] https://crrev.com/0f108c08b8916d7915aaecca25c365d24f6438aa/cc/trees/render_frame_metadata.cc

Project Member

Comment 308 by bugdroid1@chromium.org, Jun 26 2018

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

commit de0528bcb0fc216f92c838b83d05eb87657e4453
Author: Tien-Ren Chen <trchen@chromium.org>
Date: Tue Jun 26 00:32:06 2018

[viz] Fix deadlock due to pending frame refering to deleted frame sink

A pending frame with no deadline can never activate if it refers to
a future surface ID from a deleted sink. This blocks future valid frames
to be generated, causing a deadlock.

This CL makes SurfaceDependencyTracker to unblock dependencies to
all future surface IDs for a deleted frame sink.

BUG= 667551 , 672962 

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I90ecd75d575b41211cfd104184657f5c89ffb266
Reviewed-on: https://chromium-review.googlesource.com/1111273
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Tien-Ren Chen <trchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570266}
[modify] https://crrev.com/de0528bcb0fc216f92c838b83d05eb87657e4453/components/viz/common/surfaces/local_surface_id.h
[modify] https://crrev.com/de0528bcb0fc216f92c838b83d05eb87657e4453/components/viz/common/surfaces/surface_id.h
[modify] https://crrev.com/de0528bcb0fc216f92c838b83d05eb87657e4453/components/viz/service/frame_sinks/surface_synchronization_unittest.cc
[modify] https://crrev.com/de0528bcb0fc216f92c838b83d05eb87657e4453/components/viz/service/surfaces/surface_dependency_tracker.cc
[modify] https://crrev.com/de0528bcb0fc216f92c838b83d05eb87657e4453/components/viz/service/surfaces/surface_dependency_tracker.h
[modify] https://crrev.com/de0528bcb0fc216f92c838b83d05eb87657e4453/components/viz/service/surfaces/surface_manager.cc

Project Member

Comment 309 by bugdroid1@chromium.org, Jun 26 2018

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

commit 66c0789b3a82178c1b2b868192ff750fc1b6a096
Author: Fady Samuel <fsamuel@chromium.org>
Date: Tue Jun 26 18:56:57 2018

Surface Syncrhonization: Move HasAlwaysUpdateMetadataChanged out of RenderFrameMetadata

In a subsequent patch, the decision whether to ship a RenderFrameMetadata
to the browser process depends on on the scroll position relative to the edge
of the root layer. This complexity doesn't really belong in
cc::RenderFrameMetadata itself but rather its consumer. This CL moves the
code of whether to ship a RenderFrameMetadata out of the struct itself.

Bug:  672962 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I59b001088d3d83df7bd0dc9fe569e439af3fdeb9
Reviewed-on: https://chromium-review.googlesource.com/1115348
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570478}
[modify] https://crrev.com/66c0789b3a82178c1b2b868192ff750fc1b6a096/cc/trees/render_frame_metadata.cc
[modify] https://crrev.com/66c0789b3a82178c1b2b868192ff750fc1b6a096/cc/trees/render_frame_metadata.h
[modify] https://crrev.com/66c0789b3a82178c1b2b868192ff750fc1b6a096/content/renderer/OWNERS
[modify] https://crrev.com/66c0789b3a82178c1b2b868192ff750fc1b6a096/content/renderer/render_frame_metadata_observer_impl.cc
[modify] https://crrev.com/66c0789b3a82178c1b2b868192ff750fc1b6a096/content/renderer/render_frame_metadata_observer_impl.h

Project Member

Comment 310 by bugdroid1@chromium.org, Jun 26 2018

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

commit 7ef308d638cb792d9465488adba34393f35732de
Author: Fady Samuel <fsamuel@chromium.org>
Date: Tue Jun 26 23:11:23 2018

Surface Synchronization: Optimize top bar controls and overscroll glow

Shipping scroll offset every frame during a scroll to the browser process
is very expensive. It has been shown to negatively impact performance.

Instead, this CL will only ship the scroll offset if it's close to an
edge of the root layer where glow needs to be calculated. Furthermore,
blocking frames on top bar control synchronization introduces jank
and can affect FPS during scroll. This CL moves over synchronization to
best effort.

All (best-effort) synchronized visuals are moved to
OnRenderFrameMetadataChangedBeforeActivation (which is always called
BEFORE OnDidUpdateVisualPropertiesComplete) and will be called even
if the LocalSurfaceId does not change.

Bug:  672962 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I63953227e49a6b5b1b543b4fa16bac2cb336f100
Reviewed-on: https://chromium-review.googlesource.com/1115590
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570572}
[modify] https://crrev.com/7ef308d638cb792d9465488adba34393f35732de/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/7ef308d638cb792d9465488adba34393f35732de/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/7ef308d638cb792d9465488adba34393f35732de/content/renderer/render_frame_metadata_observer_impl.cc

Project Member

Comment 311 by bugdroid1@chromium.org, Jun 28 2018

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

commit 5185f1dd535accae2438bcd570e6d30fcca7b695
Author: Fady Samuel <fsamuel@chromium.org>
Date: Thu Jun 28 02:25:10 2018

Surface Synchronization: Skip activation messages from Viz on Android

Android use cases do not need to worry about
OnRenderFrameMetadataChangedAfterActivation notifications because
changes either do not require synchronization or are synchronized
directly via surface synchronization. This CL saves a few IPCs and should
improve scrolling performance on Android when surface synchronization is
enabled.

Bug:  672962 
Change-Id: I3fe26c56dde75394677ee2dee53f50f79e99f8f9
Reviewed-on: https://chromium-review.googlesource.com/1117198
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571003}
[modify] https://crrev.com/5185f1dd535accae2438bcd570e6d30fcca7b695/content/browser/renderer_host/render_frame_metadata_provider_impl.cc
[modify] https://crrev.com/5185f1dd535accae2438bcd570e6d30fcca7b695/content/renderer/render_frame_metadata_observer_impl.cc
[modify] https://crrev.com/5185f1dd535accae2438bcd570e6d30fcca7b695/content/renderer/render_frame_metadata_observer_impl.h
[add] https://crrev.com/5185f1dd535accae2438bcd570e6d30fcca7b695/content/renderer/render_frame_metadata_observer_impl_unittest.cc
[modify] https://crrev.com/5185f1dd535accae2438bcd570e6d30fcca7b695/content/test/BUILD.gn

Project Member

Comment 312 by bugdroid1@chromium.org, Jun 29 2018

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

commit 3b77b8a50cd6a869c187fb8bd41f669a7052def4
Author: Fady Samuel <fsamuel@chromium.org>
Date: Fri Jun 29 06:53:53 2018

Surface Synchronization: Don't use CompositorLock if enabled

Don't use the CompositorLock if surface synchronization is enabled.
This was causing issues on telemetry bots where the CompositorLock
was preventing the UI from submitting CompositorFrames for a long
period of time and so input latency was very high.

Before:
https://pinpoint-dot-chromeperf.appspot.com/job/11675b9b240000
After:
https://pinpoint-dot-chromeperf.appspot.com/job/152a7367240000

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I95407531216b543fb4c192387bd67fe690bee0b4
Bug:  672962 
Reviewed-on: https://chromium-review.googlesource.com/1119191
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571414}
[modify] https://crrev.com/3b77b8a50cd6a869c187fb8bd41f669a7052def4/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/3b77b8a50cd6a869c187fb8bd41f669a7052def4/ui/android/delegated_frame_host_android.cc
[modify] https://crrev.com/3b77b8a50cd6a869c187fb8bd41f669a7052def4/ui/android/delegated_frame_host_android.h
[modify] https://crrev.com/3b77b8a50cd6a869c187fb8bd41f669a7052def4/ui/android/delegated_frame_host_android_unittest.cc

Project Member

Comment 313 by bugdroid1@chromium.org, Jun 29 2018

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

commit 51b9df85a56b4f7aeca337dab4841ac5ef9a358b
Author: Fady Samuel <fsamuel@chromium.org>
Date: Fri Jun 29 19:48:16 2018

Surface Synchronization: Re-enable ScrollLatencyBrowserTest.SmoothWheelScroll

This test appears to be running fine locally now. I suspect the hang was
caused by the incorrect flag being checked that was fixed here:

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

Bug:  855920 ,  672962 
Change-Id: Ic9b27c80b3070011e5a06c3ba9153a7d464108e4
Reviewed-on: https://chromium-review.googlesource.com/1120147
Reviewed-by: Eric Karl <ericrk@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571599}
[modify] https://crrev.com/51b9df85a56b4f7aeca337dab4841ac5ef9a358b/testing/buildbot/filters/surface_sync.content_browsertests.filter

Project Member

Comment 314 by bugdroid1@chromium.org, Jul 4

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

commit c76cc200c163d7f39242ddfab675158c2169fca8
Author: Fady Samuel <fsamuel@chromium.org>
Date: Wed Jul 04 01:23:34 2018

Surface Synchronization: Implement SurfaceId flows

This CL implements two types of tracing flows:

1. Embed Flow: The flow from allocating a LocalSurfaceId to embedding it in
a SurfaceLayer and SurfaceLayerImpl for the first time until a reference is
added for the first time to a parent CompositorFrame.

2. Submission Flow: The flow from allocating a LocalSurfaceId to passing it
to the child to use to submit CompositorFrames.

These flows let us debug surface synchronization performance regressions and
lets one visually see the parallelism achieved with surface synchronization.

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I02b7baec283e60528faa24dffdca74b641135d2e
Bug:  672962 
Reviewed-on: https://chromium-review.googlesource.com/1124976
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572429}
[modify] https://crrev.com/c76cc200c163d7f39242ddfab675158c2169fca8/cc/layers/surface_layer.cc
[modify] https://crrev.com/c76cc200c163d7f39242ddfab675158c2169fca8/cc/layers/surface_layer_impl.cc
[modify] https://crrev.com/c76cc200c163d7f39242ddfab675158c2169fca8/cc/mojo_embedder/async_layer_tree_frame_sink.cc
[modify] https://crrev.com/c76cc200c163d7f39242ddfab675158c2169fca8/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/c76cc200c163d7f39242ddfab675158c2169fca8/components/viz/common/surfaces/child_local_surface_id_allocator.cc
[modify] https://crrev.com/c76cc200c163d7f39242ddfab675158c2169fca8/components/viz/common/surfaces/local_surface_id.h
[modify] https://crrev.com/c76cc200c163d7f39242ddfab675158c2169fca8/components/viz/common/surfaces/parent_local_surface_id_allocator.cc
[modify] https://crrev.com/c76cc200c163d7f39242ddfab675158c2169fca8/components/viz/service/frame_sinks/compositor_frame_sink_support.cc
[modify] https://crrev.com/c76cc200c163d7f39242ddfab675158c2169fca8/components/viz/service/surfaces/surface.cc
[modify] https://crrev.com/c76cc200c163d7f39242ddfab675158c2169fca8/components/viz/service/surfaces/surface_manager.cc

Project Member

Comment 315 by bugdroid1@chromium.org, Jul 4

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

commit 29e08ac9297d16b9222f9952e2e59a13b6a1a531
Author: Fady Samuel <fsamuel@chromium.org>
Date: Wed Jul 04 18:02:03 2018

Surface Synchronization: Don't force commit on content_source_id

It should never be necessary to force a commit when a content_source_id changes.

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I58bd6d840feca9df58bd674e2a42cf987bba2ed5
Bug: 847383,  672962 
Reviewed-on: https://chromium-review.googlesource.com/1124760
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572619}
[modify] https://crrev.com/29e08ac9297d16b9222f9952e2e59a13b6a1a531/cc/trees/layer_tree_host.cc

Cc: -varkha@chromium.org gklassen@chromium.org
Project Member

Comment 317 by bugdroid1@chromium.org, Jul 4

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

commit d79254017d169d80ba115721f307c8072416a605
Author: Fady Samuel <fsamuel@chromium.org>
Date: Wed Jul 04 18:14:30 2018

Surface Synchronization: Avoid spurious commits

Prior to this CL, an impl-side child allocated LocalSurfaceId would cause
a spurious commit on the main thread as the new LocalSurfaceId propagates its
way back to the main thread from the parent renderer.

LayerTreeHost::SetLocalSurfaceIdFromParent really only cares about changes
to the parent_sequence_number and embed_token to propagate to the impl thread.
Other changes are irrelevant and should not cause commits.

This CL fixes the issue by early exiting in
LayerTreeHost::SetLocalSurfaceIdFromParent. This problem was cause by looking
at SurfaceId flows. This will likely improve scrolling performance a bit on
Android.

Bug:  672962 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ide4f9df9d89c70216e9a54dfc6ccd298cf046821
Reviewed-on: https://chromium-review.googlesource.com/1126325
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572622}
[modify] https://crrev.com/d79254017d169d80ba115721f307c8072416a605/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/d79254017d169d80ba115721f307c8072416a605/cc/trees/layer_tree_host_unittest.cc

Project Member

Comment 318 by bugdroid1@chromium.org, Jul 4

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

commit 0056bbe81475742420c1cdc069497e190b245c95
Author: Fady Samuel <fsamuel@chromium.org>
Date: Wed Jul 04 19:33:45 2018

Surface Synchronization: Improve Embed Flow

Prior to this CL, the embed flow used in trace viewer terminated when
the parent updated its surface references. This did not really model
the user experience very well. What we'd actually like to measure is the
time it takes for a flow to make it on screen. This CL introduces a
FirstSurfaceEmbedding trace event as the new terminating step for
the embed flow to better capture the user experience.

Bug:  672962 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I3fa57973e739e65907ab7922c1c957566820f0bd
Reviewed-on: https://chromium-review.googlesource.com/1126209
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572633}
[modify] https://crrev.com/0056bbe81475742420c1cdc069497e190b245c95/components/viz/service/surfaces/surface.cc
[modify] https://crrev.com/0056bbe81475742420c1cdc069497e190b245c95/components/viz/service/surfaces/surface.h
[modify] https://crrev.com/0056bbe81475742420c1cdc069497e190b245c95/components/viz/service/surfaces/surface_manager.cc

Project Member

Comment 319 by bugdroid1@chromium.org, Jul 4

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

commit 61506912f15f5b15cf8ada0256aa65bcb904b6df
Author: Fady Samuel <fsamuel@chromium.org>
Date: Wed Jul 04 20:56:17 2018

Surface synchronization: Remove check in LayerTreeImpl

We still get the occasional surface invariants violations in the wild and
we don't necessarily want to crash the renderer in this case. We will already
log things appropriately.

Bug:  672962 , 828741
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I262850f2cf704cd59366da6dd4d154227aab5220
Reviewed-on: https://chromium-review.googlesource.com/1112491
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#569781}(cherry picked from commit e59b611ddde7200d14b873183bc1ac48f5747a41)
Reviewed-on: https://chromium-review.googlesource.com/1126500
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#599}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/61506912f15f5b15cf8ada0256aa65bcb904b6df/cc/trees/layer_tree_impl.cc

Project Member

Comment 320 by bugdroid1@chromium.org, Jul 4

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

commit 28a215cd0d4c11142cb0dea93a2a856dccd4287b
Author: Fady Samuel <fsamuel@chromium.org>
Date: Wed Jul 04 23:44:54 2018

Surface Synchronization: add viz.surface_lifetime category for traces

It's often useful to take a look at the lifetime of a Surface to see how much
things change over time. This CL introduces async trace events to track the
lifetime of a surface.

Bug:  672962 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: Ibde1e49d0c602b3ad3066b48fd21af172a327b3b
Reviewed-on: https://chromium-review.googlesource.com/1126495
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572677}
[modify] https://crrev.com/28a215cd0d4c11142cb0dea93a2a856dccd4287b/components/viz/service/surfaces/surface.cc

Project Member

Comment 321 by bugdroid1@chromium.org, Jul 9

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

commit d320d43b02f19c6e8ac67df41aa98b94d3a2548c
Author: Fady Samuel <fsamuel@chromium.org>
Date: Mon Jul 09 18:00:23 2018

Surface Synchronization: Update terminology for OOPIF and GuestView

Prior to this CL, we used to call "FirstSurfaceActivation"
"SetChildFrameSurface" when plumbed up to the parent renderer for
OOPIF and GuestView.

On the one hand, what we plumb up isn't always "FirstSurfaceActivation"
because we plumb up SetChildFrameSurface on reparenting too.

That's wrong though. We shouldn't be reusing the same child surface on
reparenting. Instead, we should be requesting a new LocalSurfaceId with
a new embed_token from the parent on reparenting.

By renaming this method to "FirstSurfaceActivation", these oddities
are exposed and I've added corresponding TODOs to fix them.

Bug:  672962 
TBR: creis@chromium.org for cross_process_frame_connector
Change-Id: I583f74bbb3d3b909604728cb606130b0386af6dd
Reviewed-on: https://chromium-review.googlesource.com/1127139
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573364}
[modify] https://crrev.com/d320d43b02f19c6e8ac67df41aa98b94d3a2548c/content/browser/browser_plugin/browser_plugin_guest.cc
[modify] https://crrev.com/d320d43b02f19c6e8ac67df41aa98b94d3a2548c/content/browser/browser_plugin/browser_plugin_guest.h
[modify] https://crrev.com/d320d43b02f19c6e8ac67df41aa98b94d3a2548c/content/browser/frame_host/cross_process_frame_connector.cc
[modify] https://crrev.com/d320d43b02f19c6e8ac67df41aa98b94d3a2548c/content/browser/frame_host/cross_process_frame_connector.h
[modify] https://crrev.com/d320d43b02f19c6e8ac67df41aa98b94d3a2548c/content/browser/frame_host/render_widget_host_view_guest.cc
[modify] https://crrev.com/d320d43b02f19c6e8ac67df41aa98b94d3a2548c/content/browser/frame_host/render_widget_host_view_guest.h
[modify] https://crrev.com/d320d43b02f19c6e8ac67df41aa98b94d3a2548c/content/browser/frame_host/render_widget_host_view_guest_unittest.cc
[modify] https://crrev.com/d320d43b02f19c6e8ac67df41aa98b94d3a2548c/content/browser/renderer_host/frame_connector_delegate.h
[modify] https://crrev.com/d320d43b02f19c6e8ac67df41aa98b94d3a2548c/content/browser/renderer_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/d320d43b02f19c6e8ac67df41aa98b94d3a2548c/content/browser/renderer_host/render_widget_host_view_child_frame.h
[modify] https://crrev.com/d320d43b02f19c6e8ac67df41aa98b94d3a2548c/content/browser/renderer_host/render_widget_host_view_child_frame_unittest.cc
[modify] https://crrev.com/d320d43b02f19c6e8ac67df41aa98b94d3a2548c/content/common/browser_plugin/browser_plugin_messages.h
[modify] https://crrev.com/d320d43b02f19c6e8ac67df41aa98b94d3a2548c/content/common/frame_messages.h
[modify] https://crrev.com/d320d43b02f19c6e8ac67df41aa98b94d3a2548c/content/renderer/browser_plugin/browser_plugin.cc
[modify] https://crrev.com/d320d43b02f19c6e8ac67df41aa98b94d3a2548c/content/renderer/browser_plugin/browser_plugin.h
[modify] https://crrev.com/d320d43b02f19c6e8ac67df41aa98b94d3a2548c/content/renderer/render_frame_proxy.cc
[modify] https://crrev.com/d320d43b02f19c6e8ac67df41aa98b94d3a2548c/content/renderer/render_frame_proxy.h

Project Member

Comment 322 by bugdroid1@chromium.org, Jul 11

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

commit bcac6f000270046070d25367009e69b2ee67d49a
Author: Fady Samuel <fsamuel@chromium.org>
Date: Wed Jul 11 02:02:42 2018

Surface Synchronization: Ensure primary always updated with fallback when off

There is a case on Android where we'll update the fallback surface without
updating the primary surface ID as well when surface synchronization is off.

This might be the cause of the occasional deadlines triggering when surface
sync is off.

Bug:  672962 
Change-Id: I87133cc11b369a2097adc3b2a2bcd7f2781e1b30
Reviewed-on: https://chromium-review.googlesource.com/1131816
Reviewed-by: Khushal <khushalsagar@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574037}
[modify] https://crrev.com/bcac6f000270046070d25367009e69b2ee67d49a/ui/android/delegated_frame_host_android.cc
[modify] https://crrev.com/bcac6f000270046070d25367009e69b2ee67d49a/ui/android/delegated_frame_host_android.h
[modify] https://crrev.com/bcac6f000270046070d25367009e69b2ee67d49a/ui/android/delegated_frame_host_android_unittest.cc

Project Member

Comment 323 by bugdroid1@chromium.org, Jul 11

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

commit 852c66f84007b6c5ee4eb6150ec2534d5797b5b4
Author: Fady Samuel <fsamuel@chromium.org>
Date: Wed Jul 11 23:38:32 2018

Surface Synchronization: Make Submission Flow Trace Less Noisy

Only log an event if the LocalSurfaceId has changed instead of on
every submit and receipt of CompositorFrames.

Bug:  672962 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I4814ec0aaa9c204864d5c5df0c4a359d4ef3bb28
Reviewed-on: https://chromium-review.googlesource.com/1134027
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574409}
[modify] https://crrev.com/852c66f84007b6c5ee4eb6150ec2534d5797b5b4/cc/mojo_embedder/async_layer_tree_frame_sink.cc
[modify] https://crrev.com/852c66f84007b6c5ee4eb6150ec2534d5797b5b4/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/852c66f84007b6c5ee4eb6150ec2534d5797b5b4/components/viz/common/surfaces/parent_local_surface_id_allocator.cc
[modify] https://crrev.com/852c66f84007b6c5ee4eb6150ec2534d5797b5b4/components/viz/service/frame_sinks/compositor_frame_sink_support.cc
[modify] https://crrev.com/852c66f84007b6c5ee4eb6150ec2534d5797b5b4/components/viz/service/surfaces/surface.cc

Project Member

Comment 324 by bugdroid1@chromium.org, Jul 16

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

commit f1a9e88d8e5e7cbe5945790e2a135125a756f97b
Author: Fady Samuel <fsamuel@chromium.org>
Date: Mon Jul 16 20:28:24 2018

Surface Synchronization: Only defer commits if frame exists

Right now, with surface sync off on Android, we will defer commits
any time there's a size change, even if we've evicted the current
surface.

Change-Id: I7e1ae394e20eef2a46bca5ae9a8fc8a4cdf35e0b
Bug:  848292 ,  672962 
Reviewed-on: https://chromium-review.googlesource.com/1121098
Reviewed-by: Saman Sami <samans@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575405}
[modify] https://crrev.com/f1a9e88d8e5e7cbe5945790e2a135125a756f97b/ui/android/delegated_frame_host_android.cc

Project Member

Comment 325 by bugdroid1@chromium.org, Jul 16

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

commit 99ec37a96ac67df53307ddf437bd170cd7be2b24
Author: Fady Samuel <fsamuel@chromium.org>
Date: Mon Jul 16 21:43:47 2018

Surface synchronization: Use longer deadlines when appropriate

When surface synchronization is off, we lock the UI compositor on
Android during the first frame and resize in order to avoid producing
a CompositorFrame until the renderer has an appropriate frame ready.

Prior to this CL, with surface sync on, we used the default deadline
for synchronization. With this CL, we use the same deadlines that were
used prior to surface sync on versions of Android O or newer.

On versions older than O, we set the deadline for resize to 0.

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: Idb8f2b3891a9ae1bd43be4d81dbec3ccdb5ff436
Bug:  672962 ,  857542 
Reviewed-on: https://chromium-review.googlesource.com/1132324
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Reviewed-by: Eric Karl <ericrk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575436}
[modify] https://crrev.com/99ec37a96ac67df53307ddf437bd170cd7be2b24/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/99ec37a96ac67df53307ddf437bd170cd7be2b24/ui/android/delegated_frame_host_android.cc
[modify] https://crrev.com/99ec37a96ac67df53307ddf437bd170cd7be2b24/ui/android/delegated_frame_host_android.h

Project Member

Comment 326 by bugdroid1@chromium.org, Jul 19

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

commit 0c2f5b89f413fdff8e4d4c0f447705170fa462fd
Author: Fady Samuel <fsamuel@chromium.org>
Date: Thu Jul 19 00:33:46 2018

Pass FrameSinkId by const ref

RenderWidgetHostViewBase::GetFrameSinkId passed the viz::FrameSinkId
by value and was not a const method.

This CL returns a const ref of FrameSinkId and makes GetFrameSinkId a
const method so it can be accessed even if the caller has a const
pointer or reference to the RenderWidgetHostViewBase object.

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I5142a75f21a1c0396ba868bf66699a2fb85a2c33
Bug:  672962 
Reviewed-on: https://chromium-review.googlesource.com/1141985
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576275}
[modify] https://crrev.com/0c2f5b89f413fdff8e4d4c0f447705170fa462fd/components/viz/common/BUILD.gn
[add] https://crrev.com/0c2f5b89f413fdff8e4d4c0f447705170fa462fd/components/viz/common/surfaces/frame_sink_id_allocator.cc
[modify] https://crrev.com/0c2f5b89f413fdff8e4d4c0f447705170fa462fd/components/viz/common/surfaces/frame_sink_id_allocator.h
[modify] https://crrev.com/0c2f5b89f413fdff8e4d4c0f447705170fa462fd/components/viz/common/surfaces/parent_local_surface_id_allocator.cc
[modify] https://crrev.com/0c2f5b89f413fdff8e4d4c0f447705170fa462fd/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/0c2f5b89f413fdff8e4d4c0f447705170fa462fd/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/0c2f5b89f413fdff8e4d4c0f447705170fa462fd/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/0c2f5b89f413fdff8e4d4c0f447705170fa462fd/content/browser/renderer_host/render_widget_host_view_aura.h
[modify] https://crrev.com/0c2f5b89f413fdff8e4d4c0f447705170fa462fd/content/browser/renderer_host/render_widget_host_view_base.cc
[modify] https://crrev.com/0c2f5b89f413fdff8e4d4c0f447705170fa462fd/content/browser/renderer_host/render_widget_host_view_base.h
[modify] https://crrev.com/0c2f5b89f413fdff8e4d4c0f447705170fa462fd/content/browser/renderer_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/0c2f5b89f413fdff8e4d4c0f447705170fa462fd/content/browser/renderer_host/render_widget_host_view_child_frame.h
[modify] https://crrev.com/0c2f5b89f413fdff8e4d4c0f447705170fa462fd/content/browser/renderer_host/render_widget_host_view_mac.h
[modify] https://crrev.com/0c2f5b89f413fdff8e4d4c0f447705170fa462fd/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/0c2f5b89f413fdff8e4d4c0f447705170fa462fd/content/test/test_render_view_host.cc
[modify] https://crrev.com/0c2f5b89f413fdff8e4d4c0f447705170fa462fd/content/test/test_render_view_host.h
[modify] https://crrev.com/0c2f5b89f413fdff8e4d4c0f447705170fa462fd/ui/android/delegated_frame_host_android.cc
[modify] https://crrev.com/0c2f5b89f413fdff8e4d4c0f447705170fa462fd/ui/android/delegated_frame_host_android.h

Project Member

Comment 327 by bugdroid1@chromium.org, Jul 19

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

commit e10d9a58d0de4c31837b9c854949529c38e753a8
Author: Fady Samuel <fsamuel@chromium.org>
Date: Thu Jul 19 00:39:26 2018

Pass LocalSurfaceId by const ref

RenderWidgetHostViewBase::GetLocalSurfaceId passed the viz::LocalSurfaceId
by value.

LocalSurfaceId is large and so copying it around is expensive.
This CL returns a const ref of LocalSurfaceId instead to reduce copying.

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: Ic775202ef5572676e056bd41419edbaefb7e44da

Bug:  672962 
Change-Id: Ic775202ef5572676e056bd41419edbaefb7e44da
Reviewed-on: https://chromium-review.googlesource.com/1141990
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576281}
[modify] https://crrev.com/e10d9a58d0de4c31837b9c854949529c38e753a8/components/viz/common/surfaces/parent_local_surface_id_allocator.cc
[modify] https://crrev.com/e10d9a58d0de4c31837b9c854949529c38e753a8/components/viz/common/surfaces/parent_local_surface_id_allocator.h
[modify] https://crrev.com/e10d9a58d0de4c31837b9c854949529c38e753a8/content/browser/frame_host/render_widget_host_view_guest.cc
[modify] https://crrev.com/e10d9a58d0de4c31837b9c854949529c38e753a8/content/browser/frame_host/render_widget_host_view_guest.h
[modify] https://crrev.com/e10d9a58d0de4c31837b9c854949529c38e753a8/content/browser/renderer_host/render_widget_host_unittest.cc
[modify] https://crrev.com/e10d9a58d0de4c31837b9c854949529c38e753a8/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/e10d9a58d0de4c31837b9c854949529c38e753a8/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/e10d9a58d0de4c31837b9c854949529c38e753a8/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/e10d9a58d0de4c31837b9c854949529c38e753a8/content/browser/renderer_host/render_widget_host_view_aura.h
[modify] https://crrev.com/e10d9a58d0de4c31837b9c854949529c38e753a8/content/browser/renderer_host/render_widget_host_view_base.cc
[modify] https://crrev.com/e10d9a58d0de4c31837b9c854949529c38e753a8/content/browser/renderer_host/render_widget_host_view_base.h
[modify] https://crrev.com/e10d9a58d0de4c31837b9c854949529c38e753a8/content/browser/renderer_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/e10d9a58d0de4c31837b9c854949529c38e753a8/content/browser/renderer_host/render_widget_host_view_child_frame.h
[modify] https://crrev.com/e10d9a58d0de4c31837b9c854949529c38e753a8/content/browser/renderer_host/render_widget_host_view_mac.h
[modify] https://crrev.com/e10d9a58d0de4c31837b9c854949529c38e753a8/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/e10d9a58d0de4c31837b9c854949529c38e753a8/content/test/test_render_view_host.cc
[modify] https://crrev.com/e10d9a58d0de4c31837b9c854949529c38e753a8/content/test/test_render_view_host.h

Project Member

Comment 328 by bugdroid1@chromium.org, Jul 20

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

commit 120c7e2f4e05e00cc7ae4ab11c71c609e77ca1d2
Author: Fady Samuel <fsamuel@chromium.org>
Date: Fri Jul 20 05:19:28 2018

Surface Synchronization: Use SurfaceRange instead of primary and fallback

This CL replaces primary and fallback SurfaceIds stored internally in
SurfaceLayer and SurfaceLayerImpl with a SurfaceRange.

This simplifies code substantially and makes updating surface layers faster
because it involves creating fewer temporary objects.

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ib0d0e96210366f5127c34ccd2a46f103a6c208a3
Bug:  672962 
Reviewed-on: https://chromium-review.googlesource.com/1144207
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576787}
[modify] https://crrev.com/120c7e2f4e05e00cc7ae4ab11c71c609e77ca1d2/cc/layers/surface_layer.cc
[modify] https://crrev.com/120c7e2f4e05e00cc7ae4ab11c71c609e77ca1d2/cc/layers/surface_layer.h
[modify] https://crrev.com/120c7e2f4e05e00cc7ae4ab11c71c609e77ca1d2/cc/layers/surface_layer_impl.cc
[modify] https://crrev.com/120c7e2f4e05e00cc7ae4ab11c71c609e77ca1d2/cc/layers/surface_layer_impl.h
[modify] https://crrev.com/120c7e2f4e05e00cc7ae4ab11c71c609e77ca1d2/cc/layers/surface_layer_impl_unittest.cc
[modify] https://crrev.com/120c7e2f4e05e00cc7ae4ab11c71c609e77ca1d2/cc/layers/surface_layer_unittest.cc
[modify] https://crrev.com/120c7e2f4e05e00cc7ae4ab11c71c609e77ca1d2/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/120c7e2f4e05e00cc7ae4ab11c71c609e77ca1d2/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/120c7e2f4e05e00cc7ae4ab11c71c609e77ca1d2/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/120c7e2f4e05e00cc7ae4ab11c71c609e77ca1d2/ui/android/delegated_frame_host_android.cc
[modify] https://crrev.com/120c7e2f4e05e00cc7ae4ab11c71c609e77ca1d2/ui/android/delegated_frame_host_android.h
[modify] https://crrev.com/120c7e2f4e05e00cc7ae4ab11c71c609e77ca1d2/ui/compositor/layer.cc

Project Member

Comment 329 by bugdroid1@chromium.org, Jul 20

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

commit f709b5aa95d70eb6c0af09a8ab9f6746fcac4adf
Author: Fady Samuel <fsamuel@chromium.org>
Date: Fri Jul 20 17:27:42 2018

Surface Synchronization: Use SurfaceRange in SurfaceDrawQuad

Now that we have a SurfaceRange object, this CL replaces the
primary and fallback SurfaceIds in SurfaceDrawQuad with SurfaceRange
and updates all usages appropriately. This results in a modest line
count reduction and improves readability.

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I2d5a6e5b173772a44e39f2093d13c9c56d6c6cb1
Bug:  672962 
TBR: yfriedman@chromium.org, boliu@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/1145044
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576910}
[modify] https://crrev.com/f709b5aa95d70eb6c0af09a8ab9f6746fcac4adf/android_webview/browser/surfaces_instance.cc
[modify] https://crrev.com/f709b5aa95d70eb6c0af09a8ab9f6746fcac4adf/cc/layers/surface_layer_impl.cc
[modify] https://crrev.com/f709b5aa95d70eb6c0af09a8ab9f6746fcac4adf/cc/layers/surface_layer_impl.h
[modify] https://crrev.com/f709b5aa95d70eb6c0af09a8ab9f6746fcac4adf/cc/layers/surface_layer_impl_unittest.cc
[modify] https://crrev.com/f709b5aa95d70eb6c0af09a8ab9f6746fcac4adf/components/viz/client/hit_test_data_provider_draw_quad.cc
[modify] https://crrev.com/f709b5aa95d70eb6c0af09a8ab9f6746fcac4adf/components/viz/client/hit_test_data_provider_draw_quad_unittest.cc
[modify] https://crrev.com/f709b5aa95d70eb6c0af09a8ab9f6746fcac4adf/components/viz/common/quads/draw_quad_unittest.cc
[modify] https://crrev.com/f709b5aa95d70eb6c0af09a8ab9f6746fcac4adf/components/viz/common/quads/surface_draw_quad.cc
[modify] https://crrev.com/f709b5aa95d70eb6c0af09a8ab9f6746fcac4adf/components/viz/common/quads/surface_draw_quad.h
[modify] https://crrev.com/f709b5aa95d70eb6c0af09a8ab9f6746fcac4adf/components/viz/service/display/display_unittest.cc
[modify] https://crrev.com/f709b5aa95d70eb6c0af09a8ab9f6746fcac4adf/components/viz/service/display/surface_aggregator.cc
[modify] https://crrev.com/f709b5aa95d70eb6c0af09a8ab9f6746fcac4adf/components/viz/service/display/surface_aggregator_perftest.cc
[modify] https://crrev.com/f709b5aa95d70eb6c0af09a8ab9f6746fcac4adf/components/viz/service/display/surface_aggregator_pixeltest.cc
[modify] https://crrev.com/f709b5aa95d70eb6c0af09a8ab9f6746fcac4adf/components/viz/service/display/surface_aggregator_unittest.cc
[modify] https://crrev.com/f709b5aa95d70eb6c0af09a8ab9f6746fcac4adf/components/viz/service/frame_sinks/direct_layer_tree_frame_sink.cc
[modify] https://crrev.com/f709b5aa95d70eb6c0af09a8ab9f6746fcac4adf/components/viz/service/frame_sinks/direct_layer_tree_frame_sink_unittest.cc
[modify] https://crrev.com/f709b5aa95d70eb6c0af09a8ab9f6746fcac4adf/components/viz/service/frame_sinks/video_detector_unittest.cc
[modify] https://crrev.com/f709b5aa95d70eb6c0af09a8ab9f6746fcac4adf/components/viz/service/surfaces/surface_hittest.cc
[modify] https://crrev.com/f709b5aa95d70eb6c0af09a8ab9f6746fcac4adf/components/viz/test/surface_hittest_test_helpers.cc
[modify] https://crrev.com/f709b5aa95d70eb6c0af09a8ab9f6746fcac4adf/content/browser/renderer_host/render_widget_host_input_event_router.cc
[modify] https://crrev.com/f709b5aa95d70eb6c0af09a8ab9f6746fcac4adf/content/renderer/android/synchronous_layer_tree_frame_sink.cc
[modify] https://crrev.com/f709b5aa95d70eb6c0af09a8ab9f6746fcac4adf/services/ui/ws/frame_generator.cc
[modify] https://crrev.com/f709b5aa95d70eb6c0af09a8ab9f6746fcac4adf/services/viz/public/cpp/compositing/quads_struct_traits.cc
[modify] https://crrev.com/f709b5aa95d70eb6c0af09a8ab9f6746fcac4adf/services/viz/public/cpp/compositing/quads_struct_traits.h
[modify] https://crrev.com/f709b5aa95d70eb6c0af09a8ab9f6746fcac4adf/services/viz/public/cpp/compositing/struct_traits_unittest.cc
[modify] https://crrev.com/f709b5aa95d70eb6c0af09a8ab9f6746fcac4adf/services/viz/public/interfaces/compositing/quads.mojom

Project Member

Comment 330 by bugdroid1@chromium.org, Jul 24

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

commit e8a2d456495a49231d6de95bb2bc1554e89c7660
Author: Fady Samuel <fsamuel@chromium.org>
Date: Tue Jul 24 00:42:00 2018

Surface Synchronization: Simplify Fallback Surface Closure

We close fallback surfaces in order to speed up the arrival of the primary
surface from a child client. That logic was complex and difficult to follow.
Now with SurfaceRanges, that logic can be simplified somewhat.

Bug:  672962 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: Ied2efb4eac61f297fe6784825084f0a88e716dab
Reviewed-on: https://chromium-review.googlesource.com/1147114
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577381}
[modify] https://crrev.com/e8a2d456495a49231d6de95bb2bc1554e89c7660/components/viz/service/frame_sinks/surface_synchronization_unittest.cc
[modify] https://crrev.com/e8a2d456495a49231d6de95bb2bc1554e89c7660/components/viz/service/surfaces/surface.cc

Project Member

Comment 331 by bugdroid1@chromium.org, Jul 24

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

commit 7dc41be640111208fc5cc6f949ac230a41c53de7
Author: Fady Samuel <fsamuel@chromium.org>
Date: Tue Jul 24 15:10:37 2018

Surface Synchronization: Simplify SurfaceDependencyTracker

Prior to this CL, SurfaceDependencyTracker read activation_dependencies
from the pending CompositorFrame's CompositorFrameMetadata. This meant
that it had to look up already resolved dependencies. It's more efficient
to use the remaining dependencies set tracked in Surface directly. The code
is also more compact and easier to read.

Bug:  672962 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I97acf5637fd92eaa6485f15601582dc75dd69c35
Reviewed-on: https://chromium-review.googlesource.com/1148226
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577544}
[modify] https://crrev.com/7dc41be640111208fc5cc6f949ac230a41c53de7/components/viz/service/surfaces/surface_dependency_tracker.cc

Project Member

Comment 332 by bugdroid1@chromium.org, Jul 24

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

commit 98ebae32e83fd6e06c699a373e5d8e36a39229aa
Author: Fady Samuel <fsamuel@chromium.org>
Date: Tue Jul 24 20:32:35 2018

Revert "Surface Synchronization: Simplify SurfaceDependencyTracker"

This reverts commit 7dc41be640111208fc5cc6f949ac230a41c53de7.

Reason for revert: activation_dependencies() can change while iterating on updating deadlines or activating late surfaces.

Original change's description:
> Surface Synchronization: Simplify SurfaceDependencyTracker
> 
> Prior to this CL, SurfaceDependencyTracker read activation_dependencies
> from the pending CompositorFrame's CompositorFrameMetadata. This meant
> that it had to look up already resolved dependencies. It's more efficient
> to use the remaining dependencies set tracked in Surface directly. The code
> is also more compact and easier to read.
> 
> Bug:  672962 
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
> Change-Id: I97acf5637fd92eaa6485f15601582dc75dd69c35
> Reviewed-on: https://chromium-review.googlesource.com/1148226
> Reviewed-by: Saman Sami <samans@chromium.org>
> Commit-Queue: Fady Samuel <fsamuel@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#577544}

TBR=fsamuel@chromium.org,kylechar@chromium.org,samans@chromium.org

Change-Id: I1f3ef525f1de8fab329d86507269ee26b6239675
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  672962 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/1148607
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577668}
[modify] https://crrev.com/98ebae32e83fd6e06c699a373e5d8e36a39229aa/components/viz/service/surfaces/surface_dependency_tracker.cc

Project Member

Comment 333 by bugdroid1@chromium.org, Jul 25

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

commit e65f318c604d2159e65edda605863b98d36e2073
Author: Fady Samuel <fsamuel@chromium.org>
Date: Wed Jul 25 20:48:23 2018

Surface synchronization: Make sure viewport size is updated with DSF/LocalSurfaceId

Surface synchronization expects to maintain the invariant that if the viewport size
or device scale factor changes, then we must allocate a new LocalSurfaceId.

The problem is we don't plumb these quantities through the same flow today. LocalSurfaceId
and device scale factor are plumbed through the pending/active tree flow but viewport
size changes go directly from LayerTreeHost to LayerTreeHostImpl, bypassing the activation
flow. This introduces the possibility for a race where a draw might happen with the new
viewport before picking up the new LocalSurfaceId. This CL refactors viewport size so
that it follows the same flow as the other two surface sync quantities.

Subsequent CLs will move all cc properties to follow through the same flow.

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: Iab21c2fd108c856dcdeee2800ba3bacf1bd17907
Bug:  672962 
Reviewed-on: https://chromium-review.googlesource.com/959222
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578050}
[modify] https://crrev.com/e65f318c604d2159e65edda605863b98d36e2073/cc/layers/picture_layer_impl.cc
[modify] https://crrev.com/e65f318c604d2159e65edda605863b98d36e2073/cc/layers/picture_layer_impl_perftest.cc
[modify] https://crrev.com/e65f318c604d2159e65edda605863b98d36e2073/cc/layers/picture_layer_impl_unittest.cc
[modify] https://crrev.com/e65f318c604d2159e65edda605863b98d36e2073/cc/layers/render_surface_impl_unittest.cc
[modify] https://crrev.com/e65f318c604d2159e65edda605863b98d36e2073/cc/layers/render_surface_unittest.cc
[modify] https://crrev.com/e65f318c604d2159e65edda605863b98d36e2073/cc/layers/video_layer_impl_unittest.cc
[modify] https://crrev.com/e65f318c604d2159e65edda605863b98d36e2073/cc/test/fake_layer_tree_host_impl.cc
[modify] https://crrev.com/e65f318c604d2159e65edda605863b98d36e2073/cc/test/fake_picture_layer_impl.cc
[modify] https://crrev.com/e65f318c604d2159e65edda605863b98d36e2073/cc/test/test_layer_tree_host_base.cc
[modify] https://crrev.com/e65f318c604d2159e65edda605863b98d36e2073/cc/tiles/tile_manager_perftest.cc
[modify] https://crrev.com/e65f318c604d2159e65edda605863b98d36e2073/cc/tiles/tile_manager_unittest.cc
[modify] https://crrev.com/e65f318c604d2159e65edda605863b98d36e2073/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/e65f318c604d2159e65edda605863b98d36e2073/cc/trees/layer_tree_host_common_perftest.cc
[modify] https://crrev.com/e65f318c604d2159e65edda605863b98d36e2073/cc/trees/layer_tree_host_common_unittest.cc
[modify] https://crrev.com/e65f318c604d2159e65edda605863b98d36e2073/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/e65f318c604d2159e65edda605863b98d36e2073/cc/trees/layer_tree_host_impl.h
[modify] https://crrev.com/e65f318c604d2159e65edda605863b98d36e2073/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/e65f318c604d2159e65edda605863b98d36e2073/cc/trees/layer_tree_host_unittest.cc
[modify] https://crrev.com/e65f318c604d2159e65edda605863b98d36e2073/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/e65f318c604d2159e65edda605863b98d36e2073/cc/trees/layer_tree_impl.h
[modify] https://crrev.com/e65f318c604d2159e65edda605863b98d36e2073/cc/trees/layer_tree_impl_unittest.cc
[modify] https://crrev.com/e65f318c604d2159e65edda605863b98d36e2073/cc/trees/proxy_impl.cc
[modify] https://crrev.com/e65f318c604d2159e65edda605863b98d36e2073/components/viz/service/display/bsp_tree_perftest.cc

Project Member

Comment 334 by bugdroid1@chromium.org, Jul 26

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

commit fe9d772edb272a0281937c609ea384b732aac546
Author: Fady Samuel <fsamuel@chromium.org>
Date: Thu Jul 26 15:13:21 2018

RELAND: Surface Synchronization: Simplify SurfaceDependencyTracker

Prior to this CL, SurfaceDependencyTracker read activation_dependencies
from the pending CompositorFrame's CompositorFrameMetadata. This meant
that it had to look up already resolved dependencies. It's more efficient
to use the remaining dependencies set tracked in Surface directly. The code
is also more compact and easier to read.

In the first attempt to land, the CL iterated over activation_dependencies
as they were changing. This resulted in a crash. The solution is to
copy over the set before iteration.

Bug:  672962 
Change-Id: I3264f75b3fc71d7cd0368a63fcd38f7afcaf1cb0
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/1148226
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1150675
Cr-Commit-Position: refs/heads/master@{#578307}
[modify] https://crrev.com/fe9d772edb272a0281937c609ea384b732aac546/components/viz/service/surfaces/surface_dependency_tracker.cc

Project Member

Comment 335 by bugdroid1@chromium.org, Jul 26

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

commit 14fba8362d1d971445897793ce3a3bb4e00ada59
Author: Fady Samuel <fsamuel@chromium.org>
Date: Thu Jul 26 21:11:43 2018

cc: Plumb the viewport visible rect alongside the activation flow

Most compositing properties should follow the activation flow
instead of being plumbed directly from LayerTreeHost to LayerTreeHostImpl.
This CL is one of many to move properties over to following the activation
flow.

This CL moves the viewport_visible_rect property used to determine how much
to raster to follow the activation flow.

Bug:  672962 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I05ac2b28b8d593eee01904b0bba7f3809cc0492e
Reviewed-on: https://chromium-review.googlesource.com/1150734
Reviewed-by: vmpstr <vmpstr@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578440}
[modify] https://crrev.com/14fba8362d1d971445897793ce3a3bb4e00ada59/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/14fba8362d1d971445897793ce3a3bb4e00ada59/cc/trees/layer_tree_host_common_unittest.cc
[modify] https://crrev.com/14fba8362d1d971445897793ce3a3bb4e00ada59/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/14fba8362d1d971445897793ce3a3bb4e00ada59/cc/trees/layer_tree_host_impl.h
[modify] https://crrev.com/14fba8362d1d971445897793ce3a3bb4e00ada59/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/14fba8362d1d971445897793ce3a3bb4e00ada59/cc/trees/layer_tree_impl.h

Blockedon: 868008
Project Member

Comment 337 by bugdroid1@chromium.org, Aug 1

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

commit 920c0ed0fda97855c5389b5fcc87163ff0d3c529
Author: Fady Samuel <fsamuel@chromium.org>
Date: Wed Aug 01 21:25:30 2018

Surface Synchronization: DeviceScaleFactor plumbed alongside Viewport

This CL ensures that device scale factor is plumbed alongside the viewport
size from commit time through pending and then the active trees.

In a subsequent CL, I will introduce diagnostic checks that verify that
if device scale factor and viewport change, then the LocalSurfaceId must
also change.

Bug:  672962 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ica059593c9890609deb76ad44e1e58e0ff30edbd
Reviewed-on: https://chromium-review.googlesource.com/1159115
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579948}
[modify] https://crrev.com/920c0ed0fda97855c5389b5fcc87163ff0d3c529/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/920c0ed0fda97855c5389b5fcc87163ff0d3c529/cc/trees/layer_tree_impl.cc

Project Member

Comment 338 by bugdroid1@chromium.org, Aug 6

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

commit 2441a576a2bc2ed6878f931e9ad5b8769290ff9f
Author: Fady Samuel <fsamuel@chromium.org>
Date: Mon Aug 06 22:39:27 2018

Surface Synchronization: Simplify SurfaceDependencyTracker::OnSurfaceDiscarded

OnSurfaceDiscarded cosists of two operations:

1. Drop tracking of dependencies of the discarded surface.
2. Unblock surfaces blocked ont he discarded surface.

1. is equivalent to listing all activation dependencies as
'removed_dependencies' and calling OnSurfaceDependenciesChanged.

This CL does just that, reducing the number of lines of code in
SurfaceDependencyTracker further.

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I8191f1be81d04cce0c2181ac4fc341ebb8e3cf3f
Bug:  672962 
Reviewed-on: https://chromium-review.googlesource.com/1163632
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581016}
[modify] https://crrev.com/2441a576a2bc2ed6878f931e9ad5b8769290ff9f/components/viz/service/surfaces/surface_dependency_tracker.cc

Blocking: 871768
Blocking: 871790
Project Member

Comment 341 by bugdroid1@chromium.org, Aug 13

Labels: merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ecb9bbc425005f7f3934626be7b3d5ee75a39c44

commit ecb9bbc425005f7f3934626be7b3d5ee75a39c44
Author: Fady Samuel <fsamuel@chromium.org>
Date: Mon Aug 13 19:18:50 2018

RELAND: Surface Synchronization: Simplify SurfaceDependencyTracker

Prior to this CL, SurfaceDependencyTracker read activation_dependencies
from the pending CompositorFrame's CompositorFrameMetadata. This meant
that it had to look up already resolved dependencies. It's more efficient
to use the remaining dependencies set tracked in Surface directly. The code
is also more compact and easier to read.

In the first attempt to land, the CL iterated over activation_dependencies
as they were changing. This resulted in a crash. The solution is to
copy over the set before iteration.

Bug:  672962 
Change-Id: I3264f75b3fc71d7cd0368a63fcd38f7afcaf1cb0
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/1148226
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1150675
Cr-Original-Commit-Position: refs/heads/master@{#578307}(cherry picked from commit fe9d772edb272a0281937c609ea384b732aac546)
Reviewed-on: https://chromium-review.googlesource.com/1173074
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#574}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/ecb9bbc425005f7f3934626be7b3d5ee75a39c44/components/viz/service/surfaces/surface_dependency_tracker.cc

CL listed at #341 got merged to M69 without merge request and approval.
Project Member

Comment 343 by bugdroid1@chromium.org, Aug 13

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

commit cf66d71a0647932bf553788fbc9db0658fbf3882
Author: Fady Samuel <fsamuel@chromium.org>
Date: Mon Aug 13 19:23:59 2018

Surface Synchronization: Simplify SurfaceDependencyTracker::OnSurfaceDiscarded

OnSurfaceDiscarded cosists of two operations:

1. Drop tracking of dependencies of the discarded surface.
2. Unblock surfaces blocked ont he discarded surface.

1. is equivalent to listing all activation dependencies as
'removed_dependencies' and calling OnSurfaceDependenciesChanged.

This CL does just that, reducing the number of lines of code in
SurfaceDependencyTracker further.

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I8191f1be81d04cce0c2181ac4fc341ebb8e3cf3f
Bug:  672962 
Reviewed-on: https://chromium-review.googlesource.com/1163632
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#581016}(cherry picked from commit 2441a576a2bc2ed6878f931e9ad5b8769290ff9f)
Reviewed-on: https://chromium-review.googlesource.com/1173075
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#575}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/cf66d71a0647932bf553788fbc9db0658fbf3882/components/viz/service/surfaces/surface_dependency_tracker.cc

#342: Sorry, I was chatting with Ben Mason about this CL:

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

I couldn't merge it cleanly without the CL in #341. Ben Mason said okay.

+benmason@, apologies for the confusion!
Labels: Merge-Approved-69
Merged approved.
Labels: -Merge-Approved-69
Removing merge approved label as this has been merged.
Labels: -Proj-Mustash Proj-Mash-MultiProcess
Labels: -Proj-Mustash-Mus-GPU
Cleaning up old Proj-Mustash labels.
Project Member

Comment 349 by bugdroid1@chromium.org, Aug 16

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

commit 90dea8d48077f7dad685dd0bb1d9ce60135039e6
Author: Fady Samuel <fsamuel@chromium.org>
Date: Thu Aug 16 02:29:54 2018

Revert "Surface Synchronization: Only defer commits if frame exists"

This reverts commit f1a9e88d8e5e7cbe5945790e2a135125a756f97b.

Reason for revert: crbug.com/868605

Original change's description:
> Surface Synchronization: Only defer commits if frame exists
> 
> Right now, with surface sync off on Android, we will defer commits
> any time there's a size change, even if we've evicted the current
> surface.
> 
> Change-Id: I7e1ae394e20eef2a46bca5ae9a8fc8a4cdf35e0b
> Bug:  848292 ,  672962 
> Reviewed-on: https://chromium-review.googlesource.com/1121098
> Reviewed-by: Saman Sami <samans@chromium.org>
> Reviewed-by: Khushal <khushalsagar@chromium.org>
> Commit-Queue: Fady Samuel <fsamuel@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#575405}

TBR=fsamuel@chromium.org,khushalsagar@chromium.org,samans@chromium.org

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

Bug:  848292 ,  672962 
Change-Id: Ie558844df649cea7f3560f61d7077907e73e20a9
Reviewed-on: https://chromium-review.googlesource.com/1176761
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583508}
[modify] https://crrev.com/90dea8d48077f7dad685dd0bb1d9ce60135039e6/ui/android/delegated_frame_host_android.cc

Cc: benmason@chromium.org
Is revert listed at #349 need to merge to M69?
Labels: Merge-Request-69
#350, Yes, Krishna, we would like to merge #349 back into M69! Thanks!
Cl listed at #350 is not in canary yet. Pls update the bug with canary result tomorrow and justify the the merge. 

I will let benmason@ do the merge review for this as he had more context on this. Thank you.
Project Member

Comment 353 by sheriffbot@chromium.org, Aug 17

Labels: -Merge-Request-69 Merge-Review-69
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 354 by bugdroid1@chromium.org, Aug 20

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

commit 01f5bfbf492aa7d9ca153b394e5fb4d457c1fc57
Author: Fady Samuel <fsamuel@chromium.org>
Date: Mon Aug 20 22:04:22 2018

Revert "Surface Synchronization: Only defer commits if frame exists"

This reverts commit f1a9e88d8e5e7cbe5945790e2a135125a756f97b.

Reason for revert: crbug.com/868605

Original change's description:
> Surface Synchronization: Only defer commits if frame exists
> 
> Right now, with surface sync off on Android, we will defer commits
> any time there's a size change, even if we've evicted the current
> surface.
> 
> Change-Id: I7e1ae394e20eef2a46bca5ae9a8fc8a4cdf35e0b
> Bug:  848292 ,  672962 
> Reviewed-on: https://chromium-review.googlesource.com/1121098
> Reviewed-by: Saman Sami <samans@chromium.org>
> Reviewed-by: Khushal <khushalsagar@chromium.org>
> Commit-Queue: Fady Samuel <fsamuel@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#575405}

TBR=fsamuel@chromium.org,khushalsagar@chromium.org,samans@chromium.org

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

Bug:  848292 ,  672962 
Change-Id: Ie558844df649cea7f3560f61d7077907e73e20a9
Reviewed-on: https://chromium-review.googlesource.com/1176761
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#583508}(cherry picked from commit 90dea8d48077f7dad685dd0bb1d9ce60135039e6)
Reviewed-on: https://chromium-review.googlesource.com/1182206
Cr-Commit-Position: refs/branch-heads/3497@{#726}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/01f5bfbf492aa7d9ca153b394e5fb4d457c1fc57/ui/android/delegated_frame_host_android.cc

Labels: CommitLog-Audit-Violation Merge-Without-Approval M-69
Here's a summary of the rules that were executed: 
 - OnlyMergeApprovedChange: Rule Failed -- Revision 01f5bfbf492aa7d9ca153b394e5fb4d457c1fc57 was merged to refs/branch-heads/3497 branch with no merge approval from a TPM! 
Please explain why this change was merged to the branch!
Labels: -Merge-Review-69
Project Member

Comment 357 by bugdroid1@chromium.org, Aug 24

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

commit 5479bee2ba20c0527e19137d8056b539d30125d1
Author: Eric Karl <ericrk@chromium.org>
Date: Fri Aug 24 23:10:28 2018

Surface Synchronization: Enable by default on Android

Surface synchronization has launched on desktop but has not shipped
on Android yet. We'd like to attempt to launch it soon. This CL turns
on surface sync by default on bots on Android.

Bug:  672962 
Change-Id: I24e8eda65a090d16aa349167e7b805be924bae1d
Reviewed-on: https://chromium-review.googlesource.com/1162438
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Eric Karl <ericrk@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586039}
[modify] https://crrev.com/5479bee2ba20c0527e19137d8056b539d30125d1/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/5479bee2ba20c0527e19137d8056b539d30125d1/testing/variations/fieldtrial_testing_config.json
[modify] https://crrev.com/5479bee2ba20c0527e19137d8056b539d30125d1/ui/android/delegated_frame_host_android.cc
[modify] https://crrev.com/5479bee2ba20c0527e19137d8056b539d30125d1/ui/android/delegated_frame_host_android.h

Project Member

Comment 358 by bugdroid1@chromium.org, Aug 29

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

commit 316ed3b5c3ec228afdaaf567ed2317df54934834
Author: Fady Samuel <fsamuel@chromium.org>
Date: Wed Aug 29 20:22:23 2018

Surface Synchronization: Remove no fallback error state

Not having an available surface to show in SurfaceAggregator is no longer
really an error state because the start of a SurfaceRange is optional.

This CL removes the displaying of a magenta solid color in that case.

Bug:  672962 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: Iadb58ac9a0791071df59702afbfe3dd373d9ef62
Reviewed-on: https://chromium-review.googlesource.com/1194959
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587271}
[modify] https://crrev.com/316ed3b5c3ec228afdaaf567ed2317df54934834/components/viz/service/display/surface_aggregator.cc
[modify] https://crrev.com/316ed3b5c3ec228afdaaf567ed2317df54934834/components/viz/service/display/surface_aggregator_unittest.cc

Labels: -CommitLog-Audit-Violation -Merge-Without-Approval
Project Member

Comment 360 by bugdroid1@chromium.org, Sep 11

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

commit c68cd4c5e9f3aab66baa3638183d06d2ae5da8fa
Author: Fady Samuel <fsamuel@chromium.org>
Date: Tue Sep 11 02:29:22 2018

Surface Synchronization: Detect invariants violations in LayerTreeHost

Currently some invariants violation checks are in LayerTreeHost for mac
but not for other platforms. This CL enables the check on other platforms.

Bug:  672962 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Iade0eee8322f9126a5ee7409b2e79aa00593b2d1
Reviewed-on: https://chromium-review.googlesource.com/1217598
Reviewed-by: ccameron <ccameron@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590175}
[modify] https://crrev.com/c68cd4c5e9f3aab66baa3638183d06d2ae5da8fa/cc/trees/layer_tree_host.cc

Project Member

Comment 361 by bugdroid1@chromium.org, Sep 19

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

commit b6a59f2e683bfe1fe0f316afcfc34f9fb7745578
Author: Fady Samuel <fsamuel@chromium.org>
Date: Wed Sep 19 18:53:53 2018

BrowserPlugin: Delete non-surface-sync code path

BrowserPlugin is only on on Chrome desktop and and surface sync is always
on on desktop. This CL deletes the old code path.

Bug:  672962 
Change-Id: If0394bd7b8eed4b04d23fede8e31aff1ee3417b4
Reviewed-on: https://chromium-review.googlesource.com/1234323
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592497}
[modify] https://crrev.com/b6a59f2e683bfe1fe0f316afcfc34f9fb7745578/content/browser/browser_plugin/browser_plugin_guest.cc
[modify] https://crrev.com/b6a59f2e683bfe1fe0f316afcfc34f9fb7745578/content/browser/browser_plugin/browser_plugin_guest.h
[modify] https://crrev.com/b6a59f2e683bfe1fe0f316afcfc34f9fb7745578/content/browser/frame_host/render_widget_host_view_guest.cc
[modify] https://crrev.com/b6a59f2e683bfe1fe0f316afcfc34f9fb7745578/content/browser/frame_host/render_widget_host_view_guest.h
[modify] https://crrev.com/b6a59f2e683bfe1fe0f316afcfc34f9fb7745578/content/browser/frame_host/render_widget_host_view_guest_unittest.cc
[modify] https://crrev.com/b6a59f2e683bfe1fe0f316afcfc34f9fb7745578/content/common/browser_plugin/browser_plugin_messages.h
[modify] https://crrev.com/b6a59f2e683bfe1fe0f316afcfc34f9fb7745578/content/renderer/browser_plugin/browser_plugin.cc
[modify] https://crrev.com/b6a59f2e683bfe1fe0f316afcfc34f9fb7745578/content/renderer/browser_plugin/browser_plugin.h

Project Member

Comment 362 by bugdroid1@chromium.org, Sep 26

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

commit 7bb351e8fd63a0727c768cce22a28597c127005a
Author: Fady Samuel <fsamuel@chromium.org>
Date: Wed Sep 26 15:12:23 2018

Surface Synchronization: Simplify Deadlines

This CL simplifies deadline setup in viz::Surface a bit in anticipation
for throttling of child-initiated synchronization.

Prior to this CL, a Surface did not always have a SurfaceDependencyDeadline
object. One was only available if unlimited deadlines were not specified
by command line. This made deadline logic a bit more difficult to read
in Surface.

Furthermore, UpdateActivationDependencies didn't just update dependencies,
it also resolved the FrameDeadline into a wall time deadline. This CL
splits up that responsibility into a separate object in anticipation of
adding additional functionality in UpdateActivationDependencies in another
CL.

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: Ib280a25922600a36265b705bac25a820043756bf
Bug:  672962 
Reviewed-on: https://chromium-review.googlesource.com/1240506
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594319}
[modify] https://crrev.com/7bb351e8fd63a0727c768cce22a28597c127005a/components/viz/service/surfaces/surface.cc
[modify] https://crrev.com/7bb351e8fd63a0727c768cce22a28597c127005a/components/viz/service/surfaces/surface.h
[modify] https://crrev.com/7bb351e8fd63a0727c768cce22a28597c127005a/components/viz/service/surfaces/surface_dependency_deadline.cc
[modify] https://crrev.com/7bb351e8fd63a0727c768cce22a28597c127005a/components/viz/service/surfaces/surface_manager.cc

Project Member

Comment 363 by bugdroid1@chromium.org, Sep 27

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

commit 6b7ebb1e1908c72c171de5303a0c82e7749b17c8
Author: Fady Samuel <fsamuel@chromium.org>
Date: Thu Sep 27 01:31:56 2018

Revert "Surface Synchronization: Detect invariants violations in LayerTreeHost"

This reverts commit c68cd4c5e9f3aab66baa3638183d06d2ae5da8fa.

Reason for revert: There are some lingering invariants violations this is hitting.

Bug: 884148

Original change's description:
> Surface Synchronization: Detect invariants violations in LayerTreeHost
> 
> Currently some invariants violation checks are in LayerTreeHost for mac
> but not for other platforms. This CL enables the check on other platforms.
> 
> Bug:  672962 
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
> Change-Id: Iade0eee8322f9126a5ee7409b2e79aa00593b2d1
> Reviewed-on: https://chromium-review.googlesource.com/1217598
> Reviewed-by: ccameron <ccameron@chromium.org>
> Commit-Queue: Fady Samuel <fsamuel@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#590175}

TBR=ccameron@chromium.org,fsamuel@chromium.org

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

Bug:  672962 
Change-Id: If795714a072608b9f6c0fe613004c71fa3c9da6f
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/1247403
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594555}
[modify] https://crrev.com/6b7ebb1e1908c72c171de5303a0c82e7749b17c8/cc/trees/layer_tree_host.cc

Project Member

Comment 364 by bugdroid1@chromium.org, Oct 3

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

commit f96f0ea3b55efe11726a463a08e34cee8de57686
Author: Fady Samuel <fsamuel@chromium.org>
Date: Wed Oct 03 18:49:09 2018

Surface Synchronization: Throttle child synchronization events

This CL enables throttling of child synchronization events. In a child-
initiated synchronization, the child's frame will likely activate
before the dependent parent's CompositorFrame arrives. Upon activation,
the child will see that the new surface does not cause any damage to the
display (because it's not embedded by any parent yet) and calls the
draw callback immediately, thereby sending back a DidReceiveCompositorFrameAck
to the client immediately.

This, in turn, results in the client sending another CompositorFrame to Viz,
and another and so on, letting the child get further and further ahead of the
parent. This results in a lot of wasted work because the child's surface will
not be shown on screen until the parent embeds it and so multiple child
initiated synchronization events are pointless.

This CL throttles DidReceiveCompositorFrameAck of the child on child-initiated
synchronization events until the parent references the child.

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I62416d87f8d871b32f4caad565cff515a9c55445
Bug:  672962 ,  879207 
Reviewed-on: https://chromium-review.googlesource.com/1236931
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596300}
[modify] https://crrev.com/f96f0ea3b55efe11726a463a08e34cee8de57686/components/viz/service/frame_sinks/compositor_frame_sink_support.cc
[modify] https://crrev.com/f96f0ea3b55efe11726a463a08e34cee8de57686/components/viz/service/frame_sinks/compositor_frame_sink_support.h
[modify] https://crrev.com/f96f0ea3b55efe11726a463a08e34cee8de57686/components/viz/service/frame_sinks/compositor_frame_sink_support_unittest.cc
[modify] https://crrev.com/f96f0ea3b55efe11726a463a08e34cee8de57686/components/viz/service/frame_sinks/surface_synchronization_unittest.cc
[modify] https://crrev.com/f96f0ea3b55efe11726a463a08e34cee8de57686/components/viz/service/surfaces/surface.cc
[modify] https://crrev.com/f96f0ea3b55efe11726a463a08e34cee8de57686/components/viz/service/surfaces/surface.h
[modify] https://crrev.com/f96f0ea3b55efe11726a463a08e34cee8de57686/components/viz/service/surfaces/surface_dependency_tracker.cc
[modify] https://crrev.com/f96f0ea3b55efe11726a463a08e34cee8de57686/components/viz/service/surfaces/surface_dependency_tracker.h
[modify] https://crrev.com/f96f0ea3b55efe11726a463a08e34cee8de57686/components/viz/service/surfaces/surface_manager.cc
[modify] https://crrev.com/f96f0ea3b55efe11726a463a08e34cee8de57686/components/viz/service/surfaces/surface_manager.h

Project Member

Comment 365 by bugdroid1@chromium.org, Oct 4

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

commit 6f72fd2c91cfbe156601a2e74acdb873187d0d36
Author: Fady Samuel <fsamuel@chromium.org>
Date: Thu Oct 04 18:00:14 2018

Surface Synchronization: Allow out-of-order activation

With child-initiated synchronization throttling, it's possible for
frames from a given client to activate out-of-order.

For example, a child-initiated synchronization that is blocked on the
parent may be superceded by a parent-initiated synchronization.

That parent-inititaed surface will activate first, before the child-
initiated one, causing surfaces to activate out of order.

That's okay! This CL fixes some CHECKs and marks older surfaces
at activation time as candidates for garbage collection.

Bug:  672962 ,  879207 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I19c033c6400f467b29658c706556a99c5eaf0791
Reviewed-on: https://chromium-review.googlesource.com/c/1260019
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596750}
[modify] https://crrev.com/6f72fd2c91cfbe156601a2e74acdb873187d0d36/components/viz/service/frame_sinks/compositor_frame_sink_support.cc
[modify] https://crrev.com/6f72fd2c91cfbe156601a2e74acdb873187d0d36/components/viz/service/frame_sinks/surface_synchronization_unittest.cc

Project Member

Comment 366 by bugdroid1@chromium.org, Oct 11

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

commit 4d26c9df588fc6459eeb9fe70e0c49c33412ccfc
Author: Saman Sami <samans@chromium.org>
Date: Thu Oct 11 15:07:50 2018

Add DCHECK to SurfaceLayer::SetFallbackSurfaceId

While solving  crbug.com/878372  I ran into a case where
having two different locations for setting the fallback
caused it to go backwards. Add a DCHECK to prevent such
bugs.

Bug:  672962 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: If883bf03169135b3cbf0a5274a30a18ed149b08a
Reviewed-on: https://chromium-review.googlesource.com/c/1274610
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598763}
[modify] https://crrev.com/4d26c9df588fc6459eeb9fe70e0c49c33412ccfc/cc/layers/surface_layer.cc

Project Member

Comment 367 by bugdroid1@chromium.org, Oct 16

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

commit e525f7cabf63e659eebd885cf2e71fc44582f1de
Author: Fady Samuel <fsamuel@chromium.org>
Date: Tue Oct 16 15:07:16 2018

Surface Synchronization: Child Sync Immediately Activates if Deadline in Past

Prior to this CL, if a child initiates a synchronization event and submits a
CompositorFrame with a deadline in the past then the CompositorFrame will
immediately activate, but the surface will not be marked and so future
CompositorFrames to the same surface will result in blocking until embedding
or deadline again and again.

This could slow down activation and skew deadline duration reporting. This CL
marks a surface such that additional CompositorFrames submitted to the surface
will not block on embedding.

Bug:  672962 , 890767
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: Ia942e39266ac11dd20cea3f3ae4a087511a98c1d
Reviewed-on: https://chromium-review.googlesource.com/c/1281885
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599983}
[modify] https://crrev.com/e525f7cabf63e659eebd885cf2e71fc44582f1de/components/viz/service/frame_sinks/surface_synchronization_unittest.cc
[modify] https://crrev.com/e525f7cabf63e659eebd885cf2e71fc44582f1de/components/viz/service/surfaces/surface.cc
[modify] https://crrev.com/e525f7cabf63e659eebd885cf2e71fc44582f1de/components/viz/service/surfaces/surface_dependency_tracker.cc
[modify] https://crrev.com/e525f7cabf63e659eebd885cf2e71fc44582f1de/components/viz/service/surfaces/surface_dependency_tracker.h

Project Member

Comment 368 by bugdroid1@chromium.org, Oct 16

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

commit 0cfa6cbaddb75c5cf9e739c1a6d70eea27267cf8
Author: Fady Samuel <fsamuel@chromium.org>
Date: Tue Oct 16 20:05:43 2018

Surface Synchronization: Use system default deadline for child throttling

If a child surface's activation is throttled then use the system default
deadline as the lower bound for the throttling. This ensures that even if a
child does not have activation dependencies, it will still be throttled
by at least the system default deadline.

This CL also adds some debugging methods and comments on FrameDeadline.

Bug:  672962 , 890767
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: If675908fb2657d5d83b5717339a4060e1eb95f77
Reviewed-on: https://chromium-review.googlesource.com/c/1284050
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600093}
[modify] https://crrev.com/0cfa6cbaddb75c5cf9e739c1a6d70eea27267cf8/components/viz/common/BUILD.gn
[add] https://crrev.com/0cfa6cbaddb75c5cf9e739c1a6d70eea27267cf8/components/viz/common/quads/frame_deadline.cc
[modify] https://crrev.com/0cfa6cbaddb75c5cf9e739c1a6d70eea27267cf8/components/viz/common/quads/frame_deadline.h
[modify] https://crrev.com/0cfa6cbaddb75c5cf9e739c1a6d70eea27267cf8/components/viz/service/frame_sinks/surface_synchronization_unittest.cc
[modify] https://crrev.com/0cfa6cbaddb75c5cf9e739c1a6d70eea27267cf8/components/viz/service/surfaces/surface.cc
[modify] https://crrev.com/0cfa6cbaddb75c5cf9e739c1a6d70eea27267cf8/components/viz/service/surfaces/surface.h
[modify] https://crrev.com/0cfa6cbaddb75c5cf9e739c1a6d70eea27267cf8/components/viz/service/surfaces/surface_dependency_deadline.cc

Project Member

Comment 369 by bugdroid1@chromium.org, Oct 17

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

commit 128e369e5c2ab7e2a4c496f216a16b996154cd1d
Author: Fady Samuel <fsamuel@chromium.org>
Date: Wed Oct 17 18:44:10 2018

Deprecate Compositing.SurfaceDependencyDeadline.DeadlineHit

This metric wasn't really providing a useful signal for anything.

Bug:  672962 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: Ia4c5086f1d9705277455c149e1c8138b3a3085b3
Reviewed-on: https://chromium-review.googlesource.com/c/1286899
Reviewed-by: Saman Sami <samans@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600495}
[modify] https://crrev.com/128e369e5c2ab7e2a4c496f216a16b996154cd1d/components/viz/service/surfaces/surface_dependency_deadline.cc
[modify] https://crrev.com/128e369e5c2ab7e2a4c496f216a16b996154cd1d/components/viz/service/surfaces/surface_dependency_deadline.h
[modify] https://crrev.com/128e369e5c2ab7e2a4c496f216a16b996154cd1d/tools/metrics/histograms/histograms.xml

Project Member

Comment 370 by bugdroid1@chromium.org, Oct 18

Labels: merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f5e51574cf726723274c89187f901dc90dba2c2e

commit f5e51574cf726723274c89187f901dc90dba2c2e
Author: Fady Samuel <fsamuel@chromium.org>
Date: Thu Oct 18 18:17:12 2018

Surface Synchronization: Child Sync Immediately Activates if Deadline in Past

Prior to this CL, if a child initiates a synchronization event and submits a
CompositorFrame with a deadline in the past then the CompositorFrame will
immediately activate, but the surface will not be marked and so future
CompositorFrames to the same surface will result in blocking until embedding
or deadline again and again.

This could slow down activation and skew deadline duration reporting. This CL
marks a surface such that additional CompositorFrames submitted to the surface
will not block on embedding.

Bug:  672962 , 890767
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: Ia942e39266ac11dd20cea3f3ae4a087511a98c1d
Reviewed-on: https://chromium-review.googlesource.com/c/1281885
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#599983}(cherry picked from commit e525f7cabf63e659eebd885cf2e71fc44582f1de)
Reviewed-on: https://chromium-review.googlesource.com/c/1289291
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#125}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/f5e51574cf726723274c89187f901dc90dba2c2e/components/viz/service/frame_sinks/surface_synchronization_unittest.cc
[modify] https://crrev.com/f5e51574cf726723274c89187f901dc90dba2c2e/components/viz/service/surfaces/surface.cc
[modify] https://crrev.com/f5e51574cf726723274c89187f901dc90dba2c2e/components/viz/service/surfaces/surface_dependency_tracker.cc
[modify] https://crrev.com/f5e51574cf726723274c89187f901dc90dba2c2e/components/viz/service/surfaces/surface_dependency_tracker.h

Project Member

Comment 371 by bugdroid1@chromium.org, Oct 18

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

commit c290a845092d95dcc3d510b832f0d1bbdaf673b7
Author: Fady Samuel <fsamuel@chromium.org>
Date: Thu Oct 18 18:19:03 2018

Surface Synchronization: Use system default deadline for child throttling

If a child surface's activation is throttled then use the system default
deadline as the lower bound for the throttling. This ensures that even if a
child does not have activation dependencies, it will still be throttled
by at least the system default deadline.

This CL also adds some debugging methods and comments on FrameDeadline.

Bug:  672962 , 890767
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: If675908fb2657d5d83b5717339a4060e1eb95f77
Reviewed-on: https://chromium-review.googlesource.com/c/1284050
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#600093}(cherry picked from commit 0cfa6cbaddb75c5cf9e739c1a6d70eea27267cf8)
Reviewed-on: https://chromium-review.googlesource.com/c/1289292
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#126}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/c290a845092d95dcc3d510b832f0d1bbdaf673b7/components/viz/common/BUILD.gn
[add] https://crrev.com/c290a845092d95dcc3d510b832f0d1bbdaf673b7/components/viz/common/quads/frame_deadline.cc
[modify] https://crrev.com/c290a845092d95dcc3d510b832f0d1bbdaf673b7/components/viz/common/quads/frame_deadline.h
[modify] https://crrev.com/c290a845092d95dcc3d510b832f0d1bbdaf673b7/components/viz/service/frame_sinks/surface_synchronization_unittest.cc
[modify] https://crrev.com/c290a845092d95dcc3d510b832f0d1bbdaf673b7/components/viz/service/surfaces/surface.cc
[modify] https://crrev.com/c290a845092d95dcc3d510b832f0d1bbdaf673b7/components/viz/service/surfaces/surface.h
[modify] https://crrev.com/c290a845092d95dcc3d510b832f0d1bbdaf673b7/components/viz/service/surfaces/surface_dependency_deadline.cc

Project Member

Comment 372 by bugdroid1@chromium.org, Oct 19

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

commit 2a84daa6439e4f342b58f0eac44ef09d110a84b9
Author: Fady Samuel <fsamuel@chromium.org>
Date: Fri Oct 19 16:37:47 2018

Surface Synchronization: Delete OnSurfaceCreated

Now that we don't do temporary reference ownership,
SurfaceObserver::OnSurfaceCreated doesn't do anything useful.

Bug:  672962 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I6e9cf4557c10f50f097cd02c78fb3bd688cfc9d6
Reviewed-on: https://chromium-review.googlesource.com/c/1291216
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601181}
[modify] https://crrev.com/2a84daa6439e4f342b58f0eac44ef09d110a84b9/components/viz/service/display/display_scheduler.cc
[modify] https://crrev.com/2a84daa6439e4f342b58f0eac44ef09d110a84b9/components/viz/service/display/display_scheduler.h
[modify] https://crrev.com/2a84daa6439e4f342b58f0eac44ef09d110a84b9/components/viz/service/frame_sinks/frame_sink_manager_impl.cc
[modify] https://crrev.com/2a84daa6439e4f342b58f0eac44ef09d110a84b9/components/viz/service/frame_sinks/frame_sink_manager_impl.h
[modify] https://crrev.com/2a84daa6439e4f342b58f0eac44ef09d110a84b9/components/viz/service/frame_sinks/video_detector.h
[modify] https://crrev.com/2a84daa6439e4f342b58f0eac44ef09d110a84b9/components/viz/service/hit_test/hit_test_manager.h
[modify] https://crrev.com/2a84daa6439e4f342b58f0eac44ef09d110a84b9/components/viz/service/surfaces/surface_manager.cc
[modify] https://crrev.com/2a84daa6439e4f342b58f0eac44ef09d110a84b9/components/viz/service/surfaces/surface_observer.h
[modify] https://crrev.com/2a84daa6439e4f342b58f0eac44ef09d110a84b9/components/viz/test/fake_surface_observer.h

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/c290a845092d95dcc3d510b832f0d1bbdaf673b7

Commit: c290a845092d95dcc3d510b832f0d1bbdaf673b7
Author: fsamuel@chromium.org
Commiter: fsamuel@chromium.org
Date: 2018-10-18 18:19:03 +0000 UTC

Surface Synchronization: Use system default deadline for child throttling

If a child surface's activation is throttled then use the system default
deadline as the lower bound for the throttling. This ensures that even if a
child does not have activation dependencies, it will still be throttled
by at least the system default deadline.

This CL also adds some debugging methods and comments on FrameDeadline.

Bug:  672962 , 890767
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: If675908fb2657d5d83b5717339a4060e1eb95f77
Reviewed-on: https://chromium-review.googlesource.com/c/1284050
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#600093}(cherry picked from commit 0cfa6cbaddb75c5cf9e739c1a6d70eea27267cf8)
Reviewed-on: https://chromium-review.googlesource.com/c/1289292
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#126}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/f5e51574cf726723274c89187f901dc90dba2c2e

Commit: f5e51574cf726723274c89187f901dc90dba2c2e
Author: fsamuel@chromium.org
Commiter: fsamuel@chromium.org
Date: 2018-10-18 18:17:12 +0000 UTC

Surface Synchronization: Child Sync Immediately Activates if Deadline in Past

Prior to this CL, if a child initiates a synchronization event and submits a
CompositorFrame with a deadline in the past then the CompositorFrame will
immediately activate, but the surface will not be marked and so future
CompositorFrames to the same surface will result in blocking until embedding
or deadline again and again.

This could slow down activation and skew deadline duration reporting. This CL
marks a surface such that additional CompositorFrames submitted to the surface
will not block on embedding.

Bug:  672962 , 890767
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: Ia942e39266ac11dd20cea3f3ae4a087511a98c1d
Reviewed-on: https://chromium-review.googlesource.com/c/1281885
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#599983}(cherry picked from commit e525f7cabf63e659eebd885cf2e71fc44582f1de)
Reviewed-on: https://chromium-review.googlesource.com/c/1289291
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#125}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Project Member

Comment 375 by bugdroid1@chromium.org, Oct 23

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

commit f56feae05c48b4ee5797df2c72548ffee14b1405
Author: Fady Samuel <fsamuel@chromium.org>
Date: Tue Oct 23 16:56:10 2018

Surface Synchronization: Eliminate surface resurrection

In the past under certain circumstances (frame eviction or losing a
CompositorFrameSink), a Surface in the garbage collection queue could
get pulled out and "resurrected" accepting CompositorFrames again.

With the introduction of sequence number based frame eviction, surface
resurrection is no longer really necessary and makes the system more
difficult to reason about.

To deal with the lost CompositorFrameSink case, anytime a new
CompositorFrameSink/LayerTreeFrameSink is passed to LayerTreeHostImpl,
a new viz::LocalSurfaceId is allocated.

Some old, irrelevant tests have been deleted.

Change-Id: I09402e71f5aacd0afb90335a2ee3240d6772c176
Bug:  672962 
Reviewed-on: https://chromium-review.googlesource.com/c/1293756
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601990}
[modify] https://crrev.com/f56feae05c48b4ee5797df2c72548ffee14b1405/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/f56feae05c48b4ee5797df2c72548ffee14b1405/components/viz/service/frame_sinks/compositor_frame_sink_support.cc
[modify] https://crrev.com/f56feae05c48b4ee5797df2c72548ffee14b1405/components/viz/service/frame_sinks/surface_synchronization_unittest.cc
[modify] https://crrev.com/f56feae05c48b4ee5797df2c72548ffee14b1405/components/viz/service/surfaces/surface.cc
[modify] https://crrev.com/f56feae05c48b4ee5797df2c72548ffee14b1405/components/viz/service/surfaces/surface.h
[modify] https://crrev.com/f56feae05c48b4ee5797df2c72548ffee14b1405/components/viz/service/surfaces/surface_manager.cc
[modify] https://crrev.com/f56feae05c48b4ee5797df2c72548ffee14b1405/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc

Status: Fixed (was: Assigned)
This is launching in M71 on Android so I'm marking this bug as FIXED after 2 long years! :D
Blocking: 876512
Showing comments 278 - 377 of 377 Older

Sign in to add a comment