Added Destroy() functions for GPUTimer and GPUTracer/GPUTrace.

BUG=467202
TEST=Shut down during gpu.device trace has no warnings.

Review URL: https://codereview.chromium.org/1018773002

Cr-Commit-Position: refs/heads/master@{#322304}

gpu/command_buffer/service/gles2_cmd_decoder.cc

@@ -3755,6 +3755,10 @@ void GLES2DecoderImpl::Destroy(bool have_context) {
   // by the context group.
   async_pixel_transfer_manager_.reset();
 
+  // Destroy the GPU Tracer which may own some in process GPU Timings.
+  gpu_tracer_->Destroy(have_context);
+  gpu_tracer_ = NULL;
+
   if (group_.get()) {
     framebuffer_manager()->RemoveObserver(this);
     group_->Destroy(this, have_context);

gpu/command_buffer/service/gpu_tracer.cc

@@ -99,6 +99,12 @@ GPUTrace::GPUTrace(scoped_refptr<Outputter> outputter,
 GPUTrace::~GPUTrace() {
 }
 
+void GPUTrace::Destroy(bool have_context) {
+  if (gpu_timer_.get()) {
+    gpu_timer_->Destroy(have_context);
+  }
+}
+
 void GPUTrace::Start(bool trace_service) {
   if (trace_service) {
     outputter_->TraceServiceBegin(category_, name_);
@@ -152,6 +158,20 @@ GPUTracer::GPUTracer(gles2::GLES2Decoder* decoder)
 GPUTracer::~GPUTracer() {
 }
 
+void GPUTracer::Destroy(bool have_context) {
+  for (int n = 0; n < NUM_TRACER_SOURCES; n++) {
+    for (size_t i = 0; i < markers_[n].size(); i++) {
+      TraceMarker& marker = markers_[n][i];
+      if (marker.trace_.get()) {
+        marker.trace_->Destroy(have_context);
+        marker.trace_ = 0;
+      }
+    }
+  }
+
+  ClearFinishedTraces(have_context);
+}
+
 bool GPUTracer::BeginDecoding() {
   if (gpu_executing_)
     return false;
@@ -194,10 +214,9 @@ bool GPUTracer::EndDecoding() {
         TraceMarker& marker = markers_[n][i];
         if (marker.trace_.get()) {
           marker.trace_->End(*gpu_trace_srv_category != 0);
-          if (marker.trace_->IsEnabled())
-            traces_.push_back(marker.trace_);
 
-          markers_[n][i].trace_ = 0;
+          finished_traces_.push_back(marker.trace_);
+          marker.trace_ = 0;
         }
       }
     }
@@ -241,14 +260,14 @@ bool GPUTracer::End(GpuTracerSource source) {
 
   // Pop last marker with matching 'source'
   if (!markers_[source].empty()) {
-    if (IsTracing()) {
-      scoped_refptr<GPUTrace> trace = markers_[source].back().trace_;
-      if (trace.get()) {
+    scoped_refptr<GPUTrace> trace = markers_[source].back().trace_;
+    if (trace.get()) {
+      if (IsTracing()) {
         trace->End(*gpu_trace_srv_category != 0);
-        if (trace->IsEnabled())
-          traces_.push_back(trace);
-        IssueProcessTask();
       }
+
+      finished_traces_.push_back(trace);
+      IssueProcessTask();
     }
 
     markers_[source].pop_back();
@@ -298,7 +317,7 @@ void GPUTracer::Process() {
 
 void GPUTracer::ProcessTraces() {
   if (!gpu_timing_client_->IsAvailable()) {
-    traces_.clear();
+    ClearFinishedTraces(false);
     return;
   }
 
@@ -307,28 +326,42 @@ void GPUTracer::ProcessTraces() {
   // Make owning decoder's GL context current
   if (!decoder_->MakeCurrent()) {
     // Skip subsequent GL calls if MakeCurrent fails
-    traces_.clear();
+    ClearFinishedTraces(false);
     return;
   }
 
   // Check if timers are still valid (e.g: a disjoint operation
   // might have occurred.)
-  if (gpu_timing_client_->CheckAndResetTimerErrors())
-    traces_.clear();
+  if (gpu_timing_client_->CheckAndResetTimerErrors()) {
+    ClearFinishedTraces(true);
+  }
 
-  while (!traces_.empty() && traces_.front()->IsAvailable()) {
-    traces_.front()->Process();
-    traces_.pop_front();
+  while (!finished_traces_.empty()) {
+    scoped_refptr<GPUTrace>& trace = finished_traces_.front();
+    if (trace->IsEnabled()) {
+      if (!finished_traces_.front()->IsAvailable())
+        break;
+      finished_traces_.front()->Process();
+    }
+    finished_traces_.front()->Destroy(true);
+    finished_traces_.pop_front();
   }
 
   // Clear pending traces if there were are any errors
   GLenum err = glGetError();
   if (err != GL_NO_ERROR)
-    traces_.clear();
+    ClearFinishedTraces(true);
+}
+
+void GPUTracer::ClearFinishedTraces(bool have_context) {
+   while (!finished_traces_.empty()) {
+    finished_traces_.front()->Destroy(have_context);
+    finished_traces_.pop_front();
+   }
 }
 
 void GPUTracer::IssueProcessTask() {
-  if (traces_.empty() || process_posted_)
+  if (finished_traces_.empty() || process_posted_)
     return;
 
   process_posted_ = true;

gpu/command_buffer/service/gpu_tracer.h

@@ -56,6 +56,8 @@ class GPU_EXPORT GPUTracer
   explicit GPUTracer(gles2::GLES2Decoder* decoder);
   virtual ~GPUTracer();
 
+  void Destroy(bool have_context);
+
   // Scheduled processing in decoder begins.
   bool BeginDecoding();
 
@@ -83,13 +85,14 @@ class GPU_EXPORT GPUTracer
 
   void Process();
   void ProcessTraces();
+  void ClearFinishedTraces(bool have_context);
 
   void IssueProcessTask();
 
   scoped_refptr<gfx::GPUTimingClient> gpu_timing_client_;
   scoped_refptr<Outputter> outputter_;
   std::vector<TraceMarker> markers_[NUM_TRACER_SOURCES];
-  std::deque<scoped_refptr<GPUTrace> > traces_;
+  std::deque<scoped_refptr<GPUTrace> > finished_traces_;
 
   const unsigned char* gpu_trace_srv_category;
   const unsigned char* gpu_trace_dev_category;
@@ -155,6 +158,8 @@ class GPU_EXPORT GPUTrace
            const std::string& name,
            const bool enabled);
 
+  void Destroy(bool have_context);
+
   void Start(bool trace_service);
   void End(bool tracing_service);
   bool IsAvailable();

gpu/command_buffer/service/gpu_tracer_unittest.cc

@@ -228,6 +228,7 @@ class BaseGpuTest : public GpuServiceTest {
 
   void TearDown() override {
     outputter_ref_ = NULL;
+    gpu_timing_client_ = NULL;
 
     gl_fake_queries_.Reset();
     GpuServiceTest::TearDown();
@@ -374,6 +375,9 @@ class BaseGpuTraceTest : public BaseGpuTest {
     // Proces should output expected Trace results to MockOutputter
     trace->Process();
 
+    // Destroy trace after we are done.
+    trace->Destroy(true);
+
     outputter_ref_ = NULL;
   }
 };

gpu/perftests/measurements.cc

@@ -96,6 +96,9 @@ Measurement MeasurementTimers::GetAsMeasurement(const std::string& name) {
 }
 
 MeasurementTimers::~MeasurementTimers() {
+  if (gpu_timer_.get()) {
+    gpu_timer_->Destroy(true);
+  }
 }
 
 }  // namespace gpu

ui/gl/gpu_timing.cc

@@ -44,7 +44,16 @@ uint32_t GPUTiming::GetDisjointCount() {
 }
 
 GPUTimer::~GPUTimer() {
-  glDeleteQueries(2, queries_);
+  // Destroy() must be called before the destructor.
+  DCHECK(queries_[0] == 0);
+  DCHECK(queries_[1] == 0);
+}
+
+void GPUTimer::Destroy(bool have_context) {
+  if (have_context) {
+    glDeleteQueries(2, queries_);
+  }
+  memset(queries_, 0, sizeof(queries_));
 }
 
 void GPUTimer::Start() {

ui/gl/gpu_timing.h

@@ -76,6 +76,10 @@ class GL_EXPORT GPUTimer {
  public:
   ~GPUTimer();
 
+  // Destroy the timer object. This must be explicitly called before destroying
+  // this object.
+  void Destroy(bool have_context);
+
   void Start();
   void End();
   bool IsAvailable();