Add test for begin frames when using SingleThreadProxy.

Add test for begin frames when using SingleThreadProxy.

Follow up to https://codereview.chromium.org/1099703004/

BUG=346230
TEST=cc_unittests w/ and w/o the above patch.

[DaleCurtis, 2015-05-04 22:26:40]

dalecurtis@chromium.org changed reviewers:
+ enne@chromium.org, mithro@mithis.com, sunnyps@chromium.org

[enne (OOO), 2015-05-04 22:30:27]

https://codereview.chromium.org/1124733003/diff/1/cc/trees/layer_tree_host_un...
File cc/trees/layer_tree_host_unittest.cc (right):

https://codereview.chromium.org/1124733003/diff/1/cc/trees/layer_tree_host_un...
cc/trees/layer_tree_host_unittest.cc:2796: saw_will_begin_impl_frame_ = true;
I guess it was my understanding that it was also that BeginMainFrame happens
between this and DidBeginImplFrameDeadline?

[sunnyps, 2015-05-04 22:34:46]

On 2015/05/04 22:30:27, enne wrote:
>
https://codereview.chromium.org/1124733003/diff/1/cc/trees/layer_tree_host_un...
> File cc/trees/layer_tree_host_unittest.cc (right):
> 
>
https://codereview.chromium.org/1124733003/diff/1/cc/trees/layer_tree_host_un...
> cc/trees/layer_tree_host_unittest.cc:2796: saw_will_begin_impl_frame_ = true;
> I guess it was my understanding that it was also that BeginMainFrame happens
> between this and DidBeginImplFrameDeadline?

Yeah, this isn't the right test for mithro's CL.

[DaleCurtis, 2015-05-04 23:03:13]

On 2015/05/04 22:34:46, sunnyps wrote:
> On 2015/05/04 22:30:27, enne wrote:
> >
>
https://codereview.chromium.org/1124733003/diff/1/cc/trees/layer_tree_host_un...
> > File cc/trees/layer_tree_host_unittest.cc (right):
> > 
> >
>
https://codereview.chromium.org/1124733003/diff/1/cc/trees/layer_tree_host_un...
> > cc/trees/layer_tree_host_unittest.cc:2796: saw_will_begin_impl_frame_ =
true;
> > I guess it was my understanding that it was also that BeginMainFrame happens
> > between this and DidBeginImplFrameDeadline?
> 
> Yeah, this isn't the right test for mithro's CL.

It was my, admittedly naive, understanding that the problem for image based
layout tests was that no begin frame was issued at all, so this test just
verifies that one is sent. Sunny to be clear are you saying this is the wrong
test period, or the wrong test for mithro's change? :) Either way, feedback on
how to make it the right test, welcome!

[sunnyps, 2015-05-04 23:19:37]

The bug fixed by the mithro's CL was that SingleThreadProxy wasn't calling
WillBeginImplFrame like ThreadProxy was. For that we need to test that
WillBeginImplFrame is called when using SingleThreadProxy. I think your
test kinda does this but there are unrelated conditions like getting the
synchronous compositor involved etc.

PTAL at this upcoming CL: https://codereview.chromium.org/1111743002/

That CL has a test which should test this CL as well.

On Mon, May 4, 2015 at 4:03 PM, <dalecurtis@chromium.org> wrote:

> On 2015/05/04 22:34:46, sunnyps wrote:
>
>> On 2015/05/04 22:30:27, enne wrote:
>> >
>>
>
>
>
https://codereview.chromium.org/1124733003/diff/1/cc/trees/layer_tree_host_un...
>
>> > File cc/trees/layer_tree_host_unittest.cc (right):
>> >
>> >
>>
>
>
>
https://codereview.chromium.org/1124733003/diff/1/cc/trees/layer_tree_host_un...
>
>> > cc/trees/layer_tree_host_unittest.cc:2796: saw_will_begin_impl_frame_ =
>>
> true;
>
>> > I guess it was my understanding that it was also that BeginMainFrame
>> happens
>> > between this and DidBeginImplFrameDeadline?
>>
>
>  Yeah, this isn't the right test for mithro's CL.
>>
>
> It was my, admittedly naive, understanding that the problem for image based
> layout tests was that no begin frame was issued at all, so this test just
> verifies that one is sent. Sunny to be clear are you saying this is the
> wrong
> test period, or the wrong test for mithro's change? :) Either way,
> feedback on
> how to make it the right test, welcome!
>
> https://codereview.chromium.org/1124733003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[DaleCurtis, 2015-05-05 17:31:41]

Sunny says my test is terrible and mithro's upcoming test is much more awesome,
so closing this unless anyone has an objection.

[DaleCurtis, 2015-05-05 17:32:00]

Oh, I forgot to :)