Add UMA stats for open audio input device failure.

media/audio/audio_input_controller.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 "media/audio/audio_input_controller.h"
 
 #include "base/bind.h"
 #include "base/strings/stringprintf.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time/time.h"
@@ skipping 29 matching lines @@
 // A large smoothing factor corresponds to a faster filter response to input
 // changes since y(n)=alpha*x(n)+(1-alpha)*y(n-1), where x(n) is the input
 // and y(n) is the output.
 const int kPowerMeasurementTimeConstantMilliseconds = 10;
 
 // Time in seconds between two successive measurements of audio power levels.
 const int kPowerMonitorLogIntervalSeconds = 5;
 #endif
 }
 
+// Used to log the result of capture startup.
+// This was previously logged as a boolean with only the no callback and OK
+// options. The enum order is kept to ensure backwards compatibility.
+// Elements in this enum should not be deleted or rearranged; the only
+// permitted operation is to add new elements before NUM_STREAM_OPEN_RESULT.

[Ilya Sherman, 2014/08/12 17:30:48]

nit: Please update the name "NUM_STREAM_OPEN_RESULT" to
"CAPTURE_STARTUP_RESULT_MAX".

[Henrik Grunell, 2014/08/13 12:31:30]

Done.

+enum CaptureStartupResult {
+  CAPTURE_STARTUP_NO_DATA_CALLBACK = 0,
+  CAPTURE_STARTUP_OK = 1,
+  CAPTURE_STARTUP_CREATE_STREAM_FAILED = 2,
+  CAPTURE_STARTUP_OPEN_STREAM_FAILED = 3,
+  CAPTURE_STARTUP_RESULT_MAX = CAPTURE_STARTUP_OPEN_STREAM_FAILED

[Ilya Sherman, 2014/08/12 17:30:48]

Optional nit: It's a little cleaner, IMO, to have this enum member be
CAPTURE_STARTUP_RESULT_COUNT, and have that be equal to 4.  That way you can
avoid the "+ 1" below; and also anyone adding values to this enum doesn't need
to remember to update this line as well.  For that second reason, I'd also omit
the explicit value assignments for all of the enum members -- the compiler is
better at assigning values in a linear sequence than humans are.

[Henrik Grunell, 2014/08/13 12:31:30]

The presubmit check actually complained about that, it had to be done this way.
It seems it could be specific for //media.
I think it's easier to see the match with the enum in histograms.xml when
written explicitly.

+};
+
+void LogStreamOpenResult(CaptureStartupResult result) {
+  UMA_HISTOGRAM_ENUMERATION("Media.AudioInputControllerCaptureStartupSuccess",
+                            result,
+                            CAPTURE_STARTUP_RESULT_MAX + 1);
+
+}
+
 namespace media {
 
 // static
 AudioInputController::Factory* AudioInputController::factory_ = NULL;
 
 AudioInputController::AudioInputController(EventHandler* handler,
                                            SyncWriter* sync_writer,
                                            UserInputMonitor* user_input_monitor)
     : creator_task_runner_(base::MessageLoopProxy::current()),
       handler_(handler),
@@ skipping 90 matching lines @@
       new AudioInputController(event_handler, sync_writer, user_input_monitor));
   controller->task_runner_ = task_runner;
 
   // TODO(miu): See TODO at top of file.  Until that's resolved, we need to
   // disable the error auto-detection here (since the audio mirroring
   // implementation will reliably report error and close events).  Note, of
   // course, that we're assuming CreateForStream() has been called for the audio
   // mirroring use case only.
   if (!controller->task_runner_->PostTask(
           FROM_HERE,
-          base::Bind(&AudioInputController::DoCreateForStream, controller,
-                     stream, false))) {
+          base::Bind(&AudioInputController::DoCreateForStream,
+                     controller,
+                     stream))) {
     controller = NULL;
   }
 
   return controller;
 }
 
 void AudioInputController::Record() {
   task_runner_->PostTask(FROM_HERE, base::Bind(
       &AudioInputController::DoRecord, this));
 }
@@ skipping 30 matching lines @@
   audio_level_.reset(new media::AudioPowerMonitor(
       params.sample_rate(),
       TimeDelta::FromMilliseconds(kPowerMeasurementTimeConstantMilliseconds)));
   audio_params_ = params;
 #endif
 
   // TODO(miu): See TODO at top of file.  Until that's resolved, assume all
   // platform audio input requires the |no_data_timer_| be used to auto-detect
   // errors.  In reality, probably only Windows needs to be treated as
   // unreliable here.
