Add native log and uma stat when all input shared memory buffers are full

content/browser/renderer_host/media/audio_input_sync_writer.cc

Index: content/browser/renderer_host/media/audio_input_sync_writer.cc
diff --git a/content/browser/renderer_host/media/audio_input_sync_writer.cc b/content/browser/renderer_host/media/audio_input_sync_writer.cc
index 117fbe8fedbbec72e086e7a22597ef126ff2b238..f42790fbfda3f1f930d7030e03fdc6621ffb404a 100644
--- a/content/browser/renderer_host/media/audio_input_sync_writer.cc
+++ b/content/browser/renderer_host/media/audio_input_sync_writer.cc
@@ -7,12 +7,38 @@
 #include <algorithm>
 
 #include "base/memory/shared_memory.h"
+#include "base/metrics/histogram.h"
+#include "base/strings/string_number_conversions.h"
 #include "content/browser/renderer_host/media/media_stream_manager.h"
 
 using media::AudioBus;
 
 namespace content {
 
+namespace {
+
+enum AudioInputBufferType {
+  MEDIA_INPUT_SINGLE_BUFFER = 0,

[tommi (sloooow) - chröme, 2014/08/29 09:16:43]

nit: can we include 'AUDIO' in the enum names?  E.g. 
MEDIA_AUDIO_INPUT_SINGLE_BUFFER etc.
I'd also be fine with dropping "MEDIA" from the names.

[no longer working on chromium, 2014/08/29 11:14:16]

Ah, good catch. I changed the name from MediaInputBufferType to
AudioInputBufferType, but forgot the contents.

Done with changing MEDIA to AUDIO

+  MEDIA_INPUT_CIRCULAR_BUFFER = 1,
+  NUM_MEDIA_INPUT_BUFFER
+};
+
+// Log to native log and uma stats that all the buffers are full.
+void LogAllBuffersAreFull(int shared_memory_segment_size) {

[tommi (sloooow) - chröme, 2014/08/29 09:16:42]

|shared_memory_segment_size| sounds to me to represent the size in bytes of a
single memory segment.  However, this is being logged as "number of buffers",
which sounds like it would be a completely different number?  which is it?

[no longer working on chromium, 2014/08/29 11:14:16]

shared_memory_segment_size_ is used by AudioInputSyncWriter as a member variable
for the number of shared memory buffers.
But I agree it is better to change number_of_buffers for this helper method.

Done. Hope it is fine now.

+  AudioInputBufferType type = MEDIA_INPUT_SINGLE_BUFFER;
+  if (shared_memory_segment_size != 1) {
+    type =  MEDIA_INPUT_CIRCULAR_BUFFER;

[tommi (sloooow) - chröme, 2014/08/29 09:16:42]

spacing

[no longer working on chromium, 2014/08/29 11:14:16]

Done.

+    const std::string log =
+        "AIS, circular buffers is full. Number of buffers is " +

[tommi (sloooow) - chröme, 2014/08/29 09:16:42]

grammar nit: circular buffers are full

[no longer working on chromium, 2014/08/29 11:14:16]

Done.

+        base::IntToString(shared_memory_segment_size);

[tommi (sloooow) - chröme, 2014/08/29 09:16:42]

nit: use StringPrintf() (fewer string objects required)

[no longer working on chromium, 2014/08/29 11:14:16]

Done.

+    MediaStreamManager::SendMessageToNativeLog(log);
+  }
+  UMA_HISTOGRAM_ENUMERATION("Media.AudioInputSyncWriterBuffersAreFull",
+                            type,
+                            NUM_MEDIA_INPUT_BUFFER);
+}
+
+}
 AudioInputSyncWriter::AudioInputSyncWriter(base::SharedMemory* shared_memory,

[tommi (sloooow) - chröme, 2014/08/29 09:16:42]

empty line above this one

[no longer working on chromium, 2014/08/29 11:14:16]

Done.

                                            int shared_memory_segment_count,
                                            const media::AudioParameters& params)
@@ -44,14 +70,10 @@ AudioInputSyncWriter::AudioInputSyncWriter(base::SharedMemory* shared_memory,
 
 AudioInputSyncWriter::~AudioInputSyncWriter() {}
 
-// TODO(henrika): Combine into one method (including Write).
-void AudioInputSyncWriter::UpdateRecordedBytes(uint32 bytes) {
-  socket_->Send(&bytes, sizeof(bytes));
-}
-
 void AudioInputSyncWriter::Write(const media::AudioBus* data,
                                  double volume,
-                                 bool key_pressed) {
+                                 bool key_pressed,
+                                 uint32 bytes) {

[tommi (sloooow) - chröme, 2014/08/29 09:16:42]

is there a way to verify that |bytes| is a valid (and correct) value?

[no longer working on chromium, 2014/08/29 11:14:16]

I was confused by the comment of UpdateRecordedBytes(uint32 bytes) API, which I
remove in this CL.

We actually pass the delay value by this |bytes|, I updated the variable name.

 #if !defined(OS_ANDROID)
   static const base::TimeDelta kLogDelayThreadhold =
       base::TimeDelta::FromMilliseconds(500);
@@ -81,6 +103,12 @@ void AudioInputSyncWriter::Write(const media::AudioBus* data,
   ptr += current_segment_id_ * shared_memory_segment_size_;
   media::AudioInputBuffer* buffer =
       reinterpret_cast<media::AudioInputBuffer*>(ptr);
+
+  // The renderer should set the size to 0 after consuming the buffer. In case
+  // it is not 0, it means buffer has not been consumed yet.
+  if (buffer->params.size)
+    LogAllBuffersAreFull(shared_memory_segment_size_);
+
   buffer->params.volume = volume;
   buffer->params.size = audio_bus_memory_size_;
   buffer->params.key_pressed = key_pressed;
@@ -92,6 +120,10 @@ void AudioInputSyncWriter::Write(const media::AudioBus* data,
 
   if (++current_segment_id_ >= shared_memory_segment_count_)
     current_segment_id_ = 0;
+
+  // Notify the synchronous peer about the number of bytes in the
+  // soundcard which has been recorded.
+  socket_->Send(&bytes, sizeof(bytes));

[tommi (sloooow) - chröme, 2014/08/29 09:16:42]

can the number of bytes not be derived from the AudioBus?  If the AudioBus
buffer does not represent the same data as |bytes| does, then that needs further
clarification (and perhaps an improved variable name)

[no longer working on chromium, 2014/08/29 11:14:16]

variable name has been changed.

 }
 
 void AudioInputSyncWriter::Close() {