mus: Always use the GPU compositor for mus.

mus: Always use the GPU compositor for mus.

BUG=635613
Committed: https://crrev.com/ecad0f0ea34d1c0cc98e317e692116117840ba89
Cr-Commit-Position: refs/heads/master@{#412069}

[sadrul, 2016-08-11 16:05:32]

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

[commit-bot: I haz the power, 2016-08-11 16:05:57]

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

[sadrul, 2016-08-11 16:08:04]

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

[sadrul, 2016-08-11 16:08:05]

From the discussion at https://codereview.chromium.org/2232793002/, it sounds
like when running as a mus client, chrome should always talk to the gpu. I think
this is the right way to do that? I can then update
https://codereview.chromium.org/2232793002/ to just remove the
SoftwareOutputDeviceMus.

[commit-bot: I haz the power, 2016-08-11 17:30:01]

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

[commit-bot: I haz the power, 2016-08-11 17:30:02]

Dry run: This issue passed the CQ dry run.

[rjkroege, 2016-08-11 17:55:38]

lgtm + please add comment.

https://codereview.chromium.org/2237053002/diff/1/content/browser/gpu/gpu_dat...
File content/browser/gpu/gpu_data_manager_impl_private.cc (right):

https://codereview.chromium.org/2237053002/diff/1/content/browser/gpu/gpu_dat...
content/browser/gpu/gpu_data_manager_impl_private.cc:989: if
(shell::ShellIsRemote())
While this is fine for now, I think that it's not the right solution long term.
The browser (mus client in this case) should be asking mus about all of these
details yes? So add a TODO?

[sadrul, 2016-08-11 17:58:08]

https://codereview.chromium.org/2237053002/diff/1/content/browser/gpu/gpu_dat...
File content/browser/gpu/gpu_data_manager_impl_private.cc (right):

https://codereview.chromium.org/2237053002/diff/1/content/browser/gpu/gpu_dat...
content/browser/gpu/gpu_data_manager_impl_private.cc:989: if
(shell::ShellIsRemote())
On 2016/08/11 17:55:38, rjkroege wrote:
> While this is fine for now, I think that it's not the right solution long
term.
> The browser (mus client in this case) should be asking mus about all of these
> details yes? So add a TODO?

What sort of details do you have in mind? (we use shell::ShellIsRemote() to
detect when chrome is running in mus in a number of places).

[rjkroege, 2016-08-11 19:24:13]

On 2016/08/11 17:58:08, sadrul wrote:
>
https://codereview.chromium.org/2237053002/diff/1/content/browser/gpu/gpu_dat...
> File content/browser/gpu/gpu_data_manager_impl_private.cc (right):
> 
>
https://codereview.chromium.org/2237053002/diff/1/content/browser/gpu/gpu_dat...
> content/browser/gpu/gpu_data_manager_impl_private.cc:989: if
> (shell::ShellIsRemote())
> On 2016/08/11 17:55:38, rjkroege wrote:
> > While this is fine for now, I think that it's not the right solution long
> term.
> > The browser (mus client in this case) should be asking mus about all of
these
> > details yes? So add a TODO?
> 
> What sort of details do you have in mind? (we use shell::ShellIsRemote() to
> detect when chrome is running in mus in a number of places).

At some point in the future, the browser on some platforms (e.g. windows) is
going to need to know a) if it is using mus and b) if it ought to not be using
the mus-gpu but instead using the not-yet-implemented mus software rendering
backend. This code will then need to change? I thought some kind of note here
would help someone make that change when we get around to it.

[Fady Samuel, 2016-08-11 19:27:32]

On 2016/08/11 19:24:13, rjkroege wrote:
> On 2016/08/11 17:58:08, sadrul wrote:
> >
>
https://codereview.chromium.org/2237053002/diff/1/content/browser/gpu/gpu_dat...
> > File content/browser/gpu/gpu_data_manager_impl_private.cc (right):
> > 
> >
>
https://codereview.chromium.org/2237053002/diff/1/content/browser/gpu/gpu_dat...
> > content/browser/gpu/gpu_data_manager_impl_private.cc:989: if
> > (shell::ShellIsRemote())
> > On 2016/08/11 17:55:38, rjkroege wrote:
> > > While this is fine for now, I think that it's not the right solution long
> > term.
> > > The browser (mus client in this case) should be asking mus about all of
> these
> > > details yes? So add a TODO?
> > 
> > What sort of details do you have in mind? (we use shell::ShellIsRemote() to
> > detect when chrome is running in mus in a number of places).
> 
> At some point in the future, the browser on some platforms (e.g. windows) is
> going to need to know a) if it is using mus and b) if it ought to not be using
> the mus-gpu but instead using the not-yet-implemented mus software rendering
> backend. This code will then need to change? I thought some kind of note here
> would help someone make that change when we get around to it.

