New issue
Advanced search Search tips

Issue 785013 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 771331
issue 785986

Blocking:
issue 760181



Sign in to add a comment

FrameWatcher::WaitFrames Doesn't work in --enable-viz

Project Member Reported by jonr...@chromium.org, Nov 14 2017

Issue description

content::FrameWatcher::WaitFrames currently times out as the DidReceiveCompositorFrames is not setup in --enable-viz

Once FrameToken passing is ready ( issue 771331 ) this can be updated.
We may need a new test API to listen for compositor frame submission.

This affects the following content_browsertests:
  CompositedScrollingBrowserTest.*
  NonBlockingEventBrowserTest.*
  ScrollLatencyBrowserTest.SmoothWheelScroll
  TouchActionBrowserTest.TouchActionNone
  WheelScrollLatchingBrowserTest.WheelEventTarget
  WheelScrollLatchingDisabledBrowserTest.WheelEventTarget

 
FrameToken work landed in: https://chromium-review.googlesource.com/c/chromium/src/+/748771

With this RenderWidgetHostImpl is no longer responsible for passing around the FrameToken for a CompositorFrame. However the delegate there which notifies FrameWatcher of submissions is not being called.

With this change HostFrameSinkManager::OnFrameTokenChanged is called when a new active CompositorFrame has been submitted. We should look at tieing FrameWatcher to this.
Also: RenderWidgetHostViewBrowserTestBase.CompositorWorksWhenReusingRenderer
FrameWatcher also provides access to the last viz::CompositorFrameMetadata which will not be available
Blockedon: 785986
Status: Started (was: Untriaged)
But for tests that only wait for a frame to progress calls can now be replaced with MainThreadFrameObserver.

Some tests actually care about hit test data being available. Those will need to switch to the api made in  issue 785986 
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 17 2017

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

commit 42cc524af0049f3cd4ff978e2b22c87d80faf575
Author: Jonathan <jonross@chromium.org>
Date: Fri Nov 17 22:19:30 2017

Update tests which don't need FrameWatcher

With --enable-viz FrameWatcher does not work. The signal is no longer there.
A second test api MainThreadFrameObserver does work.

This converts tests which are only using FrameWatcher to wait on frames. Not
those which also are waiting on hit test regions, or metadata.

TBR=sky@chromium.org
TEST=CompositedScrollingBrowserTest, ScrollLatencyBrowserTest,
RenderWidgetHostViewBrowserTestBase, WheelScrollLatchingBrowserTest,
WheelScrollLatchingDisabledBrowserTest

Bug:  785013 
Change-Id: Ica8d780e81fb1a25ea0778af95381d042b112255
Reviewed-on: https://chromium-review.googlesource.com/775987
Commit-Queue: Jonathan Ross <jonross@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517584}
[modify] https://crrev.com/42cc524af0049f3cd4ff978e2b22c87d80faf575/content/browser/renderer_host/input/composited_scrolling_browsertest.cc
[modify] https://crrev.com/42cc524af0049f3cd4ff978e2b22c87d80faf575/content/browser/renderer_host/input/scroll_latency_browsertest.cc
[modify] https://crrev.com/42cc524af0049f3cd4ff978e2b22c87d80faf575/content/browser/renderer_host/input/wheel_scroll_latching_browsertest.cc
[modify] https://crrev.com/42cc524af0049f3cd4ff978e2b22c87d80faf575/content/browser/renderer_host/render_widget_host_view_browsertest.cc
[modify] https://crrev.com/42cc524af0049f3cd4ff978e2b22c87d80faf575/content/public/test/browser_test_utils.h
[modify] https://crrev.com/42cc524af0049f3cd4ff978e2b22c87d80faf575/testing/buildbot/filters/mojo.fyi.viz.content_browsertests.filter
[modify] https://crrev.com/42cc524af0049f3cd4ff978e2b22c87d80faf575/testing/buildbot/filters/viz.content_browsertests.filter

