Getting rid of DelegatedFrameData

Getting rid of DelegatedFrameData

delegated_frame_data in CompositorFrame creates an unnecessary level of
indirection. This CL removes DelegatedFrameData and stores the contents of
delegated_frame_host (namely resource_list and render_pass_list) directly inside
CompositorFrame.

BUG=653741
Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Committed: https://crrev.com/ce5c3c3facaf0240ab9fdb7bbf05116af9cab5d5
Cr-Original-Commit-Position: refs/heads/master@{#428591}
Cr-Commit-Position: refs/heads/master@{#433186}

[Saman Sami, 2016-10-25 20:28:56]

Description was changed from

==========
Getting rid of DelegatedFrameData
BUG=653741
==========

to

==========
Getting rid of DelegatedFrameData
BUG=653741
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Saman Sami, 2016-10-25 20:28:57]

Description was changed from

==========
Getting rid of DelegatedFrameData
BUG=653741
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Getting rid of DelegatedFrameData
BUG=653741
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[Saman Sami, 2016-10-25 20:30:08]

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

[commit-bot: I haz the power, 2016-10-25 20:30:38]

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

[Saman Sami, 2016-10-25 20:39:14]

Description was changed from

==========
Getting rid of DelegatedFrameData
BUG=653741
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Getting rid of DelegatedFrameData
BUG=653741
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[Saman Sami, 2016-10-25 20:39:14]

samans@chromium.org changed reviewers:
+ aelias@chromium.org, ccameron@chromium.org, darin@chromium.org,
dcheng@chromium.org, fsamuel@chromium.org

[Fady Samuel, 2016-10-25 21:34:30]

fsamuel@chromium.org changed reviewers:
- aelias@chromium.org, ccameron@chromium.org, darin@chromium.org,
dcheng@chromium.org

[Fady Samuel, 2016-10-25 21:39:21]

lgtm

Please pass to:

danakj@ for cc/
piman@ for content/
boliu@ for android_webview
sky@ for services/ and ui/
tsepez@ for ParamTraits and StructTraits things

[commit-bot: I haz the power, 2016-10-25 21:47:14]

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

[commit-bot: I haz the power, 2016-10-25 21:47:17]

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

[Saman Sami, 2016-10-25 21:50:24]

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

[Saman Sami, 2016-10-25 21:50:25]

Hi Bo
Can you please review android_webview?

[Saman Sami, 2016-10-25 21:56:04]

samans@chromium.org changed reviewers:
+ danakj@chromium.org, piman@chromium.org, sky@chromium.org, tsepez@chromium.org
- boliu@chromium.org

[Saman Sami, 2016-10-25 21:56:05]

danakj@: cc/
piman@: content/
sky@: services/ and ui/
tsepez@: ParamTraits and StructTraits

[Saman Sami, 2016-10-25 21:56:32]

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

[boliu, 2016-10-25 22:22:34]

I think we had this discussion before. Can you summarize somewhere how will
webview detect an "empty" frame now? In the CL description?

[danakj, 2016-10-25 22:23:08]

https://codereview.chromium.org/2449853004/diff/1/cc/ipc/cc_param_traits.cc
File cc/ipc/cc_param_traits.cc (right):

https://codereview.chromium.org/2449853004/diff/1/cc/ipc/cc_param_traits.cc#n...
cc/ipc/cc_param_traits.cc:719: p->render_pass_list.clear();
Why would they not already be empty?

https://codereview.chromium.org/2449853004/diff/1/cc/output/compositor_frame.cc
File cc/output/compositor_frame.cc (right):

https://codereview.chromium.org/2449853004/diff/1/cc/output/compositor_frame....
cc/output/compositor_frame.cc:18: return render_pass_list.empty() &&
resource_list.empty();
I find this function weird..

1) resource_list is a set of things referred to in the render_pass_list. So it
has to be empty if render_pass_list is empty.

2) empty compositor frames exist before this patch and we don't branch on them
in places that this patch now does, so why is that what we want?

[piman, 2016-10-25 22:57:31]

https://codereview.chromium.org/2449853004/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_impl.cc (right):

https://codereview.chromium.org/2449853004/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_impl.cc:1637: if
(!frame.IsEmpty())
nit: you can skip this test, ReturnResources is a noop on an empty list.

https://codereview.chromium.org/2449853004/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/2449853004/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_view_aura.cc:911: if
(frame.IsEmpty())
I don't think this test is necessary, or required. In theory the renderer could
send an empty frame while updating metadata (e.g. selection bounds).

[boliu, 2016-10-25 23:00:30]

https://codereview.chromium.org/2449853004/diff/1/content/browser/android/syn...
File content/browser/android/synchronous_compositor_browser_filter.cc (right):

https://codereview.chromium.org/2449853004/diff/1/content/browser/android/syn...
content/browser/android/synchronous_compositor_browser_filter.cc:83: if
(!compositor_frame.IsEmpty()) {
you may need to rebase this, fyi

https://codereview.chromium.org/2449853004/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_view_mac.mm (left):

https://codereview.chromium.org/2449853004/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_view_mac.mm:1417:
bad_message::RWHVM_UNEXPECTED_FRAME_TYPE);
you can mark this enum as unused in bad_message.h, also you can stop including
bad_message.h here

[Tom Sepez, 2016-10-25 23:04:59]

LGTM % the one comment by danakj

[sky, 2016-10-25 23:33:06]

LGTM

[Fady Samuel, 2016-10-26 03:13:54]

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

[Fady Samuel, 2016-10-26 03:13:56]

https://codereview.chromium.org/2449853004/diff/1/cc/ipc/cc_param_traits.cc
File cc/ipc/cc_param_traits.cc (right):

https://codereview.chromium.org/2449853004/diff/1/cc/ipc/cc_param_traits.cc#n...
cc/ipc/cc_param_traits.cc:719: p->render_pass_list.clear();
On 2016/10/25 22:23:07, danakj wrote:
> Why would they not already be empty?

You might want to overwrite an existing CompositorFrame right?

https://codereview.chromium.org/2449853004/diff/1/cc/output/compositor_frame.cc
File cc/output/compositor_frame.cc (right):