Mus-Gpu will exist even if we are doing software compositing in both tadpole and
salamander. In Tadpole, the display compositor will live in mus-gpu. In
Salamander, layers/property trees would as well. Maybe we should call Mus-Gpu,
Mus-Compositor?

[sadrul, 2016-08-11 19:54:32]

On 2016/08/11 19:27:32, Fady Samuel wrote:
> On 2016/08/11 19:24:13, rjkroege wrote:
> > On 2016/08/11 17:58:08, sadrul wrote:
> > >
> >
>
https://codereview.chromium.org/2237053002/diff/1/content/browser/gpu/gpu_dat...
> > > File content/browser/gpu/gpu_data_manager_impl_private.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2237053002/diff/1/content/browser/gpu/gpu_dat...
> > > content/browser/gpu/gpu_data_manager_impl_private.cc:989: if
> > > (shell::ShellIsRemote())
> > > On 2016/08/11 17:55:38, rjkroege wrote:
> > > > While this is fine for now, I think that it's not the right solution
long
> > > term.
> > > > The browser (mus client in this case) should be asking mus about all of
> > these
> > > > details yes? So add a TODO?
> > > 
> > > What sort of details do you have in mind? (we use shell::ShellIsRemote()
to
> > > detect when chrome is running in mus in a number of places).
> > 
> > At some point in the future, the browser on some platforms (e.g. windows) is
> > going to need to know a) if it is using mus and b) if it ought to not be
using
> > the mus-gpu but instead using the not-yet-implemented mus software rendering
> > backend. This code will then need to change? I thought some kind of note
here
> > would help someone make that change when we get around to it.
> 
> Mus-Gpu will exist even if we are doing software compositing in both tadpole
and
> salamander. In Tadpole, the display compositor will live in mus-gpu. In
> Salamander, layers/property trees would as well. Maybe we should call Mus-Gpu,
> Mus-Compositor?

It sounds like (b) does not apply, in that case? And the current check provides
(a). If that is correct, then we don't need a TODO?

[sadrul, 2016-08-11 19:55:09]

sadrul@chromium.org changed reviewers:
+ piman@chromium.org

[sadrul, 2016-08-11 19:55:09]

+piman@ for owner review

[piman, 2016-08-11 20:05:50]

On 2016/08/11 19:55:09, sadrul wrote:
> +piman@ for owner review

This seems odd - what happens if there's no usable GPU on the system?

[rjkroege, 2016-08-11 20:59:04]

On 2016/08/11 19:27:32, Fady Samuel wrote:
> On 2016/08/11 19:24:13, rjkroege wrote:
> > On 2016/08/11 17:58:08, sadrul wrote:
> > >
> >
>
https://codereview.chromium.org/2237053002/diff/1/content/browser/gpu/gpu_dat...
> > > File content/browser/gpu/gpu_data_manager_impl_private.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2237053002/diff/1/content/browser/gpu/gpu_dat...
> > > content/browser/gpu/gpu_data_manager_impl_private.cc:989: if
> > > (shell::ShellIsRemote())
> > > On 2016/08/11 17:55:38, rjkroege wrote:
> > > > While this is fine for now, I think that it's not the right solution
long
> > > term.
> > > > The browser (mus client in this case) should be asking mus about all of
> > these
> > > > details yes? So add a TODO?
> > > 
> > > What sort of details do you have in mind? (we use shell::ShellIsRemote()
to
> > > detect when chrome is running in mus in a number of places).
> > 
> > At some point in the future, the browser on some platforms (e.g. windows) is
> > going to need to know a) if it is using mus and b) if it ought to not be
using
> > the mus-gpu but instead using the not-yet-implemented mus software rendering
> > backend. This code will then need to change? I thought some kind of note
here
> > would help someone make that change when we get around to it.
> 
> Mus-Gpu will exist even if we are doing software compositing in both tadpole
and
> salamander. In Tadpole, the display compositor will live in mus-gpu. In
> Salamander, layers/property trees would as well. Maybe we should call Mus-Gpu,
> Mus-Compositor?

