Commit 4b8556b8 authored by mhasank's avatar mhasank Committed by Commit Bot

login: ignore new subscribers in session_manager_client during events

session_manager_client uses observer_list which allows reentrancy by default i.e. it allows adding more subscribers while it is firing the events on the observers.
One event that causes the problem is when the container is shutdown. When the user is attempts the retry of opt-in flow then we first shutdown the container and then restart the flow by creating a new container.
However, when we create a new container, another subscriber is added to session_manager_client and it has not yet completed firing the ArcInstanceStopped event so the new container also is incorrectly detected as shutdown.

With this change, we are going to safely invoke the observers and ignore the new subscribers during invocation of events.

Currently there are five consumers of the observer interface

1. login_display_host_webui.cc implements OnLoginPromptVisible and plays a sound
2. arc_session_manager.cc implements OnLoginPromptVisible and starts a mini instance.
3. owner_settings_service_chromeos.h implements OwnerKeySet and reloads keys
4. device_settings_service.h implements OwnerKeySet and PropertyChangeComplete and reloads settings
5. arc_container_client_adapter.cc implements ArcInstanceStopped where the problem was occurring initially

From the code inspection it is clear that the change should not cause any regression since we're not relying on the same event being fired while handling the event in question for the same instance of container.

BUG=b:164816080
TEST=deploy to dut, disable playstore, re-enable playstore, successfully
finish the authorization flow

Change-Id: Ic820d4f014d5890c68aef99512e38de5acfb0279
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2381050Reviewed-by: default avatarRyo Hashimoto <hashimoto@chromium.org>
Reviewed-by: default avatarHidehiko Abe <hidehiko@chromium.org>
Reviewed-by: default avatarYusuke Sato <yusukes@chromium.org>
Commit-Queue: Muhammad Hasan Khan <mhasank@chromium.org>
Auto-Submit: Muhammad Hasan Khan <mhasank@chromium.org>
Cr-Commit-Position: refs/heads/master@{#804121}
parent de6e899d
...@@ -1061,7 +1061,8 @@ class SessionManagerClientImpl : public SessionManagerClient { ...@@ -1061,7 +1061,8 @@ class SessionManagerClientImpl : public SessionManagerClient {
dbus::ObjectProxy* session_manager_proxy_ = nullptr; dbus::ObjectProxy* session_manager_proxy_ = nullptr;
std::unique_ptr<BlockingMethodCaller> blocking_method_caller_; std::unique_ptr<BlockingMethodCaller> blocking_method_caller_;
base::ObserverList<Observer>::Unchecked observers_; base::ObserverList<Observer>::Unchecked observers_{
base::ObserverListPolicy::EXISTING_ONLY};
// Most recent screen-lock state received from session_manager. // Most recent screen-lock state received from session_manager.
bool screen_is_locked_ = false; bool screen_is_locked_ = false;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment