DelegatedFrameHost should use CompositorFrameSinkSupport

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}
Committed: https://chromium.googlesource.com/chromium/src/+/566aae768dfb729fad06c14339dcd7dfe2e5aba5

[Saman Sami, 2017-02-16 21:23:40]

Description was changed from

==========
DelegatedFrameHost should use CompositorFrameSinkSupport

BUG=692880
==========

to

==========
DelegatedFrameHost should use CompositorFrameSinkSupport

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

[Saman Sami, 2017-02-17 15:14:03]

The CQ bit was checked by samans@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-17 15:14:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-17 15:38:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-17 15:38:24]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[Saman Sami, 2017-02-17 18:29:26]

The CQ bit was checked by samans@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-17 18:30:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-17 18:53:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-17 18:53:20]

Dry run: Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[saman, 2017-02-21 20:51:32]

The CQ bit was checked by samans@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-21 20:52:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-21 21:22:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-21 21:22:21]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Saman Sami, 2017-02-21 21:28:58]

The CQ bit was checked by samans@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-21 21:29:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Saman Sami, 2017-02-21 21:42:39]

Description was changed from

==========
DelegatedFrameHost should use CompositorFrameSinkSupport

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

to

==========
DelegatedFrameHost should use CompositorFrameSinkSupport

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

[Saman Sami, 2017-02-21 21:42:39]

samans@chromium.org changed reviewers:
- samans@google.com

[Saman Sami, 2017-02-21 21:52:10]

The CQ bit was checked by samans@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-21 21:54:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Saman Sami, 2017-02-21 21:58:06]

The CQ bit was checked by samans@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-21 21:59:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Saman Sami, 2017-02-21 22:07:45]

The CQ bit was checked by samans@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-21 22:09:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Saman Sami, 2017-02-21 22:24:54]

The CQ bit was checked by samans@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-21 22:25:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-21 23:44:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-21 23:44:51]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Saman Sami, 2017-02-22 15:17:40]

The CQ bit was checked by samans@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-22 15:17:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-22 15:29:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-22 15:29:26]

Dry run: Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Saman Sami, 2017-02-22 16:12:03]

The CQ bit was checked by samans@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-22 16:12:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-22 16:23:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-22 16:23:58]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Saman Sami, 2017-02-22 18:40:16]

The CQ bit was checked by samans@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-22 18:41:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-22 19:07:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-22 19:08:00]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Saman Sami, 2017-02-22 19:49:35]

The CQ bit was checked by samans@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-22 19:50:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-22 20:21:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-22 20:21:23]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[Saman Sami, 2017-02-22 23:36:32]

samans@chromium.org changed reviewers:
+ fsamuel@chromium.org

[Saman Sami, 2017-02-23 15:38:26]

The CQ bit was checked by samans@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-23 15:38:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-23 16:02:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-23 16:02:58]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Saman Sami, 2017-02-23 16:32:28]

The CQ bit was checked by samans@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-23 16:32:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-23 16:58:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-23 16:58:09]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[Saman Sami, 2017-02-23 17:41:55]

The CQ bit was checked by samans@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-23 17:42:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Fady Samuel, 2017-02-23 18:32:40]

https://codereview.chromium.org/2693423005/diff/220001/content/browser/render...
File content/browser/renderer_host/delegated_frame_host.cc (right):

https://codereview.chromium.org/2693423005/diff/220001/content/browser/render...
content/browser/renderer_host/delegated_frame_host.cc:62: support_ =
base::MakeUnique<cc::CompositorFrameSinkSupport>(
Maybe we can move creating a support to a helper method?

https://codereview.chromium.org/2693423005/diff/220001/content/browser/render...
content/browser/renderer_host/delegated_frame_host.cc:453:
compositor_->RemoveFrameSink(frame_sink_id_);
Remind me why you remove the FrameSink first and then add it again?

[Fady Samuel, 2017-02-23 18:32:41]

https://codereview.chromium.org/2693423005/diff/220001/content/browser/render...
File content/browser/renderer_host/delegated_frame_host.cc (right):

https://codereview.chromium.org/2693423005/diff/220001/content/browser/render...
content/browser/renderer_host/delegated_frame_host.cc:62: support_ =
base::MakeUnique<cc::CompositorFrameSinkSupport>(
Maybe we can move creating a support to a helper method?

https://codereview.chromium.org/2693423005/diff/220001/content/browser/render...
content/browser/renderer_host/delegated_frame_host.cc:453:
compositor_->RemoveFrameSink(frame_sink_id_);
Remind me why you remove the FrameSink first and then add it again?

[Saman Sami, 2017-02-23 18:34:40]

https://codereview.chromium.org/2693423005/diff/220001/content/browser/render...
File content/browser/renderer_host/delegated_frame_host.cc (right):

https://codereview.chromium.org/2693423005/diff/220001/content/browser/render...
content/browser/renderer_host/delegated_frame_host.cc:62: support_ =
base::MakeUnique<cc::CompositorFrameSinkSupport>(
On 2017/02/23 18:32:40, Fady Samuel wrote:
> Maybe we can move creating a support to a helper method?

Acknowledged.

https://codereview.chromium.org/2693423005/diff/220001/content/browser/render...
content/browser/renderer_host/delegated_frame_host.cc:453:
compositor_->RemoveFrameSink(frame_sink_id_);
On 2017/02/23 18:32:40, Fady Samuel wrote:
> Remind me why you remove the FrameSink first and then add it again?

The link already exists. We need to remove and re-create so begin frame source
propagates from compositor to delegated frame host.

[commit-bot: I haz the power, 2017-02-23 18:35:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-23 18:35:40]

Dry run: This issue passed the CQ dry run.

[Saman Sami, 2017-02-23 22:57:34]

The CQ bit was checked by samans@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-23 22:58:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Saman Sami, 2017-02-23 23:10:13]

The CQ bit was checked by samans@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-23 23:10:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-23 23:34:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-23 23:34:36]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[Saman Sami, 2017-02-24 00:59:56]

The CQ bit was checked by samans@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-24 01:00:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Fady Samuel, 2017-02-24 01:20:20]

https://codereview.chromium.org/2693423005/diff/280001/content/browser/render...
File content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
(right):

https://codereview.chromium.org/2693423005/diff/280001/content/browser/render...
content/browser/renderer_host/render_widget_host_view_aura_unittest.cc:1811:
frame.resource_list.push_back(resource);
This is dead code?

[commit-bot: I haz the power, 2017-02-24 02:25:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-24 02:25:11]

Dry run: This issue passed the CQ dry run.

[Saman Sami, 2017-02-24 21:52:27]

The CQ bit was checked by samans@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-24 21:53:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Saman Sami, 2017-02-24 21:54:30]

samans@chromium.org changed reviewers:
+ jam@chromium.org, jbauman@chromium.org

[Saman Sami, 2017-02-24 21:56:35]

fsamuel@: Please review content/
jbauman@: Please review conent/
jam@: Please review the unit test

https://codereview.chromium.org/2693423005/diff/280001/content/browser/render...
File content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
(right):

https://codereview.chromium.org/2693423005/diff/280001/content/browser/render...
content/browser/renderer_host/render_widget_host_view_aura_unittest.cc:1811:
frame.resource_list.push_back(resource);
On 2017/02/24 01:20:20, Fady Samuel wrote:
> This is dead code?

Fixed

[Saman Sami, 2017-02-24 21:57:49]

Description was changed from

==========
DelegatedFrameHost should use CompositorFrameSinkSupport

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

to

==========
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
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[Fady Samuel, 2017-02-24 21:59:47]

lgtm

[commit-bot: I haz the power, 2017-02-24 23:18:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-24 23:18:49]

Dry run: This issue passed the CQ dry run.

[Saman Sami, 2017-02-24 23:28:57]

Description was changed from

==========
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
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
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
==========

[Saman Sami, 2017-02-24 23:32:07]

The CQ bit was checked by samans@chromium.org

[commit-bot: I haz the power, 2017-02-24 23:33:26]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-24 23:39:34]

CQ is committing da patch.

Bot data: {"patchset_id": 300001, "attempt_start_ts": 1487979127343350,
"parent_rev": "b8a9a5a4dc6fddf60fb247db3fca17a42662632a", "commit_rev":
"566aae768dfb729fad06c14339dcd7dfe2e5aba5"}

[commit-bot: I haz the power, 2017-02-24 23:40:15]

Description was changed from

==========
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
==========

to

==========
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}
Committed:
https://chromium.googlesource.com/chromium/src/+/566aae768dfb729fad06c14339dc...
==========

[commit-bot: I haz the power, 2017-02-24 23:40:16]

Committed patchset #16 (id:300001) as
https://chromium.googlesource.com/chromium/src/+/566aae768dfb729fad06c14339dc...