Project Member

Comment 6 by bugdroid1@chromium.org, Feb 16 2018

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

commit a2ff4f82109df55045dee9f54985a98054f86dc4
Author: jonross <jonross@chromium.org>
Date: Fri Feb 16 17:27:46 2018

Renderer observer of frame submission

A proposal to allow the renderer to notify the browser of frame submission,
along with RenderFrameMetadata.

This change adds cc::RenderFrameObserver which LayerTreeHostImpl can notify of frame submission.
This change adds content::RenderFrameObserverImpl which implements the above. RenderWidget sets this on the LTHI.
MainThreadFrameObserver has been updated to enable observing.
NonBlockingEventBrowserTest.StartTouch has been updated to make use of this.

TEST=NonBlockingEventBrowserTest.StartTouch

Bug:  785013 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: Ic01e81331b34119a08488778f5dbe0c86a1775a3
Reviewed-on: https://chromium-review.googlesource.com/886655
Commit-Queue: Jonathan Ross <jonross@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537339}
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/cc/BUILD.gn
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/cc/test/fake_proxy.h
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/cc/trees/layer_tree_host.h
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/cc/trees/layer_tree_host_impl.h
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/cc/trees/proxy.h
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/cc/trees/proxy_impl.cc
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/cc/trees/proxy_impl.h
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/cc/trees/proxy_main.cc
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/cc/trees/proxy_main.h
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/cc/trees/render_frame_metadata.cc
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/cc/trees/render_frame_metadata.h
[add] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/cc/trees/render_frame_metadata_observer.h
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/cc/trees/single_thread_proxy.cc
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/cc/trees/single_thread_proxy.h
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/browser/BUILD.gn
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/browser/renderer_host/frame_sink_provider_impl.cc
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/browser/renderer_host/frame_sink_provider_impl.h
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/browser/renderer_host/input/non_blocking_event_browsertest.cc
[add] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/browser/renderer_host/render_frame_metadata_provider_impl.cc
[add] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/browser/renderer_host/render_frame_metadata_provider_impl.h
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/browser/renderer_host/render_widget_host_unittest.cc
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/common/BUILD.gn
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/common/frame_sink_provider.mojom
[add] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/common/render_frame_metadata.mojom
[add] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/common/render_frame_metadata.typemap
[add] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/common/render_frame_metadata_struct_traits.cc
[add] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/common/render_frame_metadata_struct_traits.h
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/common/typemaps.gni
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/public/browser/BUILD.gn
[add] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/public/browser/render_frame_metadata_provider.h
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/public/test/browser_test_utils.cc
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/public/test/browser_test_utils.h
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/renderer/BUILD.gn
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/renderer/gpu/render_widget_compositor.cc
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/renderer/gpu/render_widget_compositor.h
[add] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/renderer/render_frame_metadata_observer_impl.cc
[add] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/renderer/render_frame_metadata_observer_impl.h
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/renderer/render_thread_impl.cc
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/renderer/render_thread_impl.h
[modify] https://crrev.com/a2ff4f82109df55045dee9f54985a98054f86dc4/content/renderer/render_widget.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Feb 21 2018

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

commit c1877d6d8b4187deb729df71a04063b4b7f026ca
Author: jonross <jonross@chromium.org>
Date: Wed Feb 21 00:07:31 2018

Implement RenderFrameMetadataObserver on SingleThreadProxy

The NOTIMPLEMENTED in SingleThreadProxy is triggering on layouttests.

This change implements it by forwarding along the RenderFrameMetadataObserver.

Bug:  785013 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: Ic32b03248b9484c421c0442e66476dbe890e2cd7
Reviewed-on: https://chromium-review.googlesource.com/926561
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Jonathan Ross <jonross@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537945}
[modify] https://crrev.com/c1877d6d8b4187deb729df71a04063b4b7f026ca/cc/trees/single_thread_proxy.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Feb 21 2018

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

