Replace command buffer FlushSync with WaitForTokenInRange and WaitFor…

…GetOffsetInRange

This allows the command-buffer interface to be more clear about what's happening, so we'll later be able to create IPCs that wait specifically for one of these events to happen. Currently both function are still implemented by looping on the GetStateFast IPC.

BUG=349632

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@257771 0039d316-1c4b-4281-b951-d872f2087c98

content/common/gpu/client/command_buffer_proxy_impl.cc

@@ -218,25 +218,38 @@ void CommandBufferProxyImpl::SetLatencyInfo(
   Send(new GpuCommandBufferMsg_SetLatencyInfo(route_id_, latency_info));
 }
 
-gpu::CommandBuffer::State CommandBufferProxyImpl::FlushSync(
-    int32 put_offset,
-    int32 last_known_get) {
-  TRACE_EVENT1("gpu", "CommandBufferProxyImpl::FlushSync", "put_offset",
-               put_offset);
-  Flush(put_offset);
+void CommandBufferProxyImpl::WaitForTokenInRange(int32 start, int32 end) {
+  TRACE_EVENT2("gpu",
+               "CommandBufferProxyImpl::WaitForToken",
+               "start",
+               start,
+               "end",
+               end);
   TryUpdateState();
-  if (last_known_get == last_state_.get_offset) {
-    // Send will flag state with lost context if IPC fails.
-    if (last_state_.error == gpu::error::kNoError) {
-      gpu::CommandBuffer::State state;
-      if (Send(new GpuCommandBufferMsg_GetStateFast(route_id_,
-                                                    &state)))
-        OnUpdateState(state);
-    }
+  while (!InRange(start, end, last_state_.token) &&
+         last_state_.error == gpu::error::kNoError) {
+    gpu::CommandBuffer::State state;
+    if (Send(new GpuCommandBufferMsg_GetStateFast(route_id_, &state)))
+      OnUpdateState(state);
     TryUpdateState();
   }
+}
 
-  return last_state_;
+void CommandBufferProxyImpl::WaitForGetOffsetInRange(int32 start, int32 end) {
+  TRACE_EVENT2("gpu",
+               "CommandBufferProxyImpl::WaitForGetOffset",
+               "start",
+               start,
+               "end",
+               end);
+  TryUpdateState();
+  while (!InRange(start, end, last_state_.get_offset) &&
+         last_state_.error == gpu::error::kNoError) {
+    gpu::CommandBuffer::State state;
+    if (Send(new GpuCommandBufferMsg_GetStateFast(route_id_, &state)))
+      OnUpdateState(state);
+    TryUpdateState();
+  }
 }
 
 void CommandBufferProxyImpl::SetGetBuffer(int32 shm_id) {

content/common/gpu/client/command_buffer_proxy_impl.h

@@ -82,7 +82,8 @@ class CommandBufferProxyImpl
   virtual State GetLastState() OVERRIDE;
   virtual int32 GetLastToken() OVERRIDE;
   virtual void Flush(int32 put_offset) OVERRIDE;
-  virtual State FlushSync(int32 put_offset, int32 last_known_get) OVERRIDE;
+  virtual void WaitForTokenInRange(int32 start, int32 end) OVERRIDE;
+  virtual void WaitForGetOffsetInRange(int32 start, int32 end) OVERRIDE;
   virtual void SetGetBuffer(int32 shm_id) OVERRIDE;
   virtual void SetGetOffset(int32 get_offset) OVERRIDE;
   virtual gpu::Buffer CreateTransferBuffer(size_t size,

content/renderer/pepper/ppb_graphics_3d_impl.cc

@@ -136,15 +136,18 @@ PP_Bool PPB_Graphics3D_Impl::Flush(int32_t put_offset) {
   return PP_TRUE;
 }
 
-gpu::CommandBuffer::State PPB_Graphics3D_Impl::FlushSync(int32_t put_offset) {
-  gpu::CommandBuffer::State state = GetCommandBuffer()->GetState();
-  return GetCommandBuffer()->FlushSync(put_offset, state.get_offset);
+gpu::CommandBuffer::State PPB_Graphics3D_Impl::WaitForTokenInRange(
+    int32_t start,
+    int32_t end) {
+  GetCommandBuffer()->WaitForTokenInRange(start, end);
+  return GetCommandBuffer()->GetLastState();
 }
 
-gpu::CommandBuffer::State PPB_Graphics3D_Impl::FlushSyncFast(
-    int32_t put_offset,
-    int32_t last_known_get) {
-  return GetCommandBuffer()->FlushSync(put_offset, last_known_get);
+gpu::CommandBuffer::State PPB_Graphics3D_Impl::WaitForGetOffsetInRange(
+    int32_t start,
+    int32_t end) {
+  GetCommandBuffer()->WaitForGetOffsetInRange(start, end);
+  return GetCommandBuffer()->GetLastState();
 }
 
 uint32_t PPB_Graphics3D_Impl::InsertSyncPoint() {

content/renderer/pepper/ppb_graphics_3d_impl.h

@@ -30,10 +30,11 @@ class PPB_Graphics3D_Impl : public ppapi::PPB_Graphics3D_Shared {
                                     int* shm_handle,
                                     uint32_t* shm_size) OVERRIDE;
   virtual PP_Bool Flush(int32_t put_offset) OVERRIDE;
-  virtual gpu::CommandBuffer::State FlushSync(int32_t put_offset) OVERRIDE;
-  virtual gpu::CommandBuffer::State FlushSyncFast(
-      int32_t put_offset,
-      int32_t last_known_get) OVERRIDE;
+  virtual gpu::CommandBuffer::State WaitForTokenInRange(int32_t start,
+                                                        int32_t end) OVERRIDE;
+  virtual gpu::CommandBuffer::State WaitForGetOffsetInRange(int32_t start,
+                                                            int32_t end)
+      OVERRIDE;
   virtual uint32_t InsertSyncPoint() OVERRIDE;
 
   // Binds/unbinds the graphics of this context with the associated instance.

gpu/command_buffer/client/client_test_helper.cc

@@ -38,12 +38,11 @@ void MockCommandBufferBase::SetGetOffset(int32 get_offset) {
   state_.get_offset = get_offset;
 }
 
-CommandBuffer::State MockCommandBufferBase::FlushSync(
-    int32 put_offset, int32 last_known_get) {
-  state_.put_offset = put_offset;
-  state_.get_offset = put_offset;
+void MockCommandBufferBase::WaitForTokenInRange(int32 start, int32 end) {}
+
+void MockCommandBufferBase::WaitForGetOffsetInRange(int32 start, int32 end) {
+  state_.get_offset = state_.put_offset;
   OnFlush();
-  return state_;
 }
 
 void MockCommandBufferBase::SetGetBuffer(int transfer_buffer_id) {

gpu/command_buffer/client/client_test_helper.h

@@ -32,7 +32,8 @@ class MockCommandBufferBase : public CommandBuffer {
   virtual State GetState() OVERRIDE;
   virtual State GetLastState() OVERRIDE;
   virtual int32 GetLastToken() OVERRIDE;
-  virtual State FlushSync(int32 put_offset, int32 last_known_get) OVERRIDE;
+  virtual void WaitForTokenInRange(int32 start, int32 end) OVERRIDE;
+  virtual void WaitForGetOffsetInRange(int32 start, int32 end) OVERRIDE;
   virtual void SetGetBuffer(int transfer_buffer_id) OVERRIDE;
   virtual void SetGetOffset(int32 get_offset) OVERRIDE;
   virtual Buffer CreateTransferBuffer(size_t size, int32* id) OVERRIDE;

gpu/command_buffer/client/cmd_buffer_helper.cc

@@ -145,21 +145,12 @@ CommandBufferHelper::~CommandBufferHelper() {
   FreeResources();
 }
 
-bool CommandBufferHelper::FlushSync() {
+bool CommandBufferHelper::WaitForGetOffsetInRange(int32 start, int32 end) {
   if (!usable()) {
     return false;
   }
-
-  // Wrap put_ before flush.
-  if (put_ == total_entry_count_)
-    put_ = 0;
-
-  last_flush_time_ = clock();
-  last_put_sent_ = put_;
-  CommandBuffer::State state = command_buffer_->FlushSync(put_, get_offset());
-  ++flush_generation_;
-  CalcImmediateEntries(0);
-  return state.error == error::kNoError;
+  command_buffer_->WaitForGetOffsetInRange(start, end);
+  return command_buffer_->GetLastError() == gpu::error::kNoError;
 }
 
 void CommandBufferHelper::Flush() {
@@ -196,12 +187,12 @@ bool CommandBufferHelper::Finish() {
     return true;
   }
   DCHECK(HaveRingBuffer());
-  do {
-    // Do not loop forever if the flush fails, meaning the command buffer reader
-    // has shutdown.
-    if (!FlushSync())
-      return false;
-  } while (put_ != get_offset());
+  Flush();
+  if (!WaitForGetOffsetInRange(put_, put_))
+    return false;
+  DCHECK_EQ(get_offset(), put_);
+
+  CalcImmediateEntries(0);
 
   return true;
 }
@@ -242,16 +233,10 @@ void CommandBufferHelper::WaitForToken(int32 token) {
   if (token < 0)
     return;
   if (token > token_) return;  // we wrapped
-  while (last_token_read() < token) {
-    if (get_offset() == put_) {
-      LOG(FATAL) << "Empty command buffer while waiting on a token.";
-      return;
-    }
-    // Do not loop forever if the flush fails, meaning the command buffer reader
-    // has shutdown.
-    if (!FlushSync())
-      return;
-  }
+  if (last_token_read() > token)
+    return;
+  Flush();
+  command_buffer_->WaitForTokenInRange(token, token_);
 }
 
 // Waits for available entries, basically waiting until get >= put + count + 1.
@@ -275,13 +260,12 @@ void CommandBufferHelper::WaitForAvailableEntries(int32 count) {
     int32 curr_get = get_offset();
     if (curr_get > put_ || curr_get == 0) {
       TRACE_EVENT0("gpu", "CommandBufferHelper::WaitForAvailableEntries");
-      while (curr_get > put_ || curr_get == 0) {
-        // Do not loop forever if the flush fails, meaning the command buffer
-        // reader has shutdown.
-        if (!FlushSync())
-          return;
-        curr_get = get_offset();
-      }
+      Flush();
+      if (!WaitForGetOffsetInRange(1, put_))
+        return;
+      curr_get = get_offset();
+      DCHECK_LE(curr_get, put_);
+      DCHECK_NE(0, curr_get);
     }
     // Insert Noops to fill out the buffer.
     int32 num_entries = total_entry_count_ - put_;
@@ -303,13 +287,10 @@ void CommandBufferHelper::WaitForAvailableEntries(int32 count) {
     if (immediate_entry_count_ < count) {
       // Buffer is full.  Need to wait for entries.
       TRACE_EVENT0("gpu", "CommandBufferHelper::WaitForAvailableEntries1");
-      while (immediate_entry_count_ < count) {
-        // Do not loop forever if the flush fails, meaning the command buffer
-        // reader has shutdown.
-        if (!FlushSync())
-          return;
-        CalcImmediateEntries(count);
-      }
+      if (!WaitForGetOffsetInRange(put_ + count + 1, put_))
+        return;
+      CalcImmediateEntries(count);
+      DCHECK_GE(immediate_entry_count_, count);
     }
   }
 }

gpu/command_buffer/client/cmd_buffer_helper.h

@@ -64,14 +64,6 @@ class GPU_EXPORT CommandBufferHelper {
   // returns, the command buffer service is aware of all pending commands.
   void Flush();
 
-  // Flushes the commands, setting the put pointer to let the buffer interface
-  // know that new commands have been added. After a flush returns, the command
-  // buffer service is aware of all pending commands and it is guaranteed to
-  // have made some progress in processing them. Returns whether the flush was
-  // successful. The flush will fail if the command buffer service has
-  // disconnected.
-  bool FlushSync();
-
   // Waits until all the commands have been executed. Returns whether it
   // was successful. The function will fail if the command buffer service has
   // disconnected.
@@ -274,9 +266,6 @@ class GPU_EXPORT CommandBufferHelper {
   }
 
  private:
-  // Waits until get changes, updating the value of get_.
-  void WaitForGetChange();
-
   // Returns the number of available entries (they may not be contiguous).
   int32 AvailableEntries() {
     return (get_offset() - put_ - 1 + total_entry_count_) % total_entry_count_;
@@ -286,6 +275,10 @@ class GPU_EXPORT CommandBufferHelper {
   bool AllocateRingBuffer();
   void FreeResources();
 
+  // Waits for the get offset to be in a specific range, inclusive. Returns
+  // false if there was an error.
+  bool WaitForGetOffsetInRange(int32 start, int32 end);
+
 #if defined(CMD_HELPER_PERIODIC_FLUSH_CHECK)
   // Calls Flush if automatic flush conditions are met.
   void PeriodicFlushCheck();

gpu/command_buffer/client/cmd_buffer_helper_test.cc

@@ -43,20 +43,34 @@ class CommandBufferServiceLocked : public CommandBufferService {
   explicit CommandBufferServiceLocked(
       TransferBufferManagerInterface* transfer_buffer_manager)
       : CommandBufferService(transfer_buffer_manager),
-        flush_locked_(false) {}
+        flush_locked_(false),
+        last_flush_(-1) {}
   virtual ~CommandBufferServiceLocked() {}
 
   virtual void Flush(int32 put_offset) OVERRIDE {
-    if (!flush_locked_)
+    if (!flush_locked_) {
+      last_flush_ = -1;
       CommandBufferService::Flush(put_offset);
+    } else {
+      last_flush_ = put_offset;
+    }
   }
 
   void LockFlush() { flush_locked_ = true; }
 
   void UnlockFlush() { flush_locked_ = false; }
 
+  virtual void WaitForGetOffsetInRange(int32 start, int32 end) OVERRIDE {
+    if (last_flush_ != -1) {
+      CommandBufferService::Flush(last_flush_);
+      last_flush_ = -1;
+    }
+    CommandBufferService::WaitForGetOffsetInRange(start, end);
+  }
+
  private:
   bool flush_locked_;
+  int last_flush_;
   DISALLOW_COPY_AND_ASSIGN(CommandBufferServiceLocked);
 };
 
@@ -630,15 +644,6 @@ TEST_F(CommandBufferHelperTest, TestFlushGeneration) {
   EXPECT_EQ(gen2, gen1);
   EXPECT_NE(gen3, gen2);
 
-  // Generation should change after FlushSync() but not before.
-  gen1 = GetHelperFlushGeneration();
-  AddUniqueCommandWithExpect(error::kNoError, 2);
-  gen2 = GetHelperFlushGeneration();
-  helper_->FlushSync();
-  gen3 = GetHelperFlushGeneration();
-  EXPECT_EQ(gen2, gen1);
-  EXPECT_NE(gen3, gen2);
-
   // Generation should change after Finish() but not before.
   gen1 = GetHelperFlushGeneration();
   AddUniqueCommandWithExpect(error::kNoError, 2);

gpu/command_buffer/common/command_buffer.h

@@ -70,6 +70,15 @@ class GPU_EXPORT CommandBuffer {
   virtual ~CommandBuffer() {
   }
 
+  // Check if a value is between a start and end value, inclusive, allowing
+  // for wrapping if start > end.
+  static bool InRange(int32 start, int32 end, int32 value) {
+    if (start <= end)
+      return start <= value && value <= end;
+    else
+      return start <= value || value <= end;
+  }
+
   // Initialize the command buffer with the given size.
   virtual bool Initialize() = 0;
 
@@ -92,10 +101,13 @@ class GPU_EXPORT CommandBuffer {
   // subsequent Flushes on the same GpuChannel.
   virtual void Flush(int32 put_offset) = 0;
 
-  // The writer calls this to update its put offset. This function returns the
-  // reader's most recent get offset. Does not return until all pending commands
-  // have been executed.
-  virtual State FlushSync(int32 put_offset, int32 last_known_get) = 0;
+  // The writer calls this to wait until the current token is within a
+  // specific range, inclusive. Can return early if an error is generated.
+  virtual void WaitForTokenInRange(int32 start, int32 end) = 0;
+
+  // The writer calls this to wait until the current get offset is within a
+  // specific range, inclusive. Can return early if an error is generated.
+  virtual void WaitForGetOffsetInRange(int32 start, int32 end) = 0;
 
   // Sets the buffer commands are read from.
   // Also resets the get and put offsets to 0.

gpu/command_buffer/common/command_buffer_mock.h

@@ -26,7 +26,8 @@ class MockCommandBuffer : public CommandBuffer {
   MOCK_METHOD0(GetLastState, State());
   MOCK_METHOD0(GetLastToken, int32());
   MOCK_METHOD1(Flush, void(int32 put_offset));
-  MOCK_METHOD2(FlushSync, State(int32 put_offset, int32 last_known_get));
+  MOCK_METHOD2(WaitForTokenInRange, void(int32 start, int32 end));
+  MOCK_METHOD2(WaitForGetOffsetInRange, void(int32 start, int32 end));
   MOCK_METHOD1(SetGetBuffer, void(int32 transfer_buffer_id));
   MOCK_METHOD1(SetGetOffset, void(int32 get_offset));
   MOCK_METHOD2(CreateTransferBuffer, Buffer(size_t size, int32* id));

gpu/command_buffer/service/command_buffer_service.cc

@@ -65,19 +65,12 @@ void CommandBufferService::UpdateState() {
   }
 }
 
-CommandBufferService::State CommandBufferService::FlushSync(
-    int32 put_offset, int32 last_known_get) {
-  if (put_offset < 0 || put_offset > num_entries_) {
-    error_ = gpu::error::kOutOfBounds;
-    return GetState();
-  }
-
-  put_offset_ = put_offset;
-
-  if (!put_offset_change_callback_.is_null())
-    put_offset_change_callback_.Run();
+void CommandBufferService::WaitForTokenInRange(int32 start, int32 end) {
+  DCHECK(error_ != error::kNoError || InRange(start, end, token_));
+}
 
-  return GetState();
+void CommandBufferService::WaitForGetOffsetInRange(int32 start, int32 end) {
+  DCHECK(error_ != error::kNoError || InRange(start, end, get_offset_));
 }
 
 void CommandBufferService::Flush(int32 put_offset) {