There would still be a mus-gpu process and it will, as you say, contain the
compositor.
But I think that this function also serves to let the browser know if it may
connect to the
GPU service running in the mus-gpu process. And if the mus-gpu process is on a
platform where the GPU has been black-listed, then perhaps the browser should
not
be connecting to this services?

[rjkroege, 2016-08-11 21:17:16]

On 2016/08/11 20:05:50, piman wrote:
> On 2016/08/11 19:55:09, sadrul wrote:
> > +piman@ for owner review
> 
> This seems odd - what happens if there's no usable GPU on the system?

Current mus has a sad. Which is (per badly-worded http://crbug.com/540929) a
bug. But it seemed like a reasonable prioritization to defer software
compositing given that mus/tadpole is CrOS first (where I thought we always had
a GPU). But this is why I wanted a TODO added to the code.

[piman, 2016-08-11 22:25:58]

On Thu, Aug 11, 2016 at 2:17 PM, <rjkroege@chromium.org> wrote:

> On 2016/08/11 20:05:50, piman wrote:
> > On 2016/08/11 19:55:09, sadrul wrote:
> > > +piman@ for owner review
> >
> > This seems odd - what happens if there's no usable GPU on the system?
>
> Current mus has a sad. Which is (per badly-worded http://crbug.com/540929)
> a
> bug. But it seemed like a reasonable prioritization to defer software
> compositing given that mus/tadpole is CrOS first (where I thought we
> always had
> a GPU). But this is why I wanted a TODO added to the code.
>

Ok, I guess I don't want to overload this function, and in general the
GpuDataManager - which is meant to decide what GPU features we use (and
blacklist, etc.), rather than which process we instantiate/talk to.
For example, even in Chrome, when using SwiftShader, we'll return false
here, but we'll still instantiate a GPU process if the renderer wants to
use WebGL.
So I think encoding the logic "we always want to use a mus-gpu process even
if GPU is disabled" belongs to something like GpuProcessTransportFactory.

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

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sadrul, 2016-08-12 16:44:43]

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

[commit-bot: I haz the power, 2016-08-12 16:45:00]

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

[sadrul, 2016-08-12 16:46:04]

On 2016/08/11 22:25:58, piman OOO back 2016-8-15 wrote:
> On Thu, Aug 11, 2016 at 2:17 PM, <mailto:rjkroege@chromium.org> wrote:
> 
> > On 2016/08/11 20:05:50, piman wrote:
> > > On 2016/08/11 19:55:09, sadrul wrote:
> > > > +piman@ for owner review
> > >
> > > This seems odd - what happens if there's no usable GPU on the system?
> >
> > Current mus has a sad. Which is (per badly-worded http://crbug.com/540929)
> > a
> > bug. But it seemed like a reasonable prioritization to defer software
> > compositing given that mus/tadpole is CrOS first (where I thought we
> > always had
> > a GPU). But this is why I wanted a TODO added to the code.
> >
> 
> Ok, I guess I don't want to overload this function, and in general the
> GpuDataManager - which is meant to decide what GPU features we use (and
> blacklist, etc.), rather than which process we instantiate/talk to.
> For example, even in Chrome, when using SwiftShader, we'll return false
> here, but we'll still instantiate a GPU process if the renderer wants to
> use WebGL.
> So I think encoding the logic "we always want to use a mus-gpu process even
> if GPU is disabled" belongs to something like GpuProcessTransportFactory.

I have moved the logic into GpuProcessTransportFactory. PTAL

[sadrul, 2016-08-12 16:48:26]

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

[sadrul, 2016-08-12 16:48:26]

-->danakj@ for owner since piman is ooo

[commit-bot: I haz the power, 2016-08-12 17:24:39]

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

[commit-bot: I haz the power, 2016-08-12 17:24:40]

Dry run: This issue passed the CQ dry run.

[danakj, 2016-08-12 17:48:28]

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

[danakj, 2016-08-12 17:48:29]

https://codereview.chromium.org/2237053002/diff/20001/content/browser/composi...
File content/browser/compositor/gpu_process_transport_factory.cc (right):

https://codereview.chromium.org/2237053002/diff/20001/content/browser/composi...
content/browser/compositor/gpu_process_transport_factory.cc:138:
DCHECK(ShouldCreateGpuOutputSurface(nullptr));
This is tied to the change for shared main thread context so explanation there
should explain this.

https://codereview.chromium.org/2237053002/diff/20001/content/browser/composi...
content/browser/compositor/gpu_process_transport_factory.cc:342:
create_gpu_output_surface = false;
You probably want to not do this on mus too no?

https://codereview.chromium.org/2237053002/diff/20001/content/browser/composi...
content/browser/compositor/gpu_process_transport_factory.cc:369: if
(ShouldCreateGpuOutputSurface(nullptr))
I don't think this should change. The fact that we're here means that
ShouldCreateGpuOutputSurface was previously true already.

I think that CanUseGpuBrowserCompositor can change tho, due to gpu crashes and
stuff. But the other things in ShouldCreateGpuOutputSurface can't. So this is
checking a bunch of redundant things which obfuscates things.

Is CanUseGpuBrowserCompositor not returning true for mus? Cuz that sounds
problematic if this code does not agree with the GpuDataManager.

https://codereview.chromium.org/2237053002/diff/20001/content/browser/composi...
content/browser/compositor/gpu_process_transport_factory.cc:775: if
(!ShouldCreateGpuOutputSurface(nullptr))
This might be right but it has nothing to do with the patch description, which
is using gpu on mus. I'd like to know an explanation for this being correct
either way..

[danakj, 2016-08-12 17:50:51]

On 2016/08/11 21:17:16, rjkroege wrote:
> On 2016/08/11 20:05:50, piman wrote:
> > On 2016/08/11 19:55:09, sadrul wrote:
> > > +piman@ for owner review
> > 
> > This seems odd - what happens if there's no usable GPU on the system?
> 
> Current mus has a sad. Which is (per badly-worded http://crbug.com/540929) a
> bug. But it seemed like a reasonable prioritization to defer software
> compositing given that mus/tadpole is CrOS first (where I thought we always
had
> a GPU). But this is why I wanted a TODO added to the code.

But for mus to always use GPU on chromeos requires 0 changes cuz we already
always use gpu on chromeos.

[danakj, 2016-08-12 17:52:16]

On 2016/08/11 22:25:58, piman OOO back 2016-8-15 wrote:
> On Thu, Aug 11, 2016 at 2:17 PM, <mailto:rjkroege@chromium.org> wrote:
> 
> > On 2016/08/11 20:05:50, piman wrote:
> > > On 2016/08/11 19:55:09, sadrul wrote:
> > > > +piman@ for owner review
> > >
> > > This seems odd - what happens if there's no usable GPU on the system?
> >
> > Current mus has a sad. Which is (per badly-worded http://crbug.com/540929)
> > a
> > bug. But it seemed like a reasonable prioritization to defer software
> > compositing given that mus/tadpole is CrOS first (where I thought we
> > always had
> > a GPU). But this is why I wanted a TODO added to the code.
> >
> 
> Ok, I guess I don't want to overload this function, and in general the
> GpuDataManager - which is meant to decide what GPU features we use (and
> blacklist, etc.), rather than which process we instantiate/talk to.
> For example, even in Chrome, when using SwiftShader, we'll return false
> here, but we'll still instantiate a GPU process if the renderer wants to
> use WebGL.
> So I think encoding the logic "we always want to use a mus-gpu process even
> if GPU is disabled" belongs to something like GpuProcessTransportFactory.

I read thru previous comments to try and understand this patch but I think I'm
going to need more explanations to be able to review this in place of piman. I
don't get why "use the gpu process" is being equated to "use gpu contexts" here.

[sadrul, 2016-08-12 18:15:06]

> Is CanUseGpuBrowserCompositor not returning true for mus? Cuz that sounds
> problematic if this code does not agree with the GpuDataManager.

It returns false because the gpu gets black-listed (weirdly, it doesn't happen
consistently. The blacklisting seems to always happen when running
browser_tests, and sometimes when running chrome)

[sadrul, 2016-08-12 18:16:07]

On 2016/08/12 18:15:06, sadrul wrote:
> > Is CanUseGpuBrowserCompositor not returning true for mus? Cuz that sounds
> > problematic if this code does not agree with the GpuDataManager.
> 
> It returns false because the gpu gets black-listed (weirdly, it doesn't happen
> consistently. The blacklisting seems to always happen when running
> browser_tests, and sometimes when running chrome)

Correction: not 'gpu gets blacklisted', but 'the
GPU_FEATURE_TYPE_GPU_COMPOSITING feature gets blacklisted'.

[danakj, 2016-08-12 18:24:24]

On Fri, Aug 12, 2016 at 11:16 AM, <sadrul@chromium.org> wrote:

> On 2016/08/12 18:15:06, sadrul wrote:
> > > Is CanUseGpuBrowserCompositor not returning true for mus? Cuz that
> sounds
> > > problematic if this code does not agree with the GpuDataManager.
> >
> > It returns false because the gpu gets black-listed (weirdly, it doesn't
> happen
> > consistently. The blacklisting seems to always happen when running
> > browser_tests, and sometimes when running chrome)
>
> Correction: not 'gpu gets blacklisted', but 'the
> GPU_FEATURE_TYPE_GPU_COMPOSITING feature gets blacklisted'.
>

There are a few software compositing browser tests. But they are disabled
on chromeos since we don't do software compositing there. Is it those?


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

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sadrul, 2016-08-12 18:40:55]

On 2016/08/12 18:24:24, danakj wrote:
> On Fri, Aug 12, 2016 at 11:16 AM, <mailto:sadrul@chromium.org> wrote:
> 
> > On 2016/08/12 18:15:06, sadrul wrote:
> > > > Is CanUseGpuBrowserCompositor not returning true for mus? Cuz that
> > sounds
> > > > problematic if this code does not agree with the GpuDataManager.
> > >
> > > It returns false because the gpu gets black-listed (weirdly, it doesn't
> > happen
> > > consistently. The blacklisting seems to always happen when running
> > > browser_tests, and sometimes when running chrome)
> >
> > Correction: not 'gpu gets blacklisted', but 'the
> > GPU_FEATURE_TYPE_GPU_COMPOSITING feature gets blacklisted'.
> >
> 
> There are a few software compositing browser tests. But they are disabled
> on chromeos since we don't do software compositing there. Is it those?

I don't think so (the only test I have tried (and the only browser_tests we
currently run in mash) is BrowserTest.Title:
https://cs.chromium.org/chromium/src/chrome/browser/ui/browser_browsertest.cc...)

[danakj, 2016-08-12 18:42:24]

On Fri, Aug 12, 2016 at 11:40 AM, <sadrul@chromium.org> wrote:

> On 2016/08/12 18:24:24, danakj wrote:
> > On Fri, Aug 12, 2016 at 11:16 AM, <mailto:sadrul@chromium.org> wrote:
> >
> > > On 2016/08/12 18:15:06, sadrul wrote:
> > > > > Is CanUseGpuBrowserCompositor not returning true for mus? Cuz that
> > > sounds
> > > > > problematic if this code does not agree with the GpuDataManager.
> > > >
> > > > It returns false because the gpu gets black-listed (weirdly, it
> doesn't
> > > happen
> > > > consistently. The blacklisting seems to always happen when running
> > > > browser_tests, and sometimes when running chrome)
> > >
> > > Correction: not 'gpu gets blacklisted', but 'the
> > > GPU_FEATURE_TYPE_GPU_COMPOSITING feature gets blacklisted'.
> > >
> >
> > There are a few software compositing browser tests. But they are disabled
> > on chromeos since we don't do software compositing there. Is it those?
>
> I don't think so (the only test I have tried (and the only browser_tests we
> currently run in mash) is BrowserTest.Title:
> https://cs.chromium.org/chromium/src/chrome/browser/
> ui/browser_browsertest.cc?sq=package:chromium&l=432&dr=C)
>

Why's it blacklisted? browser tests should be using gpu, it's osmesa.


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

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sadrul, 2016-08-12 20:07:49]

On 2016/08/12 18:42:24, danakj wrote:
> On Fri, Aug 12, 2016 at 11:40 AM, <mailto:sadrul@chromium.org> wrote:
> 
> > On 2016/08/12 18:24:24, danakj wrote:
> > > On Fri, Aug 12, 2016 at 11:16 AM, <mailto:sadrul@chromium.org> wrote:
> > >
> > > > On 2016/08/12 18:15:06, sadrul wrote:
> > > > > > Is CanUseGpuBrowserCompositor not returning true for mus? Cuz that
> > > > sounds
> > > > > > problematic if this code does not agree with the GpuDataManager.
> > > > >
> > > > > It returns false because the gpu gets black-listed (weirdly, it
> > doesn't
> > > > happen
> > > > > consistently. The blacklisting seems to always happen when running
> > > > > browser_tests, and sometimes when running chrome)
> > > >
> > > > Correction: not 'gpu gets blacklisted', but 'the
> > > > GPU_FEATURE_TYPE_GPU_COMPOSITING feature gets blacklisted'.
> > > >
> > >
> > > There are a few software compositing browser tests. But they are disabled
> > > on chromeos since we don't do software compositing there. Is it those?
> >
> > I don't think so (the only test I have tried (and the only browser_tests we
> > currently run in mash) is BrowserTest.Title:
> > https://cs.chromium.org/chromium/src/chrome/browser/
> > ui/browser_browsertest.cc?sq=package:chromium&l=432&dr=C)
> >
> 
> Why's it blacklisted? browser tests should be using gpu, it's osmesa.

Here's one weird trick to avoid the blacklisting: use --use-gpu-in-tests
--disable-gpu-driver-bug-workarounds flags (i.e. the hardcoded blacklists hate
my gpu). Is this expected? (a chromeos build on the same workstation does work
correctly, so not sure why blacklisting is necessary in a linux build) Does this
make sense?

[danakj, 2016-08-12 21:09:50]

On Fri, Aug 12, 2016 at 1:07 PM, <sadrul@chromium.org> wrote:

> On 2016/08/12 18:42:24, danakj wrote:
> > On Fri, Aug 12, 2016 at 11:40 AM, <mailto:sadrul@chromium.org> wrote:
> >
> > > On 2016/08/12 18:24:24, danakj wrote:
> > > > On Fri, Aug 12, 2016 at 11:16 AM, <mailto:sadrul@chromium.org>
> wrote:
> > > >
> > > > > On 2016/08/12 18:15:06, sadrul wrote:
> > > > > > > Is CanUseGpuBrowserCompositor not returning true for mus? Cuz
> that
> > > > > sounds
> > > > > > > problematic if this code does not agree with the
> GpuDataManager.
> > > > > >
> > > > > > It returns false because the gpu gets black-listed (weirdly, it
> > > doesn't
> > > > > happen
> > > > > > consistently. The blacklisting seems to always happen when
> running
> > > > > > browser_tests, and sometimes when running chrome)
> > > > >
> > > > > Correction: not 'gpu gets blacklisted', but 'the
> > > > > GPU_FEATURE_TYPE_GPU_COMPOSITING feature gets blacklisted'.
> > > > >
> > > >
> > > > There are a few software compositing browser tests. But they are
> disabled
> > > > on chromeos since we don't do software compositing there. Is it
> those?
> > >
> > > I don't think so (the only test I have tried (and the only
> browser_tests we
> > > currently run in mash) is BrowserTest.Title:
> > > https://cs.chromium.org/chromium/src/chrome/browser/
> > > ui/browser_browsertest.cc?sq=package:chromium&l=432&dr=C)
> > >
> >
> > Why's it blacklisted? browser tests should be using gpu, it's osmesa.
>
> Here's one weird trick to avoid the blacklisting: use --use-gpu-in-tests
> --disable-gpu-driver-bug-workarounds flags (i.e. the hardcoded blacklists
> hate
> my gpu). Is this expected? (a chromeos build on the same workstation does
> work
> correctly, so not sure why blacklisting is necessary in a linux build)
> Does this
> make sense?
>

