[basemgr] Rely on basemgr to set presentation of base shell and session
shell. More notes below:
- This will remove duplicate code in topaz's base shell model, where
they were implementing session shell switching logic and settings
parsing.
- Basemgr will now receive session shell settings in its constructor, so
a slight optimization in how many times we read/parse session shell
settings.
- Basemgr also sets the presentation of the base shell being launched to
match the session shell settings. This is also duplicate code in topaz
that will be deleted.
MF-110 #comment
TESTED: paved, integration testing
Change-Id: Ibd2f4748e856684ebadb9f676927959ad797e403
diff --git a/bin/basemgr/basemgr_impl.cc b/bin/basemgr/basemgr_impl.cc
index 891c5e8..28b761e 100644
--- a/bin/basemgr/basemgr_impl.cc
+++ b/bin/basemgr/basemgr_impl.cc
@@ -35,15 +35,19 @@
BasemgrImpl::BasemgrImpl(
const modular::BasemgrSettings& settings,
+ const std::vector<SessionShellSettings>& session_shell_settings,
std::shared_ptr<component::StartupContext> const context,
std::function<void()> on_shutdown)
: settings_(settings),
+ session_shell_settings_(session_shell_settings),
user_provider_impl_("UserProviderImpl"),
context_(std::move(context)),
on_shutdown_(std::move(on_shutdown)),
base_shell_context_binding_(this),
account_provider_context_binding_(this),
authentication_context_provider_binding_(this) {
+ update_session_shell_config();
+
if (!context_->has_environment_services()) {
FXL_LOG(ERROR) << "Failed to receive services from the environment.";
exit(1);
@@ -98,6 +102,37 @@
AddGlobalKeyboardShortcuts(presentation_state_.presentation);
SetShadowTechnique(presentation_state_.shadow_technique);
+
+ // Set the presentation of the given view to the settings of the active
+ // session shell.
+ if (active_session_shell_settings_index_ >= session_shell_settings_.size()) {
+ FXL_LOG(ERROR) << "Active session shell index is "
+ << active_session_shell_settings_index_ << ", but only "
+ << session_shell_settings_.size()
+ << " session shell settings exist.";
+ return;
+ }
+
+ auto active_session_shell_settings =
+ session_shell_settings_[active_session_shell_settings_index_];
+ if (active_session_shell_settings.display_usage !=
+ fuchsia::ui::policy::DisplayUsage::kUnknown) {
+ FXL_DLOG(INFO) << "Setting display usage: "
+ << fidl::ToUnderlying(
+ active_session_shell_settings.display_usage);
+ presentation_state_.presentation->SetDisplayUsage(
+ active_session_shell_settings.display_usage);
+ }
+
+ if (!std::isnan(active_session_shell_settings.screen_width) &&
+ !std::isnan(active_session_shell_settings.screen_height)) {
+ FXL_DLOG(INFO) << "Setting display size: "
+ << active_session_shell_settings.screen_width << " x "
+ << active_session_shell_settings.screen_height;
+ presentation_state_.presentation->SetDisplaySizeInMm(
+ active_session_shell_settings.screen_width,
+ active_session_shell_settings.screen_height);
+ }
}
void BasemgrImpl::StartBaseShell() {
@@ -236,7 +271,7 @@
account_provider_context_binding_.NewBinding());
user_provider_impl_.reset(new UserProviderImpl(
- context_, settings_.sessionmgr, settings_.session_shell,
+ context_, settings_.sessionmgr, session_shell_config_,
settings_.story_shell, account_provider_->primary_service().get(),
token_manager_factory_.get(),
authentication_context_provider_binding_.NewBinding().Bind(),
@@ -326,16 +361,6 @@
}
InitializePresentation(session_shell_view_owner_);
-
- const auto& settings_vector = SessionShellSettings::GetSystemSettings();
- if (active_session_shell_index_ >= settings_vector.size()) {
- FXL_LOG(ERROR) << "Active session shell index is "
- << active_session_shell_index_ << ", but only "
- << settings_vector.size() << " session shells exist.";
- return;
- }
-
- UpdatePresentation(settings_vector[active_session_shell_index_]);
}
void BasemgrImpl::DidLogout() {
@@ -410,45 +435,23 @@
keyboard_capture_listener_bindings_.AddBinding(this));
}
-void BasemgrImpl::UpdatePresentation(const SessionShellSettings& settings) {
- if (settings.display_usage != fuchsia::ui::policy::DisplayUsage::kUnknown) {
- FXL_DLOG(INFO) << "Setting display usage: "
- << fidl::ToUnderlying(settings.display_usage);
- presentation_state_.presentation->SetDisplayUsage(settings.display_usage);
- }
-
- if (!std::isnan(settings.screen_width) &&
- !std::isnan(settings.screen_height)) {
- FXL_DLOG(INFO) << "Setting display size: " << settings.screen_width << " x "
- << settings.screen_height;
- presentation_state_.presentation->SetDisplaySizeInMm(
- settings.screen_width, settings.screen_height);
- }
-}
-
void BasemgrImpl::SwapSessionShell() {
- if (SessionShellSettings::GetSystemSettings().empty()) {
+ if (session_shell_settings_.empty()) {
FXL_DLOG(INFO) << "No session shells has been defined";
return;
}
-
- auto shell_count = SessionShellSettings::GetSystemSettings().size();
-
+ auto shell_count = session_shell_settings_.size();
if (shell_count <= 1) {
- FXL_DLOG(INFO) << "Only one session shell has been defined so switch is disabled";
+ FXL_DLOG(INFO)
+ << "Only one session shell has been defined so switch is disabled";
return;
}
+ active_session_shell_settings_index_ =
+ (active_session_shell_settings_index_ + 1) % shell_count;
- active_session_shell_index_ =
- (active_session_shell_index_ + 1) %
- shell_count;
- const auto& settings =
- SessionShellSettings::GetSystemSettings().at(active_session_shell_index_);
+ update_session_shell_config();
- auto session_shell_config = fuchsia::modular::AppConfig::New();
- session_shell_config->url = settings.name;
-
- user_provider_impl_->SwapSessionShell(std::move(*session_shell_config))
+ user_provider_impl_->SwapSessionShell(CloneStruct(session_shell_config_))
->Then([] { FXL_DLOG(INFO) << "Swapped session shell"; });
}
diff --git a/bin/basemgr/basemgr_impl.h b/bin/basemgr/basemgr_impl.h
index 2eb063a..734d640 100644
--- a/bin/basemgr/basemgr_impl.h
+++ b/bin/basemgr/basemgr_impl.h
@@ -25,6 +25,7 @@
#include "peridot/bin/basemgr/basemgr_settings.h"
#include "peridot/bin/basemgr/cobalt/cobalt.h"
#include "peridot/bin/basemgr/user_provider_impl.h"
+#include "peridot/lib/fidl/clone.h"
#include "peridot/lib/session_shell_settings/session_shell_settings.h"
namespace modular {
@@ -42,9 +43,11 @@
fuchsia::ui::policy::KeyboardCaptureListenerHACK,
modular::UserProviderImpl::Delegate {
public:
- explicit BasemgrImpl(const modular::BasemgrSettings& settings,
- std::shared_ptr<component::StartupContext> context,
- std::function<void()> on_shutdown);
+ explicit BasemgrImpl(
+ const modular::BasemgrSettings& settings,
+ const std::vector<modular::SessionShellSettings>& session_shell_settings,
+ std::shared_ptr<component::StartupContext> context,
+ std::function<void()> on_shutdown);
~BasemgrImpl() override;
@@ -113,7 +116,30 @@
void ToggleClipping();
+ // Updates the session shell app config to the active session shell. Done once
+ // on initialization and every time the session shells are swapped.
+ void update_session_shell_config() {
+ // The session shell settings overrides the session_shell flag passed via
+ // command line. TODO(MF-113): Consolidate the session shell settings.
+ fuchsia::modular::AppConfig session_shell_config;
+ if (session_shell_settings_.empty()) {
+ session_shell_config = CloneStruct(settings_.session_shell);
+ } else {
+ const auto& settings =
+ session_shell_settings_[active_session_shell_settings_index_];
+ session_shell_config.url = settings.name;
+ }
+
+ session_shell_config_ = std::move(session_shell_config);
+ }
+
const modular::BasemgrSettings& settings_; // Not owned nor copied.
+ const std::vector<SessionShellSettings>& session_shell_settings_;
+ fuchsia::modular::AppConfig session_shell_config_;
+ // Used to indicate which settings in |session_shell_settings_| is currently
+ // active.
+ std::vector<SessionShellSettings>::size_type
+ active_session_shell_settings_index_{};
AsyncHolder<UserProviderImpl> user_provider_impl_;
@@ -153,8 +179,6 @@
component::ServiceNamespace service_namespace_;
- std::vector<SessionShellSettings>::size_type active_session_shell_index_{};
-
enum class State {
// normal mode of operation
RUNNING,
diff --git a/bin/basemgr/main.cc b/bin/basemgr/main.cc
index 9fe9fde..03ecbf3 100644
--- a/bin/basemgr/main.cc
+++ b/bin/basemgr/main.cc
@@ -16,6 +16,7 @@
#include "peridot/bin/basemgr/basemgr_impl.h"
#include "peridot/bin/basemgr/basemgr_settings.h"
#include "peridot/bin/basemgr/cobalt/cobalt.h"
+#include "peridot/lib/session_shell_settings/session_shell_settings.h"
fit::deferred_action<fit::closure> SetupCobalt(
modular::BasemgrSettings& settings, async_dispatcher_t* dispatcher,
@@ -34,6 +35,8 @@
}
modular::BasemgrSettings settings(command_line);
+ auto session_shell_settings =
+ modular::SessionShellSettings::GetSystemSettings();
async::Loop loop(&kAsyncLoopConfigAttachToThread);
trace::TraceProvider trace_provider(loop.dispatcher());
auto context = std::shared_ptr<component::StartupContext>(
@@ -44,10 +47,11 @@
// TODO(MF-98): Assess feasibility of injecting the service dependencies
// explicitly rather than passing the entire startup context, for easier
// testing.
- modular::BasemgrImpl basemgr(settings, context, [&loop, &cobalt_cleanup] {
- cobalt_cleanup.call();
- loop.Quit();
- });
+ modular::BasemgrImpl basemgr(settings, session_shell_settings, context,
+ [&loop, &cobalt_cleanup] {
+ cobalt_cleanup.call();
+ loop.Quit();
+ });
loop.Run();
return 0;
diff --git a/bin/basemgr/user_provider_impl.cc b/bin/basemgr/user_provider_impl.cc
index ac796b4..c7f62d7 100644
--- a/bin/basemgr/user_provider_impl.cc
+++ b/bin/basemgr/user_provider_impl.cc
@@ -111,7 +111,7 @@
UserProviderImpl::UserProviderImpl(
std::shared_ptr<component::StartupContext> context,
const fuchsia::modular::AppConfig& sessionmgr,
- const fuchsia::modular::AppConfig& default_session_shell,
+ const fuchsia::modular::AppConfig& session_shell,
const fuchsia::modular::AppConfig& story_shell,
fuchsia::modular::auth::AccountProvider* account_provider,
fuchsia::auth::TokenManagerFactory* token_manager_factory,
@@ -120,7 +120,7 @@
bool use_token_manager_factory, Delegate* const delegate)
: context_(std::move(context)),
sessionmgr_(sessionmgr),
- default_session_shell_(default_session_shell),
+ session_shell_(session_shell),
story_shell_(story_shell),
account_provider_(account_provider),
token_manager_factory_(token_manager_factory),
@@ -577,10 +577,6 @@
agent_token_manager = CreateTokenManager(account_id);
}
- auto session_shell = params.session_shell_config
- ? std::move(*params.session_shell_config)
- : CloneStruct(default_session_shell_);
-
auto view_owner =
delegate_->GetSessionShellViewOwner(std::move(params.view_owner));
auto service_provider =
@@ -588,7 +584,7 @@
auto controller = std::make_unique<UserControllerImpl>(
context_->launcher().get(), CloneStruct(sessionmgr_),
- std::move(session_shell), CloneStruct(story_shell_),
+ CloneStruct(session_shell_), CloneStruct(story_shell_),
std::move(token_provider_factory), std::move(ledger_token_manager),
std::move(agent_token_manager), std::move(account), std::move(view_owner),
std::move(service_provider), std::move(params.user_controller),
diff --git a/bin/basemgr/user_provider_impl.h b/bin/basemgr/user_provider_impl.h
index 3079fa5..3c13c5a 100644
--- a/bin/basemgr/user_provider_impl.h
+++ b/bin/basemgr/user_provider_impl.h
@@ -59,7 +59,7 @@
UserProviderImpl(
std::shared_ptr<component::StartupContext> context,
const fuchsia::modular::AppConfig& sessionmgr,
- const fuchsia::modular::AppConfig& default_session_shell,
+ const fuchsia::modular::AppConfig& session_shell,
const fuchsia::modular::AppConfig& story_shell,
fuchsia::modular::auth::AccountProvider* account_provider,
fuchsia::auth::TokenManagerFactory* token_manager_factory,
@@ -132,7 +132,7 @@
std::shared_ptr<component::StartupContext> context_;
const fuchsia::modular::AppConfig& sessionmgr_; // Neither owned nor copied.
const fuchsia::modular::AppConfig&
- default_session_shell_; // Neither owned nor copied.
+ session_shell_; // Neither owned nor copied.
const fuchsia::modular::AppConfig& story_shell_; // Neither owned nor copied.
fuchsia::modular::auth::AccountProvider* const
account_provider_; // Neither owned nor copied.