https://codereview.chromium.org/2449853004/diff/1/cc/output/compositor_frame....
cc/output/compositor_frame.cc:18: return render_pass_list.empty() &&
resource_list.empty();
On 2016/10/25 22:23:07, danakj wrote:
> I find this function weird..
> 
> 1) resource_list is a set of things referred to in the render_pass_list. So it
> has to be empty if render_pass_list is empty.
> 
> 2) empty compositor frames exist before this patch and we don't branch on them
> in places that this patch now does, so why is that what we want?

I agree we probably don't need this or just return render_pass_list.empty() and
DCHECK that resource_list.empty()) || !render_pass_list.empty()

[Saman Sami, 2016-10-26 13:25:54]

Description was changed from

==========
Getting rid of DelegatedFrameData
BUG=653741
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Getting rid of DelegatedFrameData

Note: Since delegated_frame_data no longer exists in CompositorFrame, in order
to detect empty compositor frames, call the method IsEmpty which basically
checks whether render_pass_list is empty.

BUG=653741
CQ_INCLUDE_TRYBOTS=mastr.tryserver.blink:linux_precise_blink_rel
==========

[Saman Sami, 2016-10-26 14:39:38]

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

[commit-bot: I haz the power, 2016-10-26 14:39:49]

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

[Saman Sami, 2016-10-26 14:41:18]

https://codereview.chromium.org/2449853004/diff/1/cc/ipc/cc_param_traits.cc
File cc/ipc/cc_param_traits.cc (right):

https://codereview.chromium.org/2449853004/diff/1/cc/ipc/cc_param_traits.cc#n...
cc/ipc/cc_param_traits.cc:719: p->render_pass_list.clear();
On 2016/10/26 03:13:55, Fady Samuel wrote:
> On 2016/10/25 22:23:07, danakj wrote:
> > Why would they not already be empty?
> 
> You might want to overwrite an existing CompositorFrame right?

Since the original code assumes the frame is empty, I will make the same
assumption.

https://codereview.chromium.org/2449853004/diff/1/cc/output/compositor_frame.cc
File cc/output/compositor_frame.cc (right):

https://codereview.chromium.org/2449853004/diff/1/cc/output/compositor_frame....
cc/output/compositor_frame.cc:18: return render_pass_list.empty() &&
resource_list.empty();
On 2016/10/25 22:23:07, danakj wrote:
> I find this function weird..
> 
> 1) resource_list is a set of things referred to in the render_pass_list. So it
> has to be empty if render_pass_list is empty.
> 
> 2) empty compositor frames exist before this patch and we don't branch on them
> in places that this patch now does, so why is that what we want?

1) I'll fix it.
2) There are certain places that we check for empty frames. See
content/browser/android/synchronous_compositor_host.cc:146 for an example. Can
you be more specific about where I have introduced new branches?

[commit-bot: I haz the power, 2016-10-26 15:30:30]

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

[commit-bot: I haz the power, 2016-10-26 15:30:31]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  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_...)

[Fady Samuel, 2016-10-26 16:17:41]

https://codereview.chromium.org/2449853004/diff/20001/cc/output/compositor_fr...
File cc/output/compositor_frame.cc (right):

https://codereview.chromium.org/2449853004/diff/20001/cc/output/compositor_fr...
cc/output/compositor_frame.cc:18: DCHECK(resource_list.empty() ==
render_pass_list.empty());
This DCHECK is wrong:

DCHECK(resource_list.empty() || !render_pass_list.empty());

[Saman Sami, 2016-10-26 16:51:40]

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

[commit-bot: I haz the power, 2016-10-26 16:52:05]

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

[Saman Sami, 2016-10-26 18:03:13]

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

[Saman Sami, 2016-10-26 18:03:18]

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

[commit-bot: I haz the power, 2016-10-26 18:03:53]

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

[danakj, 2016-10-26 18:47:44]

On Wed, Oct 26, 2016 at 7:41 AM, <samans@chromium.org> wrote:

>
> https://codereview.chromium.org/2449853004/diff/1/cc/ipc/
> cc_param_traits.cc
> File cc/ipc/cc_param_traits.cc (right):
>
> https://codereview.chromium.org/2449853004/diff/1/cc/ipc/
> cc_param_traits.cc#newcode719
> cc/ipc/cc_param_traits.cc:719: p->render_pass_list.clear();
> On 2016/10/26 03:13:55, Fady Samuel wrote:
> > On 2016/10/25 22:23:07, danakj wrote:
> > > Why would they not already be empty?
> >
> > You might want to overwrite an existing CompositorFrame right?
>
> Since the original code assumes the frame is empty, I will make the same
> assumption.
>
> https://codereview.chromium.org/2449853004/diff/1/cc/
> output/compositor_frame.cc
> File cc/output/compositor_frame.cc (right):
>
> https://codereview.chromium.org/2449853004/diff/1/cc/
> output/compositor_frame.cc#newcode18
> cc/output/compositor_frame.cc:18: return render_pass_list.empty() &&
> resource_list.empty();
> On 2016/10/25 22:23:07, danakj wrote:
> > I find this function weird..
> >
> > 1) resource_list is a set of things referred to in the
> render_pass_list. So it
> > has to be empty if render_pass_list is empty.
> >
> > 2) empty compositor frames exist before this patch and we don't branch
> on them
> > in places that this patch now does, so why is that what we want?
>
> 1) I'll fix it.
> 2) There are certain places that we check for empty frames. See
> content/browser/android/synchronous_compositor_host.cc:146 for an
> example. Can you be more specific about where I have introduced new
> branches?
>

Hm, what I mean to say is, code before was checking for a non-null frame
data to say that a frame had been received or was "null". It could have a
non-null frame data that's empty, so "null" vs "empty" were different.

This collapses them so no we take the "null" branch when we had an "empty"
frame. How intentional is this?

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[danakj, 2016-10-26 18:48:29]

On Wed, Oct 26, 2016 at 7:41 AM, <samans@chromium.org> wrote:

>
> https://codereview.chromium.org/2449853004/diff/1/cc/ipc/
> cc_param_traits.cc
> File cc/ipc/cc_param_traits.cc (right):
>
> https://codereview.chromium.org/2449853004/diff/1/cc/ipc/
> cc_param_traits.cc#newcode719
> cc/ipc/cc_param_traits.cc:719: p->render_pass_list.clear();
> On 2016/10/26 03:13:55, Fady Samuel wrote:
> > On 2016/10/25 22:23:07, danakj wrote:
> > > Why would they not already be empty?
> >
> > You might want to overwrite an existing CompositorFrame right?
>
> Since the original code assumes the frame is empty, I will make the same
> assumption.
>
> https://codereview.chromium.org/2449853004/diff/1/cc/
> output/compositor_frame.cc
> File cc/output/compositor_frame.cc (right):
>
> https://codereview.chromium.org/2449853004/diff/1/cc/
> output/compositor_frame.cc#newcode18
> cc/output/compositor_frame.cc:18: return render_pass_list.empty() &&
> resource_list.empty();
> On 2016/10/25 22:23:07, danakj wrote:
> > I find this function weird..
> >
> > 1) resource_list is a set of things referred to in the
> render_pass_list. So it
> > has to be empty if render_pass_list is empty.
> >
> > 2) empty compositor frames exist before this patch and we don't branch
> on them
> > in places that this patch now does, so why is that what we want?
>
> 1) I'll fix it.
> 2) There are certain places that we check for empty frames. See
> content/browser/android/synchronous_compositor_host.cc:146 for an
> example. Can you be more specific about where I have introduced new
> branches?
>

Hm, what I mean to say is, code before was checking for a non-null frame
data to say that a frame had been received or was "null". It could have a
non-null frame data that's empty, so "null" vs "empty" were different.

This collapses them so no we take the "null" branch when we had an "empty"
frame. How intentional is this?

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[boliu, 2016-10-26 18:50:49]

> Hm, what I mean to say is, code before was checking for a non-null frame data
to say that a frame had been received or was "null". It could have a non-null
frame data that's empty, so "null" vs "empty" were different.
>
> This collapses them so no we take the "null" branch when we had an "empty"
frame. How intentional is this?

That's a good way to put it.

For webview, ie sync compositor, we needed the null check before at a bunch of
places, because we have to send back something to browser even if renderer
didn't swap. So now webview checks for empty instead of null. But webview could
probably done outside of CompositorFrame, like pass back a bool along with a
CompositorFrame, and don't access the frame if bool is false.

I'm guessing collapsing null and empty in general is unintended.

[Saman Sami, 2016-10-26 23:45:39]

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

[commit-bot: I haz the power, 2016-10-26 23:46:07]

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

[Saman Sami, 2016-10-27 00:08:08]

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

[commit-bot: I haz the power, 2016-10-27 00:08:30]

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

[Saman Sami, 2016-10-27 00:20:14]

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

[commit-bot: I haz the power, 2016-10-27 00:20:41]

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

[Saman Sami, 2016-10-27 01:32:17]

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

[commit-bot: I haz the power, 2016-10-27 01:32:36]

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

[Saman Sami, 2016-10-27 13:37:47]

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

[commit-bot: I haz the power, 2016-10-27 13:38: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, 2016-10-27 14:52:46]

PTAL

[Saman Sami, 2016-10-27 14:58:17]

Description was changed from

==========
Getting rid of DelegatedFrameData

Note: Since delegated_frame_data no longer exists in CompositorFrame, in order
to detect empty compositor frames, call the method IsEmpty which basically
checks whether render_pass_list is empty.

BUG=653741
CQ_INCLUDE_TRYBOTS=mastr.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Getting rid of DelegatedFrameData

Note: From now on, in order to check if a frame is empty, check whether
render_pass_list is empty. The concept of "null" compositor frame is gone (a
compositor frame with a null delegated frame data) You need to either use
base::Optional<CompositorFrame> instead (See cc/surfaces/surface.h) or when
passing around frames using IPC, also pass a bool indicating whether the
compositor frame is "null" (see
content/renderer/android/synchronous_compositor_proxy.cc)

BUG=653741
CQ_INCLUDE_TRYBOTS=mastr.tryserver.blink:linux_precise_blink_rel
==========

[boliu, 2016-10-27 16:28:40]

On 2016/10/27 14:52:46, samans wrote:
> PTAL

what changed..?

[Saman Sami, 2016-10-27 16:52:57]

PTAL
In addition to addressing inline comments, I also fixed the null-empty
distinction problem. I got rid of IsEmpty function in compositor frame which was
introduced in prior patches, and instead passed a boolean parameter through ipc
in synchronous_compositor_* to signal whether a swap has happened or not instead
of relying on "null" compositor frames.

[boliu, 2016-10-27 17:04:58]

On 2016/10/27 16:52:57, samans wrote:
> PTAL
> In addition to addressing inline comments, I also fixed the null-empty
> distinction problem. I got rid of IsEmpty function in compositor frame which
was
> introduced in prior patches, and instead passed a boolean parameter through
ipc
> in synchronous_compositor_* to signal whether a swap has happened or not
instead
> of relying on "null" compositor frames.

Please pull out the synchronous compositor changes in to a separate CL so I can
review it separately from this big refactor

[Saman Sami, 2016-10-27 22:04:44]

Description was changed from

==========
Getting rid of DelegatedFrameData

Note: From now on, in order to check if a frame is empty, check whether
render_pass_list is empty. The concept of "null" compositor frame is gone (a
compositor frame with a null delegated frame data) You need to either use
base::Optional<CompositorFrame> instead (See cc/surfaces/surface.h) or when
passing around frames using IPC, also pass a bool indicating whether the
compositor frame is "null" (see
content/renderer/android/synchronous_compositor_proxy.cc)

BUG=653741
CQ_INCLUDE_TRYBOTS=mastr.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Getting rid of DelegatedFrameData

Note: From now on, in order to check if a frame is empty, check whether
render_pass_list is empty. The concept of "null" compositor frame (a compositor
frame with a null delegated frame data) is gone. You should use
base::Optional<CompositorFrame> instead (See cc/surfaces/surface.h).

BUG=653741
CQ_INCLUDE_TRYBOTS=mastr.tryserver.blink:linux_precise_blink_rel
==========

[Saman Sami, 2016-10-27 22:05:25]

Description was changed from

==========
Getting rid of DelegatedFrameData

Note: From now on, in order to check if a frame is empty, check whether
render_pass_list is empty. The concept of "null" compositor frame (a compositor
frame with a null delegated frame data) is gone. You should use
base::Optional<CompositorFrame> instead (See cc/surfaces/surface.h).

