ui + mus: Split ContextFactory into ContextFactory(Client) and ContextFactoryPrivate

ui + mus: Split ContextFactory into ContextFactory(Client) and ContextFactoryPrivate

ui::ContextFactory is an interface that contains both privileged and
unprivileged operations. This CL dices ContextFactory into two, clearly
delineating operations that can be performed by Mus clients, and operations that
are reserved for mus-ws and mus-gpu and should be refactored.

Mus clients other than Chrome do not need to inject a
ContextFactoryPrivate into ui::Compositor. Chrome still needs
ContextFactoryPrivate because it has its own display compositor for
now.

Once we complete the unified display compositor work across all Chrome
clients, ContextFactoryPrivate can go away entirely.

BUG=670710
TBR=reveman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[Fady Samuel, 2016-12-08 21:58:45]

Description was changed from

==========
ui + mus: Split ContextFactory into ContextFactory(Client) and
ContextFactoryPrivate

ui::ContextFactory is an interface that contains both privileged and
unprivileged operations. This CL dices ContextFactory into two, clearly
delineating operations that can be performed by Mus clients, and operations that
are reserved for mus-ws and mus-gpu and should be refactored.

BUG=
==========

to

==========
ui + mus: Split ContextFactory into ContextFactory(Client) and
ContextFactoryPrivate

ui::ContextFactory is an interface that contains both privileged and
unprivileged operations. This CL dices ContextFactory into two, clearly
delineating operations that can be performed by Mus clients, and operations that
are reserved for mus-ws and mus-gpu and should be refactored.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Fady Samuel, 2016-12-08 22:01:01]

fsamuel@chromium.org changed reviewers:
+ rjkroege@chromium.org, sadrul@chromium.org

[Fady Samuel, 2016-12-08 22:01:02]

fsamuel@chromium.org changed reviewers:
+ rjkroege@chromium.org, sadrul@chromium.org

[Fady Samuel, 2016-12-08 22:01:02]

This CL is still WIP but I was hoping to get some early feedback, Sadrul, Rob. I
think once I'm done, this will be a net negative LoC patch because Mus clients
will no longer need a bunch of NOTIMPLEMENTED() methods. Chrome without Mus will
have a ContextFactoryPrivate, whereas Chrome in Mus will use ContextFactory
only. WDYT?

[Fady Samuel, 2016-12-08 22:01:03]

This CL is still WIP but I was hoping to get some early feedback, Sadrul, Rob. I
think once I'm done, this will be a net negative LoC patch because Mus clients
will no longer need a bunch of NOTIMPLEMENTED() methods. Chrome without Mus will
have a ContextFactoryPrivate, whereas Chrome in Mus will use ContextFactory
only. WDYT?

[Fady Samuel, 2016-12-08 22:03:50]

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

[commit-bot: I haz the power, 2016-12-08 22:04:35]

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

[Fady Samuel, 2016-12-08 22:25:00]

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

[commit-bot: I haz the power, 2016-12-08 22:25:37]

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-12-08 22:47:14]

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

[commit-bot: I haz the power, 2016-12-08 22:47:15]

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

[rjkroege, 2016-12-08 23:42:59]

the general approach looks reasonable to me.

https://codereview.chromium.org/2563783002/diff/20001/content/browser/context...
File content/browser/context_factory.cc (right):

https://codereview.chromium.org/2563783002/diff/20001/content/browser/context...
content/browser/context_factory.cc:16: ui::ContextFactoryPrivate*
GetContextFactoryPrivate() {
presuming that I am correct in my understanding that this should never be
invoked from a mus client, could it have a DCHECK enforcing this -- i.e. test if
we're running in --mash mode and check?

https://codereview.chromium.org/2563783002/diff/20001/content/public/browser/...
File content/public/browser/context_factory.h (right):

https://codereview.chromium.org/2563783002/diff/20001/content/public/browser/...
content/public/browser/context_factory.h:25: CONTENT_EXPORT
ui::ContextFactoryPrivate* GetContextFactoryPrivate();
so in a mus context, there should be no calls to this code from within Chrome
browser process?

[sadrul, 2016-12-09 00:11:03]

https://codereview.chromium.org/2563783002/diff/20001/ui/compositor/composito...
File ui/compositor/compositor.cc (right):

https://codereview.chromium.org/2563783002/diff/20001/ui/compositor/composito...
ui/compositor/compositor.cc:251: if (!context_factory_private_)
Probably should be DCHECK()?

[Fady Samuel, 2016-12-09 18:05:12]

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

[commit-bot: I haz the power, 2016-12-09 18:05:53]

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-12-09 18:09:37]

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

