Remove CommandBufferEngine

The only non-test implementation of CommandBufferEngine
(CommandExecutor) directly forwards to CommandBufferServiceBase, so
eliminate CommandBufferEngine and have CommonDecoder use
CommandBufferServiceBase directly.

Make CommandBufferServiceBase not derive from CommandBuffer (since
neither CommonDecoder nor CommandExecutor need it), and instead make the
relevant implementations (e.g. CommandBufferService) explicitly
implement CommandBuffer.

Rename the test class MockCommandBufferBase into
FakeCommandBufferServiceBase (since it's a fake, not a mock), and move
the CommandBuffer part of its implementation onto
MockClientCommandBuffer. Use it instead of various CommandBufferEngine
mocks/fakes.

Bug: 723770
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ia1386fa7f8617962739a36a397d508e61e4514e2
Reviewed-on: https://chromium-review.googlesource.com/508114
Commit-Queue: Antoine Labour <piman@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#473056}

gpu/command_buffer/client/client_test_helper.cc

@@ -20,49 +20,25 @@ using ::testing::Invoke;
 
 namespace gpu {
 
-MockCommandBufferBase::MockCommandBufferBase() : put_offset_(0) {}
+FakeCommandBufferServiceBase::FakeCommandBufferServiceBase() : put_offset_(0) {}
 
-MockCommandBufferBase::~MockCommandBufferBase() {}
+FakeCommandBufferServiceBase::~FakeCommandBufferServiceBase() {}
 
-CommandBuffer::State MockCommandBufferBase::GetLastState() {
+CommandBuffer::State FakeCommandBufferServiceBase::GetState() {
   return state_;
 }
 
-void MockCommandBufferBase::SetGetOffset(int32_t get_offset) {
+void FakeCommandBufferServiceBase::SetGetOffset(int32_t get_offset) {
   state_.get_offset = get_offset;
 }
 
-void MockCommandBufferBase::SetReleaseCount(uint64_t release_count) {
+void FakeCommandBufferServiceBase::SetReleaseCount(uint64_t release_count) {
   state_.release_count = release_count;
 }
 
-CommandBuffer::State MockCommandBufferBase::WaitForTokenInRange(int32_t start,
-                                                                int32_t end) {
-  return state_;
-}
-
-CommandBuffer::State MockCommandBufferBase::WaitForGetOffsetInRange(
-    uint32_t set_get_buffer_count,
-    int32_t start,
-    int32_t end) {
-  EXPECT_EQ(set_get_buffer_count, state_.set_get_buffer_count);
-  if (state_.get_offset != put_offset_) {
-    state_.get_offset = put_offset_;
-    OnFlush();
-  }
-  return state_;
-}
-
-void MockCommandBufferBase::SetGetBuffer(int transfer_buffer_id) {
-  ++state_.set_get_buffer_count;
-  state_.get_offset = 0;
-  put_offset_ = 0;
-  state_.token = 10000;  // All token checks in the tests should pass.
-}
-
 // Get's the Id of the next transfer buffer that will be returned
 // by CreateTransferBuffer. This is useful for testing expected ids.
-int32_t MockCommandBufferBase::GetNextFreeTransferBufferId() {
+int32_t FakeCommandBufferServiceBase::GetNextFreeTransferBufferId() {
   for (size_t ii = 0; ii < arraysize(transfer_buffer_buffers_); ++ii) {
     if (!transfer_buffer_buffers_[ii].get()) {
       return kTransferBufferBaseId + ii;
@@ -71,9 +47,16 @@ int32_t MockCommandBufferBase::GetNextFreeTransferBufferId() {
   return -1;
 }
 
-scoped_refptr<gpu::Buffer> MockCommandBufferBase::CreateTransferBuffer(
-    size_t size,
-    int32_t* id) {
+void FakeCommandBufferServiceBase::SetGetBufferHelper(int transfer_buffer_id) {
+  ++state_.set_get_buffer_count;
+  state_.get_offset = 0;
+  put_offset_ = 0;
+  state_.token = 10000;  // All token checks in the tests should pass.
+}
+
+scoped_refptr<gpu::Buffer>
+FakeCommandBufferServiceBase::CreateTransferBufferHelper(size_t size,
+                                                         int32_t* id) {
   *id = GetNextFreeTransferBufferId();
   if (*id >= 0) {
     int32_t ndx = *id - kTransferBufferBaseId;
@@ -85,44 +68,46 @@ scoped_refptr<gpu::Buffer> MockCommandBufferBase::CreateTransferBuffer(
   return GetTransferBuffer(*id);
 }
 
-void MockCommandBufferBase::DestroyTransferBufferHelper(int32_t id) {
+void FakeCommandBufferServiceBase::DestroyTransferBufferHelper(int32_t id) {
   DCHECK_GE(id, kTransferBufferBaseId);
   DCHECK_LT(id, kTransferBufferBaseId + kMaxTransferBuffers);
   id -= kTransferBufferBaseId;
   transfer_buffer_buffers_[id] = NULL;
 }
 
-scoped_refptr<Buffer> MockCommandBufferBase::GetTransferBuffer(int32_t id) {
-  DCHECK_GE(id, kTransferBufferBaseId);
-  DCHECK_LT(id, kTransferBufferBaseId + kMaxTransferBuffers);
+scoped_refptr<Buffer> FakeCommandBufferServiceBase::GetTransferBuffer(
+    int32_t id) {
+  if ((id < kTransferBufferBaseId) ||
+      (id >= kTransferBufferBaseId + kMaxTransferBuffers))
+    return nullptr;
   return transfer_buffer_buffers_[id - kTransferBufferBaseId];
 }
 
-void MockCommandBufferBase::FlushHelper(int32_t put_offset) {
+void FakeCommandBufferServiceBase::FlushHelper(int32_t put_offset) {
   put_offset_ = put_offset;
 }
 
-void MockCommandBufferBase::SetToken(int32_t token) {
+void FakeCommandBufferServiceBase::SetToken(int32_t token) {
   state_.token = token;
 }
 
-void MockCommandBufferBase::SetParseError(error::Error error) {
+void FakeCommandBufferServiceBase::SetParseError(error::Error error) {
   state_.error = error;
 }
 
-void MockCommandBufferBase::SetContextLostReason(
+void FakeCommandBufferServiceBase::SetContextLostReason(
     error::ContextLostReason reason) {
   state_.context_lost_reason = reason;
 }
 
-int32_t MockCommandBufferBase::GetPutOffset() {
+int32_t FakeCommandBufferServiceBase::GetPutOffset() {
   return put_offset_;
 }
 
 // GCC requires these declarations, but MSVC requires they not be present
 #ifndef _MSC_VER
-const int32_t MockCommandBufferBase::kTransferBufferBaseId;
-const int32_t MockCommandBufferBase::kMaxTransferBuffers;
+const int32_t FakeCommandBufferServiceBase::kTransferBufferBaseId;
+const int32_t FakeCommandBufferServiceBase::kMaxTransferBuffers;
 #endif
 
 MockClientCommandBuffer::MockClientCommandBuffer() {
@@ -131,6 +116,39 @@ MockClientCommandBuffer::MockClientCommandBuffer() {
 
 MockClientCommandBuffer::~MockClientCommandBuffer() {}
 
+CommandBuffer::State MockClientCommandBuffer::GetLastState() {
+  return GetState();
+}
+
+CommandBuffer::State MockClientCommandBuffer::WaitForTokenInRange(int32_t start,
+                                                                  int32_t end) {
+  return GetState();
+}
+
+CommandBuffer::State MockClientCommandBuffer::WaitForGetOffsetInRange(
+    uint32_t set_get_buffer_count,
+    int32_t start,
+    int32_t end) {
+  State state = GetState();
+  EXPECT_EQ(set_get_buffer_count, state.set_get_buffer_count);
+  if (state.get_offset != GetPutOffset()) {
+    SetGetOffset(GetPutOffset());
+    OnFlush();
+    state = GetState();
+  }
+  return state;
+}
+
+void MockClientCommandBuffer::SetGetBuffer(int transfer_buffer_id) {
+  SetGetBufferHelper(transfer_buffer_id);
+}
+
+scoped_refptr<gpu::Buffer> MockClientCommandBuffer::CreateTransferBuffer(
+    size_t size,
+    int32_t* id) {
+  return CreateTransferBufferHelper(size, id);
+}
+
 void MockClientCommandBuffer::Flush(int32_t put_offset) {
   FlushHelper(put_offset);
 }
@@ -141,8 +159,8 @@ void MockClientCommandBuffer::OrderingBarrier(int32_t put_offset) {
 
 void MockClientCommandBuffer::DelegateToFake() {
   ON_CALL(*this, DestroyTransferBuffer(_))
-      .WillByDefault(
-          Invoke(this, &MockCommandBufferBase::DestroyTransferBufferHelper));
+      .WillByDefault(Invoke(
+          this, &FakeCommandBufferServiceBase::DestroyTransferBufferHelper));
 }
 
 MockClientCommandBufferMockFlush::MockClientCommandBufferMockFlush() {
@@ -154,7 +172,7 @@ MockClientCommandBufferMockFlush::~MockClientCommandBufferMockFlush() {}
 void MockClientCommandBufferMockFlush::DelegateToFake() {
   MockClientCommandBuffer::DelegateToFake();
   ON_CALL(*this, Flush(_))
-      .WillByDefault(Invoke(this, &MockCommandBufferBase::FlushHelper));
+      .WillByDefault(Invoke(this, &FakeCommandBufferServiceBase::FlushHelper));
 }
 
 MockClientGpuControl::MockClientGpuControl() {}

gpu/command_buffer/client/client_test_helper.h

@@ -25,24 +25,17 @@
 
 namespace gpu {
 
-class MockCommandBufferBase : public CommandBufferServiceBase {
+class FakeCommandBufferServiceBase : public CommandBufferServiceBase {
  public:
   static const int32_t kTransferBufferBaseId = 0x123;
   static const int32_t kMaxTransferBuffers = 32;
 
-  MockCommandBufferBase();
-  ~MockCommandBufferBase() override;
+  FakeCommandBufferServiceBase();
+  ~FakeCommandBufferServiceBase() override;
 
-  State GetLastState() override;
-  State WaitForTokenInRange(int32_t start, int32_t end) override;
-  State WaitForGetOffsetInRange(uint32_t set_get_buffer_count,
-                                int32_t start,
-                                int32_t end) override;
-  void SetGetBuffer(int transfer_buffer_id) override;
+  CommandBuffer::State GetState() override;
   void SetGetOffset(int32_t get_offset) override;
   void SetReleaseCount(uint64_t release_count) override;
-  scoped_refptr<gpu::Buffer> CreateTransferBuffer(size_t size,
-                                                  int32_t* id) override;
   scoped_refptr<gpu::Buffer> GetTransferBuffer(int32_t id) override;
   void SetToken(int32_t token) override;
   void SetParseError(error::Error error) override;
@@ -54,21 +47,32 @@ class MockCommandBufferBase : public CommandBufferServiceBase {
   int32_t GetNextFreeTransferBufferId();
 
   void FlushHelper(int32_t put_offset);
+  void SetGetBufferHelper(int transfer_buffer_id);
+  scoped_refptr<gpu::Buffer> CreateTransferBufferHelper(size_t size,
+                                                        int32_t* id);
   void DestroyTransferBufferHelper(int32_t id);
 
-  virtual void OnFlush() = 0;
-
  private:
   scoped_refptr<Buffer> transfer_buffer_buffers_[kMaxTransferBuffers];
-  State state_;
+  CommandBuffer::State state_;
   int32_t put_offset_;
 };
 
-class MockClientCommandBuffer : public MockCommandBufferBase {
+class MockClientCommandBuffer : public CommandBuffer,
+                                public FakeCommandBufferServiceBase {
  public:
   MockClientCommandBuffer();
   ~MockClientCommandBuffer() override;
 
+  State GetLastState() override;
+  State WaitForTokenInRange(int32_t start, int32_t end) override;
+  State WaitForGetOffsetInRange(uint32_t set_get_buffer_count,
+                                int32_t start,
+                                int32_t end) override;
+  void SetGetBuffer(int transfer_buffer_id) override;
+  scoped_refptr<gpu::Buffer> CreateTransferBuffer(size_t size,
+                                                  int32_t* id) override;
+
   // This is so we can use all the gmock functions when Flush is called.
   MOCK_METHOD0(OnFlush, void());
   MOCK_METHOD1(DestroyTransferBuffer, void(int32_t id));

gpu/command_buffer/client/cmd_buffer_helper_test.cc

@@ -92,8 +92,8 @@ class CommandBufferServiceLocked : public CommandBufferService {
 };
 
 // Test fixture for CommandBufferHelper test - Creates a CommandBufferHelper,
-// using a CommandBufferEngine with a mock AsyncAPIInterface for its interface
-// (calling it directly, not through the RPC mechanism).
+// using a CommandBufferServiceLocked with a mock AsyncAPIInterface for its
+// interface (calling it directly, not through the RPC mechanism).
 class CommandBufferHelperTest : public testing::Test {
  protected:
   virtual void SetUp() {
@@ -115,7 +115,7 @@ class CommandBufferHelperTest : public testing::Test {
     command_buffer_->SetGetBufferChangeCallback(base::Bind(
         &CommandExecutor::SetGetBuffer, base::Unretained(executor_.get())));
 
-    api_mock_->set_engine(executor_.get());
+    api_mock_->set_command_buffer_service(command_buffer_.get());
 
     helper_.reset(new CommandBufferHelper(command_buffer_.get()));
     helper_->Initialize(kCommandBufferSizeBytes);

gpu/command_buffer/client/fenced_allocator_test.cc

@@ -16,7 +16,6 @@
 #include "base/run_loop.h"
 #include "gpu/command_buffer/client/cmd_buffer_helper.h"
 #include "gpu/command_buffer/client/fenced_allocator.h"
-#include "gpu/command_buffer/service/cmd_buffer_engine.h"
 #include "gpu/command_buffer/service/command_buffer_service.h"
 #include "gpu/command_buffer/service/command_executor.h"
 #include "gpu/command_buffer/service/mocks.h"
@@ -61,7 +60,7 @@ class BaseFencedAllocatorTest : public testing::Test {
     command_buffer_->SetGetBufferChangeCallback(base::Bind(
         &CommandExecutor::SetGetBuffer, base::Unretained(executor_.get())));
 
-    api_mock_->set_engine(executor_.get());
+    api_mock_->set_command_buffer_service(command_buffer_.get());
 
     helper_.reset(new CommandBufferHelper(command_buffer_.get()));
     helper_->Initialize(kBufferSize);

gpu/command_buffer/client/mapped_memory_unittest.cc

@@ -57,7 +57,7 @@ class MappedMemoryTestBase : public testing::Test {
     command_buffer_->SetGetBufferChangeCallback(base::Bind(
         &CommandExecutor::SetGetBuffer, base::Unretained(executor_.get())));
 
-    api_mock_->set_engine(executor_.get());
+    api_mock_->set_command_buffer_service(command_buffer_.get());
 
     helper_.reset(new CommandBufferHelper(command_buffer_.get()));
     helper_->Initialize(kBufferSize);

gpu/command_buffer/client/ring_buffer_test.cc

@@ -16,7 +16,6 @@
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "gpu/command_buffer/client/cmd_buffer_helper.h"
-#include "gpu/command_buffer/service/cmd_buffer_engine.h"
 #include "gpu/command_buffer/service/command_buffer_service.h"
 #include "gpu/command_buffer/service/command_executor.h"
 #include "gpu/command_buffer/service/mocks.h"
@@ -83,7 +82,7 @@ class BaseRingBufferTest : public testing::Test {
     command_buffer_->SetGetBufferChangeCallback(base::Bind(
         &CommandExecutor::SetGetBuffer, base::Unretained(executor_.get())));
 
-    api_mock_->set_engine(executor_.get());
+    api_mock_->set_command_buffer_service(command_buffer_.get());
 
     helper_.reset(new CommandBufferHelper(command_buffer_.get()));
     helper_->Initialize(kBufferSize);

gpu/command_buffer/client/transfer_buffer_unittest.cc

@@ -235,7 +235,7 @@ class MockClientCommandBufferCanFail : public MockClientCommandBufferMockFlush {
 
   scoped_refptr<gpu::Buffer> RealCreateTransferBuffer(size_t size,
                                                       int32_t* id) {
-    return MockCommandBufferBase::CreateTransferBuffer(size, id);
+    return MockClientCommandBufferMockFlush::CreateTransferBuffer(size, id);
   }
 };
 

gpu/command_buffer/service/BUILD.gn

@@ -33,7 +33,6 @@ target(link_target_type, "service_sources") {
     "buffer_manager.cc",
     "buffer_manager.h",
     "client_service_map.h",
-    "cmd_buffer_engine.h",
     "command_buffer_service.cc",
     "command_buffer_service.h",
     "command_executor.cc",