BUG=653741
CQ_INCLUDE_TRYBOTS=mastr.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Getting rid of DelegatedFrameData

Note: From now on, in order to check if a frame is empty, check whether
render_pass_list is empty. The concept of "null" compositor frame (a compositor
frame with a null delegated frame data) is gone. You should use
base::Optional<CompositorFrame> instead.

BUG=653741
CQ_INCLUDE_TRYBOTS=mastr.tryserver.blink:linux_precise_blink_rel
==========

[Saman Sami, 2016-10-27 22:27:02]

samans@chromium.org changed reviewers:
+ kmarshall@chromium.org, reveman@chromium.org

[Saman Sami, 2016-10-27 22:37:00]

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

[commit-bot: I haz the power, 2016-10-27 22:37:39]

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

[Saman Sami, 2016-10-27 22:58:34]

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

[commit-bot: I haz the power, 2016-10-27 22:59:13]

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, 2016-10-27 23:05:20]

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

[commit-bot: I haz the power, 2016-10-27 23:05:22]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  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_...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[danakj, 2016-10-27 23:06:57]

https://codereview.chromium.org/2449853004/diff/160001/android_webview/browse...
File android_webview/browser/hardware_renderer.cc (left):

https://codereview.chromium.org/2449853004/diff/160001/android_webview/browse...
android_webview/browser/hardware_renderer.cc:179: cc::CompositorFrame
empty_frame;
no need for this variable, just pass cc::CompositorFrame() directly

https://codereview.chromium.org/2449853004/diff/160001/cc/ipc/cc_param_traits.cc
File cc/ipc/cc_param_traits.cc (left):

