[MediaController] Fix for bug onChange events

In server side, all onchange*request events should be fired only after
requesting server state change from client side.
Clients should receive on*changed events only after server state change.

[Verification] TCT passed 100%

Change-Id: I85e5eda12acf6de74bc653a9ccc8a283862eeb16
Signed-off-by: Szymon Jastrzebski <s.jastrzebsk@partner.samsung.com>

diff --git a/src/mediacontroller/mediacontroller_client.cc b/src/mediacontroller/mediacontroller_client.cc
index 950ab6f..43e3c89 100644 (file)
--- a/src/mediacontroller/mediacontroller_client.cc
+++ b/src/mediacontroller/mediacontroller_client.cc
@@ -278,11 +278,6 @@ void MediaControllerClient::OnServerStatusUpdate(const char* server_name, mc_ser
   ScopeLogger();
   MediaControllerClient* client = static_cast<MediaControllerClient*>(user_data);
 
-  if (!client->server_status_listener_) {
-    LoggerD("No server status listener registered, skipping");
-    return;
-  }
-
   // server state
   std::string state_str;
   PlatformResult result = Types::PlatformEnumToString(Types::kMediaControllerServerState,
@@ -305,10 +300,16 @@ PlatformResult MediaControllerClient::SetPlaybackInfoListener(const JsonCallback
   ScopeLogger();
   int failed_setter = 0;
   SCOPE_EXIT {
+    // Lambda function used to clean all set listeners (in case of failure of
+    // SetPlaybackInfoListener method).
+    // The purpose of this lambda is to unset as many setters as we can in case of failure.
+
     int (*unsetters[])(mc_client_h) = {
         mc_client_unset_playback_update_cb, mc_client_unset_shuffle_mode_update_cb,
         mc_client_unset_repeat_mode_update_cb,
         /*mc_client_unset_metadata_update_cb the last unsetter will never be used*/};
+
+    // This loop is no-op in case of success.
     for (int i = 0; i < failed_setter; ++i) {
       auto ret = unsetters[i](handle_);
       if (MEDIA_CONTROLLER_ERROR_NONE != ret) {
@@ -374,11 +375,6 @@ void MediaControllerClient::OnPlaybackUpdate(const char* server_name, mc_playbac
   ScopeLogger();
   MediaControllerClient* client = static_cast<MediaControllerClient*>(user_data);
 
-  if (!client->playback_info_listener_) {
-    LoggerD("No playback info listener registered, skipping");
-    return;
-  }
-
   // playback state
   std::string state;
   PlatformResult result = Types::ConvertPlaybackState(playback, &state);
@@ -411,11 +407,6 @@ void MediaControllerClient::OnShuffleModeUpdate(const char* server_name, mc_shuf
   ScopeLogger();
   MediaControllerClient* client = static_cast<MediaControllerClient*>(user_data);
 
-  if (!client->playback_info_listener_) {
-    LoggerD("No playback info listener registered, skipping");
-    return;
-  }
-
   picojson::value data = picojson::value(picojson::object());
   picojson::object& data_o = data.get<picojson::object>();
 
@@ -431,11 +422,6 @@ void MediaControllerClient::OnRepeatModeUpdate(const char* server_name, mc_repea
   ScopeLogger();
   MediaControllerClient* client = static_cast<MediaControllerClient*>(user_data);
 
-  if (!client->playback_info_listener_) {
-    LoggerD("No playback info listener registered, skipping");
-    return;
-  }
-
   picojson::value data = picojson::value(picojson::object());
   picojson::object& data_o = data.get<picojson::object>();
 
@@ -451,11 +437,6 @@ void MediaControllerClient::OnMetadataUpdate(const char* server_name, mc_metadat
   ScopeLogger();
   MediaControllerClient* client = static_cast<MediaControllerClient*>(user_data);
 
-  if (!client->playback_info_listener_) {
-    LoggerD("No playback info listener registered, skipping");
-    return;
-  }
-
   picojson::value data = picojson::value(picojson::object());
   picojson::object& data_o = data.get<picojson::object>();
 

diff --git a/src/mediacontroller/mediacontroller_server.cc b/src/mediacontroller/mediacontroller_server.cc
index 76aab28..dd6449a 100644 (file)
--- a/src/mediacontroller/mediacontroller_server.cc
+++ b/src/mediacontroller/mediacontroller_server.cc
@@ -27,17 +27,15 @@
 namespace extension {
 namespace mediacontroller {
 
-namespace {
-// The privileges that are required in Application API
-const std::string kInternalCommandSendPlaybackPosition = "__internal_sendPlaybackPosition";
-const std::string kInternalCommandSendShuffleMode = "__internal_sendShuffleMode";
-const std::string kInternalCommandSendRepeatMode = "__internal_sendRepeatMode";
-}  // namespace
-
 using common::PlatformResult;
 using common::ErrorCode;
 
-MediaControllerServer::MediaControllerServer() : handle_(nullptr) {
+MediaControllerServer::MediaControllerServer()
+    : handle_(nullptr),
+      playback_state_(MC_PLAYBACK_STATE_STOPPED),
+      position_(0ULL),
+      shuffle_mode_(MC_SHUFFLE_MODE_OFF),
+      repeat_mode_(MC_REPEAT_MODE_OFF) {
   ScopeLogger();
 }
 
@@ -82,6 +80,11 @@ PlatformResult MediaControllerServer::SetPlaybackState(const std::string& state)
     return result;
   }
 
+  if (static_cast<mc_playback_states_e>(state_int) == playback_state_) {
+    LoggerD("No change in playback state requested, skipping");
+    return PlatformResult(ErrorCode::NO_ERROR);
+  }
+
   int ret = mc_server_set_playback_state(handle_, static_cast<mc_playback_states_e>(state_int));
   if (MEDIA_CONTROLLER_ERROR_NONE != ret) {
     return LogAndCreateResult(
@@ -89,6 +92,8 @@ PlatformResult MediaControllerServer::SetPlaybackState(const std::string& state)
         ("mc_server_set_playback_state() error: %d, message: %s", ret, get_error_message(ret)));
   }
 
+  playback_state_ = static_cast<mc_playback_states_e>(state_int);
+
   ret = mc_server_update_playback_info(handle_);
   if (MEDIA_CONTROLLER_ERROR_NONE != ret) {
     return LogAndCreateResult(
@@ -102,6 +107,11 @@ PlatformResult MediaControllerServer::SetPlaybackState(const std::string& state)
 PlatformResult MediaControllerServer::SetPlaybackPosition(double position) {
   ScopeLogger();
 
+  if (static_cast<unsigned long long>(position) == position_) {
+    LoggerD("No change in playback position requested, skipping");
+    return PlatformResult(ErrorCode::NO_ERROR);
+  }
+
   int ret = mc_server_set_playback_position(handle_, static_cast<unsigned long long>(position));
   if (MEDIA_CONTROLLER_ERROR_NONE != ret) {
     return LogAndCreateResult(
@@ -109,6 +119,8 @@ PlatformResult MediaControllerServer::SetPlaybackPosition(double position) {
         ("mc_server_set_playback_position() error: %d, message: %s", ret, get_error_message(ret)));
   }
 
+  position_ = static_cast<unsigned long long>(position);
+
   ret = mc_server_update_playback_info(handle_);
   if (MEDIA_CONTROLLER_ERROR_NONE != ret) {
     return LogAndCreateResult(
@@ -122,26 +134,42 @@ PlatformResult MediaControllerServer::SetPlaybackPosition(double position) {
 PlatformResult MediaControllerServer::SetShuffleMode(bool mode) {
   ScopeLogger();
 
-  int ret = mc_server_update_shuffle_mode(handle_, mode ? MC_SHUFFLE_MODE_ON : MC_SHUFFLE_MODE_OFF);
+  mc_shuffle_mode_e shuffle_mode = mode ? MC_SHUFFLE_MODE_ON : MC_SHUFFLE_MODE_OFF;
+  if (shuffle_mode == shuffle_mode_) {
+    LoggerD("No change in shuffle mode requested, skipping");
+    return PlatformResult(ErrorCode::NO_ERROR);
+  }
+
+  int ret = mc_server_update_shuffle_mode(handle_, shuffle_mode);
   if (MEDIA_CONTROLLER_ERROR_NONE != ret) {
     return LogAndCreateResult(
         ErrorCode::UNKNOWN_ERR, "Error updating shuffle mode",
         ("mc_server_update_shuffle_mode() error: %d, message: %s", ret, get_error_message(ret)));
   }
 
+  shuffle_mode_ = shuffle_mode;
+
   return PlatformResult(ErrorCode::NO_ERROR);
 }
 
 PlatformResult MediaControllerServer::SetRepeatMode(bool mode) {
   ScopeLogger();
 
-  int ret = mc_server_update_repeat_mode(handle_, mode ? MC_REPEAT_MODE_ON : MC_REPEAT_MODE_OFF);
+  mc_repeat_mode_e repeat_mode = mode ? MC_REPEAT_MODE_ON : MC_REPEAT_MODE_OFF;
+  if (repeat_mode == repeat_mode_) {
+    LoggerD("No change in repeat mode requested, skipping");
+    return PlatformResult(ErrorCode::NO_ERROR);
+  }
+
+  int ret = mc_server_update_repeat_mode(handle_, repeat_mode);
   if (MEDIA_CONTROLLER_ERROR_NONE != ret) {
     return LogAndCreateResult(
         ErrorCode::UNKNOWN_ERR, "Error updating repeat mode",
         ("mc_server_update_repeat_mode() error: %d, message: %s", ret, get_error_message(ret)));
   }
 
+  repeat_mode_ = repeat_mode;
+
   return PlatformResult(ErrorCode::NO_ERROR);
 }
 
@@ -289,11 +317,17 @@ PlatformResult MediaControllerServer::SetChangeRequestPlaybackInfoListener(
   ScopeLogger();
   int failed_setter = 0;
   SCOPE_EXIT {
+    // Lambda function used to clean all set listeners (in case of failure of
+    // SetPlaybackInfoListener method).
+    // The purpose of this lambda is to unset as many setters as we can in case of failure.
+
     int (*unsetters[])(mc_server_h) = {
         mc_server_unset_playback_state_command_received_cb,
         mc_server_unset_playback_position_command_received_cb,
         mc_server_unset_shuffle_mode_command_received_cb,
         /*mc_server_unset_repeat_mode_command_received_cb the last unsetter will never be used*/};
+
+    // This loop is no-op in case of success.
     for (int i = 0; i < failed_setter; ++i) {
       auto ret = unsetters[i](handle_);
       if (MEDIA_CONTROLLER_ERROR_NONE != ret) {
@@ -389,8 +423,8 @@ void MediaControllerServer::OnPlaybackStateCommand(const char* client_name,
 
   MediaControllerServer* server = static_cast<MediaControllerServer*>(user_data);
 
-  if (!server->change_request_playback_info_listener_) {
-    LoggerD("No change request playback info listener registered, skipping");
+  if (server->playback_state_ == state_e) {
+    LoggerD("The media playback state did not change, skipping");
     return;
   }
 
@@ -418,8 +452,8 @@ void MediaControllerServer::OnPlaybackPositionCommand(const char* client_name,
 
   MediaControllerServer* server = static_cast<MediaControllerServer*>(user_data);
 
-  if (!server->change_request_playback_info_listener_) {
-    LoggerD("No change request playback info listener registered, skipping");
+  if (server->position_ == position) {
+    LoggerD("The position did not change, skipping");
     return;
   }
 
@@ -438,8 +472,8 @@ void MediaControllerServer::OnShuffleModeCommand(const char* client_name, mc_shu
 
   MediaControllerServer* server = static_cast<MediaControllerServer*>(user_data);
 
-  if (!server->change_request_playback_info_listener_) {
-    LoggerD("No change request playback info listener registered, skipping");
+  if (server->shuffle_mode_ == mode) {
+    LoggerD("The shuffle mode did not change, skipping");
     return;
   }
 
@@ -458,8 +492,8 @@ void MediaControllerServer::OnRepeatModeCommand(const char* client_name, mc_repe
 
   MediaControllerServer* server = static_cast<MediaControllerServer*>(user_data);
 
-  if (!server->change_request_playback_info_listener_) {
-    LoggerD("No change request playback info listener registered, skipping");
+  if (server->repeat_mode_ == mode) {
+    LoggerD("The repeat mode did not change, skipping");
     return;
   }
 

diff --git a/src/mediacontroller/mediacontroller_server.h b/src/mediacontroller/mediacontroller_server.h
index 3110560..679bb4b 100644 (file)
--- a/src/mediacontroller/mediacontroller_server.h
+++ b/src/mediacontroller/mediacontroller_server.h
@@ -51,6 +51,11 @@ class MediaControllerServer {
   mc_server_h handle_;
 
   JsonCallback change_request_playback_info_listener_;
+  mc_playback_states_e playback_state_;
+  unsigned long long position_;
+  mc_shuffle_mode_e shuffle_mode_;
+  mc_repeat_mode_e repeat_mode_;
+
   JsonCallback command_listener_;
 
   static void OnPlaybackStateCommand(const char* client_name, mc_playback_states_e state_e,