Fixed compile- and link-time errors with the "enable_vulkan" GN-argument

Fixed compile- and link-time errors with the "enable_vulkan" GN-argument
set to true:

I.

Got rid of the VulkanBrowserCompositorSurface::OnGpuSwapBuffersCompleted
function due to it was dropped from the BrowserCompositorSurface parent
type.

II.

Brought appearance of the VulkanBrowserCompositorSurface class in
accordance with the most recent cc::OutputSurface class definition.

III.

Fixed 3rd parameter within the VulkanBrowserCompositorOutputSurface
c-tor calling from GpuProcessTransportFactory::EstablishedGpuChannel.

IV.

Updated the VulkanBrowserCompositorOutputSurface c-tor signature
according to the declaration provided within corresponding header.

V.

Starting from version 1.0.13 the Vulkan SDK installer no longer installs
files to system locations that require administrative privileges. Thus,
to support newer versions of the SDK, the VULKAN_SDK environment
variable shall be read to locate corresponding shared library instead of
making GN to look for the latter among system folders.

BUG=582558, 582564
CQ_INCLUDE_TRYBOTS=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

Committed: https://crrev.com/1f226f5a91c999f0cd248a807a15e2038bf57cc3
Cr-Commit-Position: refs/heads/master@{#430354}

[Sergey.Kipet, 2016-11-01 17:39:03]

Description was changed from

==========
Fixed compile- and link-time errors with the "enable_vulkan" GN-argument
set to true:

I.

Got rid of the VulkanBrowserCompositorSurface::OnGpuSwapBuffersCompleted
function due to it was dropped from the BrowserCompositorSurface parent
type.

II.

Brought appearance of the VulkanBrowserCompositorSurface class in
accordance with the most recent cc::OutputSurface class definition.

III.

Fixed 3rd parameter within the VulkanBrowserCompositorOutputSurface
c-tor calling from GpuProcessTransportFactory::EstablishedGpuChannel.

IV.

Updated the VulkanBrowserCompositorOutputSurface c-tor signature
according to the declaration provided within corresponding header.

V.

Starting from version 1.0.13 the Vulkan SDK installer no longer installs
files to system locations that require administrative privileges. Thus,
to support newer versions of the SDK, the VULKAN_SDK environment
variable shall be read to locate corresponding shared library instead of
making GN to look for the latter among system folders.

BUG=582558, 582564
==========

to

==========
Fixed compile- and link-time errors with the "enable_vulkan" GN-argument
set to true:

I.

Got rid of the VulkanBrowserCompositorSurface::OnGpuSwapBuffersCompleted
function due to it was dropped from the BrowserCompositorSurface parent
type.

II.

Brought appearance of the VulkanBrowserCompositorSurface class in
accordance with the most recent cc::OutputSurface class definition.

III.

Fixed 3rd parameter within the VulkanBrowserCompositorOutputSurface
c-tor calling from GpuProcessTransportFactory::EstablishedGpuChannel.

IV.

Updated the VulkanBrowserCompositorOutputSurface c-tor signature
according to the declaration provided within corresponding header.

V.

Starting from version 1.0.13 the Vulkan SDK installer no longer installs
files to system locations that require administrative privileges. Thus,
to support newer versions of the SDK, the VULKAN_SDK environment
variable shall be read to locate corresponding shared library instead of
making GN to look for the latter among system folders.

BUG=582558, 582564
CQ_INCLUDE_TRYBOTS=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
==========

[Sergey.Kipet, 2016-11-01 18:01:20]

sergey.kipet@gmail.com changed reviewers:
+ danakj@chromium.org, enne@chromium.org, fsamuel@chromium.org,
piman@chromium.org

[Sergey.Kipet, 2016-11-01 18:01:21]

Hello,

Please, participate the first code review of mine.

Thank you so much in advance and have a nice day!

BRs,
Sergey Kipet.

[danakj, 2016-11-01 18:48:24]

LGTM

https://codereview.chromium.org/2472433002/diff/1/content/browser/compositor/...
File content/browser/compositor/vulkan_browser_compositor_output_surface.cc
(right):

https://codereview.chromium.org/2472433002/diff/1/content/browser/compositor/...
content/browser/compositor/vulkan_browser_compositor_output_surface.cc:17: :
BrowserCompositorOutputSurface(context,
std::move

https://codereview.chromium.org/2472433002/diff/1/content/browser/compositor/...
content/browser/compositor/vulkan_browser_compositor_output_surface.cc:18:
vsync_manager,
same

https://codereview.chromium.org/2472433002/diff/1/content/browser/compositor/...
content/browser/compositor/vulkan_browser_compositor_output_surface.cc:89:
surface_->SwapBuffers();
This needs to post-task back a call to client_->DidReceiveSwapBuffersAck() or
the client will be swap-throttled.

[piman, 2016-11-01 19:45:13]

https://codereview.chromium.org/2472433002/diff/1/gpu/vulkan/BUILD.gn
File gpu/vulkan/BUILD.gn (right):

https://codereview.chromium.org/2472433002/diff/1/gpu/vulkan/BUILD.gn#newcode13
gpu/vulkan/BUILD.gn:13: vulkan_lib_dir = getenv("VULKAN_SDK") + "/lib"
What is this for? The expectation is that the vulkan run-time is installed in
the normal location (/usr/lib/x86_64-linux-gnu) as defined by the package.

[Sergey.Kipet, 2016-11-04 22:15:37]

Hello,

Please, find the issues previously found fixed in patch set 2 of the code review
of mine.

Thanks a lot in advance!

Look forward to hearing from you.

BRs,
Sergey Kipet.

https://codereview.chromium.org/2472433002/diff/1/content/browser/compositor/...
File content/browser/compositor/vulkan_browser_compositor_output_surface.cc
(right):

https://codereview.chromium.org/2472433002/diff/1/content/browser/compositor/...
content/browser/compositor/vulkan_browser_compositor_output_surface.cc:17: :
BrowserCompositorOutputSurface(context,
On 2016/11/01 18:48:23, danakj_OOO_back_11_9 wrote:
> std::move

Done.

https://codereview.chromium.org/2472433002/diff/1/content/browser/compositor/...
content/browser/compositor/vulkan_browser_compositor_output_surface.cc:18:
vsync_manager,
On 2016/11/01 18:48:23, danakj_OOO_back_11_9 wrote:
> same

Done.

https://codereview.chromium.org/2472433002/diff/1/content/browser/compositor/...
content/browser/compositor/vulkan_browser_compositor_output_surface.cc:89:
surface_->SwapBuffers();
On 2016/11/01 18:48:23, danakj_OOO_back_11_9 wrote:
> This needs to post-task back a call to client_->DidReceiveSwapBuffersAck() or
> the client will be swap-throttled.

Done.

https://codereview.chromium.org/2472433002/diff/1/gpu/vulkan/BUILD.gn
File gpu/vulkan/BUILD.gn (right):

https://codereview.chromium.org/2472433002/diff/1/gpu/vulkan/BUILD.gn#newcode13
gpu/vulkan/BUILD.gn:13: vulkan_lib_dir = getenv("VULKAN_SDK") + "/lib"
On 2016/11/01 19:45:13, piman wrote:
> What is this for? The expectation is that the vulkan run-time is installed in
> the normal location (/usr/lib/x86_64-linux-gnu) as defined by the package.

Well, seems like starting from the SDK version 1.0.13, it is not assumed as
location for installing Vulkan run-time anymore (please, refer to the
'Administrative privileges' section at
https://vulkan.lunarg.com/doc/sdk/latest/linux/getting_started.html).
Particularly, the 'libvulkan_wrapper.so' sub-target is successfully linked if
that kind of workaround is provided while having the most recent SDK installed
only.

[piman, 2016-11-05 00:39:06]

Please add your name to the AUTHORS file, then we should be good to go.
(note to other reviewers: I checked the CLA was signed).

https://codereview.chromium.org/2472433002/diff/1/gpu/vulkan/BUILD.gn
File gpu/vulkan/BUILD.gn (right):

https://codereview.chromium.org/2472433002/diff/1/gpu/vulkan/BUILD.gn#newcode13
gpu/vulkan/BUILD.gn:13: vulkan_lib_dir = getenv("VULKAN_SDK") + "/lib"
On 2016/11/04 22:15:36, Sergey.Kipet wrote:
> On 2016/11/01 19:45:13, piman wrote:
> > What is this for? The expectation is that the vulkan run-time is installed
in
> > the normal location (/usr/lib/x86_64-linux-gnu) as defined by the package.
> 
> Well, seems like starting from the SDK version 1.0.13, it is not assumed as
> location for installing Vulkan run-time anymore (please, refer to the
> 'Administrative privileges' section at
> https://vulkan.lunarg.com/doc/sdk/latest/linux/getting_started.html).
> Particularly, the 'libvulkan_wrapper.so' sub-target is successfully linked if
> that kind of workaround is provided while having the most recent SDK installed
> only.

Ok. This is not a great long-term situation, but because it's all behind a flag,
no need to block on it.

[Sergey.Kipet, 2016-11-05 08:43:13]

On 2016/11/05 00:39:06, piman wrote:
> Please add your name to the AUTHORS file, then we should be good to go.
> (note to other reviewers: I checked the CLA was signed).
> 
> https://codereview.chromium.org/2472433002/diff/1/gpu/vulkan/BUILD.gn
> File gpu/vulkan/BUILD.gn (right):
> 
>
https://codereview.chromium.org/2472433002/diff/1/gpu/vulkan/BUILD.gn#newcode13
> gpu/vulkan/BUILD.gn:13: vulkan_lib_dir = getenv("VULKAN_SDK") + "/lib"
> On 2016/11/04 22:15:36, Sergey.Kipet wrote:
> > On 2016/11/01 19:45:13, piman wrote:
> > > What is this for? The expectation is that the vulkan run-time is installed
> in
> > > the normal location (/usr/lib/x86_64-linux-gnu) as defined by the package.
> > 
> > Well, seems like starting from the SDK version 1.0.13, it is not assumed as
> > location for installing Vulkan run-time anymore (please, refer to the
> > 'Administrative privileges' section at
> > https://vulkan.lunarg.com/doc/sdk/latest/linux/getting_started.html).
> > Particularly, the 'libvulkan_wrapper.so' sub-target is successfully linked
if
> > that kind of workaround is provided while having the most recent SDK
installed
> > only.
> 
> Ok. This is not a great long-term situation, but because it's all behind a
flag,
> no need to block on it.

Agree. Thank you.

[Sergey.Kipet, 2016-11-05 09:10:11]

Hello,

Please, find the AUTHORS file updated as requested.

Thank you.

BRs,
Sergey Kipet.

https://codereview.chromium.org/2472433002/diff/1/gpu/vulkan/BUILD.gn
File gpu/vulkan/BUILD.gn (right):

https://codereview.chromium.org/2472433002/diff/1/gpu/vulkan/BUILD.gn#newcode13
gpu/vulkan/BUILD.gn:13: vulkan_lib_dir = getenv("VULKAN_SDK") + "/lib"
On 2016/11/05 00:39:06, piman wrote:
> On 2016/11/04 22:15:36, Sergey.Kipet wrote:
> > On 2016/11/01 19:45:13, piman wrote:
> > > What is this for? The expectation is that the vulkan run-time is installed
> in
> > > the normal location (/usr/lib/x86_64-linux-gnu) as defined by the package.
> > 
> > Well, seems like starting from the SDK version 1.0.13, it is not assumed as
> > location for installing Vulkan run-time anymore (please, refer to the
> > 'Administrative privileges' section at
> > https://vulkan.lunarg.com/doc/sdk/latest/linux/getting_started.html).
> > Particularly, the 'libvulkan_wrapper.so' sub-target is successfully linked
if
> > that kind of workaround is provided while having the most recent SDK
installed
> > only.
> 
> Ok. This is not a great long-term situation, but because it's all behind a
flag,
> no need to block on it.

Acknowledged.

[piman, 2016-11-07 19:00:00]

lgtm

[piman, 2016-11-07 19:00:04]

The CQ bit was checked by piman@chromium.org

[piman, 2016-11-07 19:00:04]

The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org
Link to the patchset: https://codereview.chromium.org/2472433002/#ps40001
(title: "Augmented the AUTHORS file per reviewers request.")

[commit-bot: I haz the power, 2016-11-07 19:00:49]

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-11-07 20:10:24]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-07 20:22:11]

Description was changed from

==========
Fixed compile- and link-time errors with the "enable_vulkan" GN-argument
set to true:

I.

Got rid of the VulkanBrowserCompositorSurface::OnGpuSwapBuffersCompleted
function due to it was dropped from the BrowserCompositorSurface parent
type.

II.

Brought appearance of the VulkanBrowserCompositorSurface class in
accordance with the most recent cc::OutputSurface class definition.

III.

Fixed 3rd parameter within the VulkanBrowserCompositorOutputSurface
c-tor calling from GpuProcessTransportFactory::EstablishedGpuChannel.

IV.

Updated the VulkanBrowserCompositorOutputSurface c-tor signature
according to the declaration provided within corresponding header.

V.

Starting from version 1.0.13 the Vulkan SDK installer no longer installs
files to system locations that require administrative privileges. Thus,
to support newer versions of the SDK, the VULKAN_SDK environment
variable shall be read to locate corresponding shared library instead of
making GN to look for the latter among system folders.

BUG=582558, 582564
CQ_INCLUDE_TRYBOTS=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

==========
Fixed compile- and link-time errors with the "enable_vulkan" GN-argument
set to true:

I.

Got rid of the VulkanBrowserCompositorSurface::OnGpuSwapBuffersCompleted
function due to it was dropped from the BrowserCompositorSurface parent
type.

II.

Brought appearance of the VulkanBrowserCompositorSurface class in
accordance with the most recent cc::OutputSurface class definition.

III.

Fixed 3rd parameter within the VulkanBrowserCompositorOutputSurface
c-tor calling from GpuProcessTransportFactory::EstablishedGpuChannel.

IV.

Updated the VulkanBrowserCompositorOutputSurface c-tor signature
according to the declaration provided within corresponding header.

V.

Starting from version 1.0.13 the Vulkan SDK installer no longer installs
files to system locations that require administrative privileges. Thus,
to support newer versions of the SDK, the VULKAN_SDK environment
variable shall be read to locate corresponding shared library instead of
making GN to look for the latter among system folders.

BUG=582558, 582564
CQ_INCLUDE_TRYBOTS=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

Committed: https://crrev.com/1f226f5a91c999f0cd248a807a15e2038bf57cc3
Cr-Commit-Position: refs/heads/master@{#430354}
==========

[commit-bot: I haz the power, 2016-11-07 20:22:12]

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