Web MIDI: add a new UMA entry for the final result code

This change adds a new UMA entry for the final MidiManager
result code on browser shutdown.

To report the result correctly, I assigned new numbers for
the result code. As a side effect, this patch can fix an
existing problem that the result code for NOT_INITIALIZED
was not propagated to renderer side correctly.

BUG=465661

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

Cr-Commit-Position: refs/heads/master@{#338148}

content/browser/media/midi_host.cc

@@ -130,9 +130,9 @@ void MidiHost::OnEndSession() {
   midi_manager_->EndSession(this);
 }
 
-void MidiHost::CompleteStartSession(media::midi::MidiResult result) {
+void MidiHost::CompleteStartSession(media::midi::Result result) {
   DCHECK(is_session_requested_);
-  if (result == media::midi::MIDI_OK) {
+  if (result == media::midi::Result::OK) {
     // ChildSecurityPolicy is set just before OnStartSession by
     // MidiDispatcherHost. So we can safely cache the policy.
     has_sys_ex_permission_ = ChildProcessSecurityPolicyImpl::GetInstance()->

content/browser/media/midi_host.h

@@ -38,7 +38,7 @@ class CONTENT_EXPORT MidiHost : public BrowserMessageFilter,
   bool OnMessageReceived(const IPC::Message& message) override;
 
   // MidiManagerClient implementation.
-  void CompleteStartSession(media::midi::MidiResult result) override;
+  void CompleteStartSession(media::midi::Result result) override;
   void AddInputPort(const media::midi::MidiPortInfo& info) override;
   void AddOutputPort(const media::midi::MidiPortInfo& info) override;
   void SetInputPortState(uint32 port,

content/common/media/midi_messages.h

@@ -10,7 +10,7 @@
 #include "ipc/ipc_message_macros.h"
 #include "ipc/param_traits_macros.h"
 #include "media/midi/midi_port_info.h"
-#include "media/midi/midi_result.h"
+#include "media/midi/result.h"
 #include "url/gurl.h"
 
 #undef IPC_MESSAGE_EXPORT
@@ -28,8 +28,7 @@ IPC_STRUCT_TRAITS_BEGIN(media::midi::MidiPortInfo)
   IPC_STRUCT_TRAITS_MEMBER(state)
 IPC_STRUCT_TRAITS_END()
 
-IPC_ENUM_TRAITS_MAX_VALUE(media::midi::MidiResult,
-                          media::midi::MIDI_RESULT_LAST)
+IPC_ENUM_TRAITS_MAX_VALUE(media::midi::Result, media::midi::Result::MAX)
 
 // Messages for IPC between MidiMessageFilter and MidiHost.
 
@@ -59,8 +58,7 @@ IPC_MESSAGE_CONTROL2(MidiMsg_SetOutputPortState,
                      uint32 /* port */,
                      media::midi::MidiPortState /* state */)
 
-IPC_MESSAGE_CONTROL1(MidiMsg_SessionStarted,
-                     media::midi::MidiResult /* result */)
+IPC_MESSAGE_CONTROL1(MidiMsg_SessionStarted, media::midi::Result /* result */)
 
 IPC_MESSAGE_CONTROL3(MidiMsg_DataReceived,
                      uint32 /* port */,

content/renderer/media/midi_message_filter.cc

@@ -29,7 +29,7 @@ MidiMessageFilter::MidiMessageFilter(
     : sender_(nullptr),
       io_task_runner_(io_task_runner),
       main_task_runner_(base::ThreadTaskRunnerHandle::Get()),
-      session_result_(media::midi::MIDI_NOT_INITIALIZED),
+      session_result_(media::midi::Result::NOT_INITIALIZED),
       unacknowledged_bytes_sent_(0u) {
 }
 
@@ -39,7 +39,7 @@ void MidiMessageFilter::AddClient(blink::WebMIDIAccessorClient* client) {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
   TRACE_EVENT0("midi", "MidiMessageFilter::AddClient");
   clients_waiting_session_queue_.push_back(client);
-  if (session_result_ != media::midi::MIDI_NOT_INITIALIZED) {
+  if (session_result_ != media::midi::Result::NOT_INITIALIZED) {
     HandleClientAdded(session_result_);
   } else if (clients_waiting_session_queue_.size() == 1u) {
     io_task_runner_->PostTask(
@@ -57,7 +57,7 @@ void MidiMessageFilter::RemoveClient(blink::WebMIDIAccessorClient* client) {
   if (it != clients_waiting_session_queue_.end())
     clients_waiting_session_queue_.erase(it);
   if (clients_.empty() && clients_waiting_session_queue_.empty()) {
-    session_result_ = media::midi::MIDI_NOT_INITIALIZED;
+    session_result_ = media::midi::Result::NOT_INITIALIZED;
     inputs_.clear();
     outputs_.clear();
     io_task_runner_->PostTask(
@@ -143,7 +143,7 @@ void MidiMessageFilter::OnChannelClosing() {
   sender_ = nullptr;
 }
 
-void MidiMessageFilter::OnSessionStarted(media::midi::MidiResult result) {
+void MidiMessageFilter::OnSessionStarted(media::midi::Result result) {
   TRACE_EVENT0("midi", "MidiMessageFilter::OnSessionStarted");
   DCHECK(io_task_runner_->BelongsToCurrentThread());
   // Handle on the main JS thread.
@@ -200,19 +200,19 @@ void MidiMessageFilter::OnAcknowledgeSentData(size_t bytes_sent) {
                             this, bytes_sent));
 }
 
-void MidiMessageFilter::HandleClientAdded(media::midi::MidiResult result) {
+void MidiMessageFilter::HandleClientAdded(media::midi::Result result) {
   TRACE_EVENT0("midi", "MidiMessageFilter::HandleClientAdded");
   DCHECK(main_task_runner_->BelongsToCurrentThread());
   session_result_ = result;
   std::string error;
   std::string message;
   switch (result) {
-    case media::midi::MIDI_OK:
+    case media::midi::Result::OK:
       break;
-    case media::midi::MIDI_NOT_SUPPORTED:
+    case media::midi::Result::NOT_SUPPORTED:
       error = "NotSupportedError";
       break;
-    case media::midi::MIDI_INITIALIZATION_ERROR:
+    case media::midi::Result::INITIALIZATION_ERROR:
       error = "InvalidStateError";
       message = "Platform dependent initialization failed.";
       break;
@@ -230,7 +230,7 @@ void MidiMessageFilter::HandleClientAdded(media::midi::MidiResult result) {
   while (!clients_waiting_session_queue_.empty()) {
     auto client = clients_waiting_session_queue_.back();
     clients_waiting_session_queue_.pop_back();
-    if (result == media::midi::MIDI_OK) {
+    if (result == media::midi::Result::OK) {
       // Add the client's input and output ports.
       for (const auto& info : inputs_) {
         client->didAddInputPort(
@@ -250,7 +250,8 @@ void MidiMessageFilter::HandleClientAdded(media::midi::MidiResult result) {
             ToBlinkState(info.state));
       }
     }
-    client->didStartSession(result == media::midi::MIDI_OK, error16, message16);
+    client->didStartSession(result == media::midi::Result::OK, error16,
+                            message16);
     clients_.insert(client);
   }
 }

content/renderer/media/midi_message_filter.h

@@ -12,7 +12,7 @@
 #include "content/common/content_export.h"
 #include "ipc/message_filter.h"
 #include "media/midi/midi_port_info.h"
-#include "media/midi/midi_result.h"
+#include "media/midi/result.h"
 #include "third_party/WebKit/public/platform/WebMIDIAccessorClient.h"
 
 namespace base {
@@ -75,7 +75,7 @@ class CONTENT_EXPORT MidiMessageFilter : public IPC::MessageFilter {
 
   // Called when the browser process has approved (or denied) access to
   // MIDI hardware.
-  void OnSessionStarted(media::midi::MidiResult result);
+  void OnSessionStarted(media::midi::Result result);
 
   // These functions are called in 2 cases:
   //  (1) Just before calling |OnSessionStarted|, to notify the recipient about
@@ -103,7 +103,7 @@ class CONTENT_EXPORT MidiMessageFilter : public IPC::MessageFilter {
   void OnAcknowledgeSentData(size_t bytes_sent);
 
   // Following methods, Handle*, run on |main_task_runner_|.
-  void HandleClientAdded(media::midi::MidiResult result);
+  void HandleClientAdded(media::midi::Result result);
 
   void HandleAddInputPort(media::midi::MidiPortInfo info);
   void HandleAddOutputPort(media::midi::MidiPortInfo info);
@@ -142,7 +142,7 @@ class CONTENT_EXPORT MidiMessageFilter : public IPC::MessageFilter {
   ClientsQueue clients_waiting_session_queue_;
 
   // Represents a result on starting a session. Can be accessed only on
-  media::midi::MidiResult session_result_;
+  media::midi::Result session_result_;
 
   // Holds MidiPortInfoList for input ports and output ports.
   media::midi::MidiPortInfoList inputs_;

media/midi/midi_manager.cc

@@ -6,17 +6,20 @@
 
 #include "base/bind.h"
 #include "base/message_loop/message_loop.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/trace_event/trace_event.h"
 
 namespace media {
 namespace midi {
 
 MidiManager::MidiManager()
-    : initialized_(false),
-      result_(MIDI_NOT_SUPPORTED) {
+    : initialized_(false), result_(Result::NOT_INITIALIZED) {
 }
 
 MidiManager::~MidiManager() {
+  UMA_HISTOGRAM_ENUMERATION("Media.Midi.ResultOnShutdown",
+                            static_cast<int>(result_),
+                            static_cast<int>(Result::MAX) + 1);
 }
 
 #if !defined(OS_MACOSX) && !defined(OS_WIN) && \
@@ -64,7 +67,7 @@ void MidiManager::StartSession(MidiManagerClient* client) {
     }
     if (too_many_pending_clients_exist) {
       // Return an error immediately if there are too many requests.
-      client->CompleteStartSession(MIDI_INITIALIZATION_ERROR);
+      client->CompleteStartSession(Result::INITIALIZATION_ERROR);
       return;
     }
     // CompleteInitialization() will be called asynchronously when platform
@@ -74,10 +77,10 @@ void MidiManager::StartSession(MidiManagerClient* client) {
 
   // Platform dependent initialization was already finished for previously
   // initialized clients.
-  MidiResult result;
+  Result result;
   {
     base::AutoLock auto_lock(lock_);
-    if (result_ == MIDI_OK) {
+    if (result_ == Result::OK) {
       AddInitialPorts(client);
       clients_.insert(client);
     }
@@ -111,10 +114,10 @@ void MidiManager::DispatchSendMidiData(MidiManagerClient* client,
 }
 
 void MidiManager::StartInitialization() {
-  CompleteInitialization(MIDI_NOT_SUPPORTED);
+  CompleteInitialization(Result::NOT_SUPPORTED);
 }
 
-void MidiManager::CompleteInitialization(MidiResult result) {
+void MidiManager::CompleteInitialization(Result result) {
   DCHECK(session_thread_runner_.get());
   // It is safe to post a task to the IO thread from here because the IO thread
   // should have stopped if the MidiManager is going to be destructed.
@@ -166,7 +169,7 @@ void MidiManager::ReceiveMidiData(
     client->ReceiveMidiData(port_index, data, length, timestamp);
 }
 
-void MidiManager::CompleteInitializationInternal(MidiResult result) {
+void MidiManager::CompleteInitializationInternal(Result result) {
   TRACE_EVENT0("midi", "MidiManager::CompleteInitialization");
 
   base::AutoLock auto_lock(lock_);
@@ -176,7 +179,7 @@ void MidiManager::CompleteInitializationInternal(MidiResult result) {
   result_ = result;
 
   for (auto client : pending_clients_) {
-    if (result_ == MIDI_OK) {
+    if (result_ == Result::OK) {
       AddInitialPorts(client);
       clients_.insert(client);
     }

media/midi/midi_manager.h

@@ -14,7 +14,7 @@
 #include "base/time/time.h"
 #include "media/midi/midi_export.h"
 #include "media/midi/midi_port_info.h"
-#include "media/midi/midi_result.h"
+#include "media/midi/result.h"
 
 namespace base {
 class SingleThreadTaskRunner;
@@ -43,7 +43,7 @@ class MIDI_EXPORT MidiManagerClient {
 
   // CompleteStartSession() is called when platform dependent preparation is
   // finished.
-  virtual void CompleteStartSession(MidiResult result) = 0;
+  virtual void CompleteStartSession(Result result) = 0;
 
   // ReceiveMidiData() is called when MIDI data has been received from the
   // MIDI system.
@@ -78,8 +78,8 @@ class MIDI_EXPORT MidiManager {
   // A client calls StartSession() to receive and send MIDI data.
   // If the session is ready to start, the MIDI system is lazily initialized
   // and the client is registered to receive MIDI data.
-  // CompleteStartSession() is called with MIDI_OK if the session is started.
-  // Otherwise CompleteStartSession() is called with proper MidiResult code.
+  // CompleteStartSession() is called with Result::OK if the session is started.
+  // Otherwise CompleteStartSession() is called with proper Result code.
   // StartSession() and EndSession() can be called on the Chrome_IOThread.
   // CompleteStartSession() will be invoked on the same Chrome_IOThread.
   void StartSession(MidiManagerClient* client);
@@ -110,20 +110,20 @@ class MIDI_EXPORT MidiManager {
 
   // Initializes the platform dependent MIDI system. MidiManager class has a
   // default implementation that synchronously calls CompleteInitialization()
-  // with MIDI_NOT_SUPPORTED on the caller thread. A derived class for a
+  // with Result::NOT_SUPPORTED on the caller thread. A derived class for a
   // specific platform should override this method correctly.
   // This method is called on Chrome_IOThread thread inside StartSession().
   // Platform dependent initialization can be processed synchronously or
   // asynchronously. When the initialization is completed,
   // CompleteInitialization() should be called with |result|.
-  // |result| should be MIDI_OK on success, otherwise a proper MidiResult.
+  // |result| should be Result::OK on success, otherwise a proper Result.
   virtual void StartInitialization();
 
   // Called from a platform dependent implementation of StartInitialization().
   // It invokes CompleteInitializationInternal() on the thread that calls
   // StartSession() and distributes |result| to MIDIManagerClient objects in
   // |pending_clients_|.
-  void CompleteInitialization(MidiResult result);
+  void CompleteInitialization(Result result);
 
   void AddInputPort(const MidiPortInfo& info);
   void AddOutputPort(const MidiPortInfo& info);
@@ -152,7 +152,7 @@ class MIDI_EXPORT MidiManager {
   }
 
  private:
-  void CompleteInitializationInternal(MidiResult result);
+  void CompleteInitializationInternal(Result result);
   void AddInitialPorts(MidiManagerClient* client);
 
   // Keeps track of all clients who wish to receive MIDI data.
@@ -170,8 +170,8 @@ class MIDI_EXPORT MidiManager {
   bool initialized_;
 
   // Keeps the platform dependent initialization result if initialization is
-  // completed. Otherwise keeps MIDI_NOT_SUPPORTED.
-  MidiResult result_;
+  // completed. Otherwise keeps Result::NOT_INITIALIZED.
+  Result result_;
 
   // Keeps all MidiPortInfo.
   MidiPortInfoList input_ports_;

media/midi/midi_manager_alsa.cc

@@ -185,7 +185,7 @@ void MidiManagerAlsa::StartInitialization() {
       snd_seq_open(&in_client, kAlsaHw, SND_SEQ_OPEN_INPUT, SND_SEQ_NONBLOCK);
   if (err != 0) {
     VLOG(1) << "snd_seq_open fails: " << snd_strerror(err);
-    return CompleteInitialization(MIDI_INITIALIZATION_ERROR);
+    return CompleteInitialization(Result::INITIALIZATION_ERROR);
   }
   in_client_.reset(in_client);
   in_client_id_ = snd_seq_client_id(in_client_.get());
@@ -194,7 +194,7 @@ void MidiManagerAlsa::StartInitialization() {
   err = snd_seq_open(&out_client, kAlsaHw, SND_SEQ_OPEN_OUTPUT, 0);
   if (err != 0) {
     VLOG(1) << "snd_seq_open fails: " << snd_strerror(err);
-    return CompleteInitialization(MIDI_INITIALIZATION_ERROR);
+    return CompleteInitialization(Result::INITIALIZATION_ERROR);
   }
   out_client_.reset(out_client);
   out_client_id_ = snd_seq_client_id(out_client_.get());
@@ -203,12 +203,12 @@ void MidiManagerAlsa::StartInitialization() {
   err = snd_seq_set_client_name(in_client_.get(), "Chrome (input)");
   if (err != 0) {
     VLOG(1) << "snd_seq_set_client_name fails: " << snd_strerror(err);
-    return CompleteInitialization(MIDI_INITIALIZATION_ERROR);
+    return CompleteInitialization(Result::INITIALIZATION_ERROR);
   }
   err = snd_seq_set_client_name(out_client_.get(), "Chrome (output)");
   if (err != 0) {
     VLOG(1) << "snd_seq_set_client_name fails: " << snd_strerror(err);
-    return CompleteInitialization(MIDI_INITIALIZATION_ERROR);
+    return CompleteInitialization(Result::INITIALIZATION_ERROR);
   }
 
   // Create input port.
@@ -217,7 +217,7 @@ void MidiManagerAlsa::StartInitialization() {
   if (in_port_id_ < 0) {
     VLOG(1) << "snd_seq_create_simple_port fails: "
             << snd_strerror(in_port_id_);
-    return CompleteInitialization(MIDI_INITIALIZATION_ERROR);
+    return CompleteInitialization(Result::INITIALIZATION_ERROR);
   }
 
   // Subscribe to the announce port.
@@ -235,7 +235,7 @@ void MidiManagerAlsa::StartInitialization() {
   if (err != 0) {
     VLOG(1) << "snd_seq_subscribe_port on the announce port fails: "
             << snd_strerror(err);
-    return CompleteInitialization(MIDI_INITIALIZATION_ERROR);
+    return CompleteInitialization(Result::INITIALIZATION_ERROR);
   }
 
   // Generate hotplug events for existing ports.
@@ -247,20 +247,20 @@ void MidiManagerAlsa::StartInitialization() {
       device::udev_monitor_new_from_netlink(udev_.get(), kUdev));
   if (!udev_monitor_.get()) {
     VLOG(1) << "udev_monitor_new_from_netlink fails";
-    return CompleteInitialization(MIDI_INITIALIZATION_ERROR);
+    return CompleteInitialization(Result::INITIALIZATION_ERROR);
   }
   err = device::udev_monitor_filter_add_match_subsystem_devtype(
       udev_monitor_.get(), kUdevSubsystemSound, nullptr);
   if (err != 0) {
     VLOG(1) << "udev_monitor_add_match_subsystem fails: "
             << base::safe_strerror(-err);
-    return CompleteInitialization(MIDI_INITIALIZATION_ERROR);
+    return CompleteInitialization(Result::INITIALIZATION_ERROR);
   }
   err = device::udev_monitor_enable_receiving(udev_monitor_.get());
   if (err != 0) {
     VLOG(1) << "udev_monitor_enable_receiving fails: "
             << base::safe_strerror(-err);
-    return CompleteInitialization(MIDI_INITIALIZATION_ERROR);
+    return CompleteInitialization(Result::INITIALIZATION_ERROR);
   }
 
   // Generate hotplug events for existing udev devices.
@@ -272,7 +272,7 @@ void MidiManagerAlsa::StartInitialization() {
       FROM_HERE,
       base::Bind(&MidiManagerAlsa::ScheduleEventLoop, base::Unretained(this)));
 
-  CompleteInitialization(MIDI_OK);
+  CompleteInitialization(Result::OK);
 }
 
 void MidiManagerAlsa::DispatchSendMidiData(MidiManagerClient* client,