[commit-bot: I haz the power, 2016-12-09 18:09:38]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Fady Samuel, 2016-12-09 20:39:45]

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

[commit-bot: I haz the power, 2016-12-09 20:40:57]

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

[Fady Samuel, 2016-12-09 20:51:17]

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

[commit-bot: I haz the power, 2016-12-09 20:52:26]

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

[Fady Samuel, 2016-12-09 21:08:52]

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

[commit-bot: I haz the power, 2016-12-09 21:09:50]

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-12-09 21:40:20]

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

[commit-bot: I haz the power, 2016-12-09 21:40:21]

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

[Fady Samuel, 2016-12-09 22:05:38]

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

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

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

[Fady Samuel, 2016-12-09 22:08:16]

Description was changed from

==========
ui + mus: Split ContextFactory into ContextFactory(Client) and
ContextFactoryPrivate

ui::ContextFactory is an interface that contains both privileged and
unprivileged operations. This CL dices ContextFactory into two, clearly
delineating operations that can be performed by Mus clients, and operations that
are reserved for mus-ws and mus-gpu and should be refactored.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
ui + mus: Split ContextFactory into ContextFactory(Client) and
ContextFactoryPrivate

ui::ContextFactory is an interface that contains both privileged and
unprivileged operations. This CL dices ContextFactory into two, clearly
delineating operations that can be performed by Mus clients, and operations that
are reserved for mus-ws and mus-gpu and should be refactored.

BUG=670710
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Fady Samuel, 2016-12-09 22:25:38]

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

[commit-bot: I haz the power, 2016-12-09 22:26:18]

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-12-09 22:54:12]

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

[commit-bot: I haz the power, 2016-12-09 22:54:13]

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

[Fady Samuel, 2016-12-12 18:41:12]

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

[commit-bot: I haz the power, 2016-12-12 18:42:06]

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-12-12 19:05:49]

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

[commit-bot: I haz the power, 2016-12-12 19:05:50]

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

[Fady Samuel, 2016-12-12 19:29:45]

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

[commit-bot: I haz the power, 2016-12-12 19:30:55]

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-12-12 19:50:59]

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

[commit-bot: I haz the power, 2016-12-12 19:51:00]

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

[Fady Samuel, 2016-12-12 21:07:13]

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

[commit-bot: I haz the power, 2016-12-12 21:08:09]

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

[Fady Samuel, 2016-12-12 21:27:43]

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

[commit-bot: I haz the power, 2016-12-12 21:29:21]

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-12-12 21:50:39]

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

[commit-bot: I haz the power, 2016-12-12 21:50:40]

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

[Fady Samuel, 2016-12-12 21:59:50]

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

[commit-bot: I haz the power, 2016-12-12 22:01:36]

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-12-12 23:21:58]

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

[commit-bot: I haz the power, 2016-12-12 23:21:59]

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

[Fady Samuel, 2016-12-12 23:56:27]

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

[commit-bot: I haz the power, 2016-12-12 23:57:43]

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-12-13 00:41:16]

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

[commit-bot: I haz the power, 2016-12-13 00:41:17]

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

[Fady Samuel, 2016-12-13 18:25:30]

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

[commit-bot: I haz the power, 2016-12-13 18:26:02]

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-12-13 19:09:12]

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

[commit-bot: I haz the power, 2016-12-13 19:09:13]

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

[Fady Samuel, 2016-12-13 19:13:57]

fsamuel@chromium.org changed reviewers:
+ jam@chromium.org, sky@chromium.org

[Fady Samuel, 2016-12-13 19:13:58]

fsamuel@chromium.org changed reviewers:
+ jam@chromium.org, sky@chromium.org

[Fady Samuel, 2016-12-13 19:13:59]

PTAL Sadrul! This is now mostly ready for review. I still have some Mac things
to fix (working on it now), but I'm fairly confident in this CL at this point.

+sky@ for chrome and ash
+jam@ for content/

https://codereview.chromium.org/2563783002/diff/20001/content/browser/context...
File content/browser/context_factory.cc (right):

https://codereview.chromium.org/2563783002/diff/20001/content/browser/context...
content/browser/context_factory.cc:16: ui::ContextFactoryPrivate*
GetContextFactoryPrivate() {
On 2016/12/08 23:42:59, rjkroege wrote:
> presuming that I am correct in my understanding that this should never be
> invoked from a mus client, could it have a DCHECK enforcing this -- i.e. test
if
> we're running in --mash mode and check?

That is correct but unfortunately, Chrome has its own ContextFactoryPrivate for
now: GpuProcessTransportFactory. I intend to fix this in a subsequent CL.

https://codereview.chromium.org/2563783002/diff/20001/content/public/browser/...
File content/public/browser/context_factory.h (right):

https://codereview.chromium.org/2563783002/diff/20001/content/public/browser/...
content/public/browser/context_factory.h:25: CONTENT_EXPORT
ui::ContextFactoryPrivate* GetContextFactoryPrivate();
On 2016/12/08 23:42:59, rjkroege wrote:
> so in a mus context, there should be no calls to this code from within Chrome
> browser process?

There will be for now. Apps that use the Mus client lib directly, and use the
unified display compositor will not need this. Currently Chrome DOES need this
but this will change once I split out GpuProcessTransportFactory in a subsequent
CL.

https://codereview.chromium.org/2563783002/diff/20001/ui/compositor/composito...
File ui/compositor/compositor.cc (right):

https://codereview.chromium.org/2563783002/diff/20001/ui/compositor/composito...
ui/compositor/compositor.cc:251: if (!context_factory_private_)
On 2016/12/09 00:11:03, sadrul wrote:
> Probably should be DCHECK()?

I cannot unfortunately because Chrome still uses ContextFactoryPrivate.

[Fady Samuel, 2016-12-13 19:13:59]

PTAL Sadrul! This is now mostly ready for review. I still have some Mac things
to fix (working on it now), but I'm fairly confident in this CL at this point.

+sky@ for chrome and ash
+jam@ for content/

https://codereview.chromium.org/2563783002/diff/20001/content/browser/context...
File content/browser/context_factory.cc (right):

https://codereview.chromium.org/2563783002/diff/20001/content/browser/context...
content/browser/context_factory.cc:16: ui::ContextFactoryPrivate*
GetContextFactoryPrivate() {
On 2016/12/08 23:42:59, rjkroege wrote:
> presuming that I am correct in my understanding that this should never be
> invoked from a mus client, could it have a DCHECK enforcing this -- i.e. test
if
> we're running in --mash mode and check?

That is correct but unfortunately, Chrome has its own ContextFactoryPrivate for
now: GpuProcessTransportFactory. I intend to fix this in a subsequent CL.

https://codereview.chromium.org/2563783002/diff/20001/content/public/browser/...
File content/public/browser/context_factory.h (right):

https://codereview.chromium.org/2563783002/diff/20001/content/public/browser/...
content/public/browser/context_factory.h:25: CONTENT_EXPORT
ui::ContextFactoryPrivate* GetContextFactoryPrivate();
On 2016/12/08 23:42:59, rjkroege wrote:
> so in a mus context, there should be no calls to this code from within Chrome
> browser process?

There will be for now. Apps that use the Mus client lib directly, and use the
unified display compositor will not need this. Currently Chrome DOES need this
but this will change once I split out GpuProcessTransportFactory in a subsequent
CL.

https://codereview.chromium.org/2563783002/diff/20001/ui/compositor/composito...
File ui/compositor/compositor.cc (right):

https://codereview.chromium.org/2563783002/diff/20001/ui/compositor/composito...
ui/compositor/compositor.cc:251: if (!context_factory_private_)
On 2016/12/09 00:11:03, sadrul wrote:
> Probably should be DCHECK()?

I cannot unfortunately because Chrome still uses ContextFactoryPrivate.

[Fady Samuel, 2016-12-13 19:16:15]

Description was changed from

==========
ui + mus: Split ContextFactory into ContextFactory(Client) and
ContextFactoryPrivate

ui::ContextFactory is an interface that contains both privileged and
unprivileged operations. This CL dices ContextFactory into two, clearly
delineating operations that can be performed by Mus clients, and operations that
are reserved for mus-ws and mus-gpu and should be refactored.

BUG=670710
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
ui + mus: Split ContextFactory into ContextFactory(Client) and
ContextFactoryPrivate

ui::ContextFactory is an interface that contains both privileged and
unprivileged operations. This CL dices ContextFactory into two, clearly
delineating operations that can be performed by Mus clients, and operations that
are reserved for mus-ws and mus-gpu and should be refactored.

Mus clients other than Chrome do not need to inject a 
ContextFactoryPrivate into ui::Compositor. Chrome still needs 
ContextFactoryPrivate because it has its own display compositor for 
now.

Once we complete the unified display compositor work across all Chrome 
clients, ContextFactoryPrivate can go away entirely.

BUG=670710
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Fady Samuel, 2016-12-13 20:18:54]

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

[commit-bot: I haz the power, 2016-12-13 20:19:43]

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

[sky, 2016-12-13 21:29:54]

https://codereview.chromium.org/2563783002/diff/300001/ui/aura/env.h
File ui/aura/env.h (right):

https://codereview.chromium.org/2563783002/diff/300001/ui/aura/env.h#newcode91
ui/aura/env.h:91: void set_context_factory_private(
Are these temporary? I wouldn't expect a class with the name Private to be
publically exposed. But I'm not familiar with the class, if you say it makes
sense I'm fine.

Also, is there a case where you would set one but not the other? Should the
setter take and set both at the same time, e.g.
SetContentFactories(ui::ContextFactory* context_factory,
ui::ContextFactoryPrivate* context_factory_private);

[Fady Samuel, 2016-12-13 21:55:58]

https://codereview.chromium.org/2563783002/diff/300001/ui/aura/env.h
File ui/aura/env.h (right):

https://codereview.chromium.org/2563783002/diff/300001/ui/aura/env.h#newcode91
ui/aura/env.h:91: void set_context_factory_private(
On 2016/12/13 21:29:54, sky wrote:
> Are these temporary? I wouldn't expect a class with the name Private to be
> publically exposed. But I'm not familiar with the class, if you say it makes
> sense I'm fine.
> 
> Also, is there a case where you would set one but not the other? Should the
> setter take and set both at the same time, e.g.
> SetContentFactories(ui::ContextFactory* context_factory,
> ui::ContextFactoryPrivate* context_factory_private);

Yes, Mus clients other than Chrome should not have a ContextFactoryPrivate.

ContextFactoryPrivate is a temporary concept until we pull out the code into the
display compositor API.

[commit-bot: I haz the power, 2016-12-13 22:17:44]

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

[commit-bot: I haz the power, 2016-12-13 22:17:46]

Dry run: This issue passed the CQ dry run.

[sky, 2016-12-13 23:25:29]

Ok, LGTM

[rjkroege, 2016-12-14 00:15:31]

lgtm with nits

https://codereview.chromium.org/2563783002/diff/300001/components/exo/surface.cc
File components/exo/surface.cc (right):

https://codereview.chromium.org/2563783002/diff/300001/components/exo/surface...
components/exo/surface.cc:150: ->context_factory_private()
exo should not require this. Is there a separate bug tracking the removal of
this dependency?

https://codereview.chromium.org/2563783002/diff/300001/mash/test/mash_test_su...
File mash/test/mash_test_suite.cc (right):

https://codereview.chromium.org/2563783002/diff/300001/mash/test/mash_test_su...
mash/test/mash_test_suite.cc:42:
env_->set_context_factory_private(context_factory_private);
Why is this necessary? This is mash code. It's a client and so shouldn't require
this?

[jam, 2016-12-14 00:46:05]

lgtm for content files outside content/browser/compositor/, for those ones
please get specific owner

[Fady Samuel, 2016-12-14 00:49:03]

fsamuel@chromium.org changed reviewers:
+ jbauman@chromium.org

[Fady Samuel, 2016-12-14 00:49:05]

+jbauman@ for content/browser/compositor

[sadrul, 2016-12-14 00:57:34]

lgtm

[jbauman, 2016-12-14 01:06:22]

lgtm for content/browser/compositor/

[Fady Samuel, 2016-12-14 01:27:11]

fsamuel@chromium.org changed reviewers:
+ reveman@chromium.org

[Fady Samuel, 2016-12-14 01:27:12]

+reveman@ for exo change (TBR'ing since this is a small mechanical change that
matches changes elsewhere).

https://codereview.chromium.org/2563783002/diff/300001/components/exo/surface.cc
File components/exo/surface.cc (right):

https://codereview.chromium.org/2563783002/diff/300001/components/exo/surface...
components/exo/surface.cc:150: ->context_factory_private()
On 2016/12/14 00:15:31, rjkroege wrote:
> exo should not require this. Is there a separate bug tracking the removal of
> this dependency?

I added a TODO. https://bugs.chromium.org/p/chromium/issues/detail?id=672961
added info here on how to deal with this.

https://codereview.chromium.org/2563783002/diff/300001/mash/test/mash_test_su...
File mash/test/mash_test_suite.cc (right):

https://codereview.chromium.org/2563783002/diff/300001/mash/test/mash_test_su...
mash/test/mash_test_suite.cc:42:
env_->set_context_factory_private(context_factory_private);
On 2016/12/14 00:15:31, rjkroege wrote:
> Why is this necessary? This is mash code. It's a client and so shouldn't
require
> this?

I don't think it's necessary. Removed.

[Fady Samuel, 2016-12-14 01:27:28]

Description was changed from

==========
ui + mus: Split ContextFactory into ContextFactory(Client) and
ContextFactoryPrivate

ui::ContextFactory is an interface that contains both privileged and
unprivileged operations. This CL dices ContextFactory into two, clearly
delineating operations that can be performed by Mus clients, and operations that
are reserved for mus-ws and mus-gpu and should be refactored.

Mus clients other than Chrome do not need to inject a 
ContextFactoryPrivate into ui::Compositor. Chrome still needs 
ContextFactoryPrivate because it has its own display compositor for 
now.

Once we complete the unified display compositor work across all Chrome 
clients, ContextFactoryPrivate can go away entirely.

BUG=670710
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
ui + mus: Split ContextFactory into ContextFactory(Client) and
ContextFactoryPrivate

ui::ContextFactory is an interface that contains both privileged and
unprivileged operations. This CL dices ContextFactory into two, clearly
delineating operations that can be performed by Mus clients, and operations that
are reserved for mus-ws and mus-gpu and should be refactored.

Mus clients other than Chrome do not need to inject a 
ContextFactoryPrivate into ui::Compositor. Chrome still needs 
ContextFactoryPrivate because it has its own display compositor for 
now.

Once we complete the unified display compositor work across all Chrome 
clients, ContextFactoryPrivate can go away entirely.

BUG=670710
TBR=reveman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Fady Samuel, 2016-12-14 01:27:36]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2016-12-14 01:27:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
rjkroege@chromium.org, sadrul@chromium.org, jam@chromium.org, sky@chromium.org,
jbauman@chromium.org
Link to the patchset: https://codereview.chromium.org/2563783002/#ps320001
(title: "Updated")

[commit-bot: I haz the power, 2016-12-14 01:28:22]

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-12-14 02:33:49]

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

[commit-bot: I haz the power, 2016-12-14 02:33:51]

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

[Fady Samuel, 2016-12-14 04:43:06]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2016-12-14 04:43:07]

