Fix early IsRegistered() state of application (#3796)

* Fix early IsRegistered state of application

Mark application as registered only when app registration
was finalized and all corresponding requests were sent to
HMI. This prevents data races in case when language has been
changed by HMI and application is in intermediate registration
state yet.

* fixup! Fix early IsRegistered state of application

8 files changed, 108 insertions, 39 deletions

diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/hmi/on_tts_language_change_notification.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/hmi/on_tts_language_change_notification.cc
index 3c25ff5386..d693c00ac3 100644
--- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/hmi/on_tts_language_change_notification.cc
+++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/hmi/on_tts_language_change_notification.cc
@@ -88,12 +88,15 @@ void OnTTSLanguageChangeNotification::Run() {
   (*message_)[strings::params][strings::function_id] =
       static_cast<int32_t>(mobile_apis::FunctionID::OnLanguageChangeID);
 
-  const ApplicationSet& accessor =
-      application_manager_.applications().GetData();
-  ApplicationSetIt it = accessor.begin();
-  for (; accessor.end() != it;) {
-    ApplicationSharedPtr app = *it;
-    ++it;
+  const auto applications = application_manager_.applications().GetData();
+  for (const auto& app : applications) {
+    if (!app->IsRegistered()) {
+      SDL_LOG_DEBUG("Skipping app "
+                    << app->app_id()
+                    << " which has not finished the registration process");
+      continue;
+    }
+
     (*message_)[strings::params][strings::connection_key] = app->app_id();
     SendNotificationToMobile(message_);
 
diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/hmi/on_vr_language_change_notification.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/hmi/on_vr_language_change_notification.cc
index bf00291c88..23c1ec1b80 100644
--- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/hmi/on_vr_language_change_notification.cc
+++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/hmi/on_vr_language_change_notification.cc
@@ -79,10 +79,17 @@ void OnVRLanguageChangeNotification::Run() {
       static_cast<int32_t>(mobile_apis::FunctionID::OnLanguageChangeID);
 
   const auto applications = application_manager_.applications().GetData();
-
   for (auto app : applications) {
+    if (!app->IsRegistered()) {
+      SDL_LOG_DEBUG("Skipping app "
+                    << app->app_id()
+                    << " which has not finished the registration process");
+      continue;
+    }
+
     (*message_)[strings::params][strings::connection_key] = app->app_id();
     SendNotificationToMobile(message_);
+
     if (static_cast<int32_t>(app->language()) !=
         (*message_)[strings::msg_params][strings::language].asInt()) {
       application_manager_.state_controller().SetRegularState(
diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/hmi/sdl_activate_app_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/hmi/sdl_activate_app_request.cc
index a8b34bf2d4..88b3065fb0 100644
--- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/hmi/sdl_activate_app_request.cc
+++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/hmi/sdl_activate_app_request.cc
@@ -136,18 +136,10 @@ void SDLActivateAppRequest::Run() {
                       static_cast<eType>(function_id()),
                       hmi_apis::Common_Result::REJECTED,
                       "HMIDeactivate is active");
-  } else if (app && !app->IsRegistered() && app->is_cloud_app()) {
-    SDL_LOG_DEBUG("Starting cloud application.");
-    const ApplicationManagerSettings& settings =
-        application_manager_.get_settings();
-    uint32_t total_retry_timeout = (settings.cloud_app_retry_timeout() *
-                                    settings.cloud_app_max_retry_attempts());
-    application_manager_.UpdateRequestTimeout(
-        0, correlation_id(), default_timeout_ + total_retry_timeout);
-    subscribe_on_event(BasicCommunication_OnAppRegistered);
-    application_manager_.connection_handler().ConnectToDevice(app->device());
-  } else {
-    const uint32_t application_id = app_id();
+    return;
+  }
+
+  if (!app->is_cloud_app() || app->IsRegistered()) {
     auto main_state =
         app->CurrentHmiState(mobile_apis::PredefinedWindows::DEFAULT_WINDOW);
     if (mobile_apis::HMILevel::INVALID_ENUM == main_state->hmi_level()) {
@@ -156,11 +148,30 @@ void SDLActivateAppRequest::Run() {
           "yet, postpone activation");
       auto& postponed_activation_ctrl = application_manager_.state_controller()
                                             .GetPostponedActivationController();
-      postponed_activation_ctrl.AddAppToActivate(application_id,
+      postponed_activation_ctrl.AddAppToActivate(app->app_id(),
                                                  correlation_id());
       return;
     }
+  }
+
+  const uint32_t application_id = app_id();
+  if (app->IsRegistered()) {
+    SDL_LOG_DEBUG("Application is registered. Activating.");
     policy_handler_.OnActivateApp(application_id, correlation_id());
+    return;
+  }
+
+  if (app->is_cloud_app()) {
+    SDL_LOG_DEBUG("Starting cloud application.");
+    const ApplicationManagerSettings& settings =
+        application_manager_.get_settings();
+    uint32_t total_retry_timeout = (settings.cloud_app_retry_timeout() *
+                                    settings.cloud_app_max_retry_attempts());
+    application_manager_.UpdateRequestTimeout(
+        0, correlation_id(), default_timeout_ + total_retry_timeout);
+    subscribe_on_event(BasicCommunication_OnAppRegistered);
+    application_manager_.connection_handler().ConnectToDevice(app->device());
+    return;
   }
 }
 
@@ -205,8 +216,7 @@ void SDLActivateAppRequest::Run() {
     return;
   }
 
-  if (app_to_activate->IsRegistered()) {
-    SDL_LOG_DEBUG("Application is registered. Activating.");
+  if (!app_to_activate->is_cloud_app() || app_to_activate->IsRegistered()) {
     auto main_state = app_to_activate->CurrentHmiState(
         mobile_apis::PredefinedWindows::DEFAULT_WINDOW);
     if (mobile_apis::HMILevel::INVALID_ENUM == main_state->hmi_level()) {
@@ -215,13 +225,19 @@ void SDLActivateAppRequest::Run() {
           "yet, postpone activation");
       auto& postponed_activation_ctrl = application_manager_.state_controller()
                                             .GetPostponedActivationController();
-      postponed_activation_ctrl.AddAppToActivate(application_id,
+      postponed_activation_ctrl.AddAppToActivate(app_to_activate->app_id(),
                                                  correlation_id());
       return;
     }
+  }
+
+  if (app_to_activate->IsRegistered()) {
+    SDL_LOG_DEBUG("Application is registered. Activating.");
     policy_handler_.OnActivateApp(application_id, correlation_id());
     return;
-  } else if (app_to_activate->is_cloud_app()) {
+  }
+
+  if (app_to_activate->is_cloud_app()) {
     SDL_LOG_DEBUG("Starting cloud application.");
     const ApplicationManagerSettings& settings =
         application_manager_.get_settings();
diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/register_app_interface_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/register_app_interface_request.cc
index 2224f17c4e..9a9aa0c533 100644
--- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/register_app_interface_request.cc
+++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/register_app_interface_request.cc
@@ -440,6 +440,8 @@ void FinishSendingResponseToMobile(const smart_objects::SmartObject& msg_params,
                                       &(msg_params[strings::app_hmi_type]));
   }
 
+  application->MarkRegistered();
+
   // Default HMI level should be set before any permissions validation, since
   // it relies on HMI level.
   app_manager.OnApplicationRegistered(application);
@@ -732,7 +734,6 @@ void RegisterAppInterfaceRequest::Run() {
   }
 
   CheckLanguage();
-
   SendRegisterAppInterfaceResponseToMobile(
       ApplicationType::kNewApplication, status_notifier, add_info);
 }
diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/hmi/hmi_notifications_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/hmi/hmi_notifications_test.cc
index 82a5b536a8..53b1fdfc67 100644
--- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/hmi/hmi_notifications_test.cc
+++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/hmi/hmi_notifications_test.cc
@@ -1368,6 +1368,7 @@ TEST_F(HMICommandsNotificationsTest,
   EXPECT_CALL(mock_hmi_capabilities_, set_active_vr_language(_));
   EXPECT_CALL(mock_rpc_service_,
               ManageMobileCommand(_, Command::CommandSource::SOURCE_SDL));
+  EXPECT_CALL(*app_ptr_, IsRegistered()).WillOnce(Return(true));
   EXPECT_CALL(*app_ptr_, app_id()).WillOnce(Return(kAppId_));
   EXPECT_CALL(*app_ptr_, language()).WillRepeatedly(ReturnRef(kLang));
 
@@ -1412,6 +1413,7 @@ TEST_F(HMICommandsNotificationsTest,
   EXPECT_CALL(mock_hmi_capabilities_, set_active_vr_language(_));
   EXPECT_CALL(mock_rpc_service_,
               ManageMobileCommand(_, Command::CommandSource::SOURCE_SDL));
+  EXPECT_CALL(*app_ptr_, IsRegistered()).WillOnce(Return(true));
   EXPECT_CALL(*app_ptr_, app_id()).WillRepeatedly(Return(kAppId_));
   EXPECT_CALL(*app_ptr_, language()).WillRepeatedly(ReturnRef(kLang));
   EXPECT_CALL(app_mngr_, state_controller())
@@ -1697,6 +1699,7 @@ TEST_F(HMICommandsNotificationsTest,
   EXPECT_CALL(mock_hmi_capabilities_, set_active_tts_language(_));
   EXPECT_CALL(mock_rpc_service_,
               ManageMobileCommand(_, Command::CommandSource::SOURCE_SDL));
+  EXPECT_CALL(*app_ptr_, IsRegistered()).WillOnce(Return(true));
   EXPECT_CALL(*app_ptr_, app_id()).WillOnce(Return(kAppId_));
   EXPECT_CALL(*app_ptr_, language()).WillRepeatedly(ReturnRef(kLang));
 
@@ -1742,6 +1745,7 @@ TEST_F(HMICommandsNotificationsTest,
   EXPECT_CALL(mock_hmi_capabilities_, set_active_tts_language(_));
   EXPECT_CALL(mock_rpc_service_,
               ManageMobileCommand(_, Command::CommandSource::SOURCE_SDL));
+  EXPECT_CALL(*app_ptr_, IsRegistered()).WillOnce(Return(true));
   EXPECT_CALL(*app_ptr_, app_id()).WillRepeatedly(Return(kAppId_));
   EXPECT_CALL(*app_ptr_, language()).WillRepeatedly(ReturnRef(kLang));
   EXPECT_CALL(mock_message_helper_,
diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/hmi/sdl_activate_app_request_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/hmi/sdl_activate_app_request_test.cc
index 443050b47f..44bd7f4a80 100644
--- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/hmi/sdl_activate_app_request_test.cc
+++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/hmi/sdl_activate_app_request_test.cc
@@ -157,6 +157,8 @@ TEST_F(SDLActivateAppRequestTest, Run_ActivateApp_SUCCESS) {
               CurrentHmiState(mobile_apis::PredefinedWindows::DEFAULT_WINDOW))
       .WillOnce(Return(state));
 
+  EXPECT_CALL(*mock_app, IsRegistered()).WillOnce(Return(true));
+
   EXPECT_CALL(mock_policy_handler_, OnActivateApp(kAppID, kCorrelationID));
 
   command->Run();
@@ -202,8 +204,14 @@ TEST_F(SDLActivateAppRequestTest, FindAppToRegister_SUCCESS) {
               IsStateActive(am::HmiState::StateID::STATE_ID_DEACTIVATE_HMI))
       .WillOnce(Return(false));
 
-  EXPECT_CALL(*mock_app, IsRegistered()).WillOnce(Return(false));
-  EXPECT_CALL(*mock_app, is_cloud_app()).WillOnce(Return(false));
+  am::HmiStatePtr state = std::make_shared<am::HmiState>(mock_app, app_mngr_);
+  state->set_hmi_level(mobile_apis::HMILevel::HMI_NONE);
+  EXPECT_CALL(*mock_app,
+              CurrentHmiState(mobile_apis::PredefinedWindows::DEFAULT_WINDOW))
+      .WillOnce(Return(state));
+
+  ON_CALL(*mock_app, IsRegistered()).WillByDefault(Return(false));
+  ON_CALL(*mock_app, is_cloud_app()).WillByDefault(Return(false));
   ON_CALL(*mock_app, device()).WillByDefault(Return(kHandle));
 
   MockAppPtr mock_app_first(CreateMockApp());
@@ -264,8 +272,14 @@ TEST_F(SDLActivateAppRequestTest, DevicesAppsEmpty_SUCCESS) {
               IsStateActive(am::HmiState::StateID::STATE_ID_DEACTIVATE_HMI))
       .WillOnce(Return(false));
 
-  EXPECT_CALL(*mock_app, IsRegistered()).WillOnce(Return(false));
-  EXPECT_CALL(*mock_app, is_cloud_app()).WillOnce(Return(false));
+  am::HmiStatePtr state = std::make_shared<am::HmiState>(mock_app, app_mngr_);
+  state->set_hmi_level(mobile_apis::HMILevel::HMI_NONE);
+  EXPECT_CALL(*mock_app,
+              CurrentHmiState(mobile_apis::PredefinedWindows::DEFAULT_WINDOW))
+      .WillOnce(Return(state));
+
+  ON_CALL(*mock_app, IsRegistered()).WillByDefault(Return(false));
+  ON_CALL(*mock_app, is_cloud_app()).WillByDefault(Return(false));
   ON_CALL(*mock_app, device()).WillByDefault(Return(kHandle));
 
   DataAccessor<ApplicationSet> accessor(app_list_, lock_);
@@ -291,6 +305,12 @@ TEST_F(SDLActivateAppRequestTest, FirstAppActive_SUCCESS) {
       .WillOnce(Return(false));
   EXPECT_CALL(*mock_app, device()).WillOnce(Return(kHandle));
 
+  am::HmiStatePtr state = std::make_shared<am::HmiState>(mock_app, app_mngr_);
+  state->set_hmi_level(mobile_apis::HMILevel::HMI_NONE);
+  EXPECT_CALL(*mock_app,
+              CurrentHmiState(mobile_apis::PredefinedWindows::DEFAULT_WINDOW))
+      .WillOnce(Return(state));
+
   DataAccessor<ApplicationSet> accessor(app_list_, lock_);
   EXPECT_CALL(app_mngr_, applications()).WillRepeatedly(Return(accessor));
 
@@ -327,14 +347,15 @@ TEST_F(SDLActivateAppRequestTest, FirstAppNotActiveNONE_SUCCESS) {
   EXPECT_CALL(mock_state_controller_,
               IsStateActive(am::HmiState::StateID::STATE_ID_DEACTIVATE_HMI))
       .WillOnce(Return(false));
-  EXPECT_CALL(*mock_app, IsRegistered()).WillOnce(Return(true));
+
   am::HmiStatePtr state = std::make_shared<am::HmiState>(mock_app, app_mngr_);
   state->set_hmi_level(mobile_apis::HMILevel::HMI_NONE);
-
   EXPECT_CALL(*mock_app,
               CurrentHmiState(mobile_apis::PredefinedWindows::DEFAULT_WINDOW))
       .WillOnce(Return(state));
 
+  EXPECT_CALL(*mock_app, IsRegistered()).WillOnce(Return(true));
+
   EXPECT_CALL(mock_policy_handler_, OnActivateApp(kAppID, kCorrelationID));
 
   command->Run();
@@ -356,9 +377,16 @@ TEST_F(SDLActivateAppRequestTest, FirstAppIsForeground_SUCCESS) {
 
   ON_CALL(app_mngr_, application(kAppID)).WillByDefault(Return(mock_app));
 
+  am::HmiStatePtr state = std::make_shared<am::HmiState>(mock_app, app_mngr_);
+  state->set_hmi_level(mobile_apis::HMILevel::HMI_NONE);
+  EXPECT_CALL(*mock_app,
+              CurrentHmiState(mobile_apis::PredefinedWindows::DEFAULT_WINDOW))
+      .WillOnce(Return(state));
+
   EXPECT_CALL(*mock_app, device()).WillOnce(Return(kHandle));
-  EXPECT_CALL(*mock_app, IsRegistered()).WillOnce(Return(false));
-  EXPECT_CALL(*mock_app, is_cloud_app()).WillOnce(Return(false));
+  ON_CALL(*mock_app, IsRegistered()).WillByDefault(Return(false));
+  ON_CALL(*mock_app, is_cloud_app()).WillByDefault(Return(false));
+
   EXPECT_CALL(app_mngr_, state_controller())
       .WillOnce(ReturnRef(mock_state_controller_));
   EXPECT_CALL(mock_state_controller_,
@@ -397,6 +425,12 @@ TEST_F(SDLActivateAppRequestTest, FirstAppNotRegisteredAndEmpty_SUCCESS) {
               IsStateActive(am::HmiState::StateID::STATE_ID_DEACTIVATE_HMI))
       .WillOnce(Return(false));
 
+  am::HmiStatePtr state = std::make_shared<am::HmiState>(mock_app, app_mngr_);
+  state->set_hmi_level(mobile_apis::HMILevel::HMI_NONE);
+  EXPECT_CALL(*mock_app,
+              CurrentHmiState(mobile_apis::PredefinedWindows::DEFAULT_WINDOW))
+      .WillOnce(Return(state));
+
   MockAppPtr mock_app_first(CreateMockApp());
   ON_CALL(*mock_app_first, device()).WillByDefault(Return(kHandle));
   ON_CALL(*mock_app_first, is_foreground()).WillByDefault(Return(false));
@@ -429,6 +463,13 @@ TEST_F(SDLActivateAppRequestTest, FirstAppNotRegistered_SUCCESS) {
   EXPECT_CALL(mock_state_controller_,
               IsStateActive(am::HmiState::StateID::STATE_ID_DEACTIVATE_HMI))
       .WillOnce(Return(false));
+
+  am::HmiStatePtr state = std::make_shared<am::HmiState>(mock_app, app_mngr_);
+  state->set_hmi_level(mobile_apis::HMILevel::HMI_NONE);
+  EXPECT_CALL(*mock_app,
+              CurrentHmiState(mobile_apis::PredefinedWindows::DEFAULT_WINDOW))
+      .WillOnce(Return(state));
+
   DataAccessor<ApplicationSet> accessor(app_list_, lock_);
   EXPECT_CALL(app_mngr_, applications()).WillRepeatedly(Return(accessor));
 
@@ -460,8 +501,8 @@ TEST_F(SDLActivateAppRequestTest, WaitingCloudApplication_ConnectDevice) {
   MockAppPtr mock_app(CreateMockApp());
 
   EXPECT_CALL(*mock_app, device()).WillOnce(Return(kHandle));
-  EXPECT_CALL(*mock_app, IsRegistered()).WillOnce(Return(false));
-  EXPECT_CALL(*mock_app, is_cloud_app()).WillOnce(Return(true));
+  ON_CALL(*mock_app, IsRegistered()).WillByDefault(Return(false));
+  ON_CALL(*mock_app, is_cloud_app()).WillByDefault(Return(true));
 
 #ifndef EXTERNAL_PROPRIETARY_MODE
   EXPECT_CALL(app_mngr_, application(kAppID))
diff --git a/src/components/application_manager/src/application_manager_impl.cc b/src/components/application_manager/src/application_manager_impl.cc
index b0b05a3031..8d2875364d 100644
--- a/src/components/application_manager/src/application_manager_impl.cc
+++ b/src/components/application_manager/src/application_manager_impl.cc
@@ -4744,9 +4744,6 @@ void ApplicationManagerImpl::AddAppToRegisteredAppList(
   SDL_LOG_AUTO_TRACE();
   DCHECK_OR_RETURN_VOID(application);
   sync_primitives::AutoLock lock(applications_list_lock_ptr_);
-
-  // Add application to registered app list and set appropriate mark.
-  application->MarkRegistered();
   applications_.insert(application);
   SDL_LOG_DEBUG("App with app_id: "
                 << application->app_id()
diff --git a/src/components/application_manager/src/message_helper/message_helper.cc b/src/components/application_manager/src/message_helper/message_helper.cc
index 606bc4c3f0..94d0a75c7f 100644
--- a/src/components/application_manager/src/message_helper/message_helper.cc
+++ b/src/components/application_manager/src/message_helper/message_helper.cc
@@ -1848,7 +1848,7 @@ bool MessageHelper::CreateHMIApplicationStruct(
   if (file_system::FileExists(app->app_icon_path())) {
     message[strings::icon] = icon_path;
   }
-  if (app->IsRegistered()) {
+  if (!app->is_cloud_app() || app->IsRegistered()) {
     message[strings::hmi_display_language_desired] = app->ui_language();
     message[strings::is_media_application] = app->is_media_application();
   } else {