New issue
Advanced search Search tips

Issue 824510 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocked on:
issue 797137



Sign in to add a comment

Add unit tests for command buffer proxies

Project Member Reported by sunn...@chromium.org, Mar 21 2018

Issue description

At one point, the proxies would just forward calls to IPC messages, so testing wasn't important. However, since the introduction of ordering barrier, sync tokens, and multi flush, they are responsible for ensuring correctness too, so testing is necessary.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dde8877bc21f56ea610b4889f34d31f970d54fd6

commit dde8877bc21f56ea610b4889f34d31f970d54fd6
Author: Sunny Sachanandani <sunnyps@chromium.org>
Date: Fri Mar 23 03:37:10 2018

gpu: Unit tests for IPC command buffer proxy

The command buffer proxy doesn't just forward calls, but it also does
coalescing of flushes, and handles sync token verification. This needs
to be tested for correctness.

R=piman
BUG= 824510 

Cq-Include-Trybots: luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I6802b954e7bc13587ccaaba070f6ae88b0c732de
Reviewed-on: https://chromium-review.googlesource.com/974758
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545361}
[modify] https://crrev.com/dde8877bc21f56ea610b4889f34d31f970d54fd6/gpu/BUILD.gn
[add] https://crrev.com/dde8877bc21f56ea610b4889f34d31f970d54fd6/gpu/ipc/client/command_buffer_proxy_impl_unittest.cc
[modify] https://crrev.com/dde8877bc21f56ea610b4889f34d31f970d54fd6/gpu/ipc/client/gpu_channel_host.h

Labels: Postmortem-Followup
Status: Fixed (was: Assigned)
We have an initial set of unit tests which would've caught issue 797137.

Sign in to add a comment