[modular] Test ModuleController.Stop() pipelining in parent_child integration test.

Also:
* Fix a bug where ModuleController.Stop() was not causing a module to
actually stop if that module was added through ModuleContext.
* Added a comment explaining how the classification and difference in
behavior for INTERNAL/EXTERNAL mods is odd, and deserves
reconsideration. However, this change is not the place for it.

TEST=parent_child
MF-17 #comment [modular] Test ModuleCongtroller.Stop() pipelining in parent_child integration test.

Change-Id: I41b17cdafcbd5380ba9eded88dc5417d789d4f36
diff --git a/bin/sessionmgr/story_runner/story_controller_impl.cc b/bin/sessionmgr/story_runner/story_controller_impl.cc
index 113d654..5b42afc 100644
--- a/bin/sessionmgr/story_runner/story_controller_impl.cc
+++ b/bin/sessionmgr/story_runner/story_controller_impl.cc
@@ -627,11 +627,10 @@
  private:
   void Run() override {
     FlowToken flow{this};
-    if (!story_controller_impl_->IsRunning() ||
-        module_data_.module_source !=
-            fuchsia::modular::ModuleSource::EXTERNAL) {
+    if (!story_controller_impl_->IsRunning()) {
       return;
     }
+
     // Check for existing module at the given path.
     auto* const running_mod_info =
         story_controller_impl_->FindRunningModInfo(module_data_.module_path);
@@ -644,6 +643,19 @@
       return;
     }
 
+    // We do not auto-start Modules that were added through ModuleContext on
+    // other devices.
+    //
+    // TODO(thatguy): Revisit this decision. It seems wrong: we do not want
+    // to auto-start mods added through ModuleContext.EmbedModule(), because
+    // 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) {
+      return;
+    }
+
     // We reach this point only if we want to start or update an existing
     // external module.
     operation_queue_.Add(new LaunchModuleInShellCall(
@@ -1169,6 +1181,7 @@
 void StoryControllerImpl::StopModule(
     const fidl::VectorPtr<fidl::StringPtr>& module_path,
     const std::function<void()>& done) {
+  FXL_LOG(INFO) << "XXX StopModule() " << PathString(module_path);
   operation_queue_.Add(new StopModuleCall(story_storage_, module_path, done));
 }
 
diff --git a/tests/parent_child/parent_child_test_parent_module.cc b/tests/parent_child/parent_child_test_parent_module.cc
index a210643..1c1576f 100644
--- a/tests/parent_child/parent_child_test_parent_module.cc
+++ b/tests/parent_child/parent_child_test_parent_module.cc
@@ -27,7 +27,7 @@
 
 namespace {
 
-void StartModuleWithHandler(
+void AddModuleToStoryWithHandler(
     fuchsia::modular::ModuleContext* const module_context,
     fidl::InterfaceRequest<fuchsia::modular::ModuleController> request,
     fidl::StringPtr handler) {
@@ -35,7 +35,8 @@
   intent.handler = handler;
   intent.action = kChildModuleAction;
   module_context->AddModuleToStory(
-      kChildModuleName, std::move(intent), std::move(request), nullptr,
+      kChildModuleName, std::move(intent), std::move(request),
+      nullptr /* surface_relation */,
       [](const fuchsia::modular::StartModuleStatus) {});
 }
 
@@ -73,8 +74,8 @@
       "Second child module controller closed"};
 
   void StartChildModuleTwice() {
-    StartModuleWithHandler(module_host_->module_context(),
-                           child_module_.NewRequest(), kChildModuleUrl1);
+    AddModuleToStoryWithHandler(module_host_->module_context(),
+                                child_module_.NewRequest(), kChildModuleUrl1);
     child_module_.set_error_handler(
         [this](zx_status_t status) { OnFirstChildModuleStopped(); });
 
@@ -82,14 +83,15 @@
     // Intent, and then again but with a different Intent.handler. The second
     // call stops the previous module instance and starts a new one.
     Await("child_module_1_init", [this] {
-      StartModuleWithHandler(module_host_->module_context(),
-                             child_module_again_.NewRequest(),
-                             kChildModuleUrl1);
+      AddModuleToStoryWithHandler(module_host_->module_context(),
+                                  child_module_again_.NewRequest(),
+                                  kChildModuleUrl1);
       child_module_again_.set_error_handler([this](zx_status_t status) {
         second_child_module_controller_closed_.Pass();
       });
-      StartModuleWithHandler(module_host_->module_context(),
-                             child_module2_.NewRequest(), kChildModuleUrl2);
+      AddModuleToStoryWithHandler(module_host_->module_context(),
+                                  child_module2_.NewRequest(),
+                                  kChildModuleUrl2);
     });
   }
 
@@ -108,16 +110,39 @@
   }
 
   TestPoint child_module2_stopped_{"Second child module stopped"};
-
   void OnChildModule2Stopped() {
     child_module2_stopped_.Pass();
-    Signal(modular::testing::kTestShutdown);
+
+    TestPipelinedAddAndStop();
+  }
+
+  TestPoint child_module_3_started_{"Third child module started"};
+  TestPoint child_module_3_stopped_{"Third child module stopped"};
+  void TestPipelinedAddAndStop() {
+    // New test case: start a third child module, and immediately stop it.
+    // We expect that the child module will go through its full lifecycle,
+    // since we serialize these requests in sessionmgr.
+    AddModuleToStoryWithHandler(module_host_->module_context(),
+                                child_module3_.NewRequest(), kChildModuleUrl1);
+    child_module3_->Stop([this] { /* do nothing */ });
+    child_module3_.events().OnStateChange =
+        [this](fuchsia::modular::ModuleState module_state) {
+          FXL_LOG(INFO) << "XXX SDLFKJSDFLKJ " << static_cast<int>(module_state);
+          if (module_state == fuchsia::modular::ModuleState::RUNNING) {
+            child_module_3_started_.Pass();
+          }
+          if (module_state == fuchsia::modular::ModuleState::STOPPED) {
+            child_module_3_stopped_.Pass();
+            Signal(modular::testing::kTestShutdown);
+          }
+        };
   }
 
   modular::ModuleHost* module_host_;
   fuchsia::modular::ModuleControllerPtr child_module_;
   fuchsia::modular::ModuleControllerPtr child_module_again_;
   fuchsia::modular::ModuleControllerPtr child_module2_;
+  fuchsia::modular::ModuleControllerPtr child_module3_;
 
   FXL_DISALLOW_COPY_AND_ASSIGN(TestModule);
 };