allow usage of transparent visuals when using xvfb

allow usage of transparent visuals when using xvfb

For the switch kEnableTransparentVisuals to be effective it requires
a compositing manager to run. Indeed ui::ChooseVisualForWindow checks
for the presence of the Atom _NET_WM_CM_S0 before looking for a 32
bit depth compatible visual.

BUG=589509
R=dpranke@chromium.org, piman@chromium.org, sky@chromium.org, danakj@chromium.org

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

[Julien Isorce Samsung, 2016-05-24 09:00:07]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-05-24 09:00:22]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2008833002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2008833002/1

[commit-bot: I haz the power, 2016-05-24 10:19:23]

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

[commit-bot: I haz the power, 2016-05-24 10:19:24]

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

[Julien Isorce Samsung, 2016-05-24 10:55:55]

Description was changed from

==========
allow usage of transparent visuals when using xvfb

For the switch kEnableTransparentVisuals to be effective it requires
a compositing manager to run. Indeed ui::ChooseVisualForWindow checks
for the presence of the Atom _NET_WM_CM_S0 before looking for a 32
bit depth compatible visual.

BUG=589509

R=piman@chromium.org
==========

to

==========
allow usage of transparent visuals when using xvfb

For the switch kEnableTransparentVisuals to be effective it requires
a compositing manager to run. Indeed ui::ChooseVisualForWindow checks
for the presence of the Atom _NET_WM_CM_S0 before looking for a 32
bit depth compatible visual.

BUG=589509

R=dpranke@chromium.org, piman@chromium.org
==========

[Julien Isorce Samsung, 2016-05-24 10:55:55]

j.isorce@samsung.com changed reviewers:
+ dpranke@chromium.org

[Julien Isorce Samsung, 2016-05-24 10:56:53]

On 2016/05/24 10:19:24, commit-bot: I haz the power wrote:
> Dry run: Try jobs failed on following builders:
>   linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

"Failed to start Xvfb or Openbox or xcompmgr: [Errno 2] No such file or
directory"

It would require to install xcompmgr on all linux try bots.

[Julien Isorce Samsung, 2016-05-24 10:58:02]

On 2016/05/24 10:56:53, Julien Isorce wrote:
> On 2016/05/24 10:19:24, commit-bot: I haz the power wrote:
> > Dry run: Try jobs failed on following builders:
> >   linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
> 
> "Failed to start Xvfb or Openbox or xcompmgr: [Errno 2] No such file or
> directory"
> 
> It would require to install xcompmgr on all linux try bots.

See https://codereview.chromium.org/1998653002/ for an example of unit test
that uses switches::kEnableTransparentVisuals.

[Dirk Pranke, 2016-05-24 19:03:48]

Description was changed from

==========
allow usage of transparent visuals when using xvfb

For the switch kEnableTransparentVisuals to be effective it requires
a compositing manager to run. Indeed ui::ChooseVisualForWindow checks
for the presence of the Atom _NET_WM_CM_S0 before looking for a 32
bit depth compatible visual.

BUG=589509

R=dpranke@chromium.org, piman@chromium.org
==========

to

==========
allow usage of transparent visuals when using xvfb

For the switch kEnableTransparentVisuals to be effective it requires
a compositing manager to run. Indeed ui::ChooseVisualForWindow checks
for the presence of the Atom _NET_WM_CM_S0 before looking for a 32
bit depth compatible visual.

BUG=589509

R=dpranke@chromium.org, piman@chromium.org, sky@chromium.org,
danakj@chromium.org
==========

[Dirk Pranke, 2016-05-24 19:03:48]

dpranke@chromium.org changed reviewers:
+ danakj@chromium.org, sky@chromium.org

[Dirk Pranke, 2016-05-24 19:04:33]

I have no understanding of whether we want this to work or not, so I'm going to
defer to piman/sky/danakj for that first.

If we do, I'm happy to take another look at the changes ...

[piman, 2016-05-24 19:13:16]

lgtm

[piman, 2016-05-24 19:15:27]

On Tue, May 24, 2016 at 12:04 PM, <dpranke@chromium.org> wrote:

> I have no understanding of whether we want this to work or not, so I'm
> going to
> defer to piman/sky/danakj for that first.
>

