Replace command buffer FlushSync with WaitForTokenInRange and WaitForGetOffsetInRange

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

11 files changed, 85 insertions, 97 deletions

diff --git a/gpu/command_buffer/client/client_test_helper.cc b/gpu/command_buffer/client/client_test_helper.cc
index db191ce..c731edf 100644
--- a/gpu/command_buffer/client/client_test_helper.cc
+++ b/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) {
diff --git a/gpu/command_buffer/client/client_test_helper.h b/gpu/command_buffer/client/client_test_helper.h
index 88c37e8..6cf98ba 100644
--- a/gpu/command_buffer/client/client_test_helper.h
+++ b/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;
diff --git a/gpu/command_buffer/client/cmd_buffer_helper.cc b/gpu/command_buffer/client/cmd_buffer_helper.cc
index f4bb5d1..c32154f 100644
--- a/gpu/command_buffer/client/cmd_buffer_helper.cc
+++ b/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);
     }
   }
 }
diff --git a/gpu/command_buffer/client/cmd_buffer_helper.h b/gpu/command_buffer/client/cmd_buffer_helper.h
index 825cf9f..c62071b 100644
--- a/gpu/command_buffer/client/cmd_buffer_helper.h
+++ b/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();
diff --git a/gpu/command_buffer/client/cmd_buffer_helper_test.cc b/gpu/command_buffer/client/cmd_buffer_helper_test.cc
index add15ca..6ce31cf 100644
--- a/gpu/command_buffer/client/cmd_buffer_helper_test.cc
+++ b/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);
diff --git a/gpu/command_buffer/common/command_buffer.h b/gpu/command_buffer/common/command_buffer.h
index 394ccf3..ac52c8b 100644
--- a/gpu/command_buffer/common/command_buffer.h
+++ b/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.
diff --git a/gpu/command_buffer/common/command_buffer_mock.h b/gpu/command_buffer/common/command_buffer_mock.h
index 0e78b7b..6e99fee 100644
--- a/gpu/command_buffer/common/command_buffer_mock.h
+++ b/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));
diff --git a/gpu/command_buffer/service/command_buffer_service.cc b/gpu/command_buffer/service/command_buffer_service.cc
index bae2d80..76a5f51 100644
--- a/gpu/command_buffer/service/command_buffer_service.cc
+++ b/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) {
diff --git a/gpu/command_buffer/service/command_buffer_service.h b/gpu/command_buffer/service/command_buffer_service.h
index b1f8fa1..fd28653 100644
--- a/gpu/command_buffer/service/command_buffer_service.h
+++ b/gpu/command_buffer/service/command_buffer_service.h
@@ -29,7 +29,8 @@ class GPU_EXPORT CommandBufferService : public CommandBuffer {
   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 transfer_buffer_id) OVERRIDE;
   virtual void SetGetOffset(int32 get_offset) OVERRIDE;
   virtual Buffer CreateTransferBuffer(size_t size, int32* id) OVERRIDE;
diff --git a/gpu/command_buffer/service/in_process_command_buffer.cc b/gpu/command_buffer/service/in_process_command_buffer.cc
index dd59838..3e2b1fb 100644
--- a/gpu/command_buffer/service/in_process_command_buffer.cc
+++ b/gpu/command_buffer/service/in_process_command_buffer.cc
@@ -536,21 +536,22 @@ void InProcessCommandBuffer::Flush(int32 put_offset) {
   QueueTask(task);
 }
 
-CommandBuffer::State InProcessCommandBuffer::FlushSync(int32 put_offset,
-                                                       int32 last_known_get) {
+void InProcessCommandBuffer::WaitForTokenInRange(int32 start, int32 end) {
+  CheckSequencedThread();
+  while (!InRange(start, end, GetLastToken()) &&
+         last_state_.error == gpu::error::kNoError)
+    flush_event_.Wait();
+}
+
+void InProcessCommandBuffer::WaitForGetOffsetInRange(int32 start, int32 end) {
   CheckSequencedThread();
-  if (put_offset == last_known_get || last_state_.error != gpu::error::kNoError)
-    return last_state_;
 
-  Flush(put_offset);
   GetStateFast();
-  while (last_known_get == last_state_.get_offset &&
+  while (!InRange(start, end, last_state_.get_offset) &&
          last_state_.error == gpu::error::kNoError) {
     flush_event_.Wait();
     GetStateFast();
   }
-
-  return last_state_;
 }
 
 void InProcessCommandBuffer::SetGetBuffer(int32 shm_id) {
diff --git a/gpu/command_buffer/service/in_process_command_buffer.h b/gpu/command_buffer/service/in_process_command_buffer.h
index c9bfaaf..f42e19e 100644
--- a/gpu/command_buffer/service/in_process_command_buffer.h
+++ b/gpu/command_buffer/service/in_process_command_buffer.h
@@ -84,7 +84,8 @@ class GPU_EXPORT InProcessCommandBuffer : public CommandBuffer,
   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, int32* id) OVERRIDE;