The patchset sent to the CQ was uploaded after l-g-t-m from
rjkroege@chromium.org, sky@chromium.org, jam@chromium.org, jbauman@chromium.org,
sadrul@chromium.org
Link to the patchset: https://codereview.chromium.org/2563783002/#ps340001
(title: "Restore mash")

[commit-bot: I haz the power, 2016-12-14 04:43:37]

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

[Fady Samuel, 2016-12-14 04:51:02]

Filed a bug: https://bugs.chromium.org/p/chromium/issues/detail?id=674001

There's a bit of work to do there. I'll address in the next patch.

https://codereview.chromium.org/2563783002/diff/300001/mash/test/mash_test_su...
File mash/test/mash_test_suite.cc (right):

https://codereview.chromium.org/2563783002/diff/300001/mash/test/mash_test_su...
mash/test/mash_test_suite.cc:42:
env_->set_context_factory_private(context_factory_private);
On 2016/12/14 01:27:12, Fady Samuel wrote:
> On 2016/12/14 00:15:31, rjkroege wrote:
> > Why is this necessary? This is mash code. It's a client and so shouldn't
> require
> > this?
> 
> I don't think it's necessary. Removed.

I don't understand what's going on...

It seems like mash_unittests is using InProcessContextFactory and not Mus or
SurfaceContextFactory...I need to investigate further...filing a bug.