We probably want this - if anything that probably brings us a little closer
to what most linux users are running (with a compositing manager), however
I wouldn't want to sacrifice cycle time for it - we can mock the
availability of a compositing manager.


>
> If we do, I'm happy to take another look at the changes ...
>
> https://codereview.chromium.org/2008833002/
>

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

[Dirk Pranke, 2016-05-24 19:42:29]

ok, lgtm. It would be interesting to know if there was much of a increase in
cycle time.

You'll also need to file a ticket w/ Infra-Labs to get them to re-run
install-build-deps on all the bots, I think.

[sky, 2016-05-24 20:47:48]

On 2016/05/24 19:42:29, Dirk Pranke wrote:
> ok, lgtm. It would be interesting to know if there was much of a increase in
> cycle time.
> 
> You'll also need to file a ticket w/ Infra-Labs to get them to re-run
> install-build-deps on all the bots, I think.

Can we measure the impact before landing? If significant we could add a switch
for this.

[Dirk Pranke, 2016-05-24 21:08:52]

On 2016/05/24 20:47:48, sky wrote:
> On 2016/05/24 19:42:29, Dirk Pranke wrote:
> > ok, lgtm. It would be interesting to know if there was much of a increase in
> > cycle time.
> > 
> > You'll also need to file a ticket w/ Infra-Labs to get them to re-run
> > install-build-deps on all the bots, I think.
> 
> Can we measure the impact before landing? If significant we could add a switch
> for this.

Not, easily, if it requires new packages to be installed.

[sky, 2016-05-24 21:33:33]

Can't an experimental bot be set up for this? Seems bad that the only
want for measure impact of changes like this is to land it.

On Tue, May 24, 2016 at 2:08 PM,  <dpranke@chromium.org> wrote:
> On 2016/05/24 20:47:48, sky wrote:
>> On 2016/05/24 19:42:29, Dirk Pranke wrote:
>> > ok, lgtm. It would be interesting to know if there was much of a
>> > increase in
>> > cycle time.
>> >
>> > You'll also need to file a ticket w/ Infra-Labs to get them to re-run
>> > install-build-deps on all the bots, I think.
>>
>> Can we measure the impact before landing? If significant we could add a
>> switch
>> for this.
>
> Not, easily, if it requires new packages to be installed.
>
> https://codereview.chromium.org/2008833002/

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

[Dirk Pranke, 2016-05-24 21:47:16]

It's not like it's hard to revert if it causes problems, and setting up
experimental bots involves a decent amount of overhead.

On Tue, May 24, 2016 at 2:33 PM, Scott Violet <sky@chromium.org> wrote:

> Can't an experimental bot be set up for this? Seems bad that the only
> want for measure impact of changes like this is to land it.
>
> On Tue, May 24, 2016 at 2:08 PM,  <dpranke@chromium.org> wrote:
> > On 2016/05/24 20:47:48, sky wrote:
> >> On 2016/05/24 19:42:29, Dirk Pranke wrote:
> >> > ok, lgtm. It would be interesting to know if there was much of a
> >> > increase in
> >> > cycle time.
> >> >
> >> > You'll also need to file a ticket w/ Infra-Labs to get them to re-run
> >> > install-build-deps on all the bots, I think.
> >>
> >> Can we measure the impact before landing? If significant we could add a
> >> switch
> >> for this.
> >
> > Not, easily, if it requires new packages to be installed.
> >
> > https://codereview.chromium.org/2008833002/
>

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

[Julien Isorce Samsung, 2016-05-24 22:16:21]

On 2016/05/24 19:42:29, Dirk Pranke wrote:
> You'll also need to file a ticket w/ Infra-Labs to get them to re-run
> install-build-deps on all the bots, I think.

Done here: crbug.com/614512
Once the package is installed I can repeat a few dry runs and check the numbers.
Locally on my machine I have not noticed any difference but I only tried to run
base, cc, content, views unit tests and also blink layout tests, within
"python testing/xvfb.py".

[danakj, 2016-05-24 22:20:06]

On Tue, May 24, 2016 at 3:16 PM, <j.isorce@samsung.com> wrote:

> On 2016/05/24 19:42:29, Dirk Pranke wrote:
> > You'll also need to file a ticket w/ Infra-Labs to get them to re-run
> > install-build-deps on all the bots, I think.
>
> Done here: crbug.com/614512
> Once the package is installed I can repeat a few dry runs and check the
> numbers.
> Locally on my machine I have not noticed any difference but I only tried
> to run
> base, cc, content, views unit tests and also blink layout tests, within
> "python testing/xvfb.py".
>

Do any of those tests make X windows? Maybe try browser tests.


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

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

[Julien Isorce Samsung, 2016-05-24 22:56:51]

On 2016/05/24 22:20:06, danakj wrote:
> On Tue, May 24, 2016 at 3:16 PM, <mailto:j.isorce@samsung.com> wrote:
> 
> > On 2016/05/24 19:42:29, Dirk Pranke wrote:
> > > You'll also need to file a ticket w/ Infra-Labs to get them to re-run
> > > install-build-deps on all the bots, I think.
> >
> > Done here: crbug.com/614512
> > Once the package is installed I can repeat a few dry runs and check the
> > numbers.
> > Locally on my machine I have not noticed any difference but I only tried
> > to run
> > base, cc, content, views unit tests and also blink layout tests, within
> > "python testing/xvfb.py".
> >
> 
> Do any of those tests make X windows? Maybe try browser tests.

Yes at least views_unittests with its WidgetTests.
Upon your suggestion I tried
python testing/xvfb.py out/Release/ ./out/Release/browser_tests
  --test-launcher-bot-mode --gtest_filter=*ExtensionApi*
and got the same times. I also tried *Platform*.
I'll compare the full set of 4777 tests tomorrow and split the CL
as you suggested on the bug. Thx.

[danakj, 2016-05-24 22:58:41]

On Tue, May 24, 2016 at 3:56 PM, <j.isorce@samsung.com> wrote:

> On 2016/05/24 22:20:06, danakj wrote:
> > On Tue, May 24, 2016 at 3:16 PM, <mailto:j.isorce@samsung.com> wrote:
> >
> > > On 2016/05/24 19:42:29, Dirk Pranke wrote:
> > > > You'll also need to file a ticket w/ Infra-Labs to get them to re-run
> > > > install-build-deps on all the bots, I think.
> > >
> > > Done here: crbug.com/614512
> > > Once the package is installed I can repeat a few dry runs and check the
> > > numbers.
> > > Locally on my machine I have not noticed any difference but I only
> tried
> > > to run
> > > base, cc, content, views unit tests and also blink layout tests, within
> > > "python testing/xvfb.py".
> > >
> >
> > Do any of those tests make X windows? Maybe try browser tests.
>
> Yes at least views_unittests with its WidgetTests.
>

OK if views_unittests does then I'd trust that more than browser tests,
where browser tests being generally slow will dominate and make changes
hard to discern from noise.


> Upon your suggestion I tried
> python testing/xvfb.py out/Release/ ./out/Release/browser_tests
> --test-launcher-bot-mode --gtest_filter=*ExtensionApi*
> and got the same times. I also tried *Platform*.
> I'll compare the full set of 4777 tests tomorrow and split the CL
> as you suggested on the bug. Thx.
>
>
>
>
> https://codereview.chromium.org/2008833002/
>

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

[Julien Isorce Samsung, 2016-05-25 14:11:09]

On 2016/05/24 22:58:41, danakj wrote:
> On Tue, May 24, 2016 at 3:56 PM, <mailto:j.isorce@samsung.com> wrote:
> 
> > On 2016/05/24 22:20:06, danakj wrote:
> > > On Tue, May 24, 2016 at 3:16 PM, <mailto:j.isorce@samsung.com> wrote:
> > >
> > > > On 2016/05/24 19:42:29, Dirk Pranke wrote:
> > > > > You'll also need to file a ticket w/ Infra-Labs to get them to re-run
> > > > > install-build-deps on all the bots, I think.
> > > >
> > > > Done here: crbug.com/614512
> > > > Once the package is installed I can repeat a few dry runs and check the
> > > > numbers.
> > > > Locally on my machine I have not noticed any difference but I only
> > tried
> > > > to run
> > > > base, cc, content, views unit tests and also blink layout tests, within
> > > > "python testing/xvfb.py".
> > > >
> > >
> > > Do any of those tests make X windows? Maybe try browser tests.
> >
> > Yes at least views_unittests with its WidgetTests.
> >
> 
> OK if views_unittests does then I'd trust that more than browser tests,
> where browser tests being generally slow will dominate and make changes
> hard to discern from noise.

