[modular] Don't auto-start embedded modules when story reinflates.
Additional clean up:
* introduce a AddModParams struct for shuffling add-module parameters
through to various places.
* rename StoryController::StartModule -> AddModuleToStory
* AddMod story command will fail with an empty mod_name. multi-element
mod_name splits into a mod_name and a parent mod.
MF-164 #done
Test=Extend 'embed_shell' integration test to restart an existing story
and see that an embedded module is not auto-started.
Change-Id: I9652369750c61d87cc344455e56de6ded9ed3848
diff --git a/bin/sessionmgr/puppet_master/command_runners/add_mod_command_runner.cc b/bin/sessionmgr/puppet_master/command_runners/add_mod_command_runner.cc
index 55bc3b2..de39cd5 100644
--- a/bin/sessionmgr/puppet_master/command_runners/add_mod_command_runner.cc
+++ b/bin/sessionmgr/puppet_master/command_runners/add_mod_command_runner.cc
@@ -26,13 +26,33 @@
FXL_CHECK(command.is_add_mod());
auto& add_mod = command.add_mod();
+ if (add_mod.mod_name.size() == 0) {
+ fuchsia::modular::ExecuteResult result;
+ result.status = fuchsia::modular::ExecuteStatus::INVALID_COMMAND;
+ result.error_message = "A Module name must be specified";
+ done(result);
+ return;
+ }
+
+ AddModParams params;
+ params.parent_mod_path = std::move(add_mod.surface_parent_mod_name);
+ if (add_mod.mod_name.size() == 1) {
+ params.mod_name = add_mod.mod_name[0];
+ } else {
+ params.mod_name = add_mod.mod_name.back();
+
+ add_mod.mod_name.pop_back();
+ params.parent_mod_path = add_mod.mod_name;
+ }
+ params.is_embedded = false;
+ params.intent = std::move(add_mod.intent);
+ params.surface_relation = std::make_unique<fuchsia::modular::SurfaceRelation>(
+ std::move(add_mod.surface_relation));
+ params.module_source = fuchsia::modular::ModuleSource::EXTERNAL;
+
AddAddModOperation(
&operation_queue_, story_storage, module_resolver_, entity_resolver_,
- std::move(add_mod.mod_name), std::move(add_mod.intent),
- std::make_unique<fuchsia::modular::SurfaceRelation>(
- std::move(add_mod.surface_relation)),
- std::move(add_mod.surface_parent_mod_name),
- fuchsia::modular::ModuleSource::EXTERNAL,
+ std::move(params),
[done = std::move(done)](fuchsia::modular::ExecuteResult result,
fuchsia::modular::ModuleData module_data) {
done(std::move(result));
diff --git a/bin/sessionmgr/puppet_master/command_runners/add_mod_command_runner_unittest.cc b/bin/sessionmgr/puppet_master/command_runners/add_mod_command_runner_unittest.cc
index cef88ef..7337588 100644
--- a/bin/sessionmgr/puppet_master/command_runners/add_mod_command_runner_unittest.cc
+++ b/bin/sessionmgr/puppet_master/command_runners/add_mod_command_runner_unittest.cc
@@ -606,8 +606,8 @@
// Get the link path for the param of the mod we created.
fuchsia::modular::LinkPath link_path;
std::vector<std::string> full_path{"parent_mod", "mod"};
- story_storage_->ReadModuleData(full_path)
- ->Then([&](fuchsia::modular::ModuleDataPtr result) {
+ story_storage_->ReadModuleData(full_path)->Then(
+ [&](fuchsia::modular::ModuleDataPtr result) {
for (auto& entry : result->parameter_map.entries) {
if (entry.name == "param_json") {
fidl::Clone(entry.link_path, &link_path);
diff --git a/bin/sessionmgr/puppet_master/command_runners/operation_calls/add_mod_call.cc b/bin/sessionmgr/puppet_master/command_runners/operation_calls/add_mod_call.cc
index 08e7616..3afea2f 100644
--- a/bin/sessionmgr/puppet_master/command_runners/operation_calls/add_mod_call.cc
+++ b/bin/sessionmgr/puppet_master/command_runners/operation_calls/add_mod_call.cc
@@ -25,20 +25,12 @@
AddModCall(StoryStorage* const story_storage,
fuchsia::modular::ModuleResolver* const module_resolver,
fuchsia::modular::EntityResolver* const entity_resolver,
- std::vector<std::string> mod_name,
- fuchsia::modular::Intent intent,
- fuchsia::modular::SurfaceRelationPtr surface_relation,
- std::vector<std::string> surface_parent_mod_name,
- fuchsia::modular::ModuleSource module_source, ResultCall done)
+ AddModParams add_mod_params, ResultCall done)
: Operation("AddModCommandRunner::AddModCall", std::move(done)),
story_storage_(story_storage),
module_resolver_(module_resolver),
entity_resolver_(entity_resolver),
- mod_name_(std::move(mod_name)),
- intent_(std::move(intent)),
- surface_relation_(std::move(surface_relation)),
- surface_parent_mod_name_(std::move(surface_parent_mod_name)),
- module_source_(module_source) {}
+ add_mod_params_(std::move(add_mod_params)) {}
private:
void Run() override {
@@ -52,10 +44,11 @@
// resolve the (action, parameter) and the supplied optional handler to a
// module. If the module resolver doesn't recognize a supplied handler, we
// forgivingly execute the handler anyway.
- if (!intent_.action.is_null()) {
+ if (!add_mod_params_.intent.action.is_null()) {
AddFindModulesOperation(
&operation_queue_, module_resolver_, entity_resolver_,
- CloneOptional(intent_), surface_parent_mod_name_,
+ CloneOptional(add_mod_params_.intent),
+ add_mod_params_.parent_mod_path,
[this, flow](fuchsia::modular::ExecuteResult result,
fuchsia::modular::FindModulesResponse response) {
if (result.status != fuchsia::modular::ExecuteStatus::OK) {
@@ -81,7 +74,7 @@
} break;
case fuchsia::modular::FindModulesStatus::UNKNOWN_HANDLER: {
- candidate_module_.module_id = intent_.handler;
+ candidate_module_.module_id = add_mod_params_.intent.handler;
} break;
}
@@ -89,9 +82,9 @@
});
} else {
// We arrive here if the Intent has a handler, but no action.
- FXL_DCHECK(!intent_.handler.is_null())
+ FXL_DCHECK(!add_mod_params_.intent.handler.is_null())
<< "Cannot start a module without an action or a handler";
- candidate_module_.module_id = intent_.handler;
+ candidate_module_.module_id = add_mod_params_.intent.handler;
CreateLinks(flow);
}
@@ -104,9 +97,8 @@
return;
// Operation finishes since |flow| goes out of scope.
}
- auto full_module_path = surface_parent_mod_name_;
- full_module_path.insert(full_module_path.end(), mod_name_.begin(),
- mod_name_.end());
+ auto full_module_path = add_mod_params_.parent_mod_path;
+ full_module_path.push_back(add_mod_params_.mod_name);
AddInitializeChainOperation(
&operation_queue_, story_storage_, std::move(full_module_path),
std::move(parameter_info_),
@@ -130,9 +122,9 @@
std::vector<FuturePtr<fuchsia::modular::CreateModuleParameterMapEntry>>
did_get_entries;
- did_get_entries.reserve(intent_.parameters->size());
+ did_get_entries.reserve(add_mod_params_.intent.parameters->size());
- for (auto& param : *intent_.parameters) {
+ for (auto& param : *add_mod_params_.intent.parameters) {
fuchsia::modular::CreateModuleParameterMapEntry entry;
entry.key = param.name;
@@ -195,14 +187,15 @@
fuchsia::modular::ModuleParameterMapPtr map) {
fidl::Clone(*map, &out_module_data_.parameter_map);
out_module_data_.module_url = candidate_module_.module_id;
- out_module_data_.module_path = surface_parent_mod_name_;
- out_module_data_.module_path.insert(out_module_data_.module_path.end(),
- mod_name_.begin(), mod_name_.end());
- out_module_data_.module_source = module_source_;
+ out_module_data_.module_path = add_mod_params_.parent_mod_path;
+ out_module_data_.module_path.push_back(add_mod_params_.mod_name);
+ out_module_data_.module_source = add_mod_params_.module_source;
out_module_data_.module_deleted = false;
- fidl::Clone(surface_relation_, &out_module_data_.surface_relation);
- out_module_data_.intent =
- std::make_unique<fuchsia::modular::Intent>(std::move(intent_));
+ fidl::Clone(add_mod_params_.surface_relation,
+ &out_module_data_.surface_relation);
+ out_module_data_.is_embedded = add_mod_params_.is_embedded;
+ out_module_data_.intent = std::make_unique<fuchsia::modular::Intent>(
+ std::move(add_mod_params_.intent));
// Operation stays alive until flow goes out of scope.
fuchsia::modular::ModuleData module_data;
@@ -214,11 +207,7 @@
StoryStorage* const story_storage_; // Not owned.
fuchsia::modular::ModuleResolver* const module_resolver_; // Not owned.
fuchsia::modular::EntityResolver* const entity_resolver_; // Not owned.
- std::vector<std::string> mod_name_;
- fuchsia::modular::Intent intent_;
- fuchsia::modular::SurfaceRelationPtr surface_relation_;
- std::vector<std::string> surface_parent_mod_name_;
- fuchsia::modular::ModuleSource module_source_;
+ modular::AddModParams add_mod_params_;
fuchsia::modular::FindModulesResult candidate_module_;
fuchsia::modular::CreateModuleParameterMapInfoPtr parameter_info_;
fuchsia::modular::ModuleData out_module_data_;
@@ -234,21 +223,16 @@
} // namespace
-void AddAddModOperation(
- OperationContainer* const container, StoryStorage* const story_storage,
- fuchsia::modular::ModuleResolver* const module_resolver,
- fuchsia::modular::EntityResolver* const entity_resolver,
- std::vector<std::string> mod_name, fuchsia::modular::Intent intent,
- fuchsia::modular::SurfaceRelationPtr surface_relation,
- std::vector<std::string> surface_parent_mod_name,
- fuchsia::modular::ModuleSource module_source,
- std::function<void(fuchsia::modular::ExecuteResult,
- fuchsia::modular::ModuleData)>
- done) {
- container->Add(new AddModCall(
- story_storage, module_resolver, entity_resolver, std::move(mod_name),
- std::move(intent), std::move(surface_relation),
- std::move(surface_parent_mod_name), module_source, std::move(done)));
+void AddAddModOperation(OperationContainer* const container,
+ StoryStorage* const story_storage,
+ fuchsia::modular::ModuleResolver* const module_resolver,
+ fuchsia::modular::EntityResolver* const entity_resolver,
+ AddModParams add_mod_params,
+ std::function<void(fuchsia::modular::ExecuteResult,
+ fuchsia::modular::ModuleData)>
+ done) {
+ container->Add(new AddModCall(story_storage, module_resolver, entity_resolver,
+ std::move(add_mod_params), std::move(done)));
}
} // namespace modular
diff --git a/bin/sessionmgr/puppet_master/command_runners/operation_calls/add_mod_call.h b/bin/sessionmgr/puppet_master/command_runners/operation_calls/add_mod_call.h
index 75d4459..752c35e 100644
--- a/bin/sessionmgr/puppet_master/command_runners/operation_calls/add_mod_call.h
+++ b/bin/sessionmgr/puppet_master/command_runners/operation_calls/add_mod_call.h
@@ -12,17 +12,37 @@
namespace modular {
-void AddAddModOperation(
- OperationContainer* container, StoryStorage* story_storage,
- fuchsia::modular::ModuleResolver* module_resolver,
- fuchsia::modular::EntityResolver* entity_resolver,
- std::vector<std::string> mod_name, fuchsia::modular::Intent intent,
- fuchsia::modular::SurfaceRelationPtr surface_relation,
- std::vector<std::string> surface_parent_mod_name,
- fuchsia::modular::ModuleSource module_source,
- std::function<void(fuchsia::modular::ExecuteResult,
- fuchsia::modular::ModuleData)>
- done);
+// This struct contains common parameters needed to add a module to a story,
+// useful as a single place to add more parameters that need shuffling around.
+// See story_command.fidl and module_data.fidl for a detailed description of
+// what these parameters mean.
+struct AddModParams {
+ // This parent module's module path. If empty, this mod has no parent module.
+ std::vector<std::string> parent_mod_path;
+ // Module name given to this module path (parent_mod_path + mod_name is this
+ // module's module path).
+ std::string mod_name;
+
+ // True if this is an embedded mod (as opposed to being arranged by the story
+ // shell), in which case this mod's view will be embedded by its parent mod
+ // (represented by parent_mod_path).
+ bool is_embedded;
+
+ // See |fuchsia::modular::ModuleData| (module_data.fidl) for a detailed
+ // description of what these parameters mean.
+ fuchsia::modular::Intent intent;
+ fuchsia::modular::SurfaceRelationPtr surface_relation;
+ fuchsia::modular::ModuleSource module_source;
+};
+
+void AddAddModOperation(OperationContainer* container,
+ StoryStorage* story_storage,
+ fuchsia::modular::ModuleResolver* module_resolver,
+ fuchsia::modular::EntityResolver* entity_resolver,
+ AddModParams add_mod_params,
+ std::function<void(fuchsia::modular::ExecuteResult,
+ fuchsia::modular::ModuleData)>
+ done);
} // namespace modular
diff --git a/bin/sessionmgr/storage/story_storage_xdr.cc b/bin/sessionmgr/storage/story_storage_xdr.cc
index 3129040..baed8c6 100644
--- a/bin/sessionmgr/storage/story_storage_xdr.cc
+++ b/bin/sessionmgr/storage/story_storage_xdr.cc
@@ -191,8 +191,25 @@
xdr->Field("chain_data", &data->parameter_map, XdrModuleParameterMap);
}
+void XdrModuleData_v6(XdrContext* const xdr,
+ fuchsia::modular::ModuleData* const data) {
+ if (!xdr->Version(6)) {
+ return;
+ }
+ xdr->Field("url", &data->module_url);
+ xdr->Field("module_path", &data->module_path);
+ xdr->Field("module_source", &data->module_source);
+ xdr->Field("surface_relation", &data->surface_relation, XdrSurfaceRelation);
+ xdr->Field("module_deleted", &data->module_deleted);
+ xdr->Field("intent", &data->intent, XdrIntent);
+ // NOTE: the JSON field naming doesn't match the FIDL struct naming because
+ // the field name in FIDL was changed.
+ xdr->Field("chain_data", &data->parameter_map, XdrModuleParameterMap);
+ xdr->Field("is_embedded", &data->is_embedded);
+}
+
XdrFilterType<fuchsia::modular::ModuleData> XdrModuleData[] = {
- XdrModuleData_v5, XdrModuleData_v4, XdrModuleData_v3,
+ XdrModuleData_v6, XdrModuleData_v5, XdrModuleData_v4, XdrModuleData_v3,
XdrModuleData_v2, XdrModuleData_v1, nullptr,
};
diff --git a/bin/sessionmgr/story_runner/module_context_impl.cc b/bin/sessionmgr/story_runner/module_context_impl.cc
index ba5604c..ff78b8d 100644
--- a/bin/sessionmgr/story_runner/module_context_impl.cc
+++ b/bin/sessionmgr/story_runner/module_context_impl.cc
@@ -11,8 +11,8 @@
#include <lib/fxl/strings/join_strings.h>
#include "peridot/bin/sessionmgr/storage/constants_and_utils.h"
-#include "peridot/bin/sessionmgr/story_runner/story_controller_impl.h"
#include "peridot/bin/sessionmgr/story/systems/story_visibility_system.h"
+#include "peridot/bin/sessionmgr/story_runner/story_controller_impl.h"
#include "peridot/lib/fidl/clone.h"
namespace modular {
@@ -75,10 +75,16 @@
module_controller,
fidl::InterfaceRequest<fuchsia::ui::viewsv1token::ViewOwner> view_owner,
EmbedModuleCallback callback) {
- story_controller_impl_->EmbedModule(
- module_data_->module_path, name, fidl::MakeOptional(std::move(intent)),
- std::move(module_controller), std::move(view_owner),
- fuchsia::modular::ModuleSource::INTERNAL, callback);
+ AddModParams params;
+ params.parent_mod_path = module_data_->module_path;
+ params.mod_name = name;
+ params.intent = std::move(intent);
+ params.module_source = fuchsia::modular::ModuleSource::INTERNAL;
+ params.surface_relation = nullptr;
+ params.is_embedded = true;
+ story_controller_impl_->EmbedModule(std::move(params),
+ std::move(module_controller),
+ std::move(view_owner), callback);
}
void ModuleContextImpl::AddModuleToStory(
@@ -87,10 +93,15 @@
module_controller,
fuchsia::modular::SurfaceRelationPtr surface_relation,
AddModuleToStoryCallback callback) {
- story_controller_impl_->StartModule(
- module_data_->module_path, name, fidl::MakeOptional(std::move(intent)),
- std::move(module_controller), std::move(surface_relation),
- fuchsia::modular::ModuleSource::INTERNAL, callback);
+ AddModParams params;
+ params.parent_mod_path = module_data_->module_path;
+ params.mod_name = name;
+ params.intent = std::move(intent);
+ params.module_source = fuchsia::modular::ModuleSource::INTERNAL;
+ params.surface_relation = std::move(surface_relation);
+ params.is_embedded = false;
+ story_controller_impl_->AddModuleToStory(
+ std::move(params), std::move(module_controller), callback);
}
void ModuleContextImpl::StartContainerInShell(
diff --git a/bin/sessionmgr/story_runner/module_context_impl.h b/bin/sessionmgr/story_runner/module_context_impl.h
index 7150642..35cb4ca 100644
--- a/bin/sessionmgr/story_runner/module_context_impl.h
+++ b/bin/sessionmgr/story_runner/module_context_impl.h
@@ -17,6 +17,7 @@
#include <lib/fxl/macros.h>
#include "peridot/bin/sessionmgr/component_context_impl.h"
+#include "peridot/bin/sessionmgr/puppet_master/command_runners/operation_calls/add_mod_call.h"
namespace modular {
diff --git a/bin/sessionmgr/story_runner/story_controller_impl.cc b/bin/sessionmgr/story_runner/story_controller_impl.cc
index f7ac3af..487e4eb 100644
--- a/bin/sessionmgr/story_runner/story_controller_impl.cc
+++ b/bin/sessionmgr/story_runner/story_controller_impl.cc
@@ -140,8 +140,7 @@
module_data_(std::move(module_data)),
module_controller_request_(std::move(module_controller_request)),
view_owner_request_(std::move(view_owner_request)),
- start_time_(zx_clock_get(ZX_CLOCK_UTC)) {
- }
+ start_time_(zx_clock_get(ZX_CLOCK_UTC)) {}
private:
void Run() override {
@@ -393,11 +392,13 @@
}
void LoadModuleManifest(FlowToken flow) {
- story_controller_impl_->story_provider_impl_->module_facet_reader()->GetModuleManifest(
- module_data_.module_url, [this, flow] (fuchsia::modular::ModuleManifestPtr manifest) {
- module_manifest_ = std::move(manifest);
- MaybeConnectViewToStoryShell(flow);
- });
+ story_controller_impl_->story_provider_impl_->module_facet_reader()
+ ->GetModuleManifest(
+ module_data_.module_url,
+ [this, flow](fuchsia::modular::ModuleManifestPtr manifest) {
+ module_manifest_ = std::move(manifest);
+ MaybeConnectViewToStoryShell(flow);
+ });
}
void MaybeConnectViewToStoryShell(FlowToken flow) {
@@ -431,8 +432,13 @@
auto manifest_clone = fuchsia::modular::ModuleManifest::New();
fidl::Clone(module_manifest_, &manifest_clone);
- auto surface_relation_clone = fuchsia::modular::SurfaceRelation::New();
- module_data_.surface_relation->Clone(surface_relation_clone.get());
+
+ fuchsia::modular::SurfaceRelationPtr surface_relation_clone;
+ if (module_data_.surface_relation) {
+ surface_relation_clone = fuchsia::modular::SurfaceRelation::New();
+ module_data_.surface_relation->Clone(surface_relation_clone.get());
+ }
+
story_controller_impl_->pending_views_.emplace(
ModulePathToSurfaceID(module_data_.module_path),
PendingView{module_data_.module_path, std::move(manifest_clone),
@@ -716,8 +722,8 @@
// we do not have the necessary capabilities (the ViewOwner). However,
// mods added through ModuleContext.AddModuleToStory() can be started
// automatically.
- if (module_data_.module_source !=
- fuchsia::modular::ModuleSource::EXTERNAL) {
+ if (module_data_.module_source ==
+ fuchsia::modular::ModuleSource::INTERNAL) {
return;
}
@@ -798,25 +804,17 @@
: public Operation<fuchsia::modular::StartModuleStatus> {
public:
AddIntentCall(StoryControllerImpl* const story_controller_impl,
- std::vector<std::string> requesting_module_path,
- const std::string& module_name,
- fuchsia::modular::IntentPtr intent,
+ AddModParams add_mod_params,
fidl::InterfaceRequest<fuchsia::modular::ModuleController>
module_controller_request,
- fuchsia::modular::SurfaceRelationPtr surface_relation,
fidl::InterfaceRequest<fuchsia::ui::viewsv1token::ViewOwner>
view_owner_request,
- const fuchsia::modular::ModuleSource module_source,
ResultCall result_call)
: Operation("StoryControllerImpl::AddIntentCall", std::move(result_call)),
story_controller_impl_(story_controller_impl),
- requesting_module_path_(std::move(requesting_module_path)),
- module_name_(module_name),
- intent_(std::move(intent)),
+ add_mod_params_(std::move(add_mod_params)),
module_controller_request_(std::move(module_controller_request)),
- surface_relation_(std::move(surface_relation)),
- view_owner_request_(std::move(view_owner_request)),
- module_source_(module_source) {}
+ view_owner_request_(std::move(view_owner_request)) {}
private:
void Run() {
@@ -825,9 +823,7 @@
&operation_queue_, story_controller_impl_->story_storage_,
story_controller_impl_->story_provider_impl_->module_resolver(),
story_controller_impl_->story_provider_impl_->entity_resolver(),
- std::vector<std::string>({module_name_}), std::move(*intent_),
- std::move(surface_relation_), std::move(requesting_module_path_),
- module_source_,
+ std::move(add_mod_params_),
[this, flow](fuchsia::modular::ExecuteResult result,
fuchsia::modular::ModuleData module_data) {
if (result.status ==
@@ -842,11 +838,11 @@
<< "error response with message: " << result.error_message;
}
module_data_ = std::move(module_data);
- MaybeLaunchModule(flow);
+ LaunchModuleIfStoryRunning(flow);
});
}
- void MaybeLaunchModule(FlowToken flow) {
+ void LaunchModuleIfStoryRunning(FlowToken flow) {
if (story_controller_impl_->IsRunning()) {
// TODO(thatguy): Should we be checking surface_relation also?
if (!view_owner_request_) {
@@ -879,17 +875,13 @@
OperationQueue operation_queue_;
StoryControllerImpl* const story_controller_impl_;
- // Arguments passed in from the constructor. Some are used to initialize
+ // Some of the fields in add_mod_params_ are used to initializel
// module_data_ in AddModuleFromResult().
- std::vector<std::string> requesting_module_path_;
- const std::string module_name_;
- fuchsia::modular::IntentPtr intent_;
+ AddModParams add_mod_params_;
fidl::InterfaceRequest<fuchsia::modular::ModuleController>
module_controller_request_;
- fuchsia::modular::SurfaceRelationPtr surface_relation_;
fidl::InterfaceRequest<fuchsia::ui::viewsv1token::ViewOwner>
view_owner_request_;
- const fuchsia::modular::ModuleSource module_source_;
// Created by AddModuleFromResult, and ultimately written to story state.
fuchsia::modular::ModuleData module_data_;
@@ -902,9 +894,8 @@
class StoryControllerImpl::StartCall : public Operation<> {
public:
- StartCall(
- StoryControllerImpl* const story_controller_impl,
- StoryStorage* const storage)
+ StartCall(StoryControllerImpl* const story_controller_impl,
+ StoryStorage* const storage)
: Operation("StoryControllerImpl::StartCall", [] {}),
story_controller_impl_(story_controller_impl),
storage_(storage) {}
@@ -927,9 +918,10 @@
storage_->ReadAllModuleData()->Then(
[this, flow](std::vector<fuchsia::modular::ModuleData> data) {
story_controller_impl_->InitStoryEnvironment();
-
for (auto& module_data : data) {
- if (module_data.module_deleted) {
+ // Don't start the module if it is embedded, or if it has been
+ // marked deleted.
+ if (module_data.module_deleted || module_data.is_embedded) {
continue;
}
FXL_CHECK(module_data.intent);
@@ -1230,34 +1222,25 @@
}
void StoryControllerImpl::EmbedModule(
- const std::vector<std::string>& parent_module_path,
- std::string module_name, fuchsia::modular::IntentPtr intent,
+ AddModParams add_mod_params,
fidl::InterfaceRequest<fuchsia::modular::ModuleController>
module_controller_request,
fidl::InterfaceRequest<fuchsia::ui::viewsv1token::ViewOwner>
view_owner_request,
- fuchsia::modular::ModuleSource module_source,
std::function<void(fuchsia::modular::StartModuleStatus)> callback) {
operation_queue_.Add(new AddIntentCall(
- this, parent_module_path, module_name, std::move(intent),
- std::move(module_controller_request), nullptr /* surface_relation */,
- std::move(view_owner_request), std::move(module_source),
- std::move(callback)));
+ this, std::move(add_mod_params), std::move(module_controller_request),
+ std::move(view_owner_request), std::move(callback)));
}
-void StoryControllerImpl::StartModule(
- const std::vector<std::string>& parent_module_path,
- std::string module_name, fuchsia::modular::IntentPtr intent,
+void StoryControllerImpl::AddModuleToStory(
+ AddModParams add_mod_params,
fidl::InterfaceRequest<fuchsia::modular::ModuleController>
module_controller_request,
- fuchsia::modular::SurfaceRelationPtr surface_relation,
- fuchsia::modular::ModuleSource module_source,
std::function<void(fuchsia::modular::StartModuleStatus)> callback) {
operation_queue_.Add(new AddIntentCall(
- this, parent_module_path, module_name, std::move(intent),
- std::move(module_controller_request), std::move(surface_relation),
- nullptr /* view_owner_request */, std::move(module_source),
- std::move(callback)));
+ this, std::move(add_mod_params), std::move(module_controller_request),
+ nullptr /* view_owner_request */, std::move(callback)));
}
void StoryControllerImpl::ProcessPendingViews() {
@@ -1328,7 +1311,6 @@
void StoryControllerImpl::OnModuleDataUpdated(
fuchsia::modular::ModuleData module_data) {
- // Control reaching here means that this update came from a remote device.
operation_queue_.Add(
new OnModuleDataUpdatedCall(this, std::move(module_data)));
}
diff --git a/bin/sessionmgr/story_runner/story_controller_impl.h b/bin/sessionmgr/story_runner/story_controller_impl.h
index c72b43c..1560544 100644
--- a/bin/sessionmgr/story_runner/story_controller_impl.h
+++ b/bin/sessionmgr/story_runner/story_controller_impl.h
@@ -30,6 +30,7 @@
#include <lib/fidl/cpp/interface_request.h>
#include <lib/fxl/macros.h>
+#include "peridot/bin/sessionmgr/puppet_master/command_runners/operation_calls/add_mod_call.h"
#include "peridot/bin/sessionmgr/storage/session_storage.h"
#include "peridot/bin/sessionmgr/story/model/story_mutator.h"
#include "peridot/bin/sessionmgr/story/model/story_observer.h"
@@ -106,34 +107,27 @@
// Called by ModuleContextImpl.
fuchsia::modular::LinkPathPtr GetLinkPathForParameterName(
- const std::vector<std::string>& module_path,
- std::string name);
+ const std::vector<std::string>& module_path, std::string name);
// Called by ModuleContextImpl.
void EmbedModule(
- const std::vector<std::string>& parent_module_path,
- std::string module_name, fuchsia::modular::IntentPtr intent,
+ AddModParams add_mod_params,
fidl::InterfaceRequest<fuchsia::modular::ModuleController>
module_controller_request,
fidl::InterfaceRequest<fuchsia::ui::viewsv1token::ViewOwner>
view_owner_request,
- fuchsia::modular::ModuleSource module_source,
std::function<void(fuchsia::modular::StartModuleStatus)> callback);
// Called by ModuleContextImpl.
- void StartModule(
- const std::vector<std::string>& parent_module_path,
- std::string module_name, fuchsia::modular::IntentPtr intent,
+ void AddModuleToStory(
+ AddModParams add_mod_params,
fidl::InterfaceRequest<fuchsia::modular::ModuleController>
module_controller_request,
- fuchsia::modular::SurfaceRelationPtr surface_relation,
- fuchsia::modular::ModuleSource module_source,
std::function<void(fuchsia::modular::StartModuleStatus)> callback);
// Stops the module at |module_path| in response to a call to
// |ModuleContext.RemoveSelfFromStory|.
- void RemoveModuleFromStory(
- const std::vector<std::string>& module_path);
+ void RemoveModuleFromStory(const std::vector<std::string>& module_path);
// Called by ModuleContextImpl.
void StartOngoingActivity(
@@ -214,8 +208,8 @@
const fidl::StringPtr story_id_;
StoryProviderImpl* const story_provider_impl_; // Not owned.
- SessionStorage* const session_storage_; // Not owned.
- StoryStorage* const story_storage_; // 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_;
@@ -250,7 +244,7 @@
// serialized module path) until its parent is connected to story shell. Story
// shell cannot display views whose parents are not yet displayed.
struct PendingView {
- std::vector<std::string> module_path;
+ std::vector<std::string> module_path;
fuchsia::modular::ModuleManifestPtr module_manifest;
fuchsia::modular::SurfaceRelationPtr surface_relation;
fuchsia::modular::ModuleSource module_source;
diff --git a/packages/tests/modular_integration_tests b/packages/tests/modular_integration_tests
index d5a82d3..a18cf17 100644
--- a/packages/tests/modular_integration_tests
+++ b/packages/tests/modular_integration_tests
@@ -14,6 +14,7 @@
"//peridot/tests/component_context:component_context_test_unstoppable_agent",
"//peridot/tests/embed_shell:embed_shell_test_child_module",
"//peridot/tests/embed_shell:embed_shell_test_parent_module",
+ "//peridot/tests/embed_shell:embed_shell_test_session_shell",
"//peridot/tests/embed_shell:embed_shell_test_story_shell",
"//peridot/tests/intents:intent_test_parent_module",
"//peridot/tests/intents:intent_test_child_module",
diff --git a/public/fidl/fuchsia.modular/module/module_data.fidl b/public/fidl/fuchsia.modular/module/module_data.fidl
index 31b36ba..b55eafe 100644
--- a/public/fidl/fuchsia.modular/module/module_data.fidl
+++ b/public/fidl/fuchsia.modular/module/module_data.fidl
@@ -40,16 +40,21 @@
/// required once the framework is cleaned up enough to guarantee this
/// statement.
Intent? intent;
+
+ /// If true, this module was started by a parent module using
+ /// ModuleContext.EmbedModule(), and its view is not managed by the
+ /// StoryShell.
+ bool is_embedded = false;
};
enum ModuleSource {
- /// Module that was started from within the story by another module via
- /// ModuleContext.StartModule() and ModuleContext.EmbedModule().
+ /// Module that was added to the story from within the story by another
+ /// module using ModuleContext.AddModuleToStory() or
+ /// ModuleContext.EmbedModule().
INTERNAL = 0;
- /// Module that was started from outside the story by Maxwell / SessionShell via
- /// StoryController.AddModule() or StoryProvider.CreateStory() as first
- /// module.
+ /// Module that was added to the story from outside the story using
+ /// PuppetMaster.
EXTERNAL = 1;
};
diff --git a/public/fidl/fuchsia.modular/story/puppet_master.fidl b/public/fidl/fuchsia.modular/story/puppet_master.fidl
index dfb7138..23587b9 100644
--- a/public/fidl/fuchsia.modular/story/puppet_master.fidl
+++ b/public/fidl/fuchsia.modular/story/puppet_master.fidl
@@ -18,8 +18,8 @@
// either don't add one, or remove the last one.
STORY_MUST_HAVE_MODS = 3;
- // The mod specified in AddMod, RemoveMod or SendModCommand
- // was not found.
+ // The mod name specified in AddMod, RemoveMod or SendModCommand
+ // was not found, or is invalid.
INVALID_MOD = 4;
// Resolution of the intent provided in AddMod returned zero results.
diff --git a/tests/embed_shell/BUILD.gn b/tests/embed_shell/BUILD.gn
index 7404b6f..d139e1a 100644
--- a/tests/embed_shell/BUILD.gn
+++ b/tests/embed_shell/BUILD.gn
@@ -39,6 +39,35 @@
]
}
+executable_package("embed_shell_test_session_shell") {
+ deprecated_bare_package_url = "//build"
+
+ testonly = true
+
+ meta = [
+ {
+ path = "meta/embed_shell_test_session_shell.cmx"
+ dest = "embed_shell_test_session_shell.cmx"
+ },
+ ]
+
+ sources = [
+ "embed_shell_test_session_shell.cc",
+ ]
+
+ deps = [
+ ":defs",
+ "//garnet/public/fidl/fuchsia.ui.viewsv1",
+ "//garnet/public/lib/callback",
+ "//peridot/public/lib/integration_testing/cpp",
+ "//peridot/public/fidl/fuchsia.modular",
+ "//peridot/lib/testing:component_main",
+ "//peridot/lib/testing:session_shell_base",
+ "//peridot/tests/common:defs",
+ "//zircon/public/lib/async-loop-cpp",
+ ]
+}
+
executable_package("embed_shell_test_parent_module") {
deprecated_bare_package_url = "//build"
diff --git a/tests/embed_shell/defs.h b/tests/embed_shell/defs.h
index 3d25f90..3cd0c37 100644
--- a/tests/embed_shell/defs.h
+++ b/tests/embed_shell/defs.h
@@ -7,14 +7,24 @@
namespace {
+constexpr char kStoryName[] = "story";
+
constexpr char kChildModuleName[] = "child";
// Package URLs of the test components used here.
constexpr char kChildModuleUrl[] =
- "fuchsia-pkg://fuchsia.com/embed_shell_test_child_module#meta/embed_shell_test_child_module.cmx";
+ "fuchsia-pkg://fuchsia.com/embed_shell_test_child_module#meta/"
+ "embed_shell_test_child_module.cmx";
+constexpr char kParentModuleUrl[] =
+ "fuchsia-pkg://fuchsia.com/embed_shell_test_parent_module#meta/"
+ "embed_shell_test_parent_module.cmx";
+
+constexpr char kParentModuleAction[] = "action";
constexpr char kChildModuleAction[] = "action";
+constexpr char kParentModuleDoneSignal[] = "parent_module_done";
+
} // namespace
#endif // PERIDOT_TESTS_EMBED_SHELL_DEFS_H_
diff --git a/tests/embed_shell/embed_shell_test_parent_module.cc b/tests/embed_shell/embed_shell_test_parent_module.cc
index 9a68e0a..f5f7a15 100644
--- a/tests/embed_shell/embed_shell_test_parent_module.cc
+++ b/tests/embed_shell/embed_shell_test_parent_module.cc
@@ -51,7 +51,7 @@
auto check = [this, done = std::make_shared<int>(0)] {
++*done;
if (*done == 2) {
- Signal(modular::testing::kTestShutdown);
+ Signal(kParentModuleDoneSignal);
}
};
diff --git a/tests/embed_shell/embed_shell_test_session_shell.cc b/tests/embed_shell/embed_shell_test_session_shell.cc
new file mode 100644
index 0000000..c5a2c4c
--- /dev/null
+++ b/tests/embed_shell/embed_shell_test_session_shell.cc
@@ -0,0 +1,105 @@
+// Copyright 2018 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <fuchsia/modular/cpp/fidl.h>
+#include <fuchsia/sys/cpp/fidl.h>
+#include <lib/async/cpp/task.h>
+#include <lib/component/cpp/startup_context.h>
+#include <lib/fidl/cpp/binding.h>
+#include <lib/fxl/command_line.h>
+#include <lib/fxl/logging.h>
+#include <lib/fxl/macros.h>
+
+#include "peridot/lib/testing/component_main.h"
+#include "peridot/lib/testing/session_shell_base.h"
+#include "peridot/public/lib/integration_testing/cpp/reporting.h"
+#include "peridot/public/lib/integration_testing/cpp/testing.h"
+#include "peridot/tests/common/defs.h"
+#include "peridot/tests/embed_shell/defs.h"
+
+using modular::testing::Await;
+using modular::testing::Signal;
+using modular::testing::TestPoint;
+
+namespace {
+
+// Cf. README.md for what this test does in general and how. The test cases are
+// described in detail in comments below.
+class TestApp : public modular::testing::SessionShellBase {
+ public:
+ explicit TestApp(component::StartupContext* const startup_context)
+ : SessionShellBase(startup_context) {
+ TestInit(__FILE__);
+
+ startup_context->ConnectToEnvironmentService(puppet_master_.NewRequest());
+
+ TestParentChild();
+ }
+
+ ~TestApp() override = default;
+
+ private:
+ void TestParentChild() {
+ puppet_master_->ControlStory(kStoryName, story_puppet_master_.NewRequest());
+
+ fuchsia::modular::AddMod add_mod;
+ add_mod.mod_name.push_back("root");
+ add_mod.intent.handler = kParentModuleUrl;
+ add_mod.intent.action = kParentModuleAction;
+ add_mod.intent.parameters =
+ fidl::VectorPtr<fuchsia::modular::IntentParameter>::New(0);
+
+ fuchsia::modular::StoryCommand command;
+ command.set_add_mod(std::move(add_mod));
+
+ std::vector<fuchsia::modular::StoryCommand> commands;
+ commands.push_back(std::move(command));
+ story_puppet_master_->Enqueue(std::move(commands));
+ story_puppet_master_->Execute(
+ [this](fuchsia::modular::ExecuteResult result) {
+ story_provider()->GetController(kStoryName,
+ story_controller_.NewRequest());
+ fidl::InterfaceHandle<fuchsia::ui::viewsv1token::ViewOwner>
+ root_module_view;
+ story_controller_->RequestStart();
+ });
+
+ Await(kParentModuleDoneSignal, [this] {
+ story_controller_->Stop([this] { TestModuleReinflation(); });
+ });
+ }
+
+ TestPoint modules_reinflated_correctly_{"Modules reinfated correctly"};
+ // Test that embedded modules are not reinflated.
+ void TestModuleReinflation() {
+ story_controller_->RequestStart();
+ story_controller_->GetActiveModules(
+ [this](std::vector<fuchsia::modular::ModuleData> active_modules) {
+ size_t num_embedded_mods = 0u;
+ for (const fuchsia::modular::ModuleData& mod : active_modules) {
+ num_embedded_mods += mod.is_embedded;
+ }
+ if (num_embedded_mods == 0 && active_modules.size() == 2u) {
+ modules_reinflated_correctly_.Pass();
+ }
+ Await(kParentModuleDoneSignal, [this] { Logout(); });
+ });
+ }
+
+ void Logout() { Signal(modular::testing::kTestShutdown); }
+
+ fuchsia::modular::PuppetMasterPtr puppet_master_;
+ fuchsia::modular::StoryPuppetMasterPtr story_puppet_master_;
+ fuchsia::modular::StoryControllerPtr story_controller_;
+
+ FXL_DISALLOW_COPY_AND_ASSIGN(TestApp);
+};
+
+} // namespace
+
+int main(int argc, const char** argv) {
+ auto command_line = fxl::CommandLineFromArgcArgv(argc, argv);
+ modular::testing::ComponentMain<TestApp>();
+ return 0;
+}
diff --git a/tests/embed_shell/embed_shell_test_story_shell.cc b/tests/embed_shell/embed_shell_test_story_shell.cc
index 3e6cd4f..98aff2c 100644
--- a/tests/embed_shell/embed_shell_test_story_shell.cc
+++ b/tests/embed_shell/embed_shell_test_story_shell.cc
@@ -46,15 +46,17 @@
TestPoint add_surface_{"AddSurface root:child:child root"};
// |fuchsia::modular::StoryShell|
- void AddSurface(
- fuchsia::modular::ViewConnection view_connection,
- fuchsia::modular::SurfaceInfo surface_info) override {
- FXL_LOG(INFO) << "surface_id=" << view_connection.surface_id << ", anchor_id=" << surface_info.parent_id;
- if (view_connection.surface_id == "root:child:child" && surface_info.parent_id == "root") {
+ void AddSurface(fuchsia::modular::ViewConnection view_connection,
+ fuchsia::modular::SurfaceInfo surface_info) override {
+ FXL_LOG(INFO) << "surface_id=" << view_connection.surface_id
+ << ", anchor_id=" << surface_info.parent_id;
+ if (view_connection.surface_id == "root:child:child" &&
+ surface_info.parent_id == "root") {
add_surface_.Pass();
modular::testing::GetStore()->Put("story_shell_done", "1", [] {});
} else {
- FXL_LOG(WARNING) << "AddView " << view_connection.surface_id << " anchor " << surface_info.parent_id;
+ FXL_LOG(WARNING) << "AddView " << view_connection.surface_id << " anchor "
+ << surface_info.parent_id;
}
}
@@ -63,7 +65,7 @@
// |fuchsia::modular::StoryShell|
void DefocusSurface(std::string /*surface_id*/,
- DefocusSurfaceCallback callback) override {
+ DefocusSurfaceCallback callback) override {
callback();
}
@@ -72,19 +74,19 @@
std::string /*container_name*/, fidl::StringPtr /*parent_id*/,
fuchsia::modular::SurfaceRelation /*relation*/,
std::vector<fuchsia::modular::ContainerLayout> /*layout*/,
- std::vector<
- fuchsia::modular::ContainerRelationEntry> /* relationships */,
+ std::vector<fuchsia::modular::ContainerRelationEntry> /* relationships */,
std::vector<fuchsia::modular::ContainerView> /* views */) override {}
// |fuchsia::modular::StoryShell|
void RemoveSurface(std::string /*surface_id*/) override {}
// |fuchsia::modular::StoryShell|
- void ReconnectView(fuchsia::modular::ViewConnection view_connection) override {}
+ void ReconnectView(
+ fuchsia::modular::ViewConnection view_connection) override {}
// |fuchsia::modular::StoryShell|
void UpdateSurface(fuchsia::modular::ViewConnection view_connection,
- fuchsia::modular::SurfaceInfo /*surface_info*/) override {};
+ fuchsia::modular::SurfaceInfo /*surface_info*/) override{};
fuchsia::modular::StoryShellContextPtr story_shell_context_;
diff --git a/tests/embed_shell/meta/embed_shell_test_session_shell.cmx b/tests/embed_shell/meta/embed_shell_test_session_shell.cmx
new file mode 100644
index 0000000..495da50
--- /dev/null
+++ b/tests/embed_shell/meta/embed_shell_test_session_shell.cmx
@@ -0,0 +1,25 @@
+{
+ "program": {
+ "binary": "bin/app"
+ },
+ "sandbox": {
+ "services": [
+ "fuchsia.cobalt.LoggerFactory",
+ "fuchsia.modular.ComponentContext",
+ "fuchsia.modular.ModuleContext",
+ "fuchsia.modular.PuppetMaster",
+ "fuchsia.modular.SessionShellContext",
+ "fuchsia.net.LegacySocketProvider",
+ "fuchsia.sys.Environment",
+ "fuchsia.sys.Launcher",
+ "fuchsia.sys.Loader",
+ "fuchsia.testing.runner.TestRunner",
+ "fuchsia.testing.runner.TestRunnerStore",
+ "fuchsia.tracelink.Registry",
+ "fuchsia.ui.policy.Presenter",
+ "fuchsia.ui.scenic.Scenic",
+ "fuchsia.ui.viewsv1.ViewManager",
+ "fuchsia.ui.viewsv1.ViewSnapshot"
+ ]
+ }
+}
diff --git a/tests/modular_tests.json b/tests/modular_tests.json
index 38279cb..055a281 100644
--- a/tests/modular_tests.json
+++ b/tests/modular_tests.json
@@ -34,8 +34,7 @@
"--test",
"--account_provider=fuchsia-pkg://fuchsia.com/dev_token_manager#meta/dev_token_manager.cmx",
"--base_shell=fuchsia-pkg://fuchsia.com/dev_base_shell#meta/dev_base_shell.cmx",
- "--session_shell=fuchsia-pkg://fuchsia.com/dev_session_shell#meta/dev_session_shell.cmx",
- "--session_shell_args=--root_module=fuchsia-pkg://fuchsia.com/embed_shell_test_parent_module#meta/embed_shell_test_parent_module.cmx",
+ "--session_shell=fuchsia-pkg://fuchsia.com/embed_shell_test_session_shell#meta/embed_shell_test_session_shell.cmx",
"--story_shell=fuchsia-pkg://fuchsia.com/embed_shell_test_story_shell#meta/embed_shell_test_story_shell.cmx",
"--sessionmgr_args=--use_memfs_for_ledger"
]
diff --git a/tests/module_context/README.md b/tests/module_context/README.md
index 7909dd5..2eb14bb 100644
--- a/tests/module_context/README.md
+++ b/tests/module_context/README.md
@@ -3,8 +3,7 @@
Tests that calls to ModuleContext.RemoveSelfFromStory() are handled correctly by the framework.
The session shell starts a story and adds two modules to it. It then signals the
-first module to call ModuleContext.RemoveSelfFromStory and verifies that the module is stopped.
-The session shell then signals the second module, and verifies that the story is
-stopped as well.
-
-
+first module to call ModuleContext.RemoveSelfFromStory and verifies that the
+module is stopped. The session shell then signals the second module to
+ModuleContext.RemoveSelfFromStory, and verifies that the story is stopped as
+well.
\ No newline at end of file
