Harden CommandBufferProxyImpl

content/common/gpu/client/command_buffer_proxy_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/common/gpu/client/command_buffer_proxy_impl.h"
 
 #include <utility>
 #include <vector>
 
 #include "base/callback.h"
@@ skipping 65 matching lines @@
     IPC_MESSAGE_HANDLER(GpuCommandBufferMsg_ConsoleMsg, OnConsoleMessage);
     IPC_MESSAGE_HANDLER(GpuCommandBufferMsg_SignalAck,
                         OnSignalAck);
     IPC_MESSAGE_HANDLER(GpuCommandBufferMsg_SwapBuffersCompleted,
                         OnSwapBuffersCompleted);
     IPC_MESSAGE_HANDLER(GpuCommandBufferMsg_UpdateVSyncParameters,
                         OnUpdateVSyncParameters);
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
 
-  DCHECK(handled);
+  if (!handled) {
+    DLOG(ERROR) << "Gpu process sent invalid message.";
+    InvalidGpuMessage();
+  }
   return handled;
 }
 
 void CommandBufferProxyImpl::OnChannelError() {
   scoped_ptr<base::AutoLock> lock;
   if (lock_)
     lock.reset(new base::AutoLock(*lock_));
 
   gpu::error::ContextLostReason context_lost_reason =
       gpu::error::kGpuChannelLost;
@@ skipping 15 matching lines @@
     channel_->DestroyCommandBuffer(this);
     channel_ = nullptr;
   }
 
   // When the client sees that the context is lost, they should delete this
   // CommandBufferProxyImpl and create a new one.
   last_state_.error = error;
   last_state_.context_lost_reason = reason;
 
   if (!context_lost_callback_.is_null()) {
     context_lost_callback_.Run();

[vmiura, 2016/01/14 05:02:11]

Is it OK that we will run the context lost callback in a
WaitForToken*/WaitForGet* stack (potentially from a raster worker thread)?

[piman, 2016/01/14 18:27:58]

Good catch, no it's not OK, actually whether or not it's in the same thread.
Fixed in latest patch (run directly when receiving a message, post a task when
we would re-enter).

     // Avoid calling the error callback more than once.
     context_lost_callback_.Reset();
   }
 }
 
 void CommandBufferProxyImpl::OnConsoleMessage(
     const GPUCommandBufferConsoleMessage& message) {
   if (!console_message_callback_.is_null()) {
     console_message_callback_.Run(message.message, message.id);
   }
 }
 
 void CommandBufferProxyImpl::AddDeletionObserver(DeletionObserver* observer) {
   scoped_ptr<base::AutoLock> lock;
   if (lock_)
     lock.reset(new base::AutoLock(*lock_));
   deletion_observers_.AddObserver(observer);
 }
 
 void CommandBufferProxyImpl::RemoveDeletionObserver(
     DeletionObserver* observer) {
   scoped_ptr<base::AutoLock> lock;
   if (lock_)
     lock.reset(new base::AutoLock(*lock_));
   deletion_observers_.RemoveObserver(observer);
 }
 
 void CommandBufferProxyImpl::OnSignalAck(uint32_t id) {
   SignalTaskMap::iterator it = signal_tasks_.find(id);
-  DCHECK(it != signal_tasks_.end());
+  if (it == signal_tasks_.end()) {
+    DLOG(ERROR) << "Gpu process sent invalid SignalAck.";
+    InvalidGpuMessage();
+    return;
+  }
   base::Closure callback = it->second;
   signal_tasks_.erase(it);
   callback.Run();
 }
 
 void CommandBufferProxyImpl::SetContextLostCallback(
     const base::Closure& callback) {
   CheckLock();
   context_lost_callback_ = callback;
 }
@@ skipping 134 matching lines @@
                "end",
                end);
   TryUpdateState();
   if (!InRange(start, end, last_state_.token) &&
       last_state_.error == gpu::error::kNoError) {
     gpu::CommandBuffer::State state;
     if (Send(new GpuCommandBufferMsg_WaitForTokenInRange(
             route_id_, start, end, &state)))
       OnUpdateState(state);
   }
-  DCHECK(InRange(start, end, last_state_.token) ||
-         last_state_.error != gpu::error::kNoError);
+  if (!InRange(start, end, last_state_.token) &&
+      last_state_.error == gpu::error::kNoError) {
+    DLOG(ERROR) << "GPU state invalid after WaitForTokenInRange.";
+    InvalidGpuMessage();
+  }
 }
 
 void CommandBufferProxyImpl::WaitForGetOffsetInRange(int32_t start,
                                                      int32_t end) {
   CheckLock();
   TRACE_EVENT2("gpu",
                "CommandBufferProxyImpl::WaitForGetOffset",
                "start",
                start,
                "end",
                end);
   TryUpdateState();
   if (!InRange(start, end, last_state_.get_offset) &&
       last_state_.error == gpu::error::kNoError) {
     gpu::CommandBuffer::State state;
     if (Send(new GpuCommandBufferMsg_WaitForGetOffsetInRange(
             route_id_, start, end, &state)))
       OnUpdateState(state);
   }
-  DCHECK(InRange(start, end, last_state_.get_offset) ||
-         last_state_.error != gpu::error::kNoError);
+  if (!InRange(start, end, last_state_.get_offset) &&
+      last_state_.error == gpu::error::kNoError) {
+    DLOG(ERROR) << "GPU state invalid after WaitForGetOffsetInRange.";
+    InvalidGpuMessage();
+  }
 }
 
 void CommandBufferProxyImpl::SetGetBuffer(int32_t shm_id) {
   CheckLock();
   if (last_state_.error != gpu::error::kNoError)
     return;
 
   Send(new GpuCommandBufferMsg_SetGetBuffer(route_id_, shm_id));
   last_put_offset_ = -1;
 }
@@ skipping 431 matching lines @@
     swap_buffers_completion_callback_.Run(latency_info, result);
   }
 }
 
 void CommandBufferProxyImpl::OnUpdateVSyncParameters(base::TimeTicks timebase,
                                                      base::TimeDelta interval) {
   if (!update_vsync_parameters_completion_callback_.is_null())
     update_vsync_parameters_completion_callback_.Run(timebase, interval);
 }
 
+void CommandBufferProxyImpl::InvalidGpuMessage() {
+  LOG(ERROR) << "Received invalid message or reply from the GPU process.";
+  OnDestroyed(gpu::error::kInvalidGpuMessage, gpu::error::kLostContext);
+}
+
 }  // namespace content