ok make sense.

> 
> 
> > Upon your suggestion I tried
> > python testing/xvfb.py out/Release/ ./out/Release/browser_tests
> > --test-launcher-bot-mode --gtest_filter=*ExtensionApi*
> > and got the same times. I also tried *Platform*.
> > I'll compare the full set of 4777 tests tomorrow and split the CL
> > as you suggested on the bug. Thx.

Done https://codereview.chromium.org/2008313002/.
Same time if filter is *Video* . (I cannot run the full set of browser tests
in a reasonable time with and without the CL)

[Julien Isorce Samsung, 2016-05-26 08:20:41]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-05-26 08:20:53]

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

[commit-bot: I haz the power, 2016-05-26 09:26:37]

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

[commit-bot: I haz the power, 2016-05-26 09:26:38]

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

[Julien Isorce Samsung, 2016-05-27 03:15:40]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-05-27 03:15:58]

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

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

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

[commit-bot: I haz the power, 2016-05-27 04:19:13]

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

[Julien Isorce Samsung, 2016-05-27 04:41:04]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-05-27 04:41:24]

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

[commit-bot: I haz the power, 2016-05-27 06:24:17]

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

[commit-bot: I haz the power, 2016-05-27 06:24:18]

Dry run: This issue passed the CQ dry run.

[Julien Isorce Samsung, 2016-05-27 08:00:43]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-05-27 08:01:02]

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

[commit-bot: I haz the power, 2016-05-27 08:04:17]

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

[commit-bot: I haz the power, 2016-05-27 08:04:18]

Dry run: This issue passed the CQ dry run.

[Julien Isorce Samsung, 2016-05-27 09:31:10]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-05-27 09:31:23]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2008833002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2008833002/100001

[commit-bot: I haz the power, 2016-05-27 11:04:05]

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

[commit-bot: I haz the power, 2016-05-27 11:04:07]

Dry run: This issue passed the CQ dry run.

[Julien Isorce Samsung, 2016-05-27 15:55:38]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-05-27 15:56:07]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2008833002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2008833002/120001

[commit-bot: I haz the power, 2016-05-27 18:08:11]

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

[commit-bot: I haz the power, 2016-05-27 18:08:12]

Dry run: This issue passed the CQ dry run.

[Julien Isorce Samsung, 2016-05-27 19:33:26]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-05-27 19:33:53]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2008833002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2008833002/120001

[commit-bot: I haz the power, 2016-05-27 19:37:47]

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

[commit-bot: I haz the power, 2016-05-27 19:37:48]

Dry run: This issue passed the CQ dry run.

[Julien Isorce Samsung, 2016-05-31 19:59:16]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-05-31 19:59:51]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2008833002/130001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2008833002/130001

[commit-bot: I haz the power, 2016-05-31 21:53:19]

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

[commit-bot: I haz the power, 2016-05-31 21:53:21]

Dry run: This issue passed the CQ dry run.

[Julien Isorce Samsung, 2016-05-31 22:21:34]

