[fuchsia] Report content-initiated teardowns distinctly from crashes.

When the Frame used by a web-based component self-terminates, the web
component will now report the status code passed to the channel's error-
handler as the component's OnTerminated() exit-code.

The Frame implementation is updated to close the channel with ZX_OK if
the web content itself triggers teardown (e.g. via window.close()).

In addition to reporting the exit-code via OnTerminated(), instances of
CastComponent will report the exit-code to their ApplicationContext.

Bug: fuchsia:51028, b/148594115
Change-Id: Ib3543782d81e276372f9163e4c2e51454f9fc280
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2207389
Commit-Queue: Wez <wez@chromium.org>
Reviewed-by: David Dorwin <ddorwin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#772243}

fuchsia/engine/browser/frame_impl.cc

@@ -895,7 +895,7 @@ void FrameImpl::SetPermissionState(
 
 void FrameImpl::CloseContents(content::WebContents* source) {
   DCHECK_EQ(source, web_contents_.get());
-  context_->DestroyFrame(this);
+  CloseAndDestroyFrame(ZX_OK);
 }
 
 void FrameImpl::SetBlockMediaLoading(bool blocked) {

fuchsia/runners/cast/application_controller_impl.cc

@@ -11,12 +11,12 @@
 
 ApplicationControllerImpl::ApplicationControllerImpl(
     fuchsia::web::Frame* frame,
-    fidl::InterfaceHandle<chromium::cast::ApplicationContext> context)
+    chromium::cast::ApplicationContext* context)
     : binding_(this), frame_(frame) {
   DCHECK(context);
   DCHECK(frame_);
 
-  context.Bind()->SetApplicationController(binding_.NewBinding());
+  context->SetApplicationController(binding_.NewBinding());
 
   binding_.set_error_handler([](zx_status_t status) {
     if (status != ZX_ERR_PEER_CLOSED && status != ZX_ERR_CANCELED) {

fuchsia/runners/cast/application_controller_impl.h

@@ -15,9 +15,8 @@
 
 class ApplicationControllerImpl : public chromium::cast::ApplicationController {
  public:
-  ApplicationControllerImpl(
-      fuchsia::web::Frame* frame,
-      fidl::InterfaceHandle<chromium::cast::ApplicationContext> context);
+  ApplicationControllerImpl(fuchsia::web::Frame* frame,
+                            chromium::cast::ApplicationContext* context);
   ~ApplicationControllerImpl() final;
 
  protected:

fuchsia/runners/cast/application_controller_impl_unittest.cc

@@ -41,8 +41,9 @@ class ApplicationControllerImplTest : public chromium::cast::ApplicationContext,
                                       public testing::Test {
  public:
   ApplicationControllerImplTest()
-      : application_context_(this),
-        application_(&frame_, application_context_.NewBinding()) {
+      : application_context_binding_(this),
+        application_context_(application_context_binding_.NewBinding().Bind()),
+        application_(&frame_, application_context_.get()) {
     base::RunLoop run_loop;
     wait_for_controller_callback_ = run_loop.QuitClosure();
     run_loop.Run();
@@ -51,7 +52,7 @@ class ApplicationControllerImplTest : public chromium::cast::ApplicationContext,
   ~ApplicationControllerImplTest() override = default;
 
  protected:
-  // chromium::cast::ApplicationReceiver implementation.
+  // chromium::cast::ApplicationContext implementation.
   void GetMediaSessionId(GetMediaSessionIdCallback callback) final {
     NOTREACHED();
   }
@@ -68,10 +69,12 @@ class ApplicationControllerImplTest : public chromium::cast::ApplicationContext,
       base::test::SingleThreadTaskEnvironment::MainThreadType::IO};
 
   MockFrame frame_;
-  fidl::Binding<chromium::cast::ApplicationContext> application_context_;
+  fidl::Binding<chromium::cast::ApplicationContext>
+      application_context_binding_;
+  chromium::cast::ApplicationContextPtr application_context_;
+  ApplicationControllerImpl application_;
 
   chromium::cast::ApplicationControllerPtr application_ptr_;
-  ApplicationControllerImpl application_;
   base::OnceClosure wait_for_controller_callback_;
 
  private:

fuchsia/runners/cast/cast_component.cc

@@ -134,10 +134,11 @@ void CastComponent::StartComponent() {
         application_config_.force_content_dimensions()));
   }
 
-  application_controller_ = std::make_unique<ApplicationControllerImpl>(
-      frame(),
+  application_context_ =
       agent_manager_->ConnectToAgentService<chromium::cast::ApplicationContext>(
-          application_config_.agent_url()));
+          application_config_.agent_url());
+  application_controller_ = std::make_unique<ApplicationControllerImpl>(
+      frame(), application_context_.get());
 
   // Pass application permissions to the frame.
   if (application_config_.has_permissions()) {
@@ -152,13 +153,20 @@ void CastComponent::StartComponent() {
   }
 }
 
-void CastComponent::DestroyComponent(int termination_exit_code,
+void CastComponent::DestroyComponent(int64_t exit_code,
                                      fuchsia::sys::TerminationReason reason) {
   DCHECK(!constructor_active_);
 
   std::move(on_destroyed_).Run();
 
-  WebComponent::DestroyComponent(termination_exit_code, reason);
+  // If the component EXITED then pass the |exit_code| to the Agent, to allow it
+  // to distinguish graceful termination from crashes.
+  if (reason == fuchsia::sys::TerminationReason::EXITED &&
+      application_controller_) {
+    application_context_->OnApplicationExit(exit_code);
+  }
+
+  WebComponent::DestroyComponent(exit_code, reason);
 }
 
 void CastComponent::OnRewriteRulesReceived(

fuchsia/runners/cast/cast_component.h

@@ -14,6 +14,7 @@
 #include "base/message_loop/message_pump_for_io.h"
 #include "base/message_loop/message_pump_fuchsia.h"
 #include "base/optional.h"
+#include "fuchsia/fidl/chromium/cast/cpp/fidl.h"
 #include "fuchsia/runners/cast/api_bindings_client.h"
 #include "fuchsia/runners/cast/application_controller_impl.h"
 #include "fuchsia/runners/cast/named_message_port_connector.h"
@@ -64,7 +65,7 @@ class CastComponent : public WebComponent,
 
   // WebComponent overrides.
   void StartComponent() final;
-  void DestroyComponent(int termination_exit_code,
+  void DestroyComponent(int64_t termination_exit_code,
                         fuchsia::sys::TerminationReason reason) final;
 
   const chromium::cast::ApplicationConfig& application_config() {
@@ -106,6 +107,7 @@ class CastComponent : public WebComponent,
   std::unique_ptr<NamedMessagePortConnector> connector_;
   std::unique_ptr<ApiBindingsClient> api_bindings_client_;
   std::unique_ptr<ApplicationControllerImpl> application_controller_;
+  chromium::cast::ApplicationContextPtr application_context_;
   uint64_t media_session_id_ = 0;
   zx::eventpair headless_view_token_;
   base::MessagePumpForIO::ZxHandleWatchController headless_disconnect_watch_;

fuchsia/runners/cast/cast_runner_integration_test.cc

@@ -122,6 +122,13 @@ class FakeApplicationContext : public chromium::cast::ApplicationContext {
     return controller_.get();
   }
 
+  base::Optional<int64_t> WaitForApplicationTerminated() {
+    base::RunLoop loop;
+    on_application_terminated_ = loop.QuitClosure();
+    loop.Run();
+    return application_exit_code_;
+  }
+
  private:
   // chromium::cast::ApplicationContext implementation.
   void GetMediaSessionId(GetMediaSessionIdCallback callback) final {
@@ -132,8 +139,16 @@ class FakeApplicationContext : public chromium::cast::ApplicationContext {
       final {
     controller_ = controller.Bind();
   }
+  void OnApplicationExit(int64_t exit_code) final {
+    application_exit_code_ = exit_code;
+    if (on_application_terminated_)
+      std::move(on_application_terminated_).Run();
+  }
 
   chromium::cast::ApplicationControllerPtr controller_;
+
+  base::Optional<int64_t> application_exit_code_;
+  base::OnceClosure on_application_terminated_;
 };
 
 class FakeComponentState : public cr_fuchsia::AgentImpl::ComponentStateBase {
@@ -549,8 +564,7 @@ TEST_F(CastRunnerIntegrationTest, IncorrectCastAppId) {
   CreateComponentContext(kIncorrectComponentUrl);
   StartCastComponent(kIncorrectComponentUrl);
 
-  // Run the loop until the ComponentController is dropped, or a WebComponent is
-  // created.
+  // Run the loop until the ComponentController is dropped.
   base::RunLoop run_loop;
   component_controller_.set_error_handler([&run_loop](zx_status_t status) {
     EXPECT_EQ(status, ZX_ERR_PEER_CLOSED);
@@ -885,6 +899,89 @@ TEST_F(CastRunnerIntegrationTest, LegacyMetricsRedirect) {
   run_loop.Run();
 }
 
+// Verifies that the ApplicationContext::OnApplicationTerminated() is notified
+// with the component exit code if the web content closes itself.
+TEST_F(CastRunnerIntegrationTest, OnApplicationTerminated_WindowClose) {
+  const GURL url = test_server_.GetURL(kBlankAppUrl);
+  app_config_manager_.AddApp(kTestAppId, url);
+
+  CreateComponentContextAndStartComponent();
+
+  // It is possible to observe the ComponentController close before
+  // OnApplicationTerminated() is received, so ignore that.
+  component_controller_.set_error_handler([](zx_status_t) {});
+
+  // Have the web content close itself, and wait for OnApplicationTerminated().
+  EXPECT_EQ(ExecuteJavaScript("window.close()"), "undefined");
+  base::Optional<zx_status_t> exit_code =
+      component_state_->application_context()->WaitForApplicationTerminated();
+  ASSERT_TRUE(exit_code);
+  EXPECT_EQ(exit_code.value(), ZX_OK);
+}
+
+// Verifies that the ComponentController reports TerminationReason::EXITED and
+// exit code ZX_OK if the web content terminates itself.
+// TODO(https://crbug.com/1066833): Make this a WebRunner test.
+TEST_F(CastRunnerIntegrationTest, OnTerminated_WindowClose) {
+  const GURL url = test_server_.GetURL(kBlankAppUrl);
+  app_config_manager_.AddApp(kTestAppId, url);
+
+  CreateComponentContextAndStartComponent();
+
+  // Register an handler on the ComponentController channel, for the
+  // OnTerminated event.
+  base::RunLoop exit_code_loop;
+  component_controller_.set_error_handler(
+      [quit_loop = exit_code_loop.QuitClosure()](zx_status_t) {
+        quit_loop.Run();
+        ADD_FAILURE();
+      });
+  component_controller_.events().OnTerminated =
+      [quit_loop = exit_code_loop.QuitClosure()](
+          int64_t exit_code, fuchsia::sys::TerminationReason reason) {
+        quit_loop.Run();
+        EXPECT_EQ(reason, fuchsia::sys::TerminationReason::EXITED);
+        EXPECT_EQ(exit_code, ZX_OK);
+      };
+
+  // Have the web content close itself, and wait for OnTerminated().
+  EXPECT_EQ(ExecuteJavaScript("window.close()"), "undefined");
+  exit_code_loop.Run();
+
+  component_controller_.Unbind();
+}
+
+// Verifies that the ComponentController reports
+// TerminationReason::RUNNER_TERMINATED if Kill() is used.
+// TODO(https://crbug.com/1066833): Make this a WebRunner test.
+TEST_F(CastRunnerIntegrationTest, OnTerminated_ComponentKill) {
+  const GURL url = test_server_.GetURL(kBlankAppUrl);
+  app_config_manager_.AddApp(kTestAppId, url);
+
+  CreateComponentContextAndStartComponent();
+
+  // Register an handler on the ComponentController channel, for the
+  // OnTerminated event.
+  base::RunLoop exit_code_loop;
+  component_controller_.set_error_handler(
+      [quit_loop = exit_code_loop.QuitClosure()](zx_status_t) {
+        quit_loop.Run();
+        ADD_FAILURE();
+      });
+  component_controller_.events().OnTerminated =
+      [quit_loop = exit_code_loop.QuitClosure()](
+          int64_t, fuchsia::sys::TerminationReason reason) {
+        quit_loop.Run();
+        EXPECT_EQ(reason, fuchsia::sys::TerminationReason::RUNNER_TERMINATED);
+      };
+
+  // Kill() the component and wait for OnTerminated().
+  component_controller_->Kill();
+  exit_code_loop.Run();
+
+  component_controller_.Unbind();
+}
+
 TEST_F(CastRunnerIntegrationTest, WebGLContextAbsentWithoutVulkanFeature) {
   const char kTestPath[] = "/webgl_presence.html";
   const GURL test_url = test_server_.GetURL(kTestPath);

fuchsia/runners/common/web_component.cc

@@ -67,11 +67,14 @@ void WebComponent::StartComponent() {
   create_params.set_enable_remote_debugging(enable_remote_debugging_);
   frame_ = runner_->CreateFrame(std::move(create_params));
 
-  // If the Frame unexpectedly disconnect us then tear-down this Component.
+  // If the Frame unexpectedly disconnects then tear-down this Component.
+  // ZX_OK indicates intentional termination (e.g. via window.close()).
+  // ZX_ERR_PEER_CLOSED will usually indicate a crash, reported elsewhere.
+  // Therefore only log other, more unusual, |status| codes.
   frame_.set_error_handler([this](zx_status_t status) {
-    ZX_LOG_IF(ERROR, status != ZX_ERR_PEER_CLOSED, status)
-        << " Frame disconnected";
-    DestroyComponent(0, fuchsia::sys::TerminationReason::EXITED);
+    if (status != ZX_OK && status != ZX_ERR_PEER_CLOSED)
+      ZX_LOG(ERROR, status) << " Frame disconnected";
+    DestroyComponent(status, fuchsia::sys::TerminationReason::EXITED);
   });
 
   if (startup_context()->has_outgoing_directory_request()) {
@@ -132,9 +135,9 @@ void WebComponent::CreateView(
   view_is_bound_ = true;
 }
 
-void WebComponent::DestroyComponent(int termination_exit_code,
+void WebComponent::DestroyComponent(int64_t exit_code,
                                     fuchsia::sys::TerminationReason reason) {
   termination_reason_ = reason;
-  termination_exit_code_ = termination_exit_code;
+  termination_exit_code_ = exit_code;
   runner_->DestroyComponent(this);
 }

fuchsia/runners/common/web_component.h

@@ -73,8 +73,10 @@ class WebComponent : public fuchsia::sys::ComponentController,
       override;
 
   // Reports the supplied exit-code and reason to the |controller_binding_| and
-  // requests that the |runner_| delete this component.
-  virtual void DestroyComponent(int termination_exit_code,
+  // requests that the |runner_| delete this component. The EXITED |reason| is
+  // used to indicate Frame disconnection, in which case the |exit_code| is set
+  // to the status reported by the FramePtr's error handler.
+  virtual void DestroyComponent(int64_t exit_code,
                                 fuchsia::sys::TerminationReason reason);
 
   // Returns the component's startup context (e.g. incoming services, public
@@ -105,7 +107,7 @@ class WebComponent : public fuchsia::sys::ComponentController,
   // sys::ComponentController::OnTerminated event.
   fuchsia::sys::TerminationReason termination_reason_ =
       fuchsia::sys::TerminationReason::UNKNOWN;
-  int termination_exit_code_ = 0;
+  int64_t termination_exit_code_ = 0;
 
   bool view_is_bound_ = false;