cc: Create ProxyBeginFrameSource

cc: Create ProxyBeginFrameSource

Create ProxyBeginFrameSource for delivering BeginFrame args to children.
With this, RWHV can be act as a cc::BeginFrameObserver for unified BeginFrame.

R=brianderson@chromium.org
BUG=448140
TEST=cc_unittests

[simonhong, 2015-01-13 16:48:58]

brian, would you quick look at this approach?
(unit test is not yet added.)

[simonhong, 2015-01-13 16:50:39]

Patchset #1 (id:1) has been deleted

[simonhong, 2015-01-14 15:58:20]

Patchset #2 (id:40001) has been deleted

[simonhong, 2015-01-29 16:23:03]

Kindly ping.

[simonhong, 2015-02-10 01:47:20]

On 2015/01/29 16:23:03, simonhong wrote:
> Kindly ping.

Kindly ping :)

[mithro-old, 2015-02-27 02:23:39]

mithro@mithis.com changed reviewers:
+ mithro@mithis.com

[mithro-old, 2015-03-23 06:19:42]

This patch still current? Do you still plan to land it?

[mithro-old, 2015-03-23 11:32:17]

Hi Simon,

I think I've lost some context on this patch while I was away. 

I'm trying to understand the details about why the ProxyBeginFrameSource has an
almost but not really BeginFrameSource interface? I feel like the
ProxyBeginFrameSource should be much closer to the BeginFrameSourceMultiplexer?

Tim 'mithro' Ansell

https://codereview.chromium.org/845393002/diff/130001/cc/output/begin_frame_a...
File cc/output/begin_frame_args.h (right):

