cc: Abstract the LayerTreeHost.

cc: Abstract the LayerTreeHost.

Add a LayerTreeHostInterface which is implemented by the LayerTreeHost.
This change moves the public methods on the LayerTreeHost used by the
embedder to an interface, in preparation for restricting the use of the
current LayerTreeHost implementation to the case where the compositor
thread componenets run in the same process (a.k.a single threaded and
threaded compositor), and a different implementation can be used where
these are seperated by a network boundary.

Currently only the use of the threaded compositor has been hidden
behind the interface. Subsequent changes will move the single threaded
compositor and the cc_unittests to use the interface as well.

BUG=625283
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/cebe49473f9ef76541d437833878eaae6dad268f
Cr-Commit-Position: refs/heads/master@{#417114}

[Khushal, 2016-09-06 17:32:44]

Description was changed from

==========
cc: Abstract the LayerTreeHost.

Add a LayerTreeHostInterface which is implemented by the LayerTreeHost.
This change moves the public methods on the LayerTreeHost used by the
embedder to an interface, in preparation for restricting the use of the
current LayerTreeHost implementation to the case where the compositor
thread componenets run in the same process (a.k.a single threaded and
threaded compositor), and a different implementation can be used where
these are seperated by a network boundary.

Currently only the use of the threaded compositor has been hidden
behind the interface. Subsequent changes will move the single threaded
compositor and the cc_unittests to use the interface as well.

BUG=625283
==========

to

==========
cc: Abstract the LayerTreeHost.

Add a LayerTreeHostInterface which is implemented by the LayerTreeHost.
This change moves the public methods on the LayerTreeHost used by the
embedder to an interface, in preparation for restricting the use of the
current LayerTreeHost implementation to the case where the compositor
thread componenets run in the same process (a.k.a single threaded and
threaded compositor), and a different implementation can be used where
these are seperated by a network boundary.

Currently only the use of the threaded compositor has been hidden
behind the interface. Subsequent changes will move the single threaded
compositor and the cc_unittests to use the interface as well.

BUG=625283
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[Khushal, 2016-09-06 17:32:56]

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

[commit-bot: I haz the power, 2016-09-06 17:33:30]

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-09-06 17:44:24]

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

[commit-bot: I haz the power, 2016-09-06 17:44:24]

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

[Khushal, 2016-09-06 18:09:48]

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

[commit-bot: I haz the power, 2016-09-06 18:10:22]

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-09-06 18:26:14]

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

[commit-bot: I haz the power, 2016-09-06 18:26:15]

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

[Khushal, 2016-09-06 18:30:22]

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

[commit-bot: I haz the power, 2016-09-06 18:30:58]

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-09-06 19:00:52]

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

[commit-bot: I haz the power, 2016-09-06 19:00:53]

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...)

[Khushal, 2016-09-06 20:12:46]

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

[commit-bot: I haz the power, 2016-09-06 20:13: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-09-06 21:33:34]

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

[commit-bot: I haz the power, 2016-09-06 21:33:35]

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

[Khushal, 2016-09-06 22:12:30]

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

[commit-bot: I haz the power, 2016-09-06 22:12:53]

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

[Khushal, 2016-09-06 22:45:41]

khushalsagar@chromium.org changed reviewers:
+ piman@chromium.org, vmpstr@chromium.org

[Khushal, 2016-09-06 22:45:42]

+vmpstr for cc, piman for content/ and ui/. Pretty mechanical change to turn the
LayerTreeHost into an interface.

[piman, 2016-09-06 22:55:27]

LGTM for content/ and ui/compositor

[Khushal, 2016-09-06 22:56:41]

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

[Khushal, 2016-09-06 22:56:42]

Also, dtrainor for blimp/

[commit-bot: I haz the power, 2016-09-06 23:15:56]

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

[commit-bot: I haz the power, 2016-09-06 23:15:57]

Dry run: This issue passed the CQ dry run.

[David Trainor- moved to gerrit, 2016-09-07 05:07:18]

blimp/ lgtm

[vmpstr, 2016-09-07 18:26:11]

This looks fine for what it's doing. However, I'm a bit hesitant about the
names... An interface should really be called LayerTreeHost, and a specific
implementation should be called LayerTreeHostImpl... oops? Ok, maybe it should
be called something else, but I don't really like LayerTreeHostInterface as a
name. Are there other options?

I'm also not a huge fan of having all of this virtualized, but from your renames
it doesn't seem like we're accessing the functions too much? Can you do a quick
sweep to see roughly how many non virtual calls will now be virtual? Let's also
try to minimize calling virtual getters more than once in a function.

https://codereview.chromium.org/2317753002/diff/80001/cc/trees/single_thread_...
File cc/trees/single_thread_proxy.cc (right):

