Add basic framework for splitting thread proxy.

Add basic framework for splitting thread proxy.

ProxyMain and ProxyImpl have been temporarily defined as interfaces to have the implementation in ThreadProxy and incrementally move methods to the different classes.

MainThreadOnly and BlockedMainThread are the variables owned by ProxyMain and CompositorThreadOnly are the variables owned by ProxyImpl. All access to the data variables and methods for the 2 sides will be routed through the Channel interfaces with ThreadedChannel providing the implementation for the current case where ProxyMain and ProxyImpl are running on separate threads.

BUG=527200
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/5d6eb98fd5d6acf448c153e89d09dc506f0e4210
Cr-Commit-Position: refs/heads/master@{#350935}

[Khushal, 2015-09-22 15:31:21]

khushalsagar@chromium.org changed reviewers:
+ dtrainor@chromium.org

[Khushal, 2015-09-22 18:57:10]

khushalsagar@chromium.org changed reviewers:
+ nyquist@chromium.org

[David Trainor- moved to gerrit, 2015-09-23 17:54:12]

https://chromiumcodereview.appspot.com/1357373002/diff/60001/cc/trees/channel...
File cc/trees/channel_main.h (right):

https://chromiumcodereview.appspot.com/1357373002/diff/60001/cc/trees/channel...
cc/trees/channel_main.h:11: class ThreadProxy;
Remove?

https://chromiumcodereview.appspot.com/1357373002/diff/60001/cc/trees/channel...
cc/trees/channel_main.h:18: // LayerTreeHost --> ProxyMain --> ChannelMain
Odd formatting request, but can we line these two top level arrow blocks up?

Also, it might be good to show that for things like blimp the channels talk to
each other.  The ThreadedChannel case is a bit special because ChannelMain and
ChannelImpl are the same thing.

https://chromiumcodereview.appspot.com/1357373002/diff/60001/cc/trees/thread_...
File cc/trees/thread_proxy.cc (right):

https://chromiumcodereview.appspot.com/1357373002/diff/60001/cc/trees/thread_...
cc/trees/thread_proxy.cc:76: // ProxyMain#SetChannel
. at end.

https://chromiumcodereview.appspot.com/1357373002/diff/60001/cc/trees/thread_...
cc/trees/thread_proxy.cc:77: this->SetChannel(
remove this->?

https://chromiumcodereview.appspot.com/1357373002/diff/60001/cc/trees/thread_...
File cc/trees/thread_proxy.h (right):

https://chromiumcodereview.appspot.com/1357373002/diff/60001/cc/trees/thread_...
cc/trees/thread_proxy.h:160: ChannelImpl* channel_impl;
Can we comment on the fact that ProxyImpl doesn't own ChannelImpl by design?

[Khushal, 2015-09-23 20:50:28]

https://codereview.chromium.org/1357373002/diff/60001/cc/trees/channel_main.h
File cc/trees/channel_main.h (right):

https://codereview.chromium.org/1357373002/diff/60001/cc/trees/channel_main.h...
cc/trees/channel_main.h:11: class ThreadProxy;
On 2015/09/23 17:54:11, David Trainor wrote:
> Remove?

Done.

https://codereview.chromium.org/1357373002/diff/60001/cc/trees/channel_main.h...
cc/trees/channel_main.h:18: // LayerTreeHost --> ProxyMain --> ChannelMain
On 2015/09/23 17:54:11, David Trainor wrote:
> Odd formatting request, but can we line these two top level arrow blocks up?
> 
> Also, it might be good to show that for things like blimp the channels talk to
> each other.  The ThreadedChannel case is a bit special because ChannelMain and
> ChannelImpl are the same thing.

Done. Added ChannelImpl in the communication sequence. I suppose we can define
the special case of ThreadedChannel in the comments on ThreadedChannel.h.

https://codereview.chromium.org/1357373002/diff/60001/cc/trees/thread_proxy.cc
File cc/trees/thread_proxy.cc (right):

https://codereview.chromium.org/1357373002/diff/60001/cc/trees/thread_proxy.c...
cc/trees/thread_proxy.cc:76: // ProxyMain#SetChannel
On 2015/09/23 17:54:12, David Trainor wrote:
> . at end.

Done.

https://codereview.chromium.org/1357373002/diff/60001/cc/trees/thread_proxy.c...
cc/trees/thread_proxy.cc:77: this->SetChannel(
On 2015/09/23 17:54:12, David Trainor wrote:
> remove this->?

Done.

https://codereview.chromium.org/1357373002/diff/60001/cc/trees/thread_proxy.h
File cc/trees/thread_proxy.h (right):

https://codereview.chromium.org/1357373002/diff/60001/cc/trees/thread_proxy.h...
cc/trees/thread_proxy.h:160: ChannelImpl* channel_impl;
On 2015/09/23 17:54:12, David Trainor wrote:
> Can we comment on the fact that ProxyImpl doesn't own ChannelImpl by design?

Done. ProxyImpl only holds a reference to ChannelImpl. ChannelImpl
implementation actually owns ProxyImpl. For the threaded case, its
ThreadedChannel. Updated that in the comments in channel_impl.h

[Khushal, 2015-09-23 20:57:49]

khushalsagar@chromium.org changed reviewers:
+ enne@chromium.org

[enne (OOO), 2015-09-24 22:40:44]

This all looks like exactly what I'd expect.  weiliangc: do you have any
comments here?

https://codereview.chromium.org/1357373002/diff/80001/cc/trees/channel_main.h
File cc/trees/channel_main.h (right):

https://codereview.chromium.org/1357373002/diff/80001/cc/trees/channel_main.h...
cc/trees/channel_main.h:15: // The communication sequence between the 2 sides
is:
If the second diagram is just the first reversed, you could just make the arrows
point both ways and skip the second one?

https://codereview.chromium.org/1357373002/diff/80001/cc/trees/channel_main.h...
cc/trees/channel_main.h:31: virtual void SetLayerTreeHostClientReadyOnImpl() =
0;
I assume these are just a start and you'll add the rest of these in follow-up
patches?

[Khushal, 2015-09-24 23:46:51]

Thanks Enne.

https://codereview.chromium.org/1357373002/diff/80001/cc/trees/channel_main.h
File cc/trees/channel_main.h (right):

https://codereview.chromium.org/1357373002/diff/80001/cc/trees/channel_main.h...
cc/trees/channel_main.h:15: // The communication sequence between the 2 sides
is:
On 2015/09/24 22:40:44, enne wrote:
> If the second diagram is just the first reversed, you could just make the
arrows
> point both ways and skip the second one?

That makes more sense. Done.

https://codereview.chromium.org/1357373002/diff/80001/cc/trees/channel_main.h...
cc/trees/channel_main.h:31: virtual void SetLayerTreeHostClientReadyOnImpl() =
0;
On 2015/09/24 22:40:44, enne wrote:
> I assume these are just a start and you'll add the rest of these in follow-up
> patches?

Yes. Just wanted to get set up the framework right now. I'll start adding
methods incrementally.

[weiliangc, 2015-09-25 19:42:46]

Looks good.

https://codereview.chromium.org/1357373002/diff/100001/cc/trees/thread_proxy.h
File cc/trees/thread_proxy.h (right):

https://codereview.chromium.org/1357373002/diff/100001/cc/trees/thread_proxy....
cc/trees/thread_proxy.h:20: #include "threaded_channel.h"
include path?

https://codereview.chromium.org/1357373002/diff/100001/cc/trees/threaded_chan...
File cc/trees/threaded_channel.h (right):

https://codereview.chromium.org/1357373002/diff/100001/cc/trees/threaded_chan...
cc/trees/threaded_channel.h:28: // An implementation of ChannelMain and
ChannelImpl that sends commands between
This comment is awesome! Thanks. :)

[enne (OOO), 2015-09-25 19:58:28]

lgtm % weiliangc's comments.  Just wanted to give her a chance to review it too.

[Khushal, 2015-09-25 20:48:03]

Done.

https://codereview.chromium.org/1357373002/diff/100001/cc/trees/thread_proxy.h
File cc/trees/thread_proxy.h (right):

https://codereview.chromium.org/1357373002/diff/100001/cc/trees/thread_proxy....
cc/trees/thread_proxy.h:20: #include "threaded_channel.h"
On 2015/09/25 19:42:45, weiliangc wrote:
> include path?

Missed that. Thanks.

https://codereview.chromium.org/1357373002/diff/100001/cc/trees/threaded_chan...
File cc/trees/threaded_channel.h (right):

https://codereview.chromium.org/1357373002/diff/100001/cc/trees/threaded_chan...
cc/trees/threaded_channel.h:28: // An implementation of ChannelMain and
ChannelImpl that sends commands between
On 2015/09/25 19:42:46, weiliangc wrote:
> This comment is awesome! Thanks. :)

Thanks! :)

[Khushal, 2015-09-25 20:53:36]

The CQ bit was checked by khushalsagar@chromium.org

[Khushal, 2015-09-25 20:53:36]

The patchset sent to the CQ was uploaded after l-g-t-m from enne@chromium.org
Link to the patchset: https://codereview.chromium.org/1357373002/#ps120001
(title: "Addressing comments.")

[commit-bot: I haz the power, 2015-09-25 20:54:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1357373002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1357373002/120001

[commit-bot: I haz the power, 2015-09-25 21:41:49]

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

[commit-bot: I haz the power, 2015-09-25 21:41:50]

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

[Khushal, 2015-09-25 22:06:10]

The CQ bit was checked by khushalsagar@chromium.org

[commit-bot: I haz the power, 2015-09-25 22:06:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1357373002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1357373002/120001

[commit-bot: I haz the power, 2015-09-25 22:45:04]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-09-25 22:45:48]

Patchset 7 (id:??) landed as
https://crrev.com/5d6eb98fd5d6acf448c153e89d09dc506f0e4210
Cr-Commit-Position: refs/heads/master@{#350935}

[brucedawson, 2015-10-01 00:20:31]

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

[brucedawson, 2015-10-01 00:20:32]

https://chromiumcodereview-hr.appspot.com/1357373002/diff/120001/cc/cc.gyp
File cc/cc.gyp (right):

https://chromiumcodereview-hr.appspot.com/1357373002/diff/120001/cc/cc.gyp#ne...
cc/cc.gyp:547: 'trees/proxy_main.h'
Missing commas here make for a very odd three-part path name...

I'm creating a CL to fix this and some other .gyp header file errors. Just an
FYI.