https://codereview.chromium.org/2449853004/diff/160001/cc/ipc/cc_param_traits...
cc/ipc/cc_param_traits.cc:688: WriteParam(m, static_cast<int>(NO_FRAME));
I don't think we need this NO_FRAME vs DELEGATED_FRAME concept at all anymore?
It was just for calling the right deserialize method

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface.cc
File cc/surfaces/surface.cc (right):

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface.cc...
cc/surfaces/surface.cc:12: #include "base/optional.h"
already in the .h so not needed

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface.cc...
cc/surfaces/surface.cc:132: return current_frame_.value();
DCHECK current_frame_ exists please

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface.h
File cc/surfaces/surface.h (right):

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface.h#...
cc/surfaces/surface.h:64: // If the CompositorFrame's DelegateFrameData is null
then there is
Adjust comment ot mention that HasFrame() must be true to call this.

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface_ag...
File cc/surfaces/surface_aggregator_unittest.cc (right):

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface_ag...
cc/surfaces/surface_aggregator_unittest.cc:1039:
child_pass_list.swap(child_frame.render_pass_list);
Can you maybe instead define child_frame earlier and get rid of the
child_pass_list, just use child_frame directly?

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface_ag...
cc/surfaces/surface_aggregator_unittest.cc:1067: CompositorFrame middle_frame;
same

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface_ag...
cc/surfaces/surface_aggregator_unittest.cc:1101: CompositorFrame root_frame;
same

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface_ag...
cc/surfaces/surface_aggregator_unittest.cc:1201: CompositorFrame child_frame;
same

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface_ag...
cc/surfaces/surface_aggregator_unittest.cc:1224: CompositorFrame
parent_surface_frame;
same

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface_ag...
cc/surfaces/surface_aggregator_unittest.cc:1257: CompositorFrame root_frame;
same.. ok u get the idea lol

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface_fa...
File cc/surfaces/surface_factory_unittest.cc (right):

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface_fa...
cc/surfaces/surface_factory_unittest.cc:443:
factory_->SubmitCompositorFrame(local_frame_id, std::move(frame),
just pass a CompositorFrame(), no need for the frame variable

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface_fa...
cc/surfaces/surface_factory_unittest.cc:465: local_frame_id, std::move(frame),
same

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface_hi...
File cc/surfaces/surface_hittest.cc (right):

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface_hi...
cc/surfaces/surface_hittest.cc:261: 
nit: remove whitespace

https://codereview.chromium.org/2449853004/diff/160001/cc/test/fake_composito...
File cc/test/fake_compositor_frame_sink.cc (right):

https://codereview.chromium.org/2449853004/diff/160001/cc/test/fake_composito...
cc/test/fake_compositor_frame_sink.cc:44: if
(!last_sent_frame_->resource_list.empty()) {
dont need this if at all, begin() and end() will do the right thing if empty

https://codereview.chromium.org/2449853004/diff/160001/cc/test/surface_hittes...
File cc/test/surface_hittest_test_helpers.cc (right):

https://codereview.chromium.org/2449853004/diff/160001/cc/test/surface_hittes...
cc/test/surface_hittest_test_helpers.cc:70: CompositorFrame
CreateCompositorFrameWithRenderPassList(
I think you can drop this method and just move the code inline now

https://codereview.chromium.org/2449853004/diff/160001/cc/trees/layer_tree_ho...
File cc/trees/layer_tree_host_unittest_context.cc (left):

https://codereview.chromium.org/2449853004/diff/160001/cc/trees/layer_tree_ho...
cc/trees/layer_tree_host_unittest_context.cc:898:
frame_data->render_pass_list.push_back(std::move(pass_for_quad));
these exist for DelegatedRendererLayer which doesn't exist anymore, woops. the
pass_for_quad and pass are not used anymore you can remove all of it

https://codereview.chromium.org/2449853004/diff/160001/content/browser/render...
File content/browser/renderer_host/render_widget_host_view_aura.cc (left):

https://codereview.chromium.org/2449853004/diff/160001/content/browser/render...
content/browser/renderer_host/render_widget_host_view_aura.cc:930:
delegated_frame_host_->SwapDelegatedFrame(compositor_frame_sink_id,
This will dcheck that the frame isnt empty, which we used to use null framedata
for here. So I think you want to instead return above when frame has no render
passes

https://codereview.chromium.org/2449853004/diff/160001/content/common/android...
File content/common/android/sync_compositor_messages.h (right):

https://codereview.chromium.org/2449853004/diff/160001/content/common/android...
content/common/android/sync_compositor_messages.h:130:
IPC_SYNC_MESSAGE_ROUTED1_4(SyncCompositorMsg_DemandDrawHw,
ok I got this far and now it's tgif time and i see future patch sets so ill stop
and come back after you a round trip, looks great overall!

[Saman Sami, 2016-10-28 00:26:49]

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

[commit-bot: I haz the power, 2016-10-28 00:27:13]

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, 2016-10-28 00:32:34]

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

[commit-bot: I haz the power, 2016-10-28 00:32:36]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  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_...)
  linux_chromium_compile_dbg_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_...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[Saman Sami, 2016-10-28 00:34:29]

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

[commit-bot: I haz the power, 2016-10-28 00:34: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, 2016-10-28 01:05:45]

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

[commit-bot: I haz the power, 2016-10-28 01:05:46]

Dry run: CQ has no permission to schedule in bucket mastr.tryserver.blink

[reveman, 2016-10-28 14:17:21]

components/exo lgtm

[Saman Sami, 2016-10-28 15:49:59]

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

[commit-bot: I haz the power, 2016-10-28 15:50:26]

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, 2016-10-28 15:50:51]

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

[commit-bot: I haz the power, 2016-10-28 15:50:53]

Dry run: CQ has no permission to schedule in bucket mastr.tryserver.blink

[boliu, 2016-10-28 15:52:15]

lgtm

fwiw, next time you upload a new patch set new for reviewer to look at, send a
update on the code review

[Saman Sami, 2016-10-28 16:29:45]

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

[commit-bot: I haz the power, 2016-10-28 16:30:02]

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, 2016-10-28 16:30:32]

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

[commit-bot: I haz the power, 2016-10-28 16:30:34]

Dry run: CQ has no permission to schedule in bucket mastr.tryserver.blink

[Kevin M, 2016-10-28 16:38:23]

lgtm

[Saman Sami, 2016-10-28 16:47:36]

PTAL
I have addressed all reviewers' comments.

https://codereview.chromium.org/2449853004/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_impl.cc (right):

https://codereview.chromium.org/2449853004/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_impl.cc:1637: if
(!frame.IsEmpty())
On 2016/10/25 22:57:31, piman wrote:
> nit: you can skip this test, ReturnResources is a noop on an empty list.

Done.

https://codereview.chromium.org/2449853004/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/2449853004/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_view_aura.cc:911: if
(frame.IsEmpty())
On 2016/10/25 22:57:31, piman wrote:
> I don't think this test is necessary, or required. In theory the renderer
could
> send an empty frame while updating metadata (e.g. selection bounds).

Done.

https://codereview.chromium.org/2449853004/diff/160001/android_webview/browse...
File android_webview/browser/hardware_renderer.cc (left):

https://codereview.chromium.org/2449853004/diff/160001/android_webview/browse...
android_webview/browser/hardware_renderer.cc:179: cc::CompositorFrame
empty_frame;
On 2016/10/27 23:06:56, danakj wrote:
> no need for this variable, just pass cc::CompositorFrame() directly

Done.

https://codereview.chromium.org/2449853004/diff/160001/cc/ipc/cc_param_traits.cc
File cc/ipc/cc_param_traits.cc (left):

https://codereview.chromium.org/2449853004/diff/160001/cc/ipc/cc_param_traits...
cc/ipc/cc_param_traits.cc:688: WriteParam(m, static_cast<int>(NO_FRAME));
On 2016/10/27 23:06:56, danakj wrote:
> I don't think we need this NO_FRAME vs DELEGATED_FRAME concept at all anymore?
> It was just for calling the right deserialize method

Yes. I fixed this in later patches.

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface.cc
File cc/surfaces/surface.cc (right):

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface.cc...
cc/surfaces/surface.cc:12: #include "base/optional.h"
On 2016/10/27 23:06:56, danakj wrote:
> already in the .h so not needed

Done.

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface.cc...
cc/surfaces/surface.cc:132: return current_frame_.value();
On 2016/10/27 23:06:56, danakj wrote:
> DCHECK current_frame_ exists please

Done.

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface.h
File cc/surfaces/surface.h (right):

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface.h#...
cc/surfaces/surface.h:64: // If the CompositorFrame's DelegateFrameData is null
then there is
On 2016/10/27 23:06:56, danakj wrote:
> Adjust comment ot mention that HasFrame() must be true to call this.

Done.

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface_ag...
File cc/surfaces/surface_aggregator_unittest.cc (right):

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface_ag...
cc/surfaces/surface_aggregator_unittest.cc:1039:
child_pass_list.swap(child_frame.render_pass_list);
On 2016/10/27 23:06:56, danakj wrote:
> Can you maybe instead define child_frame earlier and get rid of the
> child_pass_list, just use child_frame directly?

Done.

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface_fa...
File cc/surfaces/surface_factory_unittest.cc (right):

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface_fa...
cc/surfaces/surface_factory_unittest.cc:443:
factory_->SubmitCompositorFrame(local_frame_id, std::move(frame),
On 2016/10/27 23:06:56, danakj wrote:
> just pass a CompositorFrame(), no need for the frame variable

Done.

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface_fa...
cc/surfaces/surface_factory_unittest.cc:465: local_frame_id, std::move(frame),
On 2016/10/27 23:06:56, danakj wrote:
> same

Done.

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface_hi...
File cc/surfaces/surface_hittest.cc (right):

https://codereview.chromium.org/2449853004/diff/160001/cc/surfaces/surface_hi...
cc/surfaces/surface_hittest.cc:261: 
On 2016/10/27 23:06:57, danakj wrote:
> nit: remove whitespace

Done.

https://codereview.chromium.org/2449853004/diff/160001/cc/test/fake_composito...
File cc/test/fake_compositor_frame_sink.cc (right):

https://codereview.chromium.org/2449853004/diff/160001/cc/test/fake_composito...
cc/test/fake_compositor_frame_sink.cc:44: if
(!last_sent_frame_->resource_list.empty()) {
On 2016/10/27 23:06:57, danakj wrote:
> dont need this if at all, begin() and end() will do the right thing if empty

Done.

https://codereview.chromium.org/2449853004/diff/160001/cc/test/surface_hittes...
File cc/test/surface_hittest_test_helpers.cc (right):

https://codereview.chromium.org/2449853004/diff/160001/cc/test/surface_hittes...
cc/test/surface_hittest_test_helpers.cc:70: CompositorFrame
CreateCompositorFrameWithRenderPassList(
On 2016/10/27 23:06:57, danakj wrote:
> I think you can drop this method and just move the code inline now

Done.

https://codereview.chromium.org/2449853004/diff/160001/cc/trees/layer_tree_ho...
File cc/trees/layer_tree_host_unittest_context.cc (left):

https://codereview.chromium.org/2449853004/diff/160001/cc/trees/layer_tree_ho...
cc/trees/layer_tree_host_unittest_context.cc:898:
frame_data->render_pass_list.push_back(std::move(pass_for_quad));
On 2016/10/27 23:06:57, danakj wrote:
> these exist for DelegatedRendererLayer which doesn't exist anymore, woops. the
> pass_for_quad and pass are not used anymore you can remove all of it

Done.

https://codereview.chromium.org/2449853004/diff/160001/content/browser/render...
File content/browser/renderer_host/render_widget_host_view_aura.cc (left):

https://codereview.chromium.org/2449853004/diff/160001/content/browser/render...
content/browser/renderer_host/render_widget_host_view_aura.cc:930:
delegated_frame_host_->SwapDelegatedFrame(compositor_frame_sink_id,
On 2016/10/27 23:06:57, danakj wrote:
> This will dcheck that the frame isnt empty, which we used to use null
framedata
> for here. So I think you want to instead return above when frame has no render
> passes

Done.

https://codereview.chromium.org/2449853004/diff/160001/content/common/android...
File content/common/android/sync_compositor_messages.h (right):

https://codereview.chromium.org/2449853004/diff/160001/content/common/android...
content/common/android/sync_compositor_messages.h:130:
IPC_SYNC_MESSAGE_ROUTED1_4(SyncCompositorMsg_DemandDrawHw,
On 2016/10/27 23:06:57, danakj wrote:
> ok I got this far and now it's tgif time and i see future patch sets so ill
stop
> and come back after you a round trip, looks great overall!

Thanks. Hopefully the rest of it is good too.

[danakj, 2016-10-28 22:02:54]

LGTM but 1 thing:

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

https://codereview.chromium.org/2449853004/diff/280001/content/browser/render...
content/browser/renderer_host/render_widget_host_view_aura.cc:928: if
(!frame.render_pass_list.empty())
{} for multi-line blocks

but before the selection would not be updated if the frame was null/empty, now
it will be. i don't think you want to change behaviour like that in the middle
of this large CL, it increases its riskiness by a lot and having to deal with
reverts of things like this is a huge pain. I think you should early out above
where the old code did.

[piman, 2016-10-29 00:32:00]

lgtm

[Saman Sami, 2016-10-29 00:50:35]

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

[commit-bot: I haz the power, 2016-10-29 00:50:50]

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, 2016-10-29 00:51:11]

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

[commit-bot: I haz the power, 2016-10-29 00:51:13]

Dry run: CQ has no permission to schedule in bucket mastr.tryserver.blink

[Saman Sami, 2016-10-29 00:52:01]

Description was changed from

==========
Getting rid of DelegatedFrameData

Note: From now on, in order to check if a frame is empty, check whether
render_pass_list is empty. The concept of "null" compositor frame (a compositor
frame with a null delegated frame data) is gone. You should use
base::Optional<CompositorFrame> instead.

BUG=653741
CQ_INCLUDE_TRYBOTS=mastr.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a compositor frame
with a null delegated frame data) is gone. You should use
base::Optional<CompositorFrame> instead.

BUG=653741
CQ_INCLUDE_TRYBOTS=mastr.tryserver.blink:linux_precise_blink_rel
==========

[Saman Sami, 2016-10-29 01:12:30]

The CQ bit was checked by samans@chromium.org

[Saman Sami, 2016-10-29 01:12:31]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org,
tsepez@chromium.org, fsamuel@chromium.org, reveman@chromium.org,
boliu@chromium.org, piman@chromium.org, danakj@chromium.org,
kmarshall@chromium.org
Link to the patchset: https://codereview.chromium.org/2449853004/#ps300001
(title: "Dana's comment   rebase")

[commit-bot: I haz the power, 2016-10-29 01:12:58]

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, 2016-10-29 01:13:15]

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

[commit-bot: I haz the power, 2016-10-29 01:13:18]

CQ has no permission to schedule in bucket mastr.tryserver.blink

[Saman Sami, 2016-10-29 01:33:11]

The CQ bit was checked by samans@chromium.org

[commit-bot: I haz the power, 2016-10-29 01:33:27]

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, 2016-10-29 01:33:41]

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

[commit-bot: I haz the power, 2016-10-29 01:33:43]

CQ has no permission to schedule in bucket mastr.tryserver.blink

[Saman Sami, 2016-10-29 01:45:42]

Description was changed from

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a compositor frame
with a null delegated frame data) is gone. You should use
base::Optional<CompositorFrame> instead.

BUG=653741
CQ_INCLUDE_TRYBOTS=mastr.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a compositor frame
with a null delegated frame data) is gone. You should use
base::Optional<CompositorFrame> instead.

