[fuchsia] Restore healthy browser shutdown by adding platform teardown calls.

OzonePlatform members are intended to be leaked on browser shutdown,
so any FIDL channels contained by it (directly or indirectly) will
encounter connection errors on browser shutdown.

This CL causes some Ozone Platform objects, and their constituent
FIDL client objects, to be torn down as part of the normal browser
shutdown procedure.

Also fixes another shutdown breaking issue because the
FocusController was wrongly taking ownership of the FrameImpl.

Change-Id: I2f7bf7b9417dea33f4b4f4aa3bd763204e8e8a42
Reviewed-on: https://chromium-review.googlesource.com/c/1252901
Commit-Queue: Kevin Marshall <kmarshall@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597637}

ui/ozone/platform/scenic/ozone_platform_scenic.cc

@@ -4,9 +4,14 @@
 
 #include "ui/ozone/platform/scenic/ozone_platform_scenic.h"
 
+#include <memory>
+#include <utility>
+#include <vector>
+
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
+#include "base/message_loop/message_loop_current.h"
 #include "ui/base/cursor/ozone/bitmap_cursor_factory_ozone.h"
 #include "ui/display/manager/fake_display_delegate.h"
 #include "ui/events/ozone/layout/keyboard_layout_engine_manager.h"
@@ -46,13 +51,15 @@ class ScenicPlatformEventSource : public ui::PlatformEventSource {
 };
 
 // OzonePlatform for Scenic.
