cc: Split Proxy to eliminate unnecessary dependencies on the impl side

cc: Split Proxy to eliminate unnecessary dependencies on the impl side

The impl side of the compositor uses Proxy to only access the
task runners and make debug assertions. Move those parts to
TaskRunnerProvider to seperate these dependencies.

BUG=527200
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

[Khushal, 2015-10-16 07:42:28]

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

[Khushal, 2015-10-16 07:46:44]

I have only updated a few tests right now. Could you take a look and see if the
split looks correct.

[vmpstr, 2015-10-16 18:42:25]

vmpstr@chromium.org changed reviewers:
+ danakj@chromium.org, vmpstr@chromium.org

[vmpstr, 2015-10-16 18:42:26]

That's a nice abstraction, except my main problem is that I wouldn't call it a
ThreadVerifier. I would call it TaskRunnerProvider. WDYT?

+danakj as well

https://codereview.chromium.org/1411663002/diff/1/cc/trees/layer_tree_impl.h
File cc/trees/layer_tree_impl.h (right):

https://codereview.chromium.org/1411663002/diff/1/cc/trees/layer_tree_impl.h#...
cc/trees/layer_tree_impl.h:109: BlockingTaskRunner*
BlockingMainThreadTaskRunner() const;
I think this function can go if you just have a task_runner_provider()

then 
layer_tree()->BlockingMainThreadTaskRunner()
becomes
layer_tree()->task_runner_provider()->blocking_main_thread_task_runner()

https://codereview.chromium.org/1411663002/diff/1/cc/trees/layer_tree_impl.h#...
cc/trees/layer_tree_impl.h:269: // Useful for debug assertions, probably
shouldn't be used for anything else.
I think this comment is a lie, since we use this to get a blocking main thread
runner.

https://codereview.chromium.org/1411663002/diff/1/cc/trees/thread_verifier.h
File cc/trees/thread_verifier.h (right):

https://codereview.chromium.org/1411663002/diff/1/cc/trees/thread_verifier.h#...
cc/trees/thread_verifier.h:31: class CC_EXPORT ThreadVerifier {
I'd maybe prefer to call this something like TaskRunnerProvider instead, since
it's not so much verifying things as it provides task runners for different
threads.

[Khushal, 2015-10-17 00:06:30]

Changed the name. I was confused about what to name it myself.
TaskRunnerProvider seems a better fit.

https://codereview.chromium.org/1411663002/diff/1/cc/trees/layer_tree_impl.h
File cc/trees/layer_tree_impl.h (right):

https://codereview.chromium.org/1411663002/diff/1/cc/trees/layer_tree_impl.h#...
cc/trees/layer_tree_impl.h:109: BlockingTaskRunner*
BlockingMainThreadTaskRunner() const;
On 2015/10/16 18:42:25, vmpstr wrote:
> I think this function can go if you just have a task_runner_provider()
> 
> then 
> layer_tree()->BlockingMainThreadTaskRunner()
> becomes
> layer_tree()->task_runner_provider()->blocking_main_thread_task_runner()
> 

Done.

https://codereview.chromium.org/1411663002/diff/1/cc/trees/layer_tree_impl.h#...
cc/trees/layer_tree_impl.h:269: // Useful for debug assertions, probably
shouldn't be used for anything else.
On 2015/10/16 18:42:25, vmpstr wrote:
> I think this comment is a lie, since we use this to get a blocking main thread
> runner. 

Done. Changed it to be more accurate.

https://codereview.chromium.org/1411663002/diff/1/cc/trees/thread_verifier.h
File cc/trees/thread_verifier.h (right):

https://codereview.chromium.org/1411663002/diff/1/cc/trees/thread_verifier.h#...
cc/trees/thread_verifier.h:31: class CC_EXPORT ThreadVerifier {
On 2015/10/16 18:42:26, vmpstr wrote:
> I'd maybe prefer to call this something like TaskRunnerProvider instead, since
> it's not so much verifying things as it provides task runners for different
> threads. 

Done. That name makes much more sense.

[vmpstr, 2015-10-20 18:44:39]

lgtm, thanks!

[vmpstr, 2015-10-20 18:44:56]

On 2015/10/20 18:44:39, vmpstr wrote:
> lgtm, thanks!

Can you update the description to match the new name, please

[Khushal, 2015-10-20 19:26:36]

Description was changed from

==========
cc: Split Proxy to eliminate unnecessary dependencies on the impl side

The impl side of the compositor uses Proxy to only access the
task runners and make debug assertions. Move those parts to
ThreadVerifier to seperate these dependencies.

BUG=527200
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc: Split Proxy to eliminate unnecessary dependencies on the impl side

The impl side of the compositor uses Proxy to only access the
task runners and make debug assertions. Move those parts to
TaskRunnerProvider to seperate these dependencies.

BUG=527200
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[Khushal, 2015-10-20 19:27:07]

On 2015/10/20 18:44:56, vmpstr wrote:
> On 2015/10/20 18:44:39, vmpstr wrote:
> > lgtm, thanks!
> 
> Can you update the description to match the new name, please

Done. Thanks!

[Khushal, 2015-10-20 20:18:18]

The CQ bit was checked by khushalsagar@chromium.org

[Khushal, 2015-10-20 20:18:18]

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

[commit-bot: I haz the power, 2015-10-20 20:19:40]

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

[commit-bot: I haz the power, 2015-10-20 20:32:09]

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

[commit-bot: I haz the power, 2015-10-20 20:32:10]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Khushal, 2015-10-21 02:56:44]

A couple of perf tests needed to be updated as well. Could you please take
another look?

[vmpstr, 2015-10-21 17:44:42]

On 2015/10/21 02:56:44, Khushal wrote:
> A couple of perf tests needed to be updated as well. Could you please take
> another look?

lgtm

[Khushal, 2015-10-21 18:01:36]

The CQ bit was checked by khushalsagar@chromium.org

[commit-bot: I haz the power, 2015-10-21 18:03:13]

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

[commit-bot: I haz the power, 2015-10-21 19:25:24]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-10-21 19:26:15]

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