On 2016/05/25 14:11:09, Julien Isorce wrote:
> On 2016/05/24 22:58:41, danakj wrote:
> > On Tue, May 24, 2016 at 3:56 PM, <mailto:j.isorce@samsung.com> wrote:
> > 
> > > On 2016/05/24 22:20:06, danakj wrote:
> > > > On Tue, May 24, 2016 at 3:16 PM, <mailto:j.isorce@samsung.com> wrote:
> > > >
> > > > > On 2016/05/24 19:42:29, Dirk Pranke wrote:
> > > > > > You'll also need to file a ticket w/ Infra-Labs to get them to
re-run
> > > > > > install-build-deps on all the bots, I think.
> > > > >
> > > > > Done here: crbug.com/614512
> > > > > Once the package is installed I can repeat a few dry runs and check
the
> > > > > numbers.
> > > > > Locally on my machine I have not noticed any difference but I only
> > > tried
> > > > > to run
> > > > > base, cc, content, views unit tests and also blink layout tests,
within
> > > > > "python testing/xvfb.py".
> > > > >
> > > >
> > > > Do any of those tests make X windows? Maybe try browser tests.
> > >
> > > Yes at least views_unittests with its WidgetTests.
> > >
> > 
> > OK if views_unittests does then I'd trust that more than browser tests,
> > where browser tests being generally slow will dominate and make changes
> > hard to discern from noise.
> 
> ok make sense.
> 
> > 
> > 
> > > Upon your suggestion I tried
> > > python testing/xvfb.py out/Release/ ./out/Release/browser_tests
> > > --test-launcher-bot-mode --gtest_filter=*ExtensionApi*
> > > and got the same times. I also tried *Platform*.
> > > I'll compare the full set of 4777 tests tomorrow and split the CL
> > > as you suggested on the bug. Thx.
> 
> Done https://codereview.chromium.org/2008313002/.
> Same time if filter is *Video* . (I cannot run the full set of browser tests
> in a reasonable time with and without the CL)

The CL https://codereview.chromium.org/2008313002/ is now merged
and xcompmgr is installed in linux try bots.

So I compared time to run steps 22 to 113 on tryserver.chromium.linux with and
without using xcompmgr:
From 22 because this is the step just after "21. mark: before_tests"
To 112 because then it starts again the same tests but there are upload
steps in between each tests.

Without patch:

https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
00:24:47 - 00:23:32 = 1 min 15 secs
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
08:01:43 - 08:00:19 = 1 min 24 secs
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
11:21:28 - 11:20:13 = 1 min 15 secs
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
12:57:28 - 12:56:13 = 1 min 15 secs

with patch:

https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
03:01:09 - 03:02:29 = 1 min 20 secs
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
08:10:22 - 08:09:04 = 1 min 18 secs
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
22:40:07 - 22:38:56 = 1 min 11 secs
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
13:29:44 - 13:28:31 = 1 min 13 secs

Note that on views_unittests it is always 0 sec. 
Let me know if there are particular tests I should compare too.

This page is interesting
https://build.chromium.org/p/tryserver.chromium.linux/stats/linux_chromium_re...
but I cannot really use it without landing the CL.

@sky, @danakj do you have any objection about landing the patch ?
@piman, @dpranke are you still ok ?

[danakj, 2016-05-31 22:23:04]

