New issue
Advanced search Search tips

Issue 837378 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 28
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Task



Sign in to add a comment

Replace DirectComposition CHECKs with correct error handling

Project Member Reported by sunn...@chromium.org, Apr 26 2018

Issue description

There are a bunch of CHECKs in DirectCompositionSurfaceWin/ChildSurfaceWin, and the crashes those cause are generally non-actionable such as device removed, OOM, etc. However, some CHECKs have caught invalid API calls such as creating zero-sized swap chains.

I'm going to replace the CHECKs with error handling that propagates error up the stack causing SwapBuffers to fail which will restart the GPU process without a crash report. I'm also adding an UMA histogram to log the last error, so that we can keep track of invalid calls.

CL incoming.

 
Project Member

Comment 1 by bugdroid1@chromium.org, May 9 2018

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

commit ca96558056619efb624d54e276dd0451a67b96dc
Author: Sunny Sachanandani <sunnyps@chromium.org>
Date: Wed May 09 00:44:08 2018

Replace CHECKs with error handling in DirectComposition code

This CL replaces a bunch of CHECKs with error handling that propagates
errors up the stack. Also adds DCHECKs where error handling is missing,
but the calls aren't expected to fail.

R=zmo
BUG= 837378 

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I1c52ea5fd1355b2ce3bb8e9a18bba232ec37f397
Reviewed-on: https://chromium-review.googlesource.com/1031203
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557031}
[modify] https://crrev.com/ca96558056619efb624d54e276dd0451a67b96dc/gpu/ipc/service/direct_composition_child_surface_win.cc
[modify] https://crrev.com/ca96558056619efb624d54e276dd0451a67b96dc/gpu/ipc/service/direct_composition_child_surface_win.h
[modify] https://crrev.com/ca96558056619efb624d54e276dd0451a67b96dc/gpu/ipc/service/direct_composition_surface_win.cc
[modify] https://crrev.com/ca96558056619efb624d54e276dd0451a67b96dc/tools/metrics/histograms/histograms.xml
[modify] https://crrev.com/ca96558056619efb624d54e276dd0451a67b96dc/ui/gl/gl_image_dxgi.cc

Project Member

Comment 2 by bugdroid1@chromium.org, May 14 2018

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

commit 70e70f68db3f80c46c64a38d89e2a38bb584889b
Author: Sunny Sachanandani <sunnyps@chromium.org>
Date: Mon May 14 21:43:42 2018

Do not consider command line flags for overlay support UMA

Command line flags override checks for hardware support so they
shouldn't be counted in UMA. However, we should log the hardware
support, and initialize globals even if overlays are disabled via
command line flags. Also guard against globals not being initialized.

R=zmo
BUG= 837378 

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: Idfcab896a7e1671ebe0446e657f093a4cd1f5169
Reviewed-on: https://chromium-review.googlesource.com/1034021
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558477}
[modify] https://crrev.com/70e70f68db3f80c46c64a38d89e2a38bb584889b/gpu/ipc/service/direct_composition_surface_win.cc

Project Member

Comment 3 by bugdroid1@chromium.org, May 14 2018

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

commit 9abedd704e255901801569c14a695c228479a547
Author: Sunny Sachanandani <sunnyps@chromium.org>
Date: Mon May 14 23:49:28 2018

Error handling fixes for direct composition

Initialize create_surface_handle_function_ only once, and handle errors.
Handle error when retreiving direct composition device.

R=zmo
BUG= 837378 

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: Ibbd8d041e7111ea13936b467c4630b45c9caadcf
Reviewed-on: https://chromium-review.googlesource.com/1058173
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558535}
[modify] https://crrev.com/9abedd704e255901801569c14a695c228479a547/gpu/ipc/service/direct_composition_surface_win.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Jun 6 2018

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

commit 6e3b68c1fbd7bb2ff10bbc5c483a43977ced0946
Author: Sunny Sachanandani <sunnyps@chromium.org>
Date: Wed Jun 06 00:50:29 2018

Deprecate GPU.DirectComposition.SwapBuffersLastError UMA

GetLastError returns the HRESULT of the last win32 API call and not the
last failed call so this UMA doesn't work as expected. Replace with
GPU.DirectComposition.SwapBuffersFailed boolean UMA that just counts
number of failures.

Bug:  837378 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: Idee2ad507316823d31f95405e4431287f9aef5ee
Reviewed-on: https://chromium-review.googlesource.com/1072692
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564752}
[modify] https://crrev.com/6e3b68c1fbd7bb2ff10bbc5c483a43977ced0946/gpu/ipc/service/direct_composition_surface_win.cc
[modify] https://crrev.com/6e3b68c1fbd7bb2ff10bbc5c483a43977ced0946/tools/metrics/histograms/histograms.xml

Status: Fixed (was: Started)
The CHECKs were replaced with error handling. There are still a bunch of DCHECKs for DXGI/DC calls that are guaranteed not to fail. Implementing error handling for all of them would be too cumbersome and unnecessary.

Sign in to add a comment