Commit 656883fd authored by Christopher Cameron's avatar Christopher Cameron Committed by Commit Bot

Add crash key for crashes in GLFence::Create

Also add a glFlush before calling GLFence::Create, as it should be
sufficient to trigger the crash.

Bug: 863817
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: I589ba525d771895d00cf4aebfea57ce62f8add3f
Reviewed-on: https://chromium-review.googlesource.com/1148682
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: default avatarKenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577723}
parent 6072bcf5
...@@ -4,8 +4,11 @@ ...@@ -4,8 +4,11 @@
#include "gpu/ipc/service/image_transport_surface_overlay_mac.h" #include "gpu/ipc/service/image_transport_surface_overlay_mac.h"
#include <sstream>
#include "base/bind.h" #include "base/bind.h"
#include "base/command_line.h" #include "base/command_line.h"
#include "base/debug/crash_logging.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "base/threading/thread_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h"
#include "base/trace_event/trace_event.h" #include "base/trace_event/trace_event.h"
...@@ -78,6 +81,15 @@ bool ImageTransportSurfaceOverlayMac::IsOffscreen() { ...@@ -78,6 +81,15 @@ bool ImageTransportSurfaceOverlayMac::IsOffscreen() {
void ImageTransportSurfaceOverlayMac::ApplyBackpressure() { void ImageTransportSurfaceOverlayMac::ApplyBackpressure() {
TRACE_EVENT0("gpu", "ImageTransportSurfaceOverlayMac::ApplyBackpressure"); TRACE_EVENT0("gpu", "ImageTransportSurfaceOverlayMac::ApplyBackpressure");
static auto* crash_key = base::debug::AllocateCrashKeyString(
"mac_swap", base::debug::CrashKeySize::Size64);
std::stringstream crash_info;
crash_info << "pixel_size:" << pixel_size_.ToString() << " ";
crash_info << "scale:" << scale_factor_ << " ";
crash_info << "has_tree:"
<< ca_layer_tree_coordinator_->HasPendingCARendererLayerTree();
base::debug::SetCrashKeyString(crash_key, crash_info.str());
gl::GLContext* current_context = gl::GLContext::GetCurrent(); gl::GLContext* current_context = gl::GLContext::GetCurrent();
// TODO(ccameron): Remove these CHECKs. // TODO(ccameron): Remove these CHECKs.
CHECK(current_context); CHECK(current_context);
...@@ -88,6 +100,8 @@ void ImageTransportSurfaceOverlayMac::ApplyBackpressure() { ...@@ -88,6 +100,8 @@ void ImageTransportSurfaceOverlayMac::ApplyBackpressure() {
uint64_t this_frame_fence = current_context->BackpressureFenceCreate(); uint64_t this_frame_fence = current_context->BackpressureFenceCreate();
current_context->BackpressureFenceWait(previous_frame_fence_); current_context->BackpressureFenceWait(previous_frame_fence_);
previous_frame_fence_ = this_frame_fence; previous_frame_fence_ = this_frame_fence;
base::debug::ClearCrashKeyString(crash_key);
} }
void ImageTransportSurfaceOverlayMac::BufferPresented( void ImageTransportSurfaceOverlayMac::BufferPresented(
......
...@@ -30,6 +30,9 @@ class ACCELERATED_WIDGET_MAC_EXPORT CALayerTreeCoordinator { ...@@ -30,6 +30,9 @@ class ACCELERATED_WIDGET_MAC_EXPORT CALayerTreeCoordinator {
// The CARendererLayerTree for the pending frame. This is used to construct // The CARendererLayerTree for the pending frame. This is used to construct
// the CALayer tree for the CoreAnimation renderer. // the CALayer tree for the CoreAnimation renderer.
CARendererLayerTree* GetPendingCARendererLayerTree(); CARendererLayerTree* GetPendingCARendererLayerTree();
bool HasPendingCARendererLayerTree() const {
return !!pending_ca_renderer_layer_tree_;
}
// Commit the pending frame's OpenGL backbuffer or CALayer tree to be // Commit the pending frame's OpenGL backbuffer or CALayer tree to be
// attached to the root CALayer. // attached to the root CALayer.
......
...@@ -8,8 +8,10 @@ ...@@ -8,8 +8,10 @@
#include <OpenGL/CGLTypes.h> #include <OpenGL/CGLTypes.h>
#include <memory> #include <memory>
#include <sstream>
#include <vector> #include <vector>
#include "base/debug/crash_logging.h"
#include "base/location.h" #include "base/location.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/message_loop/message_loop.h" #include "base/message_loop/message_loop.h"
...@@ -31,13 +33,6 @@ namespace { ...@@ -31,13 +33,6 @@ namespace {
bool g_support_renderer_switching; bool g_support_renderer_switching;
struct CGLRendererInfoObjDeleter {
void operator()(CGLRendererInfoObj* x) {
if (x)
CGLDestroyRendererInfo(*x);
}
};
} // namespace } // namespace
static CGLPixelFormatObj GetPixelFormat() { static CGLPixelFormatObj GetPixelFormat() {
...@@ -83,14 +78,7 @@ static CGLPixelFormatObj GetPixelFormat() { ...@@ -83,14 +78,7 @@ static CGLPixelFormatObj GetPixelFormat() {
} }
GLContextCGL::GLContextCGL(GLShareGroup* share_group) GLContextCGL::GLContextCGL(GLShareGroup* share_group)
: GLContextReal(share_group), : GLContextReal(share_group) {}
context_(nullptr),
gpu_preference_(PreferIntegratedGpu),
discrete_pixelformat_(nullptr),
screen_(-1),
renderer_id_(-1),
safe_to_force_gpu_switch_(true) {
}
bool GLContextCGL::Initialize(GLSurface* compatible_surface, bool GLContextCGL::Initialize(GLSurface* compatible_surface,
const GLContextAttribs& attribs) { const GLContextAttribs& attribs) {
...@@ -221,6 +209,7 @@ bool GLContextCGL::ForceGpuSwitchIfNeeded() { ...@@ -221,6 +209,7 @@ bool GLContextCGL::ForceGpuSwitchIfNeeded() {
} }
} }
renderer_id_ = renderer_id; renderer_id_ = renderer_id;
has_switched_gpus_ = true;
} }
} }
return true; return true;
...@@ -246,10 +235,37 @@ uint64_t GLContextCGL::BackpressureFenceCreate() { ...@@ -246,10 +235,37 @@ uint64_t GLContextCGL::BackpressureFenceCreate() {
// TODO(ccameron): Remove this CHECK. // TODO(ccameron): Remove this CHECK.
CHECK(CGLGetCurrentContext() == context_); CHECK(CGLGetCurrentContext() == context_);
CHECK(context_); CHECK(context_);
// Set a crash key before making any GL calls.
static auto* crash_key = base::debug::AllocateCrashKeyString(
"gl_context_cgl", base::debug::CrashKeySize::Size256);
std::stringstream crash_info;
crash_info << "fence:" << next_backpressure_fence_ << " ";
crash_info << "canswitch:" << g_support_renderer_switching << " ";
crash_info << "didswitch:" << has_switched_gpus_ << " ";
crash_info << "dgpu:" << !!discrete_pixelformat_ << " ";
base::debug::SetCrashKeyString(crash_key, crash_info.str());
// Query the CGL retain count of the context.
GLuint retain_count =
CGLGetContextRetainCount(static_cast<CGLContextObj>(context_));
crash_info << "cglret:" << retain_count << " ";
base::debug::SetCrashKeyString(crash_key, crash_info.str());
// Query for errors on the GL context.
GLenum error = glGetError();
crash_info << "glerr:" << error;
base::debug::SetCrashKeyString(crash_key, crash_info.str());
// This flush will trigger the crash.
glFlush();
if (gl::GLFence::IsSupported()) { if (gl::GLFence::IsSupported()) {
backpressure_fences_[next_backpressure_fence_] = GLFence::Create(); backpressure_fences_[next_backpressure_fence_] = GLFence::Create();
return next_backpressure_fence_++; return next_backpressure_fence_++;
} }
base::debug::ClearCrashKeyString(crash_key);
return kFinishFenceId; return kFinishFenceId;
} }
......
...@@ -46,19 +46,22 @@ class GL_EXPORT GLContextCGL : public GLContextReal { ...@@ -46,19 +46,22 @@ class GL_EXPORT GLContextCGL : public GLContextReal {
void Destroy(); void Destroy();
GpuPreference GetGpuPreference(); GpuPreference GetGpuPreference();
void* context_; void* context_ = nullptr;
GpuPreference gpu_preference_; GpuPreference gpu_preference_ = PreferIntegratedGpu;
std::map<gfx::ColorSpace, std::unique_ptr<YUVToRGBConverter>> std::map<gfx::ColorSpace, std::unique_ptr<YUVToRGBConverter>>
yuv_to_rgb_converters_; yuv_to_rgb_converters_;
std::map<uint64_t, std::unique_ptr<GLFence>> backpressure_fences_; std::map<uint64_t, std::unique_ptr<GLFence>> backpressure_fences_;
uint64_t next_backpressure_fence_ = 0; uint64_t next_backpressure_fence_ = 0;
CGLPixelFormatObj discrete_pixelformat_; CGLPixelFormatObj discrete_pixelformat_ = nullptr;
int screen_; int screen_ = -1;
int renderer_id_; int renderer_id_ = -1;
bool safe_to_force_gpu_switch_; bool safe_to_force_gpu_switch_ = true;
// Debugging for https://crbug.com/863817
bool has_switched_gpus_ = false;
DISALLOW_COPY_AND_ASSIGN(GLContextCGL); DISALLOW_COPY_AND_ASSIGN(GLContextCGL);
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment