[modular][story_provider_impl] Simplify flow for notifying of story changes.
Instead of caching only StoryInfo for a loaded story, and then
async-fetching StoryData, we cache StoryData directly. This takes an
async-read out of the NotifyStoryWatchers path.
TEST=existing
Change-Id: Ibbedce1adeedfb4eac76d4cf2c78a7abcbd97898
diff --git a/bin/sessionmgr/story_runner/story_provider_impl.cc b/bin/sessionmgr/story_runner/story_provider_impl.cc
index 902ffc3..bf0723b 100644
--- a/bin/sessionmgr/story_runner/story_provider_impl.cc
+++ b/bin/sessionmgr/story_runner/story_provider_impl.cc
@@ -132,33 +132,34 @@
return;
// Operation finishes since |flow| goes out of scope.
}
- story_info_ = CloneOptional(story_data->story_info);
- Cont(flow);
+ Cont(std::move(story_data), flow);
});
}
- void Cont(FlowToken flow) {
+ void Cont(fuchsia::modular::internal::StoryDataPtr story_data,
+ FlowToken flow) {
session_storage_->GetStoryStorage(story_id_)->WeakThen(
GetWeakPtr(),
- [this, flow](std::unique_ptr<StoryStorage> story_storage) {
- struct StoryRuntimeContainer container;
- container.storage = std::move(story_storage);
- container.controller_impl = std::make_unique<StoryControllerImpl>(
- story_id_, session_storage_, container.storage.get(),
- story_provider_impl_);
- container.current_info = std::move(story_info_);
- container.entity_provider =
- std::make_unique<StoryEntityProvider>(container.storage.get());
- auto it = story_provider_impl_->story_runtime_containers_.emplace(
- story_id_, std::move(container));
- story_controller_container_ = &it.first->second;
- });
+ fxl::MakeCopyable(
+ [this, story_data = std::move(story_data),
+ flow](std::unique_ptr<StoryStorage> story_storage) mutable {
+ struct StoryRuntimeContainer container;
+ container.storage = std::move(story_storage);
+ container.controller_impl = std::make_unique<StoryControllerImpl>(
+ story_id_, session_storage_, container.storage.get(),
+ story_provider_impl_);
+ container.current_data = std::move(story_data);
+ container.entity_provider = std::make_unique<StoryEntityProvider>(
+ container.storage.get());
+ auto it = story_provider_impl_->story_runtime_containers_.emplace(
+ story_id_, std::move(container));
+ story_controller_container_ = &it.first->second;
+ }));
}
StoryProviderImpl* const story_provider_impl_; // not owned
SessionStorage* const session_storage_; // not owned
const fidl::StringPtr story_id_;
- fuchsia::modular::StoryInfoPtr story_info_;
StoryRuntimeContainer* story_controller_container_ = nullptr;
@@ -348,7 +349,7 @@
auto watcher_ptr = watcher.Bind();
for (const auto& item : story_runtime_containers_) {
const auto& container = item.second;
- watcher_ptr->OnChange(CloneStruct(*container.current_info),
+ watcher_ptr->OnChange(CloneStruct(container.current_data->story_info),
container.controller_impl->GetStoryState(),
container.controller_impl->GetStoryVisibilityState());
}
@@ -450,27 +451,15 @@
void StoryProviderImpl::NotifyStoryStateChange(
fidl::StringPtr story_id, const fuchsia::modular::StoryState story_state,
const fuchsia::modular::StoryVisibilityState story_visibility_state) {
- auto on_run =
- Future<>::Create("StoryProviderImpl.NotifyStoryStateChange.on_run");
- auto done = on_run
- ->AsyncMap([this, story_id] {
- return session_storage_->GetStoryData(story_id);
- })
- ->Then([this, story_id, story_state, story_visibility_state](
- fuchsia::modular::internal::StoryDataPtr data) {
- auto it = story_runtime_containers_.find(story_id);
- if (it == story_runtime_containers_.end()) {
- // If this call arrives while DeleteStory() is in
- // progress, the story controller might already be gone
- // from here.
- return;
- }
- NotifyStoryWatchers(data.get(), story_state,
- story_visibility_state);
- });
- std::function<void()> callback = [] {};
- operation_queue_.Add(WrapFutureAsOperation(
- "StoryProviderImpl::NotifyStoryStateChange", on_run, done, callback));
+ auto it = story_runtime_containers_.find(story_id);
+ if (it == story_runtime_containers_.end()) {
+ // If this call arrives while DeleteStory() is in
+ // progress, the story controller might already be gone
+ // from here.
+ return;
+ }
+ NotifyStoryWatchers(it->second.current_data.get(), story_state,
+ story_visibility_state);
}
void StoryProviderImpl::NotifyStoryActivityChange(
@@ -569,12 +558,10 @@
void StoryProviderImpl::OnStoryStorageUpdated(
fidl::StringPtr story_id,
fuchsia::modular::internal::StoryData story_data) {
- // HACK(jimbe) We don't have the page and it's expensive to get it, so
- // just mark it as STOPPED. We know it's not running or we'd have a
- // fuchsia::modular::StoryController.
+ // If we have a StoryRuntimeContainer for this story id, update our cached
+ // StoryData and get runtime state available from it.
//
- // If we have a StoryControllerImpl for this story id, update our cached
- // fuchsia::modular::StoryInfo.
+ // Otherwise, use defaults for an unloaded story.
fuchsia::modular::StoryState state = fuchsia::modular::StoryState::STOPPED;
fuchsia::modular::StoryVisibilityState visibility_state =
fuchsia::modular::StoryVisibilityState::DEFAULT;
@@ -582,9 +569,8 @@
if (i != story_runtime_containers_.end()) {
state = i->second.controller_impl->GetStoryState();
visibility_state = i->second.controller_impl->GetStoryVisibilityState();
- i->second.current_info = CloneOptional(story_data.story_info);
+ i->second.current_data = CloneOptional(story_data);
}
-
NotifyStoryWatchers(&story_data, state, visibility_state);
}
@@ -632,7 +618,7 @@
}
void StoryProviderImpl::NotifyStoryWatchers(
- const fuchsia::modular::internal::StoryData* const story_data,
+ const fuchsia::modular::internal::StoryData* story_data,
const fuchsia::modular::StoryState story_state,
const fuchsia::modular::StoryVisibilityState story_visibility_state) {
if (!story_data || story_data->story_options.kind_of_proto_story) {
diff --git a/bin/sessionmgr/story_runner/story_provider_impl.h b/bin/sessionmgr/story_runner/story_provider_impl.h
index 625abbe..8aac267 100644
--- a/bin/sessionmgr/story_runner/story_provider_impl.h
+++ b/bin/sessionmgr/story_runner/story_provider_impl.h
@@ -210,7 +210,7 @@
fuchsia::modular::internal::StoryData story_data);
void NotifyStoryWatchers(
- const fuchsia::modular::internal::StoryData* const story_data,
+ const fuchsia::modular::internal::StoryData* story_data,
fuchsia::modular::StoryState story_state,
fuchsia::modular::StoryVisibilityState story_visibility_state);
@@ -247,14 +247,13 @@
// Only user logout or delete story calls ever remove story controllers from
// this collection, but controllers for stopped stories stay in it.
//
- // Also keeps a cached version of the fuchsia::modular::StoryInfo for every
- // story, to send it to newly registered story provider watchers, and to story
- // provider watchers when only the story state changes.
+ // Also keeps a cached version of the StoryData for every story so it does
+ // not have to be loaded from disk when querying about this story.
struct StoryRuntimeContainer {
std::unique_ptr<StoryControllerImpl> controller_impl;
std::unique_ptr<StoryStorage> storage;
std::unique_ptr<StoryEntityProvider> entity_provider;
- fuchsia::modular::StoryInfoPtr current_info;
+ fuchsia::modular::internal::StoryDataPtr current_data;
};
std::map<std::string, StoryRuntimeContainer> story_runtime_containers_;