commit 7c62fcb1261c93325c02bfda62cd1501d997cf20
Author: jonross <jonross@chromium.org>
Date: Wed Feb 21 16:35:46 2018

Remove RenderFrameMetadata from SwapPromise

With the render_frame_metadata.mojom path for observing RenderFrameMetadata we
no longer need to have SwapPromise pass them to the browser. This change removes
that code path.

This reverts most of:
  https://chromium-review.googlesource.com/c/chromium/src/+/860994
aside from the parts that landed RenderFrameMetadata itself.

TEST=cc_unittests, content_unittests

Bug:  785013 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: Iff77b631b74b2b99241f4e0eecbbbd7306d6e269
Reviewed-on: https://chromium-review.googlesource.com/923550
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Jonathan Ross <jonross@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538130}
[modify] https://crrev.com/7c62fcb1261c93325c02bfda62cd1501d997cf20/cc/ipc/cc_param_traits_macros.h
[modify] https://crrev.com/7c62fcb1261c93325c02bfda62cd1501d997cf20/cc/trees/latency_info_swap_promise.cc
[modify] https://crrev.com/7c62fcb1261c93325c02bfda62cd1501d997cf20/cc/trees/latency_info_swap_promise.h
[modify] https://crrev.com/7c62fcb1261c93325c02bfda62cd1501d997cf20/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/7c62fcb1261c93325c02bfda62cd1501d997cf20/cc/trees/layer_tree_host_unittest.cc
[modify] https://crrev.com/7c62fcb1261c93325c02bfda62cd1501d997cf20/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/7c62fcb1261c93325c02bfda62cd1501d997cf20/cc/trees/layer_tree_impl.h
[modify] https://crrev.com/7c62fcb1261c93325c02bfda62cd1501d997cf20/cc/trees/layer_tree_impl_unittest.cc
[modify] https://crrev.com/7c62fcb1261c93325c02bfda62cd1501d997cf20/cc/trees/swap_promise.h
[modify] https://crrev.com/7c62fcb1261c93325c02bfda62cd1501d997cf20/cc/trees/swap_promise_manager_unittest.cc
[modify] https://crrev.com/7c62fcb1261c93325c02bfda62cd1501d997cf20/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/7c62fcb1261c93325c02bfda62cd1501d997cf20/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/7c62fcb1261c93325c02bfda62cd1501d997cf20/content/common/view_messages.h
[modify] https://crrev.com/7c62fcb1261c93325c02bfda62cd1501d997cf20/content/renderer/gpu/queue_message_swap_promise.cc
[modify] https://crrev.com/7c62fcb1261c93325c02bfda62cd1501d997cf20/content/renderer/gpu/queue_message_swap_promise.h
[modify] https://crrev.com/7c62fcb1261c93325c02bfda62cd1501d997cf20/content/renderer/gpu/queue_message_swap_promise_unittest.cc
[modify] https://crrev.com/7c62fcb1261c93325c02bfda62cd1501d997cf20/content/renderer/gpu/render_widget_compositor.cc
[modify] https://crrev.com/7c62fcb1261c93325c02bfda62cd1501d997cf20/content/renderer/render_view_impl.cc
[modify] https://crrev.com/7c62fcb1261c93325c02bfda62cd1501d997cf20/content/test/layouttest_support.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Feb 23 2018

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

commit 20fecc46a828ab590726fd8707da107a23b096cd
Author: jonross <jonross@chromium.org>
Date: Fri Feb 23 05:32:21 2018

Refactor FrameToken queue into own class

RenderWidgetHostImpl makes use of an internal queue holding IPC::Messages which
it synchronizes to FrameTokens.

However this will be needed for RenderFrameMetadata observing, to also
synchronize on frame submission. So this refactors it into its own class to be
used by both

TBR=piman@chromium.org