https://codereview.chromium.org/845393002/diff/130001/cc/output/begin_frame_a...
cc/output/begin_frame_args.h:88: bool operator==(const BeginFrameArgs& other)
const {
Why do we need these operators again?

https://codereview.chromium.org/845393002/diff/130001/cc/scheduler/begin_fram...
File cc/scheduler/begin_frame_source.h (right):

https://codereview.chromium.org/845393002/diff/130001/cc/scheduler/begin_fram...
cc/scheduler/begin_frame_source.h:169: class CC_EXPORT ProxyBeginFrameSource :
public BeginFrameSource {
If I understand correctly, this just takes a message from a BeginFrameSource and
forwards it to multiple observers?

https://codereview.chromium.org/845393002/diff/130001/cc/scheduler/begin_fram...
cc/scheduler/begin_frame_source.h:182: // Instead, turning BeginFrame on/off is
controlled by Add/RemoveObserver.
I think this violates the BeginFrameSource interface, if
SetNeedsBeginFrames(false) is called, we shouldn't be sending any more
BeginFrames?

Can you explain why you are controlling this via Observers rather than the
normal method?

https://codereview.chromium.org/845393002/diff/130001/cc/scheduler/begin_fram...
cc/scheduler/begin_frame_source.h:190: void set_delegate(Delegate* delegate) {
Why is this set_delegate rather than SetDelegate?

[simonhong, 2015-03-23 11:45:55]

On 2015/03/23 11:32:17, mithro wrote:
> Hi Simon,
> 
> I think I've lost some context on this patch while I was away. 
> 
> I'm trying to understand the details about why the ProxyBeginFrameSource has
an
> almost but not really BeginFrameSource interface? I feel like the
> ProxyBeginFrameSource should be much closer to the
BeginFrameSourceMultiplexer?
> 
> Tim 'mithro' Ansell
> 
>
https://codereview.chromium.org/845393002/diff/130001/cc/output/begin_frame_a...
> File cc/output/begin_frame_args.h (right):
> 
>
https://codereview.chromium.org/845393002/diff/130001/cc/output/begin_frame_a...
> cc/output/begin_frame_args.h:88: bool operator==(const BeginFrameArgs& other)
> const {
> Why do we need these operators again?
> 
>
https://codereview.chromium.org/845393002/diff/130001/cc/scheduler/begin_fram...
> File cc/scheduler/begin_frame_source.h (right):
> 
>
https://codereview.chromium.org/845393002/diff/130001/cc/scheduler/begin_fram...
> cc/scheduler/begin_frame_source.h:169: class CC_EXPORT ProxyBeginFrameSource :
> public BeginFrameSource {
> If I understand correctly, this just takes a message from a BeginFrameSource
and
> forwards it to multiple observers?
> 
>
https://codereview.chromium.org/845393002/diff/130001/cc/scheduler/begin_fram...
> cc/scheduler/begin_frame_source.h:182: // Instead, turning BeginFrame on/off
is
> controlled by Add/RemoveObserver.
> I think this violates the BeginFrameSource interface, if
> SetNeedsBeginFrames(false) is called, we shouldn't be sending any more
> BeginFrames?
> 
> Can you explain why you are controlling this via Observers rather than the
> normal method?
> 
>
https://codereview.chromium.org/845393002/diff/130001/cc/scheduler/begin_fram...
> cc/scheduler/begin_frame_source.h:190: void set_delegate(Delegate* delegate) {
> Why is this set_delegate rather than SetDelegate?

At first, I was trying to modify and used BeginFrameSourceMultiplexer.
But, it can handle only one observer.
I thought making a new BeginFrameSource is much easier and simple than modifying
BeginFrameSourceMultiplexer.

[simonhong, 2015-03-23 11:46:39]

https://codereview.chromium.org/845393002/diff/130001/cc/output/begin_frame_a...
File cc/output/begin_frame_args.h (right):

https://codereview.chromium.org/845393002/diff/130001/cc/output/begin_frame_a...
cc/output/begin_frame_args.h:88: bool operator==(const BeginFrameArgs& other)
const {
On 2015/03/23 11:32:16, mithro wrote:
> Why do we need these operators again?

Can be removed.

https://codereview.chromium.org/845393002/diff/130001/cc/scheduler/begin_fram...
File cc/scheduler/begin_frame_source.h (right):

https://codereview.chromium.org/845393002/diff/130001/cc/scheduler/begin_fram...
cc/scheduler/begin_frame_source.h:169: class CC_EXPORT ProxyBeginFrameSource :
public BeginFrameSource {
On 2015/03/23 11:32:17, mithro wrote:
> If I understand correctly, this just takes a message from a BeginFrameSource
and
> forwards it to multiple observers?

Yes, right.

https://codereview.chromium.org/845393002/diff/130001/cc/scheduler/begin_fram...
cc/scheduler/begin_frame_source.h:182: // Instead, turning BeginFrame on/off is
controlled by Add/RemoveObserver.
On 2015/03/23 11:32:17, mithro wrote:
> I think this violates the BeginFrameSource interface, if
> SetNeedsBeginFrames(false) is called, we shouldn't be sending any more
> BeginFrames?
> 
> Can you explain why you are controlling this via Observers rather than the
> normal method?

This is special login for ProxyBeginFrameSource.
We only can stop sending BeginFrame if there is no observer.
Of course, login in Add/RemoveObserver can be moved into SetNeedsBeginFrames().

https://codereview.chromium.org/845393002/diff/130001/cc/scheduler/begin_fram...
cc/scheduler/begin_frame_source.h:190: void set_delegate(Delegate* delegate) {
On 2015/03/23 11:32:17, mithro wrote:
> Why is this set_delegate rather than SetDelegate?

As I know, this style is used for just get/set method.
Am I wrong? :)

[mithro-old, 2015-04-02 05:48:34]

I think we should rebase this patch onto one of my alternative "proxy"
implementations - just need to decide which one is the right choice.

For reference, they are;
 * https://codereview.chromium.org/1026233002/ - cc: Making BeginFrameSources
support multiple BeginFrameObservers.
 * https://codereview.chromium.org/1022483003/ - cc: Adding a
BeginFrameObserverMultiplexer.

Tim

[simonhong, 2015-04-06 00:49:58]

On 2015/04/02 05:48:34, mithro wrote:
> I think we should rebase this patch onto one of my alternative "proxy"
> implementations - just need to decide which one is the right choice.
> 
> For reference, they are;
>  * https://codereview.chromium.org/1026233002/ - cc: Making BeginFrameSources
> support multiple BeginFrameObservers.
>  * https://codereview.chromium.org/1022483003/ - cc: Adding a
> BeginFrameObserverMultiplexer.
> 
> Tim

Thank you Tim for caring this cl.

Thesedays, I am re-thinking the way how to schedule BeginFrame with OOPIF.
RWHVChildFrame is handled differently from others like RWHVAura.
I am not familiar with site isolation stuff.
So, need a little time for considering BeginFrame scheduling for OOPIF.

How about restart this cl after OOPIF is handled nicely?

[mithro-old, 2015-06-01 03:42:26]

Where did we get with this?