-class OzonePlatformScenic : public OzonePlatform {
+class OzonePlatformScenic
+    : public OzonePlatform,
+      public base::MessageLoopCurrent::DestructionObserver {
  public:
-  OzonePlatformScenic() : surface_factory_(&window_manager_) {}
+  OzonePlatformScenic()
+      : window_manager_(std::make_unique<ScenicWindowManager>()),
+        surface_factory_(window_manager_.get()) {}
   ~OzonePlatformScenic() override = default;
 
-  ScenicWindowManager* window_manager() { return &window_manager_; }
-
   // OzonePlatform implementation.
   ui::SurfaceFactoryOzone* GetSurfaceFactoryOzone() override {
     return &surface_factory_;
@@ -87,7 +94,8 @@ class OzonePlatformScenic : public OzonePlatform {
       return nullptr;
     }
     return std::make_unique<ScenicWindow>(
-        &window_manager_, delegate, std::move(properties.view_owner_request));
+        window_manager_.get(), delegate,
+        std::move(properties.view_owner_request));
   }
 
   const PlatformProperties& GetPlatformProperties() override {
@@ -101,7 +109,7 @@ class OzonePlatformScenic : public OzonePlatform {
   }
 
   std::unique_ptr<PlatformScreen> CreateScreen() override {
-    return window_manager_.CreateScreen();
+    return window_manager_->CreateScreen();
   }
 
   void InitializeUI(const InitParams& params) override {
@@ -114,12 +122,20 @@ class OzonePlatformScenic : public OzonePlatform {
     input_controller_ = CreateStubInputController();
     cursor_factory_ozone_ = std::make_unique<BitmapCursorFactoryOzone>();
     gpu_platform_support_host_.reset(CreateStubGpuPlatformSupportHost());
+
+    base::MessageLoopCurrent::Get()->AddDestructionObserver(this);
   }
 
   void InitializeGPU(const InitParams& params) override {}
 
  private:
-  ScenicWindowManager window_manager_;
+  // Performs graceful cleanup tasks on main message loop teardown.
+  void Shutdown() { window_manager_.reset(); }
+
+  // base::MessageLoopCurrent::DestructionObserver implementation.
+  void WillDestroyCurrentMessageLoop() override { Shutdown(); }
+
+  std::unique_ptr<ScenicWindowManager> window_manager_;
   ScenicSurfaceFactory surface_factory_;
 
   std::unique_ptr<PlatformEventSource> platform_event_source_;

ui/ozone/platform/scenic/scenic_window.cc

@@ -260,7 +260,6 @@ void ScenicWindow::OnEvent(fuchsia::ui::input::InputEvent event,
   bool result = false;
 
   if (event.is_focus()) {
-    LOG(ERROR) << "RECEIVED FOCUS EVENT!";
     delegate_->OnActivationChanged(event.focus().focused);
     result = true;
   } else {

ui/ozone/platform/scenic/scenic_window_manager.cc

@@ -5,6 +5,7 @@
 #include "ui/ozone/platform/scenic/scenic_window_manager.h"
 
 #include "base/fuchsia/component_context.h"
+#include "ui/ozone/platform/scenic/ozone_platform_scenic.h"
 
 namespace ui {
 
@@ -22,8 +23,9 @@ fuchsia::ui::viewsv1::ViewManager* ScenicWindowManager::GetViewManager() {
   if (!view_manager_) {
     view_manager_ = base::fuchsia::ComponentContext::GetDefault()
                         ->ConnectToService<fuchsia::ui::viewsv1::ViewManager>();
-    view_manager_.set_error_handler(
-        [this]() { LOG(FATAL) << "ViewManager connection failed."; });
+    view_manager_.set_error_handler([this]() {
+      LOG(ERROR) << "The ViewManager channel was unexpectedly terminated.";
+    });
   }
 
   return view_manager_.get();
@@ -32,8 +34,9 @@ fuchsia::ui::viewsv1::ViewManager* ScenicWindowManager::GetViewManager() {
 fuchsia::ui::scenic::Scenic* ScenicWindowManager::GetScenic() {
   if (!scenic_) {
     GetViewManager()->GetScenic(scenic_.NewRequest());
-    scenic_.set_error_handler(
-        [this]() { LOG(FATAL) << "Scenic connection failed."; });
+    scenic_.set_error_handler([this]() {
+      LOG(ERROR) << "The Scenic channel was unexpectedly terminated.";
+    });
   }
   return scenic_.get();
 }

webrunner/browser/frame_impl.cc

@@ -17,6 +17,7 @@
 #include "ui/aura/window.h"
 #include "ui/aura/window_tree_host_platform.h"
 #include "ui/platform_window/platform_window_init_properties.h"
+#include "ui/wm/core/base_focus_rules.h"
 #include "url/gurl.h"
 #include "webrunner/browser/context_impl.h"
 
@@ -100,13 +101,32 @@ bool ComputeNavigationEvent(const chromium::web::NavigationEntry& old_entry,
   return is_changed;
 }
 
+class FrameFocusRules : public wm::BaseFocusRules {
+ public:
+  FrameFocusRules() = default;
+  ~FrameFocusRules() override = default;
+
+  // wm::BaseFocusRules implementation.
+  bool SupportsChildActivation(aura::Window*) const override;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(FrameFocusRules);
+};
+
+bool FrameFocusRules::SupportsChildActivation(aura::Window*) const {
+  // TODO(crbug.com/878439): Return a result based on window properties such as
+  // visibility.
+  return true;
+}
+
 }  // namespace
 
 FrameImpl::FrameImpl(std::unique_ptr<content::WebContents> web_contents,
                      ContextImpl* context,
                      fidl::InterfaceRequest<chromium::web::Frame> frame_request)
     : web_contents_(std::move(web_contents)),
-      focus_controller_(std::make_unique<wm::FocusController>(this)),
+      focus_controller_(
+          std::make_unique<wm::FocusController>(new FrameFocusRules)),
       context_(context),
       binding_(this, std::move(frame_request)) {
   web_contents_->SetDelegate(this);
@@ -117,7 +137,7 @@ FrameImpl::~FrameImpl() {
   if (window_tree_host_) {
     aura::client::SetFocusClient(root_window(), nullptr);
     wm::SetActivationClient(root_window(), nullptr);
-
+    web_contents_->ClosePage();
     window_tree_host_->Hide();
     window_tree_host_->compositor()->SetVisible(false);
 
@@ -301,10 +321,4 @@ void FrameImpl::MaybeSendNavigationEvent() {
   }
 }
 
-bool FrameImpl::SupportsChildActivation(aura::Window*) const {
-  // TODO(crbug.com/878439): Return a result based on window properties such as
-  // visibility.
-  return true;
-}
-
 }  // namespace webrunner

webrunner/browser/frame_impl.h

@@ -15,7 +15,6 @@
 #include "content/public/browser/web_contents_delegate.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "ui/aura/window_tree_host.h"
-#include "ui/wm/core/base_focus_rules.h"
 #include "ui/wm/core/focus_controller.h"
 #include "url/gurl.h"
 #include "webrunner/fidl/chromium/web/cpp/fidl.h"
@@ -37,8 +36,7 @@ class ContextImpl;
 class FrameImpl : public chromium::web::Frame,
                   public chromium::web::NavigationController,
                   public content::WebContentsObserver,
-                  public content::WebContentsDelegate,
-                  public wm::BaseFocusRules {
+                  public content::WebContentsDelegate {
  public:
   FrameImpl(std::unique_ptr<content::WebContents> web_contents,
             ContextImpl* context,
@@ -101,9 +99,6 @@ class FrameImpl : public chromium::web::Frame,
   void DidFinishLoad(content::RenderFrameHost* render_frame_host,
                      const GURL& validated_url) override;
 
-  // wm::BaseFocusRules implementation.
-  bool SupportsChildActivation(aura::Window*) const override;
-
   std::unique_ptr<aura::WindowTreeHost> window_tree_host_;
   std::unique_ptr<content::WebContents> web_contents_;
   std::unique_ptr<wm::FocusController> focus_controller_;