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

Issue 775103 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 791557
issue 775030



Sign in to add a comment

Investigate how CompositorFrameMetadata is used in the browser

Project Member Reported by samans@chromium.org, Oct 16 2017

Issue description

When the browser receives a CompositorFrame from the renderer, it looks into its metadata. See RenderWidgetHostImpl::SubmitCompositorFrame, RenderWidgetHostView*::SubmitCompositorFrame, DelegatedFrameHost(Android)::SubmitCompositorFrame.

For each member of CompositorFrameMetadata, we need to know: 

1) On what platforms it's used.
2) How it's used.
3) If we deliver this information to the browser process separate from the CompositorFrame without synchronization, what will regress and how badly.
 

Comment 1 by fsamuel@google.com, Oct 16 2017

Cc: fsam...@chromium.org jonr...@chromium.org yiyix@chromium.org samans@chromium.org thanhph@chromium.org
We should make a doc and contribute to it. Share it on this bug.
Owner: jonr...@chromium.org
Status: Assigned (was: Available)
Blocking: 791557
Status: Started (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 22 2018

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

commit 7a39fcd3405e0ec87e8224250fa32e602ac59531
Author: jonross <jonross@chromium.org>
Date: Thu Feb 22 20:48:28 2018

Remove un-used CompositorFrameMetadata root_overflow_x_hidden

The root_overflow_x_hidden component in CompositorFrameMetadata is not used
anywhere.

This change removes it.

TEST=cc_unittests, services_unittests
TBR=kenrb@chromium.org

Bug:  775103 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I889573699e46fbe02e95e4987c3539cdc7dbc067
Reviewed-on: https://chromium-review.googlesource.com/931682
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Jonathan Ross <jonross@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538553}
[modify] https://crrev.com/7a39fcd3405e0ec87e8224250fa32e602ac59531/cc/ipc/cc_param_traits_macros.h
[modify] https://crrev.com/7a39fcd3405e0ec87e8224250fa32e602ac59531/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/7a39fcd3405e0ec87e8224250fa32e602ac59531/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/7a39fcd3405e0ec87e8224250fa32e602ac59531/components/viz/common/quads/compositor_frame_metadata.h
[modify] https://crrev.com/7a39fcd3405e0ec87e8224250fa32e602ac59531/services/viz/public/cpp/compositing/compositor_frame_metadata_struct_traits.cc
[modify] https://crrev.com/7a39fcd3405e0ec87e8224250fa32e602ac59531/services/viz/public/cpp/compositing/compositor_frame_metadata_struct_traits.h
[modify] https://crrev.com/7a39fcd3405e0ec87e8224250fa32e602ac59531/services/viz/public/cpp/compositing/struct_traits_unittest.cc
[modify] https://crrev.com/7a39fcd3405e0ec87e8224250fa32e602ac59531/services/viz/public/interfaces/compositing/compositor_frame_metadata.mojom

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 8 2018

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

commit 7244a4aa9283acf307a8447a6799961b2e5b3a5e
Author: jonross <jonross@chromium.org>
Date: Thu Mar 08 19:05:35 2018

Unified FrameToken Allocation

We sometimes need FrameTokens associated with a CompositorFrame submission.
However on a given frame there may be multiple sources requesting a token. This
change creates FrameTokenAllocator which only provides one incrementation to the
FrameToken per frame submission.

Bug:  775103 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I161bee7f4bf0641ebd658cd0e0e7ca707d55f015
Reviewed-on: https://chromium-review.googlesource.com/931992
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Jonathan Ross <jonross@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541852}
[modify] https://crrev.com/7244a4aa9283acf307a8447a6799961b2e5b3a5e/cc/BUILD.gn
[add] https://crrev.com/7244a4aa9283acf307a8447a6799961b2e5b3a5e/cc/trees/frame_token_allocator.cc
[add] https://crrev.com/7244a4aa9283acf307a8447a6799961b2e5b3a5e/cc/trees/frame_token_allocator.h
[modify] https://crrev.com/7244a4aa9283acf307a8447a6799961b2e5b3a5e/cc/trees/latency_info_swap_promise.cc
[modify] https://crrev.com/7244a4aa9283acf307a8447a6799961b2e5b3a5e/cc/trees/latency_info_swap_promise.h
[modify] https://crrev.com/7244a4aa9283acf307a8447a6799961b2e5b3a5e/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/7244a4aa9283acf307a8447a6799961b2e5b3a5e/cc/trees/layer_tree_host_impl.h
[modify] https://crrev.com/7244a4aa9283acf307a8447a6799961b2e5b3a5e/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/7244a4aa9283acf307a8447a6799961b2e5b3a5e/cc/trees/layer_tree_host_unittest.cc
[modify] https://crrev.com/7244a4aa9283acf307a8447a6799961b2e5b3a5e/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/7244a4aa9283acf307a8447a6799961b2e5b3a5e/cc/trees/layer_tree_impl.h
[modify] https://crrev.com/7244a4aa9283acf307a8447a6799961b2e5b3a5e/cc/trees/layer_tree_impl_unittest.cc
[modify] https://crrev.com/7244a4aa9283acf307a8447a6799961b2e5b3a5e/cc/trees/render_frame_metadata_observer.h
[modify] https://crrev.com/7244a4aa9283acf307a8447a6799961b2e5b3a5e/cc/trees/swap_promise.h
[modify] https://crrev.com/7244a4aa9283acf307a8447a6799961b2e5b3a5e/cc/trees/swap_promise_manager_unittest.cc
[modify] https://crrev.com/7244a4aa9283acf307a8447a6799961b2e5b3a5e/content/renderer/gpu/frame_swap_message_queue.cc
[modify] https://crrev.com/7244a4aa9283acf307a8447a6799961b2e5b3a5e/content/renderer/gpu/frame_swap_message_queue.h
[modify] https://crrev.com/7244a4aa9283acf307a8447a6799961b2e5b3a5e/content/renderer/gpu/queue_message_swap_promise.cc
[modify] https://crrev.com/7244a4aa9283acf307a8447a6799961b2e5b3a5e/content/renderer/gpu/queue_message_swap_promise.h
[modify] https://crrev.com/7244a4aa9283acf307a8447a6799961b2e5b3a5e/content/renderer/gpu/queue_message_swap_promise_unittest.cc
[modify] https://crrev.com/7244a4aa9283acf307a8447a6799961b2e5b3a5e/content/renderer/gpu/render_widget_compositor.cc
[modify] https://crrev.com/7244a4aa9283acf307a8447a6799961b2e5b3a5e/content/renderer/render_frame_metadata_observer_impl.cc
[modify] https://crrev.com/7244a4aa9283acf307a8447a6799961b2e5b3a5e/content/renderer/render_frame_metadata_observer_impl.h
[modify] https://crrev.com/7244a4aa9283acf307a8447a6799961b2e5b3a5e/content/renderer/render_view_impl.cc
[modify] https://crrev.com/7244a4aa9283acf307a8447a6799961b2e5b3a5e/content/test/layouttest_support.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Mar 19 2018

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

