Migrate callbacks in MediaAnalyticsClient to DBusMethodCallback.

BUG=739622
TEST=Ran on bots.

Change-Id: I6e968439859b4409a14621a03bd3a20f8ebe36f2
Reviewed-on: https://chromium-review.googlesource.com/901682
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534718}

chromeos/dbus/fake_media_analytics_client.cc

@@ -4,6 +4,8 @@
 
 #include "chromeos/dbus/fake_media_analytics_client.h"
 
+#include <utility>
+
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/threading/thread_task_runner_handle.h"
@@ -43,20 +45,23 @@ void FakeMediaAnalyticsClient::ClearMediaPerceptionSignalHandler() {
   media_perception_signal_handler_.Reset();
 }
 
-void FakeMediaAnalyticsClient::GetState(const StateCallback& callback) {
+void FakeMediaAnalyticsClient::GetState(
+    DBusMethodCallback<mri::State> callback) {
   if (!process_running_) {
-    callback.Run(false, current_state_);
+    std::move(callback).Run(base::nullopt);
     return;
   }
   base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, base::Bind(&FakeMediaAnalyticsClient::OnState,
-                            weak_ptr_factory_.GetWeakPtr(), callback));
+      FROM_HERE,
+      base::BindOnce(&FakeMediaAnalyticsClient::OnState,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
-void FakeMediaAnalyticsClient::SetState(const mri::State& state,
-                                        const StateCallback& callback) {
+void FakeMediaAnalyticsClient::SetState(
+    const mri::State& state,
+    DBusMethodCallback<mri::State> callback) {
   if (!process_running_) {
-    callback.Run(false, current_state_);
+    std::move(callback).Run(base::nullopt);
     return;
   }
   DCHECK(state.has_status()) << "Trying to set state without status.";
@@ -67,8 +72,9 @@ void FakeMediaAnalyticsClient::SetState(const mri::State& state,
          "RESTARTING.";
   current_state_ = state;
   base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, base::Bind(&FakeMediaAnalyticsClient::OnState,
-                            weak_ptr_factory_.GetWeakPtr(), callback));
+      FROM_HERE,
+      base::BindOnce(&FakeMediaAnalyticsClient::OnState,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
 void FakeMediaAnalyticsClient::SetStateSuspended() {
@@ -80,25 +86,27 @@ void FakeMediaAnalyticsClient::SetStateSuspended() {
   current_state_ = suspended;
 }
 
-void FakeMediaAnalyticsClient::OnState(const StateCallback& callback) {
-  callback.Run(true, current_state_);
+void FakeMediaAnalyticsClient::OnState(
+    DBusMethodCallback<mri::State> callback) {
+  std::move(callback).Run(current_state_);
 }
 
 void FakeMediaAnalyticsClient::GetDiagnostics(
-    const DiagnosticsCallback& callback) {
+    DBusMethodCallback<mri::Diagnostics> callback) {
   if (!process_running_) {
     LOG(ERROR) << "Fake media analytics process not running.";
-    callback.Run(false, mri::Diagnostics());
+    std::move(callback).Run(base::nullopt);
     return;
   }
   base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, base::Bind(&FakeMediaAnalyticsClient::OnGetDiagnostics,
-                            weak_ptr_factory_.GetWeakPtr(), callback));
+      FROM_HERE,
+      base::BindOnce(&FakeMediaAnalyticsClient::OnGetDiagnostics,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
 void FakeMediaAnalyticsClient::OnGetDiagnostics(
-    const DiagnosticsCallback& callback) {
-  callback.Run(true, diagnostics_);
+    DBusMethodCallback<mri::Diagnostics> callback) {
+  std::move(callback).Run(diagnostics_);
 }
 
 void FakeMediaAnalyticsClient::OnMediaPerception(

chromeos/dbus/fake_media_analytics_client.h

@@ -22,13 +22,13 @@ class CHROMEOS_EXPORT FakeMediaAnalyticsClient : public MediaAnalyticsClient {
   ~FakeMediaAnalyticsClient() override;
 
   // Inherited from MediaAnalyticsClient.
-  void GetState(const StateCallback& callback) override;
+  void GetState(DBusMethodCallback<mri::State> callback) override;
   void SetState(const mri::State& state,
-                const StateCallback& callback) override;
+                DBusMethodCallback<mri::State> callback) override;
   void SetMediaPerceptionSignalHandler(
       const MediaPerceptionSignalHandler& handler) override;
   void ClearMediaPerceptionSignalHandler() override;
-  void GetDiagnostics(const DiagnosticsCallback& callback) override;
+  void GetDiagnostics(DBusMethodCallback<mri::Diagnostics> callback) override;
 
   // Inherited from DBusClient.
   void Init(dbus::Bus* bus) override;
@@ -48,10 +48,10 @@ class CHROMEOS_EXPORT FakeMediaAnalyticsClient : public MediaAnalyticsClient {
 
  private:
   // Echoes back the previously set state.
-  void OnState(const StateCallback& callback);
+  void OnState(DBusMethodCallback<mri::State> callback);
 
   // Runs callback with the Diagnostics proto provided in SetDiagnostics.
-  void OnGetDiagnostics(const DiagnosticsCallback& callback);
+  void OnGetDiagnostics(DBusMethodCallback<mri::Diagnostics> callback);
 
   // Runs callback with a MediaPerception proto provided in
   // FireMediaPerceptionEvent.

chromeos/dbus/media_analytics_client.cc

@@ -6,6 +6,7 @@
 
 #include <cstdint>
 #include <string>
+#include <utility>
 
 #include "base/bind.h"
 #include "base/logging.h"
@@ -36,17 +37,17 @@ class MediaAnalyticsClientImpl : public MediaAnalyticsClient {
     media_perception_signal_handler_.Reset();
   }
 
-  void GetState(const StateCallback& callback) override {
+  void GetState(DBusMethodCallback<mri::State> callback) override {
     dbus::MethodCall method_call(media_perception::kMediaPerceptionServiceName,
                                  media_perception::kStateFunction);
     dbus_proxy_->CallMethod(
         &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
         base::BindOnce(&MediaAnalyticsClientImpl::OnState,
-                       weak_ptr_factory_.GetWeakPtr(), callback));
+                       weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
   }
 
   void SetState(const mri::State& state,
-                const StateCallback& callback) override {
+                DBusMethodCallback<mri::State> callback) override {
     DCHECK(state.has_status()) << "Attempting to SetState without status set.";
 
     dbus::MethodCall method_call(media_perception::kMediaPerceptionServiceName,
@@ -58,17 +59,17 @@ class MediaAnalyticsClientImpl : public MediaAnalyticsClient {
     dbus_proxy_->CallMethod(
         &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
         base::BindOnce(&MediaAnalyticsClientImpl::OnState,
-                       weak_ptr_factory_.GetWeakPtr(), callback));
+                       weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
   }
 
-  void GetDiagnostics(const DiagnosticsCallback& callback) override {
+  void GetDiagnostics(DBusMethodCallback<mri::Diagnostics> callback) override {
     dbus::MethodCall method_call(media_perception::kMediaPerceptionServiceName,
                                  media_perception::kGetDiagnosticsFunction);
     // TODO(lasoren): Verify that this timeout setting is sufficient.
     dbus_proxy_->CallMethod(
         &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
         base::BindOnce(&MediaAnalyticsClientImpl::OnGetDiagnostics,
-                       weak_ptr_factory_.GetWeakPtr(), callback));
+                       weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
   }
 
  protected:
@@ -120,11 +121,12 @@ class MediaAnalyticsClientImpl : public MediaAnalyticsClient {
     media_perception_signal_handler_.Run(media_perception);
   }
 
-  void OnState(const StateCallback& callback, dbus::Response* response) {
+  void OnState(DBusMethodCallback<mri::State> callback,
+               dbus::Response* response) {
     mri::State state;
     if (!response) {
       LOG(ERROR) << "Call to State failed to get response.";
-      callback.Run(false, state);
+      std::move(callback).Run(base::nullopt);
       return;
     }
 
@@ -134,25 +136,25 @@ class MediaAnalyticsClientImpl : public MediaAnalyticsClient {
     dbus::MessageReader reader(response);
     if (!reader.PopArrayOfBytes(&bytes, &length)) {
       LOG(ERROR) << "Invalid D-Bus response: " << response->ToString();
-      callback.Run(false, state);
+      std::move(callback).Run(base::nullopt);
       return;
     }
 
     if (!state.ParseFromArray(bytes, length)) {
       LOG(ERROR) << "Failed to parse State message.";
-      callback.Run(false, state);
+      std::move(callback).Run(base::nullopt);
       return;
     }
 
-    callback.Run(true, state);
+    std::move(callback).Run(std::move(state));
   }
 
-  void OnGetDiagnostics(const DiagnosticsCallback& callback,
+  void OnGetDiagnostics(DBusMethodCallback<mri::Diagnostics> callback,
                         dbus::Response* response) {
     mri::Diagnostics diagnostics;
     if (!response) {
       LOG(ERROR) << "Call to GetDiagnostics failed to get response.";
-      callback.Run(false, diagnostics);
+      std::move(callback).Run(base::nullopt);
       return;
     }
 
@@ -162,17 +164,17 @@ class MediaAnalyticsClientImpl : public MediaAnalyticsClient {
     dbus::MessageReader reader(response);
     if (!reader.PopArrayOfBytes(&bytes, &length)) {
       LOG(ERROR) << "Invalid GetDiagnostics response: " << response->ToString();
-      callback.Run(false, diagnostics);
+      std::move(callback).Run(base::nullopt);
       return;
     }
 
     if (!diagnostics.ParseFromArray(bytes, length)) {
       LOG(ERROR) << "Failed to parse Diagnostics message.";
-      callback.Run(false, diagnostics);
+      std::move(callback).Run(base::nullopt);
       return;
     }
 
-    callback.Run(true, diagnostics);
+    std::move(callback).Run(std::move(diagnostics));
   }
 
   dbus::ObjectProxy* dbus_proxy_;

chromeos/dbus/media_analytics_client.h

@@ -9,6 +9,7 @@
 #include "base/macros.h"
 #include "chromeos/chromeos_export.h"
 #include "chromeos/dbus/dbus_client.h"
+#include "chromeos/dbus/dbus_method_call_status.h"
 #include "chromeos/media_perception/media_perception.pb.h"
 
 namespace chromeos {
@@ -17,27 +18,19 @@ namespace chromeos {
 // running outside of Chrome.
 class CHROMEOS_EXPORT MediaAnalyticsClient : public DBusClient {
  public:
-  // Callback type for a State proto message received from the media analytics
-  // process.
-  using StateCallback =
-      base::Callback<void(bool succeeded, const mri::State& state)>;
   // Handler type for signal received from media analytics process.
   using MediaPerceptionSignalHandler =
       base::Callback<void(const mri::MediaPerception& media_perception)>;
-  // Callback type for a Diagnostics proto message received from the media
-  // analytics process.
-  using DiagnosticsCallback =
-      base::Callback<void(bool succeeded, const mri::Diagnostics& diagnostics)>;
 
   ~MediaAnalyticsClient() override;
 
   // Gets the media analytics process state.
-  virtual void GetState(const StateCallback& callback) = 0;
+  virtual void GetState(DBusMethodCallback<mri::State> callback) = 0;
 
   // Sets the media analytics process state. |state.status| is expected to be
   // set.
   virtual void SetState(const mri::State& state,
-                        const StateCallback& callback) = 0;
+                        DBusMethodCallback<mri::State> callback) = 0;
 
   // Register event handler for the MediaPerception protos received as signal
   // from the media analytics process.
@@ -50,7 +43,8 @@ class CHROMEOS_EXPORT MediaAnalyticsClient : public DBusClient {
 
   // API for getting diagnostic information from the media analytics process
   // over D-Bus as a Diagnostics proto message.
-  virtual void GetDiagnostics(const DiagnosticsCallback& callback) = 0;
+  virtual void GetDiagnostics(
+      DBusMethodCallback<mri::Diagnostics> callback) = 0;
 
   // Factory function, creates new instance and returns ownership.
   // For normal usage, access the singleton via DbusThreadManager::Get().

extensions/browser/api/media_perception_private/media_perception_api_manager.cc

@@ -4,6 +4,10 @@
 
 #include "extensions/browser/api/media_perception_private/media_perception_api_manager.h"
 
+#include <memory>
+#include <utility>
+#include <vector>
+
 #include "base/files/file_path.h"
 #include "base/lazy_instance.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
@@ -290,28 +294,26 @@ void MediaPerceptionAPIManager::UpstartRestartCallback(
   GetState(callback);
 }
 
-void MediaPerceptionAPIManager::StateCallback(const APIStateCallback& callback,
-                                              bool succeeded,
-                                              const mri::State& state_proto) {
-  media_perception::State state;
-  if (!succeeded) {
+void MediaPerceptionAPIManager::StateCallback(
+    const APIStateCallback& callback,
+    base::Optional<mri::State> result) {
+  if (!result.has_value()) {
     callback.Run(GetStateForServiceError(
         media_perception::SERVICE_ERROR_SERVICE_UNREACHABLE));
     return;
   }
-  callback.Run(media_perception::StateProtoToIdl(state_proto));
+  callback.Run(media_perception::StateProtoToIdl(result.value()));
 }
 
 void MediaPerceptionAPIManager::GetDiagnosticsCallback(
     const APIGetDiagnosticsCallback& callback,
-    bool succeeded,
-    const mri::Diagnostics& diagnostics_proto) {
-  if (!succeeded) {
+    base::Optional<mri::Diagnostics> result) {
+  if (!result.has_value()) {
     callback.Run(GetDiagnosticsForServiceError(
         media_perception::SERVICE_ERROR_SERVICE_UNREACHABLE));
     return;
   }
-  callback.Run(media_perception::DiagnosticsProtoToIdl(diagnostics_proto));
+  callback.Run(media_perception::DiagnosticsProtoToIdl(result.value()));
 }
 
 void MediaPerceptionAPIManager::MediaPerceptionSignalHandler(

extensions/browser/api/media_perception_private/media_perception_api_manager.h

@@ -5,7 +5,10 @@
 #ifndef EXTENSIONS_BROWSER_API_MEDIA_PERCEPTION_PRIVATE_MEDIA_PERCEPTION_API_MANAGER_H_
 #define EXTENSIONS_BROWSER_API_MEDIA_PERCEPTION_PRIVATE_MEDIA_PERCEPTION_API_MANAGER_H_
 
+#include <string>
+
 #include "base/memory/weak_ptr.h"
+#include "base/optional.h"
 #include "chromeos/media_perception/media_perception.pb.h"
 #include "extensions/browser/browser_context_keyed_api_factory.h"
 #include "extensions/common/api/media_perception_private.h"
@@ -73,14 +76,12 @@ class MediaPerceptionAPIManager : public BrowserContextKeyedAPI {
 
   // Callback for State D-Bus method calls to the media analytics process.
   void StateCallback(const APIStateCallback& callback,
-                     bool succeeded,
-                     const mri::State& state);
+                     base::Optional<mri::State> state);
 
   // Callback for GetDiagnostics D-Bus method calls to the media analytics
   // process.
   void GetDiagnosticsCallback(const APIGetDiagnosticsCallback& callback,
-                              bool succeeded,
-                              const mri::Diagnostics& diagnostics);
+                              base::Optional<mri::Diagnostics> diagnostics);
 
   // Callback for Upstart command to start media analytics process.
   void UpstartStartCallback(const APIStateCallback& callback,