BUG=653741
==========

[Saman Sami, 2016-10-29 01:46:13]

The CQ bit was checked by samans@chromium.org

[commit-bot: I haz the power, 2016-10-29 01:46:32]

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, 2016-10-29 02:28:15]

Description was changed from

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a compositor frame
with a null delegated frame data) is gone. You should use
base::Optional<CompositorFrame> instead.

BUG=653741
==========

to

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a compositor frame
with a null delegated frame data) is gone. You should use
base::Optional<CompositorFrame> instead.

BUG=653741
==========

[commit-bot: I haz the power, 2016-10-29 02:28:18]

Committed patchset #16 (id:300001)

[commit-bot: I haz the power, 2016-10-29 02:32:25]

Description was changed from

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a compositor frame
with a null delegated frame data) is gone. You should use
base::Optional<CompositorFrame> instead.

BUG=653741
==========

to

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a compositor frame
with a null delegated frame data) is gone. You should use
base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
==========

[commit-bot: I haz the power, 2016-10-29 02:32:28]

Patchset 16 (id:??) landed as
https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}

[Fady Samuel, 2016-11-16 00:38:23]

https://codereview.chromium.org/2449853004/diff/300001/cc/surfaces/display.cc
File cc/surfaces/display.cc (right):

https://codereview.chromium.org/2449853004/diff/300001/cc/surfaces/display.cc...
cc/surfaces/display.cc:217: bool root_surface_resources_locked = !surface ||
!surface->HasFrame();
I think the right check here is:

root_surface_resources_locked = !surface || !surface->HasFrame() ||
surface->GetEligibleFrame().render_pass_list.empty();

[danakj, 2016-11-16 01:23:17]

https://codereview.chromium.org/2449853004/diff/300001/cc/surfaces/display.cc
File cc/surfaces/display.cc (right):

https://codereview.chromium.org/2449853004/diff/300001/cc/surfaces/display.cc...
cc/surfaces/display.cc:217: bool root_surface_resources_locked = !surface ||
!surface->HasFrame();
On 2016/11/16 00:38:23, Fady Samuel wrote:
> I think the right check here is:
> 
> root_surface_resources_locked = !surface || !surface->HasFrame() ||
> surface->GetEligibleFrame().render_pass_list.empty();

Why? The old code did not check for an empty render_pass_list.

[Saman Sami, 2016-11-16 01:40:26]

https://codereview.chromium.org/2449853004/diff/300001/cc/surfaces/display.cc
File cc/surfaces/display.cc (right):

https://codereview.chromium.org/2449853004/diff/300001/cc/surfaces/display.cc...
cc/surfaces/display.cc:217: bool root_surface_resources_locked = !surface ||
!surface->HasFrame();
On 2016/11/16 01:23:17, danakj wrote:
> On 2016/11/16 00:38:23, Fady Samuel wrote:
> > I think the right check here is:
> > 
> > root_surface_resources_locked = !surface || !surface->HasFrame() ||
> > surface->GetEligibleFrame().render_pass_list.empty();
> 
> Why? The old code did not check for an empty render_pass_list.

It didn't explicitly, what we believe it meant to check for it. Could you take a
look at John's comment here:
https://bugs.chromium.org/p/chromium/issues/detail?id=663516#c12

[danakj, 2016-11-16 01:56:17]

https://codereview.chromium.org/2449853004/diff/300001/cc/surfaces/display.cc
File cc/surfaces/display.cc (right):

https://codereview.chromium.org/2449853004/diff/300001/cc/surfaces/display.cc...
cc/surfaces/display.cc:217: bool root_surface_resources_locked = !surface ||
!surface->HasFrame();
On 2016/11/16 01:40:26, Saman Sami wrote:
> On 2016/11/16 01:23:17, danakj wrote:
> > On 2016/11/16 00:38:23, Fady Samuel wrote:
> > > I think the right check here is:
> > > 
> > > root_surface_resources_locked = !surface || !surface->HasFrame() ||
> > > surface->GetEligibleFrame().render_pass_list.empty();
> > 
> > Why? The old code did not check for an empty render_pass_list.
> 
> It didn't explicitly, what we believe it meant to check for it. Could you take
a
> look at John's comment here:
> https://bugs.chromium.org/p/chromium/issues/detail?id=663516#c12

I see, this should be false if an empty frame was submitted to reclaim
resources?

This is because ForceReclaimResources() now has to way to submit a "null" frame
and is submitting an "empty" frame instead? Perhaps it should be doing something
else that would cause HasFrame to return false.

[danakj, 2016-11-16 01:57:00]

https://codereview.chromium.org/2449853004/diff/300001/cc/surfaces/direct_com...
File cc/surfaces/direct_compositor_frame_sink.cc (right):

https://codereview.chromium.org/2449853004/diff/300001/cc/surfaces/direct_com...
cc/surfaces/direct_compositor_frame_sink.cc:112:
factory_.SubmitCompositorFrame(delegated_local_frame_id_, CompositorFrame(),
Specifically should this EvictSurface or something instead of submitting an
empty frame?

[Saman Sami, 2016-11-16 20:15:24]

Description was changed from

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a compositor frame
with a null delegated frame data) is gone. You should use
base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
==========

to

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a compositor frame
with a null delegated frame data) is gone. You should use
base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
==========

[Saman Sami, 2016-11-16 20:27:48]

Description was changed from

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a compositor frame
with a null delegated frame data) is gone. You should use
base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
==========

to

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a compositor frame
with a null delegated frame data) is gone. You should use
base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Saman Sami, 2016-11-16 20:27:50]

Description was changed from

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a compositor frame
with a null delegated frame data) is gone. You should use
base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a compositor frame
with a null delegated frame data) is gone. You should use
base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[Saman Sami, 2016-11-16 20:28:28]

Description was changed from

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a compositor frame
with a null delegated frame data) is gone. You should use
base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a compositor frame
with a null delegated frame data) is gone. You should use
base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[Saman Sami, 2016-11-16 20:28:43]

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

[commit-bot: I haz the power, 2016-11-16 20:29:26]

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, 2016-11-16 21:08:10]

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

[commit-bot: I haz the power, 2016-11-16 21:08:12]

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

[Saman Sami, 2016-11-16 22:54:11]

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

[commit-bot: I haz the power, 2016-11-16 22:54:59]

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

[Saman Sami, 2016-11-17 00:11:33]

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

[commit-bot: I haz the power, 2016-11-17 00:12: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, 2016-11-17 04:15:08]

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

[commit-bot: I haz the power, 2016-11-17 04:15:10]

Dry run: This issue passed the CQ dry run.

[Saman Sami, 2016-11-17 17:18:29]

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

[commit-bot: I haz the power, 2016-11-17 17:18:59]

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, 2016-11-17 17:22:12]

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

[commit-bot: I haz the power, 2016-11-17 17:22:14]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Saman Sami, 2016-11-17 18:13:54]

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

[commit-bot: I haz the power, 2016-11-17 18:14:56]

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, 2016-11-17 18:20:08]

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

[commit-bot: I haz the power, 2016-11-17 18:20:09]

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

[Saman Sami, 2016-11-17 20:23:02]

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

[commit-bot: I haz the power, 2016-11-17 20:23:56]

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, 2016-11-17 21:44:52]

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

[commit-bot: I haz the power, 2016-11-17 21:44:54]

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

[Saman Sami, 2016-11-17 21:52:32]

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

[commit-bot: I haz the power, 2016-11-17 21:54:18]

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

[Saman Sami, 2016-11-18 00:03:16]