This is surprising cuz browser tests don't use the real gpu, it's all
osmesa. They should always be running with GL, we should fix. Someone else
fixed another case recently: https://bugs.chromium.org/p/chromium/
issues/detail?id=621316

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sadrul, 2016-08-13 02:04:12]

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

[commit-bot: I haz the power, 2016-08-13 02:04: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-08-13 02:51:35]

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

[commit-bot: I haz the power, 2016-08-13 02:51:36]

Dry run: This issue passed the CQ dry run.

[sadrul, 2016-08-13 02:59:53]

Put up https://codereview.chromium.org/2236933008/ to investigate the gpu
blacklisting issue. Slimmed down this CL to be much simpler. PTAL

[piman, 2016-08-15 20:15:10]

lgtm

[sadrul, 2016-08-15 20:40:10]

The CQ bit was checked by sadrul@chromium.org

[sadrul, 2016-08-15 20:40:10]

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

[commit-bot: I haz the power, 2016-08-15 20:40:27]

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

[danakj, 2016-08-15 21:15:21]

LGTM 2

[commit-bot: I haz the power, 2016-08-15 22:17:04]

Committed patchset #3 (id:40001)

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

Description was changed from

==========
mus: Always use the GPU compositor for mus.

BUG=635613
==========

to

==========
mus: Always use the GPU compositor for mus.

BUG=635613

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

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

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