Remove ui_compositor_image_transport, it has no effect.

Remove ui_compositor_image_transport, it has no effect.

It's been used to add a conditional dep on ui/gl, but we chrome depends on
that unconditionally already.

It's also been used to call glFinish() under the following conditions in
the glResizeCHROMIUM handler:

    !defined(UI_COMPOSITOR_IMAGE_TRANSPORT)

UI_COMPOSITOR_IMAGE_TRANSPORT was always set on Linux and Chrome OS, which
means this code only ran on Android, and I'm not sure if we use
glResizeCHROMIUM there.

BUG=700681
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2748533002
Cr-Commit-Position: refs/heads/master@{#456746}
Committed: https://chromium.googlesource.com/chromium/src/+/81e3d61f4f273627a7fb9d6d7f2bbbc382c95d7e

[Nico, 2017-03-11 18:15:54]

Description was changed from

==========
Remove ui_compositor_image_transport, it has no effect.

It's been used to add a conditional dep on ui/gl, but we chrome depends on
that unconditionally already.

It's also been used to call glFinish() under the following conditions in
the glResizeCHROMIUM handler:

    !defined(UI_COMPOSITOR_IMAGE_TRANSPORT)

UI_COMPOSITOR_IMAGE_TRANSPORT was always set on Linux and Chrome OS, which
means this code only ran on Android, and I'm not sure if we use
glResizeCHROMIUM there.

BUG=700681
==========

to

==========
Remove ui_compositor_image_transport, it has no effect.

It's been used to add a conditional dep on ui/gl, but we chrome depends on
that unconditionally already.

It's also been used to call glFinish() under the following conditions in
the glResizeCHROMIUM handler:

    !defined(UI_COMPOSITOR_IMAGE_TRANSPORT)

UI_COMPOSITOR_IMAGE_TRANSPORT was always set on Linux and Chrome OS, which
means this code only ran on Android, and I'm not sure if we use
glResizeCHROMIUM there.

BUG=700681
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Nico, 2017-03-11 18:16:42]

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

[commit-bot: I haz the power, 2017-03-11 18:16:49]

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

[Nico, 2017-03-11 18:30:00]

thakis@chromium.org changed reviewers:
+ kbr@chromium.org

[Nico, 2017-03-11 18:30:01]

The origin of this flag are lost in the mist of time. But it looks like it's
been mostly a no-op for some time from what I can tell.

https://codereview.chromium.org/2748533002/diff/1/gpu/command_buffer/service/...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (left):

https://codereview.chromium.org/2748533002/diff/1/gpu/command_buffer/service/...
gpu/command_buffer/service/gles2_cmd_decoder.cc:5145: // Make sure that we are
done drawing to the back buffer before resizing.
I'm unsure about this bit; it might still be needed on Android? It looks like
this was added explicitly for linux here long ago:
https://codereview.chromium.org/5105006 So even though it might run on android
(not sure if we use glResizeCHROMIUM there), it's probably not intentional.

[commit-bot: I haz the power, 2017-03-11 19:07:06]

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

[commit-bot: I haz the power, 2017-03-11 19:07:06]

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

[Nico, 2017-03-11 19:29:31]

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

[commit-bot: I haz the power, 2017-03-11 19:29:39]

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, 2017-03-11 19:58:42]

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

[commit-bot: I haz the power, 2017-03-11 19:58:43]

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

[Nico, 2017-03-11 20:23:59]

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

[commit-bot: I haz the power, 2017-03-11 20:24:01]

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, 2017-03-11 21:09:53]

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

[commit-bot: I haz the power, 2017-03-11 21:09:54]

Dry run: This issue passed the CQ dry run.

[Ken Russell (switch to Gerrit), 2017-03-14 03:18:18]

Since the Linux port's been using Aura for a long time now, I suspect that the
glFinish (which was added for X11 builds) is not needed any more.

Have you tested this on desktop Linux and tried live resizing Chrome? Does it
flicker?

CC'ing a couple of other people who might have concerns, but this lgtm overall.

[Nico, 2017-03-14 03:45:40]

On 2017/03/14 03:18:18, Ken Russell wrote:
> Since the Linux port's been using Aura for a long time now, I suspect that the
> glFinish (which was added for X11 builds) is not needed any more.
> 
> Have you tested this on desktop Linux and tried live resizing Chrome? Does it
> flicker?
> 
> CC'ing a couple of other people who might have concerns, but this lgtm
overall.

Thanks! On Linux, the patch is a noop since the TRANSPORT define was set, so the
#if wasn't taken.

[Ken Russell (switch to Gerrit), 2017-03-14 04:14:29]

On 2017/03/14 03:45:40, Nico wrote:
> On 2017/03/14 03:18:18, Ken Russell wrote:
> > Since the Linux port's been using Aura for a long time now, I suspect that
the
> > glFinish (which was added for X11 builds) is not needed any more.
> > 
> > Have you tested this on desktop Linux and tried live resizing Chrome? Does
it
> > flicker?
> > 
> > CC'ing a couple of other people who might have concerns, but this lgtm
> overall.
> 
> Thanks! On Linux, the patch is a noop since the TRANSPORT define was set, so
the
> #if wasn't taken.

Ah, I see. Seems fine to me to land this.

[Nico, 2017-03-14 14:28:29]

The CQ bit was checked by thakis@chromium.org

[commit-bot: I haz the power, 2017-03-14 14:28:51]

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, 2017-03-14 16:04:03]

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

[commit-bot: I haz the power, 2017-03-14 16:04:04]

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

[Nico, 2017-03-14 16:12:34]

The CQ bit was checked by thakis@chromium.org

[commit-bot: I haz the power, 2017-03-14 16:12:59]

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, 2017-03-14 17:17:07]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1489507954484940, "parent_rev":
"1fe686d9554038284aa9a3e5da5e2f7d519859ac", "commit_rev":
"81e3d61f4f273627a7fb9d6d7f2bbbc382c95d7e"}

[commit-bot: I haz the power, 2017-03-14 17:17:48]

Description was changed from

==========
Remove ui_compositor_image_transport, it has no effect.

It's been used to add a conditional dep on ui/gl, but we chrome depends on
that unconditionally already.

It's also been used to call glFinish() under the following conditions in
the glResizeCHROMIUM handler:

    !defined(UI_COMPOSITOR_IMAGE_TRANSPORT)

UI_COMPOSITOR_IMAGE_TRANSPORT was always set on Linux and Chrome OS, which
means this code only ran on Android, and I'm not sure if we use
glResizeCHROMIUM there.

BUG=700681
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Remove ui_compositor_image_transport, it has no effect.

It's been used to add a conditional dep on ui/gl, but we chrome depends on
that unconditionally already.

It's also been used to call glFinish() under the following conditions in
the glResizeCHROMIUM handler:

    !defined(UI_COMPOSITOR_IMAGE_TRANSPORT)

UI_COMPOSITOR_IMAGE_TRANSPORT was always set on Linux and Chrome OS, which
means this code only ran on Android, and I'm not sure if we use
glResizeCHROMIUM there.

BUG=700681
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2748533002
Cr-Commit-Position: refs/heads/master@{#456746}
Committed:
https://chromium.googlesource.com/chromium/src/+/81e3d61f4f273627a7fb9d6d7f2b...
==========

[commit-bot: I haz the power, 2017-03-14 17:17:49]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/81e3d61f4f273627a7fb9d6d7f2b...