ui/compositor: Provide its own 'in process' ContextProvider.

ui/compositor: Provide its own 'in process' ContextProvider.

InProcessContextProvider is twin brother of the ContextProviderInProcess
from webkit/common/gpu. We built one here specifically for compositor
in order to remove the dependency on webkit/.

We did that for some reasons: one is that webkit/ abstraction glue is going
away, the second is that compositor can't depend on content, and thus we
couldn't simply move the code into content and make compositor use it.

BUG=338338
TEST=compositor_unittests
R=piman@chromium.org
TBR=reed@google.com (for skia/ext deps addition)

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

[piman, 2015-01-20 19:14:24]

I would go one step further and eliminate the use of WebGraphicsContext3D in
ui/compositor. We never pass this context to webkit, so we don't need to have a
WebGraphicsContext3D.
Instead, I would move the GLInProcessContext and the GLES2Implementation
directly into the InProcessContextProvider, and simplify the logic.

[tfarina, 2015-01-20 23:23:52]

https://codereview.chromium.org/853353003/diff/80001/ui/compositor/test/in_pr...
File ui/compositor/test/in_process_context_factory.cc (right):

https://codereview.chromium.org/853353003/diff/80001/ui/compositor/test/in_pr...
ui/compositor/test/in_process_context_factory.cc:97:
scoped_ptr<WebGraphicsContext3DInProcessCommandBufferImpl> context3d(
Is about this that are talking about or is it something else? Sorry, I'm not
terrible familiar with these classes.

[piman, 2015-01-21 00:10:23]

https://codereview.chromium.org/853353003/diff/80001/ui/compositor/test/in_pr...
File ui/compositor/test/in_process_context_factory.cc (right):

https://codereview.chromium.org/853353003/diff/80001/ui/compositor/test/in_pr...
ui/compositor/test/in_process_context_factory.cc:97:
scoped_ptr<WebGraphicsContext3DInProcessCommandBufferImpl> context3d(
On 2015/01/20 23:23:52, tfarina wrote:
> Is about this that are talking about or is it something else? Sorry, I'm not
> terrible familiar with these classes.

Right, the current code creates a
WebGraphicsContext3DInProcessCommandBufferImpl, that gets wrapped into a
InProcessContextProvider, that uses its GLES2Implementation.

WebGraphicsContext3D*Impl is a wrapper around the GLES2Implementation, that can
be passed to blink (e.g. for WebGL).

But for the UI, we never pass it to blink, and the
WebGraphicsContext3DInProcessCommandBufferImpl is just a container for the
GLInProcessContext and the GLES2Implementation.

So instead, we could have the InProcessContextProvider own the
GLInProcessContext and GLES2Implementation directly.

This code would change, instead of creating a
WebGraphicsContext3DInProcessCommandBufferImpl from the compositor->widget(), it
would create a InProcessContextProvider, passing it the compositor->widget, and
it would directly create the GLInProcessContext and GLES2Implementation (the
same way WebGraphicsContext3DInProcessCommandBufferImpl::CreateViewContext does
today).

That way, we could completely remove the gpu/blink dependency from
ui/compositor. It would also reduce the duplication.

[tfarina, 2015-01-21 01:34:19]

I removed the usage of GrContextForWebGraphicsContext3D.

Do you also mean we don't need LostContextCallbackProxy?

[piman, 2015-01-21 02:07:36]

https://codereview.chromium.org/853353003/diff/100001/ui/compositor/test/grco...
File ui/compositor/test/grcontext_for_webgraphicscontext3d.h (right):

https://codereview.chromium.org/853353003/diff/100001/ui/compositor/test/grco...
ui/compositor/test/grcontext_for_webgraphicscontext3d.h:22: class
GrContextForWebGraphicsContext3D {
You don't need this class any more.

https://codereview.chromium.org/853353003/diff/100001/ui/compositor/test/in_p...
File ui/compositor/test/in_process_context_provider.cc (right):

https://codereview.chromium.org/853353003/diff/100001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.cc:214:
TRACE_EVENT_INSTANT0("gpu", "GrContext::freeGpuResources", \
nit: no need for \

https://codereview.chromium.org/853353003/diff/100001/ui/compositor/test/in_p...
File ui/compositor/test/in_process_context_provider.h (right):

https://codereview.chromium.org/853353003/diff/100001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.h:64: context3d_;
The gr_context_ change is a step in the right direction.
What I meant is that instead of having InProcessContextProvider own a
WebGraphicsContext3DInProcessCommandBufferImpl, which in turns owns a
GLInProcessContext, we could simply have InProcessContextProvider own the
GLInProcessContext, without an intermediate class.

You would have InProcessContextProvider::Create take an AcceleratedWidget
instead of the WebGraphicsContext3DInProcessCommandBufferImpl.
BindToCurrentThread would internally create the GLInProcessContext directly.
ContextGL() and ContextSupport() would directly return the
GLInProcessContext::GetImplementation()

You probably won't need the LostContextCallbackProxy any more, you could set the
LostContextCallback directly on the GLInProcessContext.

[tfarina, 2015-01-21 21:33:21]

Antoine, I made more changes, but I need some help to get the other details
done. You will see a bunch of code commented out. There I don't know what to do.

PTAL

[jamesr, 2015-01-21 21:39:04]

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

[jamesr, 2015-01-21 21:39:05]

https://codereview.chromium.org/853353003/diff/180001/ui/compositor/test/in_p...
File ui/compositor/test/in_process_context_provider.cc (right):

https://codereview.chromium.org/853353003/diff/180001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.cc:24: //const int kNoLimit = 0;
delete

https://codereview.chromium.org/853353003/diff/180001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.cc:55: class
InProcessContextProvider::LostContextCallbackProxy
delete this class, you don't need it

https://codereview.chromium.org/853353003/diff/180001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.cc:77: const
blink::WebGraphicsContext3D::Attributes& attributes,
don't use this type. are there any attributes that callers really need to
toggle? if so use gpu::gles2::ContextCreationAttribHelper directly

https://codereview.chromium.org/853353003/diff/180001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.cc:150:
//context_->traceBeginCHROMIUM("gpu_toplevel",
do

context_->TraceBeginCHROMIUM(....)

https://codereview.chromium.org/853353003/diff/180001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.cc:160: //   size_t
mapped_memory_limit = context3d_->GetMappedMemoryLimit();
you don't care about this, just delete the block

[tfarina, 2015-01-21 21:53:35]

https://codereview.chromium.org/853353003/diff/180001/ui/compositor/test/in_p...
File ui/compositor/test/in_process_context_provider.cc (right):

https://codereview.chromium.org/853353003/diff/180001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.cc:24: //const int kNoLimit = 0;
On 2015/01/21 21:39:05, jamesr wrote:
> delete

Done.

https://codereview.chromium.org/853353003/diff/180001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.cc:55: class
InProcessContextProvider::LostContextCallbackProxy
On 2015/01/21 21:39:05, jamesr wrote:
> delete this class, you don't need it

Done.

https://codereview.chromium.org/853353003/diff/180001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.cc:77: const
blink::WebGraphicsContext3D::Attributes& attributes,
On 2015/01/21 21:39:05, jamesr wrote:
> don't use this type. are there any attributes that callers really need to
> toggle? if so use gpu::gles2::ContextCreationAttribHelper directly

I don't know how to answer that yet. I'm thinking about it.

https://codereview.chromium.org/853353003/diff/180001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.cc:150:
//context_->traceBeginCHROMIUM("gpu_toplevel",
On 2015/01/21 21:39:05, jamesr wrote:
> do
> 
> context_->TraceBeginCHROMIUM(....)

../../../ui/compositor/test/in_process_context_provider.cc:125:13: error: no
member named 'TraceBeginCHROMIUM' in 'gpu::GLInProcessContext'
  context_->TraceBeginCHROMIUM("gpu_toplevel", unique_context_name.c_str());

https://codereview.chromium.org/853353003/diff/180001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.cc:160: //   size_t
mapped_memory_limit = context3d_->GetMappedMemoryLimit();
On 2015/01/21 21:39:05, jamesr wrote:
> you don't care about this, just delete the block

Done.

[jamesr, 2015-01-21 21:54:50]

On 2015/01/21 21:53:35, tfarina wrote:
>
https://codereview.chromium.org/853353003/diff/180001/ui/compositor/test/in_p...
> ui/compositor/test/in_process_context_provider.cc:150:
> //context_->traceBeginCHROMIUM("gpu_toplevel",
> On 2015/01/21 21:39:05, jamesr wrote:
> > do
> > 
> > context_->TraceBeginCHROMIUM(....)
> 
> ../../../ui/compositor/test/in_process_context_provider.cc:125:13: error: no
> member named 'TraceBeginCHROMIUM' in 'gpu::GLInProcessContext'
>   context_->TraceBeginCHROMIUM("gpu_toplevel", unique_context_name.c_str());

Sorry, you need to call it on whatever the GLES2Interface is here.

[tfarina, 2015-01-21 22:00:23]

On Wed, Jan 21, 2015 at 7:54 PM, <jamesr@chromium.org> wrote:

> On 2015/01/21 21:53:35, tfarina wrote:
>
> https://codereview.chromium.org/853353003/diff/180001/ui/
> compositor/test/in_process_context_provider.cc#newcode150
>
>> ui/compositor/test/in_process_context_provider.cc:150:
>> //context_->traceBeginCHROMIUM("gpu_toplevel",
>> On 2015/01/21 21:39:05, jamesr wrote:
>> > do
>> >
>> > context_->TraceBeginCHROMIUM(....)
>>
>
>  ../../../ui/compositor/test/in_process_context_provider.cc:125:13:
>> error: no
>> member named 'TraceBeginCHROMIUM' in 'gpu::GLInProcessContext'
>>    context_->TraceBeginCHROMIUM("gpu_toplevel",
>> unique_context_name.c_str());
>>
>
> Sorry, you need to call it on whatever the GLES2Interface is here.
>
> But there is nothing to call in GLInProcessContext for trace ->
https://chromium.googlesource.com/chromium/src/+/master/gpu/command_buffer/cl...
.


> https://codereview.chromium.org/853353003/
>



-- 
Thiago Farina

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jamesr, 2015-01-21 22:01:25]

That's why I said to call it on whatever GLES2Interface you have, not on
GLInProcessContext

[tfarina, 2015-01-21 22:09:26]

On 2015/01/21 22:01:25, jamesr wrote:
> That's why I said to call it on whatever GLES2Interface you have, not on
> GLInProcessContext

Done.

[piman, 2015-01-21 22:37:39]

https://codereview.chromium.org/853353003/diff/240001/ui/compositor/test/in_p...
File ui/compositor/test/in_process_context_provider.cc (right):

https://codereview.chromium.org/853353003/diff/240001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.cc:85:
share_resources_(attributes.shareResources),
This is always true, so you can remove.

https://codereview.chromium.org/853353003/diff/240001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.cc:132: capabilities_.gpu =
context_->GetImplementation()->capabilities();
nit: you can simply do this in InProcessContextProvider::BindToCurrentThread,
avoiding an extra function.

https://codereview.chromium.org/853353003/diff/240001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.cc:209: const
LostContextCallback& lost_context_callback) {
You need do keep the lost_context_callback around as a field, so that you can
call it.

I think you still need something like the
ContextProviderInProcess::OnLostContext mechanism.

After creating the GLInProcessContext, you would set the lost context callback 
to a InProcessContextProvider::OnLostContext method, that would look similar to
ContextProviderInProcess::OnLostContext:
1- set the destroyed_ flag to true (under lock)
2- call the lost context callback
3- call gr_context_->OnLostContext()

You could then fix IsContextLost to return the value of destroyed_ (taken under
lock).

https://codereview.chromium.org/853353003/diff/240001/ui/compositor/test/in_p...
File ui/compositor/test/in_process_context_provider.h (right):

https://codereview.chromium.org/853353003/diff/240001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.h:30: const
blink::WebGraphicsContext3D::Attributes& attributes,
As James mentioned, you can pass in a ContextCreationAttribHelper here instead
of the WebGraphicsContext3D::Attributes.

[tfarina, 2015-01-22 00:05:33]

Who should call OnLostContext()?

https://codereview.chromium.org/853353003/diff/240001/ui/compositor/test/in_p...
File ui/compositor/test/in_process_context_provider.cc (right):

https://codereview.chromium.org/853353003/diff/240001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.cc:85:
share_resources_(attributes.shareResources),
On 2015/01/21 22:37:39, piman (Very slow to review) wrote:
> This is always true, so you can remove.

Done.

https://codereview.chromium.org/853353003/diff/240001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.cc:132: capabilities_.gpu =
context_->GetImplementation()->capabilities();
On 2015/01/21 22:37:39, piman (Very slow to review) wrote:
> nit: you can simply do this in InProcessContextProvider::BindToCurrentThread,
> avoiding an extra function.

Done.

https://codereview.chromium.org/853353003/diff/240001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.cc:209: const
LostContextCallback& lost_context_callback) {
On 2015/01/21 22:37:39, piman (Very slow to review) wrote:
> You need do keep the lost_context_callback around as a field, so that you can
> call it.
> 
> I think you still need something like the
> ContextProviderInProcess::OnLostContext mechanism.
> 
> After creating the GLInProcessContext, you would set the lost context callback

> to a InProcessContextProvider::OnLostContext method, that would look similar
to
> ContextProviderInProcess::OnLostContext:
> 1- set the destroyed_ flag to true (under lock)
> 2- call the lost context callback
> 3- call gr_context_->OnLostContext()
> 
> You could then fix IsContextLost to return the value of destroyed_ (taken
under
> lock).

Done.

https://codereview.chromium.org/853353003/diff/240001/ui/compositor/test/in_p...
File ui/compositor/test/in_process_context_provider.h (right):

https://codereview.chromium.org/853353003/diff/240001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.h:30: const
blink::WebGraphicsContext3D::Attributes& attributes,
On 2015/01/21 22:37:39, piman (Very slow to review) wrote:
> As James mentioned, you can pass in a ContextCreationAttribHelper here instead
> of the WebGraphicsContext3D::Attributes.

Done.

[piman, 2015-01-22 00:18:17]

https://codereview.chromium.org/853353003/diff/240001/ui/compositor/test/in_p...
File ui/compositor/test/in_process_context_provider.cc (right):

https://codereview.chromium.org/853353003/diff/240001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.cc:209: const
LostContextCallback& lost_context_callback) {
On 2015/01/22 00:05:32, tfarina wrote:
> On 2015/01/21 22:37:39, piman (Very slow to review) wrote:
> > You need do keep the lost_context_callback around as a field, so that you
can
> > call it.
> > 
> > I think you still need something like the
> > ContextProviderInProcess::OnLostContext mechanism.
> > 
> > After creating the GLInProcessContext, you would set the lost context
callback
> 
> > to a InProcessContextProvider::OnLostContext method, that would look similar
> to
> > ContextProviderInProcess::OnLostContext:
> > 1- set the destroyed_ flag to true (under lock)
> > 2- call the lost context callback
> > 3- call gr_context_->OnLostContext()
> > 
> > You could then fix IsContextLost to return the value of destroyed_ (taken
> under
> > lock).
> 
> Done.

I would expect lost_context_callback_ = lost_context_callback;

[piman, 2015-01-22 00:20:07]

On 2015/01/22 00:05:33, tfarina wrote:
> Who should call OnLostContext()?

The GLInProcessContext should, after you set it as its lost context callback,
something like:
context_->SetContextLostCallback(base::Bind(&InProcessContextProvider::OnLostContext,
base::Unretained(this)));

> 
>
https://codereview.chromium.org/853353003/diff/240001/ui/compositor/test/in_p...
> File ui/compositor/test/in_process_context_provider.cc (right):
> 
>
https://codereview.chromium.org/853353003/diff/240001/ui/compositor/test/in_p...
> ui/compositor/test/in_process_context_provider.cc:85:
> share_resources_(attributes.shareResources),
> On 2015/01/21 22:37:39, piman (Very slow to review) wrote:
> > This is always true, so you can remove.
> 
> Done.
> 
>
https://codereview.chromium.org/853353003/diff/240001/ui/compositor/test/in_p...
> ui/compositor/test/in_process_context_provider.cc:132: capabilities_.gpu =
> context_->GetImplementation()->capabilities();
> On 2015/01/21 22:37:39, piman (Very slow to review) wrote:
> > nit: you can simply do this in
InProcessContextProvider::BindToCurrentThread,
> > avoiding an extra function.
> 
> Done.
> 
>
https://codereview.chromium.org/853353003/diff/240001/ui/compositor/test/in_p...
> ui/compositor/test/in_process_context_provider.cc:209: const
> LostContextCallback& lost_context_callback) {
> On 2015/01/21 22:37:39, piman (Very slow to review) wrote:
> > You need do keep the lost_context_callback around as a field, so that you
can
> > call it.
> > 
> > I think you still need something like the
> > ContextProviderInProcess::OnLostContext mechanism.
> > 
> > After creating the GLInProcessContext, you would set the lost context
callback
> 
> > to a InProcessContextProvider::OnLostContext method, that would look similar
> to
> > ContextProviderInProcess::OnLostContext:
> > 1- set the destroyed_ flag to true (under lock)
> > 2- call the lost context callback
> > 3- call gr_context_->OnLostContext()
> > 
> > You could then fix IsContextLost to return the value of destroyed_ (taken
> under
> > lock).
> 
> Done.
> 
>
https://codereview.chromium.org/853353003/diff/240001/ui/compositor/test/in_p...
> File ui/compositor/test/in_process_context_provider.h (right):
> 
>
https://codereview.chromium.org/853353003/diff/240001/ui/compositor/test/in_p...
> ui/compositor/test/in_process_context_provider.h:30: const
> blink::WebGraphicsContext3D::Attributes& attributes,
> On 2015/01/21 22:37:39, piman (Very slow to review) wrote:
> > As James mentioned, you can pass in a ContextCreationAttribHelper here
instead
> > of the WebGraphicsContext3D::Attributes.
> 
> Done.

[tfarina, 2015-01-22 16:28:59]

PTAL!

Do we need to call setGLInterface() somewhere?

[piman, 2015-01-23 03:21:18]

https://codereview.chromium.org/853353003/diff/320001/ui/compositor/test/DEPS
File ui/compositor/test/DEPS (right):

https://codereview.chromium.org/853353003/diff/320001/ui/compositor/test/DEPS...
ui/compositor/test/DEPS:6: "+third_party/WebKit/public/platform",
nit: you don't need this one anymore

https://codereview.chromium.org/853353003/diff/320001/ui/compositor/test/in_p...
File ui/compositor/test/in_process_context_provider.cc (right):

https://codereview.chromium.org/853353003/diff/320001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.cc:110: if (context_) {
Actually we should return false if context_ is NULL (implying failure).

[piman, 2015-01-23 03:22:31]

On 2015/01/22 16:28:59, tfarina wrote:
> PTAL!
> 
> Do we need to call setGLInterface() somewhere?

You should not need it. It's only used inside of
WebGraphicsContext3DInProcessCommandBufferImpl to set the pointer to the
GLES2Interface that the WebGraphicsContext3DImpl will use.
Since you don't use a WebGraphicsContext3DInProcessCommandBufferImpl any more,
you're all set.

[tfarina, 2015-01-23 14:58:17]

ptal

https://codereview.chromium.org/853353003/diff/320001/ui/compositor/test/DEPS
File ui/compositor/test/DEPS (right):

https://codereview.chromium.org/853353003/diff/320001/ui/compositor/test/DEPS...
ui/compositor/test/DEPS:6: "+third_party/WebKit/public/platform",
On 2015/01/23 03:21:17, piman (Very slow to review) wrote:
> nit: you don't need this one anymore

Done.

https://codereview.chromium.org/853353003/diff/320001/ui/compositor/test/in_p...
File ui/compositor/test/in_process_context_provider.cc (right):

https://codereview.chromium.org/853353003/diff/320001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.cc:110: if (context_) {
On 2015/01/23 03:21:17, piman (Very slow to review) wrote:
> Actually we should return false if context_ is NULL (implying failure).

I moved the Set into the if above. I'm not sure I see a way to return false
here.

[piman, 2015-01-23 18:08:52]

https://codereview.chromium.org/853353003/diff/320001/ui/compositor/test/in_p...
File ui/compositor/test/in_process_context_provider.cc (right):

https://codereview.chromium.org/853353003/diff/320001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.cc:110: if (context_) {
On 2015/01/23 14:58:17, tfarina wrote:
> On 2015/01/23 03:21:17, piman (Very slow to review) wrote:
> > Actually we should return false if context_ is NULL (implying failure).
> 
> I moved the Set into the if above. I'm not sure I see a way to return false
> here.
> 

gpu::GLInProcessContext::Create can fail if the context fails to initialize,
returning nullptr.

If that happens, we can just return false, the caller will know to abort.

[tfarina, 2015-01-23 22:29:57]

Something like I did in patch set 20?

[piman, 2015-01-23 23:21:49]

lgtm

https://codereview.chromium.org/853353003/diff/380001/ui/compositor/test/in_p...
File ui/compositor/test/in_process_context_provider.cc (right):

https://codereview.chromium.org/853353003/diff/380001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.cc:112: context_ =
context.Pass();
nit: you don't actually need the temporary |context|

This should work:

context_.reset(gpu::GLInProcessContext::Create(...));
if (!context_)
  return false;
context_->SetContextLostCallback(...)


Either GLInProcessContext::Create returns NULL, in which case context_ will be
NULL and we early exit with false
Or it returns a valid pointer, then we will set context_ to that value anyway,
since we won't early exit.

[tfarina, 2015-01-24 14:57:12]

https://codereview.chromium.org/853353003/diff/380001/ui/compositor/test/in_p...
File ui/compositor/test/in_process_context_provider.cc (right):

https://codereview.chromium.org/853353003/diff/380001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.cc:112: context_ =
context.Pass();
On 2015/01/23 23:21:48, piman (Very slow to review) wrote:
> nit: you don't actually need the temporary |context|
> 
> This should work:
> 
> context_.reset(gpu::GLInProcessContext::Create(...));
> if (!context_)
>   return false;
> context_->SetContextLostCallback(...)
> 
> 
> Either GLInProcessContext::Create returns NULL, in which case context_ will be
> NULL and we early exit with false
> Or it returns a valid pointer, then we will set context_ to that value anyway,
> since we won't early exit.

Done.

[tfarina, 2015-01-24 14:58:17]

The CQ bit was checked by tfarina@chromium.org

[commit-bot: I haz the power, 2015-01-24 14:58:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/853353003/400001

[commit-bot: I haz the power, 2015-01-24 16:01:16]

Committed patchset #21 (id:400001)

[commit-bot: I haz the power, 2015-01-24 16:02:10]

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

[danakj, 2015-02-27 22:04:22]

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

[danakj, 2015-02-27 22:04:23]

Since this CL aura_demo has no output in a linux_chromeos build.

[piman, 2015-02-27 22:19:49]

https://codereview.chromium.org/853353003/diff/400001/ui/compositor/test/in_p...
File ui/compositor/test/in_process_context_provider.cc (right):

https://codereview.chromium.org/853353003/diff/400001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.cc:98: true, /* is_offscreen */
Oh, this may be why. Should probably be changed to !window_

[danakj, 2015-02-27 22:22:43]

https://codereview.chromium.org/853353003/diff/400001/ui/compositor/test/in_p...
File ui/compositor/test/in_process_context_provider.cc (right):

https://codereview.chromium.org/853353003/diff/400001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_provider.cc:98: true, /* is_offscreen */
On 2015/02/27 22:19:49, piman (Very slow to review) wrote:
> Oh, this may be why. Should probably be changed to !window_

Yep, fixes it. Good job :)