Description was changed from

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a compositor frame
with a null delegated frame data) is gone. You should use
base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a compositor frame
with a null delegated frame data) is gone. You should use
base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
==========

[commit-bot: I haz the power, 2016-11-18 00:06:18]

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

[commit-bot: I haz the power, 2016-11-18 00:06:19]

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

[Saman Sami, 2016-11-18 01:37:11]

Description was changed from

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a compositor frame
with a null delegated frame data) is gone. You should use
base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
==========

to

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a compositor frame
with a null delegated frame data) is gone. You should use
base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Saman Sami, 2016-11-18 01:37:12]

Description was changed from

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a compositor frame
with a null delegated frame data) is gone. You should use
base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a compositor frame
with a null delegated frame data) is gone. You should use
base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[Saman Sami, 2016-11-18 01:38:33]

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

[commit-bot: I haz the power, 2016-11-18 01:39:53]

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

[Saman Sami, 2016-11-18 01:43:05]

Description was changed from

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a compositor frame
with a null delegated frame data) is gone. You should use
base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a 
compositor frame with a null delegated frame data) is gone. You 
should use base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[Saman Sami, 2016-11-18 01:43:34]

Description was changed from

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a 
compositor frame with a null delegated frame data) is gone. You 
should use base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a 
compositor frame with a null delegated frame data) is gone. You 
should use base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
==========

[Saman Sami, 2016-11-18 01:43:47]

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

[commit-bot: I haz the power, 2016-11-18 01:45:43]

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

[Saman Sami, 2016-11-18 01:58:21]

Description was changed from

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a 
compositor frame with a null delegated frame data) is gone. You 
should use base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
==========

to

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a 
compositor frame with a null delegated frame data) is gone. You 
should use base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Saman Sami, 2016-11-18 01:58:23]

Description was changed from

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a 
compositor frame with a null delegated frame data) is gone. You 
should use base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a 
compositor frame with a null delegated frame data) is gone. You 
should use base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[Saman Sami, 2016-11-18 02:04:58]

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

[commit-bot: I haz the power, 2016-11-18 02:06:01]

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

[Fady Samuel, 2016-11-18 02:16:52]

I like it! lgtm + nit

https://codereview.chromium.org/2449853004/diff/440001/cc/surfaces/direct_com...
File cc/surfaces/direct_compositor_frame_sink.cc (left):

https://codereview.chromium.org/2449853004/diff/440001/cc/surfaces/direct_com...
cc/surfaces/direct_compositor_frame_sink.cc:112: if
(delegated_local_frame_id_.is_valid()) {
nit: remove braces

[Saman Sami, 2016-11-18 02:21:05]

Description was changed from

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a 
compositor frame with a null delegated frame data) is gone. You 
should use base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a 
compositor frame with a null delegated frame data) is gone. You 
should use base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
==========

[Saman Sami, 2016-11-18 02:21:18]

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

[commit-bot: I haz the power, 2016-11-18 02:23: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, 2016-11-18 02:36:27]

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

[commit-bot: I haz the power, 2016-11-18 02:36:30]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[Saman Sami, 2016-11-18 03:08:27]

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

[commit-bot: I haz the power, 2016-11-18 03:09:25]

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, 2016-11-18 04:12:43]

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

[commit-bot: I haz the power, 2016-11-18 04:12:45]

Dry run: This issue passed the CQ dry run.

[Saman Sami, 2016-11-18 13:41:52]

The CQ bit was checked by samans@chromium.org

[Saman Sami, 2016-11-18 13:41:53]

The patchset sent to the CQ was uploaded after l-g-t-m from
reveman@chromium.org, sky@chromium.org, boliu@chromium.org, piman@chromium.org,
danakj@chromium.org, tsepez@chromium.org, kmarshall@chromium.org,
fsamuel@chromium.org
Link to the patchset: https://codereview.chromium.org/2449853004/#ps480001
(title: "nit")

[commit-bot: I haz the power, 2016-11-18 13:42:06]

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, 2016-11-18 13:48:03]

Description was changed from

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a 
compositor frame with a null delegated frame data) is gone. You 
should use base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
==========

to

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a 
compositor frame with a null delegated frame data) is gone. You 
should use base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
==========

[commit-bot: I haz the power, 2016-11-18 13:48:07]

Committed patchset #25 (id:480001)

[commit-bot: I haz the power, 2016-11-18 13:51:44]

Description was changed from

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a 
compositor frame with a null delegated frame data) is gone. You 
should use base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Cr-Commit-Position: refs/heads/master@{#428591}
==========

to

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a
compositor frame with a null delegated frame data) is gone. You
should use base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Committed: https://crrev.com/ce5c3c3facaf0240ab9fdb7bbf05116af9cab5d5
Cr-Original-Commit-Position: refs/heads/master@{#428591}
Cr-Commit-Position: refs/heads/master@{#433186}
==========

[commit-bot: I haz the power, 2016-11-18 13:51:48]

Patchset 25 (id:??) landed as
https://crrev.com/ce5c3c3facaf0240ab9fdb7bbf05116af9cab5d5
Cr-Commit-Position: refs/heads/master@{#433186}

[Saman Sami, 2016-12-16 21:56:58]

Description was changed from

==========
Getting rid of DelegatedFrameData

Note: From now on, the concept of "null" compositor frame (a
compositor frame with a null delegated frame data) is gone. You
should use base::Optional<CompositorFrame> instead.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Committed: https://crrev.com/ce5c3c3facaf0240ab9fdb7bbf05116af9cab5d5
Cr-Original-Commit-Position: refs/heads/master@{#428591}
Cr-Commit-Position: refs/heads/master@{#433186}
==========

to

==========
Getting rid of DelegatedFrameData

delegated_frame_data in CompositorFrame creates an unnecessary level of
indirection. This CL removes DelegatedFrameData and stores the contents of
delegated_frame_host (namely resource_list and render_pass_list) directly inside
CompositorFrame.

BUG=653741

Committed: https://crrev.com/4ba3a0e9d06bf66b3d900fec44bcf8ee4a2964a2
Committed: https://crrev.com/ce5c3c3facaf0240ab9fdb7bbf05116af9cab5d5
Cr-Original-Commit-Position: refs/heads/master@{#428591}
Cr-Commit-Position: refs/heads/master@{#433186}
==========