DBus MediaAnalyticsClient and media_perception pb.

chromeos/dbus/media_analytics_client.h

+// Copyright 2017 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.
+
+#ifndef CHROMEOS_DBUS_MEDIA_ANALYTICS_CLIENT_H_
+#define CHROMEOS_DBUS_MEDIA_ANALYTICS_CLIENT_H_
+
+#include "base/callback.h"
+#include "base/macros.h"
+#include "chromeos/chromeos_export.h"
+#include "chromeos/dbus/dbus_client.h"
+#include "chromeos/media_perception/media_perception.pb.h"
+
+namespace chromeos {
+
+// MediaAnalyticsClient is used to communicate with a media analytics process
+// 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. Contains a

[tbarzic, 2017/05/05 21:10:42]

The "byte array with a serialized MediaPercetion proto" does not apply anymore
:)

[Luke Sorenson, 2017/05/08 19:06:08]

Done.

+  // byte array with a serialized MediaPerception proto.
+  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;
+
+  // API for getting and setting the state of the media analytics process over
+  // D-Bus using a State proto.
+  virtual void State(const mri::State& state,

[tbarzic, 2017/05/05 21:10:42]

can we have separate methods for getting and setting state (even if the setter
returns the final state)?

[Luke Sorenson, 2017/05/08 19:06:08]

From the perspective of the D-Bus comms, it's simpler for there to be a single
org.chromium.MediaPerception.State service than to have our media analytics
process initialize and provide services for both getState and setState. The
MediaPerceptionAPIManager separates these two into distinct function for the JS
frontend to make the interface clearer. 

[tbarzic, 2017/05/08 23:04:55]

Wouldn't you need only one service that can provide both getState and setState
method (instead of state method)?

Even if DBus provides only a single State method, I think the API provided by
client would be much cleaner with both a setter and a getter here (with
different invocations to State in the implementation). 

[Luke Sorenson, 2017/05/09 17:39:45]

Okay, I see what you're saying. Done.

+                     const StateCallback& callback) = 0;
+
+  // Register event handler for the MediaPerception protos received as signal
+  // from the media analytics process.
+  virtual void SetMediaPerceptionSignalHandler(
+      const MediaPerceptionSignalHandler& handler) = 0;
+
+  virtual void UnsetMediaPerceptionSignalHandler() = 0;

[tbarzic, 2017/05/08 23:04:55]

suggestion: Clear/Reset instead of Unset

[Luke Sorenson, 2017/05/09 17:39:45]

Done.

+
+  // 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;
+
+  // Factory function, creates new instance and returns ownership.
+  // For normal usage, access the singleton via DbusThreadManager::Get().
+  static MediaAnalyticsClient* Create();
+
+ protected:
+  MediaAnalyticsClient();
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MediaAnalyticsClient);
+};
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_DBUS_MEDIA_ANALYTICS_CLIENT_H_