On 2016/05/31 22:21:34, Julien Isorce wrote:
> On 2016/05/25 14:11:09, Julien Isorce wrote:
> > On 2016/05/24 22:58:41, danakj wrote:
> > > On Tue, May 24, 2016 at 3:56 PM, <mailto:j.isorce@samsung.com> wrote:
> > > 
> > > > On 2016/05/24 22:20:06, danakj wrote:
> > > > > On Tue, May 24, 2016 at 3:16 PM, <mailto:j.isorce@samsung.com> wrote:
> > > > >
> > > > > > On 2016/05/24 19:42:29, Dirk Pranke wrote:
> > > > > > > You'll also need to file a ticket w/ Infra-Labs to get them to
> re-run
> > > > > > > install-build-deps on all the bots, I think.
> > > > > >
> > > > > > Done here: crbug.com/614512
> > > > > > Once the package is installed I can repeat a few dry runs and check
> the
> > > > > > numbers.
> > > > > > Locally on my machine I have not noticed any difference but I only
> > > > tried
> > > > > > to run
> > > > > > base, cc, content, views unit tests and also blink layout tests,
> within
> > > > > > "python testing/xvfb.py".
> > > > > >
> > > > >
> > > > > Do any of those tests make X windows? Maybe try browser tests.
> > > >
> > > > Yes at least views_unittests with its WidgetTests.
> > > >
> > > 
> > > OK if views_unittests does then I'd trust that more than browser tests,
> > > where browser tests being generally slow will dominate and make changes
> > > hard to discern from noise.
> > 
> > ok make sense.
> > 
> > > 
> > > 
> > > > Upon your suggestion I tried
> > > > python testing/xvfb.py out/Release/ ./out/Release/browser_tests
> > > > --test-launcher-bot-mode --gtest_filter=*ExtensionApi*
> > > > and got the same times. I also tried *Platform*.
> > > > I'll compare the full set of 4777 tests tomorrow and split the CL
> > > > as you suggested on the bug. Thx.
> > 
> > Done https://codereview.chromium.org/2008313002/.
> > Same time if filter is *Video* . (I cannot run the full set of browser tests
> > in a reasonable time with and without the CL)
> 
> The CL https://codereview.chromium.org/2008313002/ is now merged
> and xcompmgr is installed in linux try bots.
> 
> So I compared time to run steps 22 to 113 on tryserver.chromium.linux with and
> without using xcompmgr:
> From 22 because this is the step just after "21. mark: before_tests"
> To 112 because then it starts again the same tests but there are upload
> steps in between each tests.
> 
> Without patch:
> 
>
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
> 00:24:47 - 00:23:32 = 1 min 15 secs
>
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
> 08:01:43 - 08:00:19 = 1 min 24 secs
>
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
> 11:21:28 - 11:20:13 = 1 min 15 secs
>
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
> 12:57:28 - 12:56:13 = 1 min 15 secs
> 
> with patch:
> 
>
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
> 03:01:09 - 03:02:29 = 1 min 20 secs
>
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
> 08:10:22 - 08:09:04 = 1 min 18 secs
>
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
> 22:40:07 - 22:38:56 = 1 min 11 secs
>
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
> 13:29:44 - 13:28:31 = 1 min 13 secs
> 
> Note that on views_unittests it is always 0 sec. 
> Let me know if there are particular tests I should compare too.
> 
> This page is interesting
>
https://build.chromium.org/p/tryserver.chromium.linux/stats/linux_chromium_re...
> but I cannot really use it without landing the CL.
> 
> @sky, @danakj do you have any objection about landing the patch ?

Nope.

> @piman, @dpranke are you still ok ?

[piman, 2016-05-31 22:40:09]

On 2016/05/31 22:23:04, danakj wrote:
> On 2016/05/31 22:21:34, Julien Isorce wrote:
> > On 2016/05/25 14:11:09, Julien Isorce wrote:
> > > On 2016/05/24 22:58:41, danakj wrote:
> > > > On Tue, May 24, 2016 at 3:56 PM, <mailto:j.isorce@samsung.com> wrote:
> > > > 
> > > > > On 2016/05/24 22:20:06, danakj wrote:
> > > > > > On Tue, May 24, 2016 at 3:16 PM, <mailto:j.isorce@samsung.com>
wrote:
> > > > > >
> > > > > > > On 2016/05/24 19:42:29, Dirk Pranke wrote:
> > > > > > > > You'll also need to file a ticket w/ Infra-Labs to get them to
> > re-run
> > > > > > > > install-build-deps on all the bots, I think.
> > > > > > >
> > > > > > > Done here: crbug.com/614512
> > > > > > > Once the package is installed I can repeat a few dry runs and
check
> > the
> > > > > > > numbers.
> > > > > > > Locally on my machine I have not noticed any difference but I only
> > > > > tried
> > > > > > > to run
> > > > > > > base, cc, content, views unit tests and also blink layout tests,
> > within
> > > > > > > "python testing/xvfb.py".
> > > > > > >
> > > > > >
> > > > > > Do any of those tests make X windows? Maybe try browser tests.
> > > > >
> > > > > Yes at least views_unittests with its WidgetTests.
> > > > >
> > > > 
> > > > OK if views_unittests does then I'd trust that more than browser tests,
> > > > where browser tests being generally slow will dominate and make changes
> > > > hard to discern from noise.
> > > 
> > > ok make sense.
> > > 
> > > > 
> > > > 
> > > > > Upon your suggestion I tried
> > > > > python testing/xvfb.py out/Release/ ./out/Release/browser_tests
> > > > > --test-launcher-bot-mode --gtest_filter=*ExtensionApi*
> > > > > and got the same times. I also tried *Platform*.
> > > > > I'll compare the full set of 4777 tests tomorrow and split the CL
> > > > > as you suggested on the bug. Thx.
> > > 
> > > Done https://codereview.chromium.org/2008313002/.
> > > Same time if filter is *Video* . (I cannot run the full set of browser
tests
> > > in a reasonable time with and without the CL)
> > 
> > The CL https://codereview.chromium.org/2008313002/ is now merged
> > and xcompmgr is installed in linux try bots.
> > 
> > So I compared time to run steps 22 to 113 on tryserver.chromium.linux with
and
> > without using xcompmgr:
> > From 22 because this is the step just after "21. mark: before_tests"
> > To 112 because then it starts again the same tests but there are upload
> > steps in between each tests.
> > 
> > Without patch:
> > 
> >
>
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
> > 00:24:47 - 00:23:32 = 1 min 15 secs
> >
>
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
> > 08:01:43 - 08:00:19 = 1 min 24 secs
> >
>
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
> > 11:21:28 - 11:20:13 = 1 min 15 secs
> >
>
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
> > 12:57:28 - 12:56:13 = 1 min 15 secs
> > 
> > with patch:
> > 
> >
>
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
> > 03:01:09 - 03:02:29 = 1 min 20 secs
> >
>
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
> > 08:10:22 - 08:09:04 = 1 min 18 secs
> >
>
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
> > 22:40:07 - 22:38:56 = 1 min 11 secs
> >
>
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
> > 13:29:44 - 13:28:31 = 1 min 13 secs
> > 
> > Note that on views_unittests it is always 0 sec. 
> > Let me know if there are particular tests I should compare too.
> > 
> > This page is interesting
> >
>
https://build.chromium.org/p/tryserver.chromium.linux/stats/linux_chromium_re...
> > but I cannot really use it without landing the CL.
> > 
> > @sky, @danakj do you have any objection about landing the patch ?
> 
> Nope.
> 
> > @piman, @dpranke are you still ok ?