https://codereview.chromium.org/2317753002/diff/80001/cc/trees/single_thread_...
cc/trees/single_thread_proxy.cc:66: if
(layer_tree_host_->GetSettings().single_thread_proxy_scheduler &&
Can you pull out settings into a const ref variable here? we don't want to be
accessing the same virtual twice in a function.

[Khushal, 2016-09-07 21:25:50]

I was planning on doing one rename patch that changed
LayerTreeHostInterface->LayerTreeHost and LayerTreeHost->LayerTreeHostInProcess.
Do you want to do that in this patch itself?

Also, not a lot of these calls are in the critical path. We moved all that to
the LayerTree for this very reason. :)

https://codereview.chromium.org/2317753002/diff/80001/cc/trees/single_thread_...
File cc/trees/single_thread_proxy.cc (right):

https://codereview.chromium.org/2317753002/diff/80001/cc/trees/single_thread_...
cc/trees/single_thread_proxy.cc:66: if
(layer_tree_host_->GetSettings().single_thread_proxy_scheduler &&
On 2016/09/07 18:26:11, vmpstr wrote:
> Can you pull out settings into a const ref variable here? we don't want to be
> accessing the same virtual twice in a function.

Done.

[vmpstr, 2016-09-07 21:29:41]

On 2016/09/07 21:25:50, Khushal wrote:
> I was planning on doing one rename patch that changed
> LayerTreeHostInterface->LayerTreeHost and
LayerTreeHost->LayerTreeHostInProcess.
> Do you want to do that in this patch itself?

I think follow up is fine in this case. lgtm

> 
> Also, not a lot of these calls are in the critical path. We moved all that to
> the LayerTree for this very reason. :)
> 
>
https://codereview.chromium.org/2317753002/diff/80001/cc/trees/single_thread_...
> File cc/trees/single_thread_proxy.cc (right):
> 
>
https://codereview.chromium.org/2317753002/diff/80001/cc/trees/single_thread_...
> cc/trees/single_thread_proxy.cc:66: if
> (layer_tree_host_->GetSettings().single_thread_proxy_scheduler &&
> On 2016/09/07 18:26:11, vmpstr wrote:
> > Can you pull out settings into a const ref variable here? we don't want to
be
> > accessing the same virtual twice in a function.
> 
> Done.

[Khushal, 2016-09-07 21:36:22]

The CQ bit was checked by khushalsagar@chromium.org

[Khushal, 2016-09-07 21:36:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org, piman@chromium.org
Link to the patchset: https://codereview.chromium.org/2317753002/#ps100001
(title: "addressed comments")

[commit-bot: I haz the power, 2016-09-07 21:37:11]

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-09-07 21:41:15]

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

[commit-bot: I haz the power, 2016-09-07 21:41:16]

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...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Khushal, 2016-09-07 22:07:13]

The CQ bit was checked by khushalsagar@chromium.org

[Khushal, 2016-09-07 22:07:13]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org, vmpstr@chromium.org, piman@chromium.org
Link to the patchset: https://codereview.chromium.org/2317753002/#ps120001
(title: "Rebase")

[commit-bot: I haz the power, 2016-09-07 22:08:04]

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-09-07 23:27:32]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-09-07 23:29:40]

Description was changed from

==========
cc: Abstract the LayerTreeHost.

Add a LayerTreeHostInterface which is implemented by the LayerTreeHost.
This change moves the public methods on the LayerTreeHost used by the
embedder to an interface, in preparation for restricting the use of the
current LayerTreeHost implementation to the case where the compositor
thread componenets run in the same process (a.k.a single threaded and
threaded compositor), and a different implementation can be used where
these are seperated by a network boundary.

Currently only the use of the threaded compositor has been hidden
behind the interface. Subsequent changes will move the single threaded
compositor and the cc_unittests to use the interface as well.

BUG=625283
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
cc: Abstract the LayerTreeHost.

Add a LayerTreeHostInterface which is implemented by the LayerTreeHost.
This change moves the public methods on the LayerTreeHost used by the
embedder to an interface, in preparation for restricting the use of the
current LayerTreeHost implementation to the case where the compositor
thread componenets run in the same process (a.k.a single threaded and
threaded compositor), and a different implementation can be used where
these are seperated by a network boundary.

Currently only the use of the threaded compositor has been hidden
behind the interface. Subsequent changes will move the single threaded
compositor and the cc_unittests to use the interface as well.

BUG=625283
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/cebe49473f9ef76541d437833878eaae6dad268f
Cr-Commit-Position: refs/heads/master@{#417114}
==========

[commit-bot: I haz the power, 2016-09-07 23:29:42]

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