ui/compositor: move the browser compositor thread to the ContextFactory

ui/compositor: move the browser compositor thread to the ContextFactory

Trying to get rid of more globals in ui/compositor.

BUG=None

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=269608

[piman, 2014-05-08 22:26:42]

It also means we don't need Compositor::Initialize/Terminate. 

danakj: please review
sky: OWNER for aura and mojo

[piman, 2014-05-08 22:33:23]

On 2014/05/08 22:26:42, piman wrote:
> It also means we don't need Compositor::Initialize/Terminate. 
> 
> danakj: please review
> sky: OWNER for aura and mojo

(oh, this is on top of https://codereview.chromium.org/271843002/)

[sky, 2014-05-08 22:37:13]

https://codereview.chromium.org/277713002/diff/1/ui/aura/env.cc
File ui/aura/env.cc (right):

https://codereview.chromium.org/277713002/diff/1/ui/aura/env.cc#newcode9
ui/aura/env.cc:9: #include "ui/compositor/compositor.h"
If you can, remove this include.

[danakj, 2014-05-08 22:37:30]

LGTM

https://codereview.chromium.org/277713002/diff/1/content/browser/compositor/g...
File content/browser/compositor/gpu_process_transport_factory.cc (right):

https://codereview.chromium.org/277713002/diff/1/content/browser/compositor/g...
content/browser/compositor/gpu_process_transport_factory.cc:85:
compositor_thread_->Stop();
Why do we need to do Stop() and reset()? The destructor does this already right?
They're redundant here, and also with the default cleanup the class does?

https://codereview.chromium.org/277713002/diff/1/ui/compositor/test/in_proces...
File ui/compositor/test/in_process_context_factory.cc (right):

https://codereview.chromium.org/277713002/diff/1/ui/compositor/test/in_proces...
ui/compositor/test/in_process_context_factory.cc:35: if
(compositor_thread_.get()) {
nit: no need .get() for scoped_ptr

https://codereview.chromium.org/277713002/diff/1/ui/compositor/test/in_proces...
ui/compositor/test/in_process_context_factory.cc:36: compositor_thread_->Stop();
Doesn't this happen in the destructor? Why do we need to do this at all? Does it
help with stacktraces?

[piman, 2014-05-08 22:48:34]

https://codereview.chromium.org/277713002/diff/1/content/browser/compositor/g...
File content/browser/compositor/gpu_process_transport_factory.cc (right):

https://codereview.chromium.org/277713002/diff/1/content/browser/compositor/g...
content/browser/compositor/gpu_process_transport_factory.cc:85:
compositor_thread_->Stop();
On 2014/05/08 22:37:30, danakj wrote:
> Why do we need to do Stop() and reset()? The destructor does this already
right?
> They're redundant here, and also with the default cleanup the class does?

Done. I was just copy&pasting from existing code that was Stop()ing before
deleting, but I see that there's no need for that.

https://codereview.chromium.org/277713002/diff/1/ui/aura/env.cc
File ui/aura/env.cc (right):

https://codereview.chromium.org/277713002/diff/1/ui/aura/env.cc#newcode9
ui/aura/env.cc:9: #include "ui/compositor/compositor.h"
On 2014/05/08 22:37:13, sky wrote:
> If you can, remove this include.

Done.

https://codereview.chromium.org/277713002/diff/1/ui/compositor/test/in_proces...
File ui/compositor/test/in_process_context_factory.cc (right):

https://codereview.chromium.org/277713002/diff/1/ui/compositor/test/in_proces...
ui/compositor/test/in_process_context_factory.cc:35: if
(compositor_thread_.get()) {
On 2014/05/08 22:37:30, danakj wrote:
> nit: no need .get() for scoped_ptr

Done.

https://codereview.chromium.org/277713002/diff/1/ui/compositor/test/in_proces...
ui/compositor/test/in_process_context_factory.cc:36: compositor_thread_->Stop();
On 2014/05/08 22:37:30, danakj wrote:
> Doesn't this happen in the destructor? Why do we need to do this at all? Does
it
> help with stacktraces?

Done.

[piman, 2014-05-09 02:24:42]

The CQ bit was checked by piman@chromium.org

[commit-bot: I haz the power, 2014-05-09 02:27:38]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/piman@chromium.org/277713002/40001

[commit-bot: I haz the power, 2014-05-09 06:01:10]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)

[commit-bot: I haz the power, 2014-05-09 06:08:40]

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

[commit-bot: I haz the power, 2014-05-09 06:08:41]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)

[sky, 2014-05-09 16:49:29]

LGTM

[piman, 2014-05-09 16:49:54]

The CQ bit was checked by piman@chromium.org

[commit-bot: I haz the power, 2014-05-09 16:54:27]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/piman@chromium.org/277713002/40001

[commit-bot: I haz the power, 2014-05-09 17:13:10]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)

[commit-bot: I haz the power, 2014-05-09 17:25:48]

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

[commit-bot: I haz the power, 2014-05-09 17:25:49]

Try jobs failed on following builders:
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)

[piman, 2014-05-09 22:10:05]

The CQ bit was checked by piman@chromium.org

[commit-bot: I haz the power, 2014-05-09 22:13:36]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/piman@chromium.org/277713002/60001

[commit-bot: I haz the power, 2014-05-10 19:00:57]

Change committed as 269608