New issue
Advanced search Search tips

Issue 692880 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

DelegatedFrameHost should use CompositorFrameSinkSupport

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

Issue description

DelegatedFrameHost should use CompositorFrameSinkSupport
 
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 17 2017

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

commit 6ede8b88d716c54c8a0d62b9c3907141137b28f8
Author: samans <samans@chromium.org>
Date: Fri Feb 17 04:22:45 2017

CompositorFrameSinkSupportClient::WillDraw needs to take in arguments

SurfaceFactory sends a LocalSurfaceId and a gfx::Rect to its client's WillDraw method, but CompositorFrameSinkSupport doesn't do so.
This is a problem because we DelegatedFrameHost needs those arguments
in WillDraw so if we want to switch to CompositorFrameSinkSupport,
CompositorFrameSinkSupport must also forward those arguments.

TBR=cpu@chromium.org

BUG= 692880 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2701663003
Cr-Commit-Position: refs/heads/master@{#451206}

[modify] https://crrev.com/6ede8b88d716c54c8a0d62b9c3907141137b28f8/android_webview/browser/surfaces_instance.cc
[modify] https://crrev.com/6ede8b88d716c54c8a0d62b9c3907141137b28f8/android_webview/browser/surfaces_instance.h
[modify] https://crrev.com/6ede8b88d716c54c8a0d62b9c3907141137b28f8/cc/ipc/mojo_compositor_frame_sink.mojom
[modify] https://crrev.com/6ede8b88d716c54c8a0d62b9c3907141137b28f8/cc/surfaces/compositor_frame_sink_support.cc
[modify] https://crrev.com/6ede8b88d716c54c8a0d62b9c3907141137b28f8/cc/surfaces/compositor_frame_sink_support_client.h
[modify] https://crrev.com/6ede8b88d716c54c8a0d62b9c3907141137b28f8/cc/surfaces/compositor_frame_sink_support_unittest.cc
[modify] https://crrev.com/6ede8b88d716c54c8a0d62b9c3907141137b28f8/components/display_compositor/gpu_compositor_frame_sink.cc
[modify] https://crrev.com/6ede8b88d716c54c8a0d62b9c3907141137b28f8/components/display_compositor/gpu_compositor_frame_sink.h
[modify] https://crrev.com/6ede8b88d716c54c8a0d62b9c3907141137b28f8/components/exo/compositor_frame_sink.cc
[modify] https://crrev.com/6ede8b88d716c54c8a0d62b9c3907141137b28f8/components/exo/compositor_frame_sink.h
[modify] https://crrev.com/6ede8b88d716c54c8a0d62b9c3907141137b28f8/components/exo/compositor_frame_sink_holder.cc
[modify] https://crrev.com/6ede8b88d716c54c8a0d62b9c3907141137b28f8/components/exo/compositor_frame_sink_holder.h
[modify] https://crrev.com/6ede8b88d716c54c8a0d62b9c3907141137b28f8/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink.cc
[modify] https://crrev.com/6ede8b88d716c54c8a0d62b9c3907141137b28f8/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink.h
[modify] https://crrev.com/6ede8b88d716c54c8a0d62b9c3907141137b28f8/content/renderer/android/synchronous_compositor_frame_sink.cc
[modify] https://crrev.com/6ede8b88d716c54c8a0d62b9c3907141137b28f8/content/renderer/android/synchronous_compositor_frame_sink.h
[modify] https://crrev.com/6ede8b88d716c54c8a0d62b9c3907141137b28f8/services/ui/public/cpp/window_compositor_frame_sink.cc
[modify] https://crrev.com/6ede8b88d716c54c8a0d62b9c3907141137b28f8/services/ui/public/cpp/window_compositor_frame_sink.h
[modify] https://crrev.com/6ede8b88d716c54c8a0d62b9c3907141137b28f8/services/ui/ws/frame_generator.cc
[modify] https://crrev.com/6ede8b88d716c54c8a0d62b9c3907141137b28f8/services/ui/ws/frame_generator.h
[modify] https://crrev.com/6ede8b88d716c54c8a0d62b9c3907141137b28f8/third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp
[modify] https://crrev.com/6ede8b88d716c54c8a0d62b9c3907141137b28f8/third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.h

Project Member

Comment 2 by bugdroid1@chromium.org, Feb 23 2017

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

commit 53815530f079d15e42d21571466156b40881ed58
Author: samans <samans@chromium.org>
Date: Thu Feb 23 04:57:51 2017

DelegatedFrameHost should not return old resources to renderer

As you can see in RendererCompositorFrameSink::OnReclaimCompositorResources,
the renderer will ignore the resources returned by DelegatedFrameHost that
have an old compositor_frame_sink_id, so we might as well not return them.
This CL also fixes a unit test that wrongfully tested that the resources must
be returned even after context lost.

BUG= 692880 
TBR=jam@chromium.org

Review-Url: https://codereview.chromium.org/2710073003
Cr-Commit-Position: refs/heads/master@{#452394}

[modify] https://crrev.com/53815530f079d15e42d21571466156b40881ed58/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/53815530f079d15e42d21571466156b40881ed58/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Feb 23 2017

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

commit 955e4e20be0b90a0b5dde5877ec19167baffa0b4
Author: samans <samans@chromium.org>
Date: Thu Feb 23 15:53:40 2017

Currently, DelegatedFrameHost sends an immediate ack when compositor is null, but we believe
this is not necessary.

BUG= 692880 
TBR=jam@chromium.org

Review-Url: https://codereview.chromium.org/2708423002
Cr-Commit-Position: refs/heads/master@{#452501}

[modify] https://crrev.com/955e4e20be0b90a0b5dde5877ec19167baffa0b4/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/955e4e20be0b90a0b5dde5877ec19167baffa0b4/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Feb 23 2017

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

commit 8e0074effadf2beceb5df755b7e61ef095cde7a1
Author: samans <samans@chromium.org>
Date: Thu Feb 23 22:05:56 2017

Getting rid of immediate ack in DelegatedFrameHost

When DelegatedFrameHost receives a frame from the renderer that does not
have the right size, it sends back an ack immediately so that the
renderer sends a new frame (hopefully with the right size) as soon as
possible. The problem is that this feature relies on SubmitCompositorFrame
taking an ack callback which will no longer be true once we switch to
CompositorFrameSinkSupport. This CL removes the immediate ack feature
altogether. I tested this change and I could not see a noticable
difference in resize latency.

BUG= 692880 

Review-Url: https://codereview.chromium.org/2710263002
Cr-Commit-Position: refs/heads/master@{#452648}

[modify] https://crrev.com/8e0074effadf2beceb5df755b7e61ef095cde7a1/content/browser/renderer_host/delegated_frame_host.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Feb 24 2017

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

commit 208c75a57f289ba9e0fc10582179320900ad77e5
Author: lazyboy <lazyboy@chromium.org>
Date: Fri Feb 24 01:43:21 2017

Revert of Getting rid of immediate ack in DelegatedFrameHost (patchset #2 id:20001 of https://codereview.chromium.org/2710263002/ )

Reason for revert:
This CL is likely causing CaptureScreenshotTest.CaptureScreenshotArea test failures. See more details at https://bugs.chromium.org/p/chromium/issues/detail?id=695718

BUG= 695718 

Original issue's description:
> Getting rid of immediate ack in DelegatedFrameHost
>
> When DelegatedFrameHost receives a frame from the renderer that does not
> have the right size, it sends back an ack immediately so that the
> renderer sends a new frame (hopefully with the right size) as soon as
> possible. The problem is that this feature relies on SubmitCompositorFrame
> taking an ack callback which will no longer be true once we switch to
> CompositorFrameSinkSupport. This CL removes the immediate ack feature
> altogether. I tested this change and I could not see a noticable
> difference in resize latency.
>
> BUG= 692880 
>
> Review-Url: https://codereview.chromium.org/2710263002
> Cr-Commit-Position: refs/heads/master@{#452648}
> Committed: https://chromium.googlesource.com/chromium/src/+/8e0074effadf2beceb5df755b7e61ef095cde7a1

TBR=fsamuel@chromium.org,jbauman@chromium.org,jam@chromium.org,samans@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 692880 

Review-Url: https://codereview.chromium.org/2713553013
Cr-Commit-Position: refs/heads/master@{#452710}

[modify] https://crrev.com/208c75a57f289ba9e0fc10582179320900ad77e5/content/browser/renderer_host/delegated_frame_host.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Feb 24 2017

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

commit 7f5ee4fc924e13877c064c4b73388cdffff76ecd
Author: samans <samans@chromium.org>
Date: Fri Feb 24 21:22:52 2017

Getting rid of immediate ack in DelegatedFrameHost (Relanding)

This is a relanding of https://codereview.chromium.org/2710263002/ with a
fix. The original CL was causing flakes with
CaptureScreenshotTest.CaptureScreenshotArea. Apparently taking screenshots
depends on latency infos and latency infos somehow get lost when the frame
size is wrong.

BUG= 692880 

Review-Url: https://codereview.chromium.org/2711263004
Cr-Commit-Position: refs/heads/master@{#452924}

[modify] https://crrev.com/7f5ee4fc924e13877c064c4b73388cdffff76ecd/content/browser/renderer_host/delegated_frame_host.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Feb 24 2017

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

commit 566aae768dfb729fad06c14339dcd7dfe2e5aba5
Author: samans <samans@chromium.org>
Date: Fri Feb 24 23:39:30 2017

DelegatedFrameHost should use CompositorFrameSinkSupport

This CL replaces SurfaceFactory with CompositorFrameSinkSupport, which takes us
one step closer to enabling SurfaceReferences in Chrome and moving the
DisplayCompositor out of the browser process.

BUG= 692880 
TBR=jam@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2693423005
Cr-Commit-Position: refs/heads/master@{#452999}

[modify] https://crrev.com/566aae768dfb729fad06c14339dcd7dfe2e5aba5/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/566aae768dfb729fad06c14339dcd7dfe2e5aba5/content/browser/renderer_host/delegated_frame_host.h
[modify] https://crrev.com/566aae768dfb729fad06c14339dcd7dfe2e5aba5/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc

Status: Fixed (was: Assigned)

Sign in to add a comment