-  DoCreateForStream(audio_manager->MakeAudioInputStream(params, device_id),
-                    true);
+  DoCreateForStream(audio_manager->MakeAudioInputStream(params, device_id));
 }
 
 void AudioInputController::DoCreateForStream(
-    AudioInputStream* stream_to_control, bool enable_nodata_timer) {
+    AudioInputStream* stream_to_control) {
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   DCHECK(!stream_);
   stream_ = stream_to_control;
 
   if (!stream_) {
     if (handler_)
       handler_->OnError(this, STREAM_CREATE_ERROR);
+    LogStreamOpenResult(CAPTURE_STARTUP_CREATE_STREAM_FAILED);
     return;
   }
 
   if (stream_ && !stream_->Open()) {
     stream_->Close();
     stream_ = NULL;
     if (handler_)
       handler_->OnError(this, STREAM_OPEN_ERROR);
+    LogStreamOpenResult(CAPTURE_STARTUP_OPEN_STREAM_FAILED);
     return;
   }
 
   DCHECK(!no_data_timer_.get());
 
+  // Create the data timer which will call FirstCheckForNoData(). The timer
+  // is started in DoRecord() and restarted in each DoCheckForNoData()
+  // callback.
   // The timer is enabled for logging purposes. The NO_DATA_ERROR triggered
   // from the timer must be ignored by the EventHandler.
   // TODO(henrika): remove usage of timer when it has been verified on Canary
   // that we are safe doing so. Goal is to get rid of |no_data_timer_| and
   // everything that is tied to it. crbug.com/357569.
-  enable_nodata_timer = true;
-
-  if (enable_nodata_timer) {
-    // Create the data timer which will call FirstCheckForNoData(). The timer
-    // is started in DoRecord() and restarted in each DoCheckForNoData()
-    // callback.
-    no_data_timer_.reset(new base::Timer(
-        FROM_HERE, base::TimeDelta::FromSeconds(kTimerInitialIntervalSeconds),
-        base::Bind(&AudioInputController::FirstCheckForNoData,
-                   base::Unretained(this)), false));
-  } else {
-    DVLOG(1) << "Disabled: timer check for no data.";
-  }
+  no_data_timer_.reset(new base::Timer(
+      FROM_HERE, base::TimeDelta::FromSeconds(kTimerInitialIntervalSeconds),
+      base::Bind(&AudioInputController::FirstCheckForNoData,
+                 base::Unretained(this)), false));
 
   state_ = CREATED;
   if (handler_)
     handler_->OnCreated(this);
 
   if (user_input_monitor_) {
     user_input_monitor_->EnableKeyPressMonitoring();
     prev_key_down_count_ = user_input_monitor_->GetKeyPressCount();
   }
 }
@@ skipping 78 matching lines @@
 
   // Ensure that the AGC state only can be modified before streaming starts.
   if (state_ != CREATED)
     return;
 
   stream_->SetAutomaticGainControl(enabled);
 }
 
 void AudioInputController::FirstCheckForNoData() {
   DCHECK(task_runner_->BelongsToCurrentThread());
-  UMA_HISTOGRAM_BOOLEAN("Media.AudioInputControllerCaptureStartupSuccess",
-                        GetDataIsActive());
+  LogStreamOpenResult(GetDataIsActive() ?
+                      CAPTURE_STARTUP_OK :
+                      CAPTURE_STARTUP_NO_DATA_CALLBACK);
   DoCheckForNoData();
 }
 
 void AudioInputController::DoCheckForNoData() {
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   if (!GetDataIsActive()) {
     // The data-is-active marker will be false only if it has been more than
     // one second since a data packet was recorded. This can happen if a
     // capture device has been removed or disabled.
@@ skipping 136 matching lines @@
 
 void AudioInputController::SetDataIsActive(bool enabled) {
   base::subtle::Release_Store(&data_is_active_, enabled);
 }
 
 bool AudioInputController::GetDataIsActive() {
   return (base::subtle::Acquire_Load(&data_is_active_) != false);
 }
 
 }  // namespace media