commit 2fcd8b8c83d73dd8cf19a07be36b0dc36958069c
Author: jonross <jonross@chromium.org>
Date: Mon Mar 19 18:26:21 2018

Implement FrakeToken Synchronizing for RenderFrameMetadata

Currently RenderFrameMetadata is communicated from the renderer to the browser.
The messages are processed right away, even if Viz may have not yet completed
the processing of the frame associated to the data. This is racy.

This change adds a FrameToken to all of the RenderFrameMetadata messages. The
browser side then uses FrameTokenMessageQueue to synchronize these messages.
They will now be processed once Viz has completed processing the frame and has
notified the browser.

TEST=existing viz_content_browsertests
TBR=piman@chromium.org

Bug:  775103 
Change-Id: I52e6d023be5591946c589400374236a2a8e979d6
Reviewed-on: https://chromium-review.googlesource.com/962711
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Reviewed-by: Greg Kerr <kerrnel@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Jonathan Ross <jonross@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544072}
[modify] https://crrev.com/2fcd8b8c83d73dd8cf19a07be36b0dc36958069c/content/browser/renderer_host/frame_token_message_queue.cc
[modify] https://crrev.com/2fcd8b8c83d73dd8cf19a07be36b0dc36958069c/content/browser/renderer_host/frame_token_message_queue.h
[add] https://crrev.com/2fcd8b8c83d73dd8cf19a07be36b0dc36958069c/content/browser/renderer_host/frame_token_message_queue_unittest.cc
[modify] https://crrev.com/2fcd8b8c83d73dd8cf19a07be36b0dc36958069c/content/browser/renderer_host/render_frame_metadata_provider_impl.cc
[modify] https://crrev.com/2fcd8b8c83d73dd8cf19a07be36b0dc36958069c/content/browser/renderer_host/render_frame_metadata_provider_impl.h
[modify] https://crrev.com/2fcd8b8c83d73dd8cf19a07be36b0dc36958069c/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/2fcd8b8c83d73dd8cf19a07be36b0dc36958069c/content/common/render_frame_metadata.mojom
[modify] https://crrev.com/2fcd8b8c83d73dd8cf19a07be36b0dc36958069c/content/renderer/render_frame_metadata_observer_impl.cc
[modify] https://crrev.com/2fcd8b8c83d73dd8cf19a07be36b0dc36958069c/content/test/BUILD.gn

Project Member

Comment 8 by bugdroid1@chromium.org, Mar 20 2018

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

commit 4fd54fe26a35e1a333c5b93707eb348512521c28
Author: jonross <jonross@chromium.org>
Date: Tue Mar 20 17:00:51 2018

Update missed RenderFrameMetadataProvider binding

In the review ~https://chromium-review.googlesource.com/c/chromium/src/+/962711
I missed a spot to use a WeakPtr. This CL adds it.

TBR=piman@chromium.org
TEST=content_browsertests

Bug:  775103 
Change-Id: I137d78e40c3f121ed8d966144318c85ea9bfe6d1
Reviewed-on: https://chromium-review.googlesource.com/970911
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Commit-Queue: Jonathan Ross <jonross@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544411}
[modify] https://crrev.com/4fd54fe26a35e1a333c5b93707eb348512521c28/content/browser/renderer_host/render_frame_metadata_provider_impl.cc

Is there anything left for this bug? We have investigated and come up with some plans for fixing the problems we know about.
It's become a bit of a misnomer, which we've been attributing the fixes to.
We could generate separate issues for each one?
Status: Fixed (was: Started)
I think we know all the usages if CompositorFrameMetadata on desktop. Marking as fixed.

Sign in to add a comment