[sessionmgr][refactor] Use StoryModel for StoryState.
TEST=run_modular_tests.sh
MF-149 #done
Change-Id: I31f2cf585cc22ce5d62755c9c983e465caf20fa0
diff --git a/bin/sessionmgr/story/model/apply_mutations.cc b/bin/sessionmgr/story/model/apply_mutations.cc
index fefa18e..21c5b11 100644
--- a/bin/sessionmgr/story/model/apply_mutations.cc
+++ b/bin/sessionmgr/story/model/apply_mutations.cc
@@ -7,6 +7,7 @@
#include "peridot/bin/sessionmgr/story/model/apply_mutations.h"
using fuchsia::modular::StoryVisibilityState;
+using fuchsia::modular::StoryState;
using fuchsia::modular::storymodel::StoryModel;
using fuchsia::modular::storymodel::StoryModelMutation;
@@ -18,6 +19,10 @@
story_model->set_visibility_state(visibility_state);
}
+void ApplySetRuntimeState(const StoryState story_state, StoryModel* story_model) {
+ story_model->set_runtime_state(story_state);
+}
+
} // namespace
StoryModel ApplyMutations(const StoryModel& current_model,
@@ -30,6 +35,9 @@
case StoryModelMutation::Tag::kSetVisibilityState:
ApplySetVisibilityState(command.set_visibility_state(), &new_model);
break;
+ case StoryModelMutation::Tag::kSetRuntimeState:
+ ApplySetRuntimeState(command.set_runtime_state(), &new_model);
+ break;
default:
FXL_LOG(FATAL) << "Unsupported StoryModelMutation: "
<< fidl::ToUnderlying(command.Which());
diff --git a/bin/sessionmgr/story/model/apply_mutations_unittest.cc b/bin/sessionmgr/story/model/apply_mutations_unittest.cc
index 3e708af..fc4ba17 100644
--- a/bin/sessionmgr/story/model/apply_mutations_unittest.cc
+++ b/bin/sessionmgr/story/model/apply_mutations_unittest.cc
@@ -10,6 +10,7 @@
#include "gtest/gtest.h"
#include "peridot/bin/sessionmgr/story/model/apply_mutations.h"
+using fuchsia::modular::StoryState;
using fuchsia::modular::StoryVisibilityState;
using fuchsia::modular::storymodel::StoryModel;
using fuchsia::modular::storymodel::StoryModelMutation;
@@ -17,9 +18,21 @@
namespace modular {
namespace {
+// Test a single StoryModelMutation.set_runtimes_state command to change
+// StoryModel.runtime_state.
+TEST(ApplyMutationsTest, set_runtime_state) {
+ StoryModel before;
+ *before.mutable_runtime_state() = StoryState::STOPPED;
+
+ std::vector<StoryModelMutation> commands(1);
+ commands[0].set_set_runtime_state(StoryState::RUNNING);
+ auto result = ApplyMutations(before, commands);
+ EXPECT_EQ(StoryState::RUNNING, *result.runtime_state());
+}
+
// Test a single StoryModelMutation.set_visibility_state command to change
// StoryModel.visibility_state.
-TEST(ApplyMutationsTest, SingleMutation_set_visibility_state) {
+TEST(ApplyMutationsTest, set_visibility_state) {
StoryModel before;
*before.mutable_visibility_state() = StoryVisibilityState::DEFAULT;
diff --git a/bin/sessionmgr/story/model/story_model_owner.cc b/bin/sessionmgr/story/model/story_model_owner.cc
index 324c81e..4d84d47 100644
--- a/bin/sessionmgr/story/model/story_model_owner.cc
+++ b/bin/sessionmgr/story/model/story_model_owner.cc
@@ -18,8 +18,11 @@
namespace modular {
namespace {
-// Sets default values for all fields of a new StoryModel.
+// Sets default values for all fields of a new StoryModel. Defaults are
+// documented in
+// peridot/lib/fidl/public/fuchsia.modular.storymodel/story_model.fidl.
void InitializeModelDefaults(StoryModel* model) {
+ model->set_runtime_state(fuchsia::modular::StoryState::STOPPED);
model->set_visibility_state(fuchsia::modular::StoryVisibilityState::DEFAULT);
model->set_modules(fidl::VectorPtr<ModuleModel>::New(0));
}
diff --git a/bin/sessionmgr/story/model/story_mutator.cc b/bin/sessionmgr/story/model/story_mutator.cc
index f89c9ca..c91c1e0 100644
--- a/bin/sessionmgr/story/model/story_mutator.cc
+++ b/bin/sessionmgr/story/model/story_mutator.cc
@@ -13,6 +13,13 @@
StoryMutator::StoryMutator() = default;
StoryMutator::~StoryMutator() = default;
+fit::consumer<> StoryMutator::set_runtime_state(
+ fuchsia::modular::StoryState state) {
+ std::vector<StoryModelMutation> commands(1);
+ commands[0].set_set_runtime_state(state);
+ return ExecuteInternal(std::move(commands));
+}
+
fit::consumer<> StoryMutator::set_visibility_state(
fuchsia::modular::StoryVisibilityState state) {
std::vector<StoryModelMutation> commands(1);
diff --git a/bin/sessionmgr/story/model/story_mutator.h b/bin/sessionmgr/story/model/story_mutator.h
index c0f24a6..58f6943 100644
--- a/bin/sessionmgr/story/model/story_mutator.h
+++ b/bin/sessionmgr/story/model/story_mutator.h
@@ -39,6 +39,10 @@
// A failure guarantees that the mutation was not applied and it is safe to
// retry.
+ // Sets the value of |StoryModel.runtime_state|.
+ fit::consumer<> set_runtime_state(
+ fuchsia::modular::StoryState state);
+
// Sets the value of |StoryModel.visibility_state|.
fit::consumer<> set_visibility_state(
fuchsia::modular::StoryVisibilityState state);
diff --git a/bin/sessionmgr/story_runner/story_controller_impl.cc b/bin/sessionmgr/story_runner/story_controller_impl.cc
index 426297d..f800ca4 100644
--- a/bin/sessionmgr/story_runner/story_controller_impl.cc
+++ b/bin/sessionmgr/story_runner/story_controller_impl.cc
@@ -506,7 +506,8 @@
return;
}
- story_controller_impl_->SetState(fuchsia::modular::StoryState::STOPPING);
+ story_controller_impl_->SetRuntimeState(
+ fuchsia::modular::StoryState::STOPPING);
// If this StopCall is part of a bulk operation of story provider that stops
// all stories at once, no DetachView() notification is given to the session
@@ -611,7 +612,7 @@
// been destroyed at this point.
FXL_DCHECK(story_controller_impl_->ongoing_activities_.size() == 0);
- story_controller_impl_->SetState(
+ story_controller_impl_->SetRuntimeState(
fuchsia::modular::StoryState::STOPPED);
story_controller_impl_->DestroyStoryEnvironment();
@@ -1050,7 +1051,7 @@
nullptr /* module_controller_request */, [flow] {}));
}
- story_controller_impl_->SetState(
+ story_controller_impl_->SetRuntimeState(
fuchsia::modular::StoryState::RUNNING);
});
}
@@ -1185,19 +1186,22 @@
};
StoryControllerImpl::StoryControllerImpl(
- fidl::StringPtr story_id, SessionStorage* const session_storage,
- StoryStorage* const story_storage,
+ SessionStorage* const session_storage, StoryStorage* const story_storage,
+ std::unique_ptr<StoryMutator> story_mutator,
+ std::unique_ptr<StoryObserver> story_observer,
StoryVisibilitySystem* const story_visibility_system,
StoryProviderImpl* const story_provider_impl)
- : story_id_(story_id),
+ : story_id_(*story_observer->model().name()),
story_provider_impl_(story_provider_impl),
session_storage_(session_storage),
story_storage_(story_storage),
+ story_mutator_(std::move(story_mutator)),
+ story_observer_(std::move(story_observer)),
story_visibility_system_(story_visibility_system),
- story_shell_context_impl_{story_id, story_provider_impl, this},
+ story_shell_context_impl_{story_id_, story_provider_impl, this},
weak_factory_(this) {
auto story_scope = fuchsia::modular::StoryScope::New();
- story_scope->story_id = story_id;
+ story_scope->story_id = story_id_;
auto scope = fuchsia::modular::ComponentScope::New();
scope->set_story_scope(std::move(*story_scope));
story_provider_impl_->user_intelligence_provider()
@@ -1207,6 +1211,11 @@
[this](fuchsia::modular::ModuleData module_data) {
OnModuleDataUpdated(std::move(module_data));
});
+
+ story_observer_->RegisterListener(
+ [this](const fuchsia::modular::storymodel::StoryModel& model) {
+ NotifyStoryWatchers(model);
+ });
}
StoryControllerImpl::~StoryControllerImpl() = default;
@@ -1217,7 +1226,7 @@
}
bool StoryControllerImpl::IsRunning() {
- switch (state_) {
+ switch (*story_observer_->model().runtime_state()) {
case fuchsia::modular::StoryState::RUNNING:
return true;
case fuchsia::modular::StoryState::STOPPING:
@@ -1226,10 +1235,6 @@
}
}
-fuchsia::modular::StoryState StoryControllerImpl::GetStoryState() const {
- return state_;
-}
-
fidl::VectorPtr<fuchsia::modular::OngoingActivityType>
StoryControllerImpl::GetOngoingActivities() {
fidl::VectorPtr<fuchsia::modular::OngoingActivityType> ongoing_activities;
@@ -1274,7 +1279,9 @@
running_mod_infos_.erase(fit);
}
-fidl::StringPtr StoryControllerImpl::GetStoryId() const { return story_id_; }
+fidl::StringPtr StoryControllerImpl::GetStoryId() const {
+ return *story_observer_->model().name();
+}
void StoryControllerImpl::RequestStoryFocus() {
story_provider_impl_->RequestStoryFocus(story_id_);
@@ -1455,12 +1462,12 @@
// Synced such that if GetInfo() is called after Start() or Stop(), the
// state after the previously invoked operation is returned.
//
- // If this call enters a race with a StoryProvider.DeleteStory() call, resulting in |this|
- // being destroyed, |callback| will be dropped.
+ // If this call enters a race with a StoryProvider.DeleteStory() call,
+ // resulting in |this| being destroyed, |callback| will be dropped.
operation_queue_.Add(new SyncCall([this, callback] {
auto story_info = story_provider_impl_->GetCachedStoryInfo(story_id_);
FXL_CHECK(story_info);
- callback(std::move(*story_info), state_);
+ callback(std::move(*story_info), *story_observer_->model().runtime_state());
}));
}
@@ -1495,7 +1502,7 @@
void StoryControllerImpl::Watch(
fidl::InterfaceHandle<fuchsia::modular::StoryWatcher> watcher) {
auto ptr = watcher.Bind();
- ptr->OnStateChange(state_);
+ NotifyOneStoryWatcher(story_observer_->model(), ptr.get());
watchers_.AddInterfacePtr(std::move(ptr));
}
@@ -1601,19 +1608,22 @@
}
}
-void StoryControllerImpl::SetState(
+void StoryControllerImpl::SetRuntimeState(
const fuchsia::modular::StoryState new_state) {
- if (new_state == state_) {
- return;
- }
+ story_mutator_->set_runtime_state(new_state);
+}
- state_ = new_state;
-
+void StoryControllerImpl::NotifyStoryWatchers(
+ const fuchsia::modular::storymodel::StoryModel& model) {
for (auto& i : watchers_.ptrs()) {
- (*i)->OnStateChange(state_);
+ NotifyOneStoryWatcher(model, (*i).get());
}
+}
- story_provider_impl_->NotifyStoryStateChange(story_id_);
+void StoryControllerImpl::NotifyOneStoryWatcher(
+ const fuchsia::modular::storymodel::StoryModel& model,
+ fuchsia::modular::StoryWatcher* watcher) {
+ watcher->OnStateChange(*model.runtime_state());
}
bool StoryControllerImpl::IsExternalModule(
diff --git a/bin/sessionmgr/story_runner/story_controller_impl.h b/bin/sessionmgr/story_runner/story_controller_impl.h
index 5c9210c..6e517fe 100644
--- a/bin/sessionmgr/story_runner/story_controller_impl.h
+++ b/bin/sessionmgr/story_runner/story_controller_impl.h
@@ -31,6 +31,8 @@
#include <lib/fxl/macros.h>
#include "peridot/bin/sessionmgr/storage/session_storage.h"
+#include "peridot/bin/sessionmgr/story/model/story_observer.h"
+#include "peridot/bin/sessionmgr/story/model/story_mutator.h"
#include "peridot/bin/sessionmgr/story_runner/link_impl.h"
#include "peridot/bin/sessionmgr/story_runner/ongoing_activity_impl.h"
#include "peridot/bin/sessionmgr/story_runner/story_shell_context_impl.h"
@@ -54,8 +56,10 @@
// clients control over the story.
class StoryControllerImpl : fuchsia::modular::StoryController {
public:
- StoryControllerImpl(fidl::StringPtr story_id, SessionStorage* session_storage,
+ StoryControllerImpl(SessionStorage* session_storage,
StoryStorage* story_storage,
+ std::unique_ptr<StoryMutator> story_mutator,
+ std::unique_ptr<StoryObserver> story_observer,
StoryVisibilitySystem* story_visibility_system,
StoryProviderImpl* story_provider_impl);
~StoryControllerImpl() override;
@@ -68,9 +72,6 @@
bool IsRunning();
// Called by StoryProviderImpl.
- fuchsia::modular::StoryState GetStoryState() const;
-
- // Called by StoryProviderImpl.
//
// Returns a list of the ongoing activities in this story.
fidl::VectorPtr<fuchsia::modular::OngoingActivityType> GetOngoingActivities();
@@ -93,7 +94,7 @@
// storage. It is the caller's responsibility to delete |controller|.
void ReleaseModule(ModuleControllerImpl* module_controller_impl);
- // Called by ModuleContextImpl and StoryProviderImpl.
+ // Called by ModuleContextImpl.
fidl::StringPtr GetStoryId() const;
// Called by ModuleContextImpl.
@@ -199,8 +200,12 @@
void OnModuleDataUpdated(fuchsia::modular::ModuleData module_data);
// Misc internal helpers.
- void SetState(fuchsia::modular::StoryState new_state);
- void UpdateStoryState(fuchsia::modular::ModuleState state);
+ void SetRuntimeState(fuchsia::modular::StoryState new_state);
+ void NotifyStoryWatchers(
+ const fuchsia::modular::storymodel::StoryModel& model);
+ void NotifyOneStoryWatcher(
+ const fuchsia::modular::storymodel::StoryModel& model,
+ fuchsia::modular::StoryWatcher* watcher);
void ProcessPendingViews();
std::set<fuchsia::modular::LinkPath> GetActiveLinksInternal();
@@ -219,8 +224,9 @@
// processes.
void DestroyStoryEnvironment();
- // The ID of the story, its state and the context to obtain it from and
- // persist it to.
+ // The ID of the story, copied from |story_observer_| for convenience in
+ // transitioning clients. TODO(thatguy): Remove users of this in favor of
+ // reading from the |story_observer_| directly.
const fidl::StringPtr story_id_;
// True once AttachView() was called. Temporarily needed during transition
@@ -228,15 +234,13 @@
// Cf. MF-121.
bool needs_detach_view_{};
- // This is the canonical source for state. This state is per device and only
- // kept in memory.
- fuchsia::modular::StoryState state_{fuchsia::modular::StoryState::STOPPED};
-
StoryProviderImpl* const story_provider_impl_; // Not owned.
SessionStorage* const session_storage_; // Not owned.
StoryStorage* const story_storage_; // Not owned.
+ std::unique_ptr<StoryMutator> story_mutator_;
+ std::unique_ptr<StoryObserver> story_observer_;
StoryVisibilitySystem* const story_visibility_system_; // Not owned.
// The application environment (which abstracts a zx::job) in which the
diff --git a/bin/sessionmgr/story_runner/story_provider_impl.cc b/bin/sessionmgr/story_runner/story_provider_impl.cc
index c0aa96b..34f09d1 100644
--- a/bin/sessionmgr/story_runner/story_provider_impl.cc
+++ b/bin/sessionmgr/story_runner/story_provider_impl.cc
@@ -166,7 +166,9 @@
container.model_owner->NewMutator());
container.controller_impl = std::make_unique<StoryControllerImpl>(
- story_id_, session_storage_, container.storage.get(),
+ session_storage_, container.storage.get(),
+ container.model_owner->NewMutator(),
+ container.model_owner->NewObserver(),
story_visibility_system.get(), story_provider_impl_);
container.entity_provider = std::make_unique<StoryEntityProvider>(
container.storage.get());
@@ -400,7 +402,7 @@
const auto& container = item.second;
watcher_ptr->OnChange(
CloneStruct(container.current_data->story_info),
- container.controller_impl->GetStoryState(),
+ *container.model_observer->model().runtime_state(),
*container.model_observer->model().visibility_state());
}
watchers_.AddInterfacePtr(std::move(watcher_ptr));
@@ -413,7 +415,7 @@
for (const auto& item : story_runtime_containers_) {
const auto& container = item.second;
watcher_ptr->OnStoryActivityChange(
- container.controller_impl->GetStoryId(),
+ *container.model_observer->model().name(),
container.controller_impl->GetOngoingActivities());
}
activity_watchers_.AddInterfacePtr(std::move(watcher_ptr));
@@ -534,7 +536,7 @@
return;
}
NotifyStoryWatchers(it->second.current_data.get(),
- it->second.controller_impl->GetStoryState(),
+ *it->second.model_observer->model().runtime_state(),
*it->second.model_observer->model().visibility_state());
}
@@ -622,16 +624,16 @@
// StoryData and get runtime state available from it.
//
// Otherwise, use defaults for an unloaded story.
- fuchsia::modular::StoryState state = fuchsia::modular::StoryState::STOPPED;
+ fuchsia::modular::StoryState runtime_state = fuchsia::modular::StoryState::STOPPED;
fuchsia::modular::StoryVisibilityState visibility_state =
fuchsia::modular::StoryVisibilityState::DEFAULT;
auto i = story_runtime_containers_.find(story_data.story_info.id);
if (i != story_runtime_containers_.end()) {
- state = i->second.controller_impl->GetStoryState();
+ runtime_state = *i->second.model_observer->model().runtime_state();
visibility_state = *i->second.model_observer->model().visibility_state();
i->second.current_data = CloneOptional(story_data);
}
- NotifyStoryWatchers(&story_data, state, visibility_state);
+ NotifyStoryWatchers(&story_data, runtime_state, visibility_state);
}
void StoryProviderImpl::OnStoryStorageDeleted(fidl::StringPtr story_id) {
diff --git a/bin/sessionmgr/story_runner/story_provider_impl.h b/bin/sessionmgr/story_runner/story_provider_impl.h
index dcb535e..2da631e 100644
--- a/bin/sessionmgr/story_runner/story_provider_impl.h
+++ b/bin/sessionmgr/story_runner/story_provider_impl.h
@@ -28,9 +28,9 @@
#include "peridot/bin/sessionmgr/agent_runner/agent_runner.h"
#include "peridot/bin/sessionmgr/component_context_impl.h"
#include "peridot/bin/sessionmgr/message_queue/message_queue_manager.h"
-#include "peridot/bin/sessionmgr/story/system.h"
-#include "peridot/bin/sessionmgr/story/model/story_model_owner.h"
#include "peridot/bin/sessionmgr/story/model/noop_story_model_storage.h"
+#include "peridot/bin/sessionmgr/story/model/story_model_owner.h"
+#include "peridot/bin/sessionmgr/story/system.h"
#include "peridot/bin/sessionmgr/story_runner/story_entity_provider.h"
#include "peridot/lib/fidl/app_client.h"
#include "peridot/lib/fidl/environment.h"
@@ -144,9 +144,6 @@
void DetachView(fidl::StringPtr story_id, std::function<void()> done);
// Called by StoryControllerImpl.
- void NotifyStoryStateChange(fidl::StringPtr story_id);
-
- // Called by StoryControllerImpl.
void NotifyStoryActivityChange(
fidl::StringPtr story_id,
fidl::VectorPtr<fuchsia::modular::OngoingActivityType>
@@ -227,6 +224,10 @@
void OnStoryStorageUpdated(fidl::StringPtr story_id,
fuchsia::modular::internal::StoryData story_data);
+ // Called indirectly through observation of loaded StoryModels. Calls
+ // NotifyStoryWatchers().
+ void NotifyStoryStateChange(fidl::StringPtr story_id);
+
void NotifyStoryWatchers(
const fuchsia::modular::internal::StoryData* story_data,
fuchsia::modular::StoryState story_state,
diff --git a/public/fidl/fuchsia.modular.storymodel/story_model.fidl b/public/fidl/fuchsia.modular.storymodel/story_model.fidl
index 8bf437d..a496a22 100644
--- a/public/fidl/fuchsia.modular.storymodel/story_model.fidl
+++ b/public/fidl/fuchsia.modular.storymodel/story_model.fidl
@@ -17,16 +17,21 @@
// Always set. Immutable.
1: string:MAX_STORY_NAME_LENGTH name;
+ // An enum describing if the story is RUNNING, STOPPING, STOPPED.
+ //
+ // Always set. Defaults to StoryState::STOPPED.
+ 2: fuchsia.modular.StoryState runtime_state;
+
// An enum describing how the story should be displayed, when focused,
// in the StoryShell.
//
// Always set. Defaults to StoryVisibilityState::DEFAULT.
- 2: fuchsia.modular.StoryVisibilityState visibility_state;
+ 3: fuchsia.modular.StoryVisibilityState visibility_state;
// A list of modules present in the story.
//
// Always set. Defaults to an empty list.
- 3: vector<ModuleModel>:MAX_MODULES_PER_STORY modules;
+ 4: vector<ModuleModel>:MAX_MODULES_PER_STORY modules;
};
table ModuleModel {
diff --git a/public/fidl/fuchsia.modular.storymodel/story_model_mutation.fidl b/public/fidl/fuchsia.modular.storymodel/story_model_mutation.fidl
index 05d6f15..9d096e6 100644
--- a/public/fidl/fuchsia.modular.storymodel/story_model_mutation.fidl
+++ b/public/fidl/fuchsia.modular.storymodel/story_model_mutation.fidl
@@ -16,4 +16,7 @@
union StoryModelMutation {
// Sets the value of |StoryModel.visibility_state|.
fuchsia.modular.StoryVisibilityState set_visibility_state;
+
+ // Sets the value of |StoryModel.runtime_state|.
+ fuchsia.modular.StoryState set_runtime_state;
};