LGTM as far as I'm concerned. Thanks for checking!

[Dirk Pranke, 2016-05-31 22:49:03]

still lgtm as well.

[Julien Isorce Samsung, 2016-06-01 06:06:01]

The CQ bit was checked by j.isorce@samsung.com

[commit-bot: I haz the power, 2016-06-01 06:06:21]

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

[commit-bot: I haz the power, 2016-06-01 06:10:10]

Description was changed from

==========
allow usage of transparent visuals when using xvfb

For the switch kEnableTransparentVisuals to be effective it requires
a compositing manager to run. Indeed ui::ChooseVisualForWindow checks
for the presence of the Atom _NET_WM_CM_S0 before looking for a 32
bit depth compatible visual.

BUG=589509

R=dpranke@chromium.org, piman@chromium.org, sky@chromium.org,
danakj@chromium.org
==========

to

==========
allow usage of transparent visuals when using xvfb

For the switch kEnableTransparentVisuals to be effective it requires
a compositing manager to run. Indeed ui::ChooseVisualForWindow checks
for the presence of the Atom _NET_WM_CM_S0 before looking for a 32
bit depth compatible visual.

BUG=589509

R=dpranke@chromium.org, piman@chromium.org, sky@chromium.org,
danakj@chromium.org
==========

[commit-bot: I haz the power, 2016-06-01 06:10:13]

Committed patchset #8 (id:130001)

[commit-bot: I haz the power, 2016-06-01 06:11:12]

Description was changed from

==========
allow usage of transparent visuals when using xvfb

For the switch kEnableTransparentVisuals to be effective it requires
a compositing manager to run. Indeed ui::ChooseVisualForWindow checks
for the presence of the Atom _NET_WM_CM_S0 before looking for a 32
bit depth compatible visual.

BUG=589509

R=dpranke@chromium.org, piman@chromium.org, sky@chromium.org,
danakj@chromium.org
==========

to

==========
allow usage of transparent visuals when using xvfb

For the switch kEnableTransparentVisuals to be effective it requires
a compositing manager to run. Indeed ui::ChooseVisualForWindow checks
for the presence of the Atom _NET_WM_CM_S0 before looking for a 32
bit depth compatible visual.

BUG=589509

R=dpranke@chromium.org, piman@chromium.org, sky@chromium.org,
danakj@chromium.org

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

[commit-bot: I haz the power, 2016-06-01 06:11:13]

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