Bug:  785013 
Change-Id: Icc392444341ed1edbe2994c05054ae305f91f884
Reviewed-on: https://chromium-review.googlesource.com/911829
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Jonathan Ross <jonross@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538707}
[modify] https://crrev.com/20fecc46a828ab590726fd8707da107a23b096cd/content/browser/BUILD.gn
[add] https://crrev.com/20fecc46a828ab590726fd8707da107a23b096cd/content/browser/renderer_host/frame_token_message_queue.cc
[add] https://crrev.com/20fecc46a828ab590726fd8707da107a23b096cd/content/browser/renderer_host/frame_token_message_queue.h
[modify] https://crrev.com/20fecc46a828ab590726fd8707da107a23b096cd/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/20fecc46a828ab590726fd8707da107a23b096cd/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/20fecc46a828ab590726fd8707da107a23b096cd/content/browser/renderer_host/render_widget_host_unittest.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Feb 23 2018

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

commit 36545cff0f6888077cc1005e6d17a74f3b7e8e5c
Author: jonross <jonross@chromium.org>
Date: Fri Feb 23 19:50:35 2018

Delete FrameWatcher

With RenderFrameSubmissionObserver browser_tests and content_browsertests can
wait for the submission of compositor frames, or changes in RenderFrameMetadata.
In a way that works for both classic and Viz DisplayCompositor.

FrameWatcher did not work with Viz.

This change ports all usage of FrameWatcher to RenderFrameSubmissionObserver.
It also updates the new API to specifiy the two forms of waiting.

FrameWatcher is also being deleted.

TEST=content_browsertests, browser_tests

Bug:  785013 
Change-Id: I4e34ab49e8a7739a105686483ff9cd6aee3e3fa7
Reviewed-on: https://chromium-review.googlesource.com/924479
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Commit-Queue: Jonathan Ross <jonross@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538870}
[modify] https://crrev.com/36545cff0f6888077cc1005e6d17a74f3b7e8e5c/chrome/browser/apps/guest_view/web_view_browsertest.cc
[modify] https://crrev.com/36545cff0f6888077cc1005e6d17a74f3b7e8e5c/content/browser/renderer_host/input/non_blocking_event_browsertest.cc
[modify] https://crrev.com/36545cff0f6888077cc1005e6d17a74f3b7e8e5c/content/browser/renderer_host/input/touch_action_browsertest.cc
[modify] https://crrev.com/36545cff0f6888077cc1005e6d17a74f3b7e8e5c/content/browser/renderer_host/render_widget_host_view_browsertest.cc
[modify] https://crrev.com/36545cff0f6888077cc1005e6d17a74f3b7e8e5c/content/browser/web_contents/web_contents_view_aura_browsertest.cc
[modify] https://crrev.com/36545cff0f6888077cc1005e6d17a74f3b7e8e5c/content/public/test/browser_test_utils.cc
[modify] https://crrev.com/36545cff0f6888077cc1005e6d17a74f3b7e8e5c/content/public/test/browser_test_utils.h

Project Member

Comment 11 by bugdroid1@chromium.org, Feb 23 2018

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

commit b9cb2eefaa8f7bb0a16ebb3d0571591d514fadcc
Author: jonross <jonross@chromium.org>
Date: Fri Feb 23 21:48:27 2018

Re-enable fixed tests on FYI bot

Re-enable tests that were broken from FrameWatcher on Viz. They seem fine, but
checking on FYI first to confirm no flakes

TBR=kylechar@chromium.org
TEST=NonBlockingEventBrowserTest, TouchActionBrowserTest

Bug:  785013 
Change-Id: Ie263e1cf41556ce1c606d1ef6ecc887d990b926c
Reviewed-on: https://chromium-review.googlesource.com/935495
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538908}
[modify] https://crrev.com/b9cb2eefaa8f7bb0a16ebb3d0571591d514fadcc/testing/buildbot/filters/mojo.fyi.viz.content_browsertests.filter

Status: Fixed (was: Started)

Sign in to add a comment