[commit-bot: I haz the power, 2016-12-14 06:21:54]

CQ is committing da patch.

Bot data: {"patchset_id": 340001, "attempt_start_ts": 1481690586818520,
"parent_rev": "a3cc103444b61a92f8a5fcd70924331e5e39e9cd", "commit_rev":
"c836dfbc44c6a33003e57171ad37aaba2a8336d9"}

[commit-bot: I haz the power, 2016-12-14 06:22:39]

Description was changed from

==========
ui + mus: Split ContextFactory into ContextFactory(Client) and
ContextFactoryPrivate

ui::ContextFactory is an interface that contains both privileged and
unprivileged operations. This CL dices ContextFactory into two, clearly
delineating operations that can be performed by Mus clients, and operations that
are reserved for mus-ws and mus-gpu and should be refactored.

Mus clients other than Chrome do not need to inject a 
ContextFactoryPrivate into ui::Compositor. Chrome still needs 
ContextFactoryPrivate because it has its own display compositor for 
now.

Once we complete the unified display compositor work across all Chrome 
clients, ContextFactoryPrivate can go away entirely.

BUG=670710
TBR=reveman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
ui + mus: Split ContextFactory into ContextFactory(Client) and
ContextFactoryPrivate

ui::ContextFactory is an interface that contains both privileged and
unprivileged operations. This CL dices ContextFactory into two, clearly
delineating operations that can be performed by Mus clients, and operations that
are reserved for mus-ws and mus-gpu and should be refactored.

Mus clients other than Chrome do not need to inject a 
ContextFactoryPrivate into ui::Compositor. Chrome still needs 
ContextFactoryPrivate because it has its own display compositor for 
now.

Once we complete the unified display compositor work across all Chrome 
clients, ContextFactoryPrivate can go away entirely.

BUG=670710
TBR=reveman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2563783002
==========

[commit-bot: I haz the power, 2016-12-14 06:22:42]

Committed patchset #18 (id:340001)

[commit-bot: I haz the power, 2016-12-14 06:25:13]

Description was changed from

==========
ui + mus: Split ContextFactory into ContextFactory(Client) and
ContextFactoryPrivate

ui::ContextFactory is an interface that contains both privileged and
unprivileged operations. This CL dices ContextFactory into two, clearly
delineating operations that can be performed by Mus clients, and operations that
are reserved for mus-ws and mus-gpu and should be refactored.

Mus clients other than Chrome do not need to inject a 
ContextFactoryPrivate into ui::Compositor. Chrome still needs 
ContextFactoryPrivate because it has its own display compositor for 
now.

Once we complete the unified display compositor work across all Chrome 
clients, ContextFactoryPrivate can go away entirely.

BUG=670710
TBR=reveman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2563783002
==========

to

==========
ui + mus: Split ContextFactory into ContextFactory(Client) and
ContextFactoryPrivate

ui::ContextFactory is an interface that contains both privileged and
unprivileged operations. This CL dices ContextFactory into two, clearly
delineating operations that can be performed by Mus clients, and operations that
are reserved for mus-ws and mus-gpu and should be refactored.

Mus clients other than Chrome do not need to inject a
ContextFactoryPrivate into ui::Compositor. Chrome still needs
ContextFactoryPrivate because it has its own display compositor for
now.

Once we complete the unified display compositor work across all Chrome
clients, ContextFactoryPrivate can go away entirely.

BUG=670710
TBR=reveman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[commit-bot: I haz the power, 2016-12-14 06:25:15]

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

[reveman, 2016-12-14 19:01:17]

lgtm