chromeos: fix DCHECK in ~WindowTreeHostManager

The DCHECK is because on observer is being added but not
removed. Specifically exo's WmHelper installs an observer on
WindowTreeHostManager but WmHelper never removes itself (because
WindowTreeHostManager is deleted before WmHelper). This patch makes
WmHelper be destroyed before WindowTreeHostManager by way of making
ChromeBrowserMainExtraPartsAsh create exo related functionality.

BUG=none
TEST=none

Change-Id: I84e7209c817e4687017edbc14db7f75cb1054fbb
Reviewed-on: https://chromium-review.googlesource.com/572427
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487556}

chrome/browser/BUILD.gn

@@ -3919,8 +3919,8 @@ split_static_library("browser") {
       "//components/exo/wayland",
     ]
     sources += [
-      "chrome_browser_main_extra_parts_exo.cc",
-      "chrome_browser_main_extra_parts_exo.h",
+      "exo_parts.cc",
+      "exo_parts.h",
     ]
   }
 

chrome/browser/chrome_content_browser_client.cc

@@ -383,10 +383,6 @@
 #include "components/printing/service/public/interfaces/pdf_compositor.mojom.h"
 #endif
 
-#if BUILDFLAG(ENABLE_WAYLAND_SERVER)
-#include "chrome/browser/chrome_browser_main_extra_parts_exo.h"
-#endif
-
 #if BUILDFLAG(ENABLE_MOJO_MEDIA)
 #include "chrome/browser/media/output_protection_impl.h"
 #if BUILDFLAG(ENABLE_MOJO_CDM) && defined(OS_ANDROID)
@@ -904,10 +900,6 @@ content::BrowserMainParts* ChromeContentBrowserClient::CreateBrowserMainParts(
   main_parts->AddParts(new ChromeBrowserMainExtraPartsX11());
 #endif
 
-#if BUILDFLAG(ENABLE_WAYLAND_SERVER)
-  main_parts->AddParts(new ChromeBrowserMainExtraPartsExo());
-#endif
-
   chrome::AddMetricsExtraParts(main_parts);
 
   return main_parts;

chrome/browser/chrome_browser_main_extra_parts_exo.cc → chrome/browser/exo_parts.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/chrome_browser_main_extra_parts_exo.h"
+#include "chrome/browser/exo_parts.h"
 
 #include "base/memory/ptr_util.h"
 
@@ -12,6 +12,7 @@
 
 #include "base/command_line.h"
 #include "base/macros.h"
+#include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
 #include "chrome/browser/ui/ash/ash_util.h"
 #include "chrome/common/chrome_switches.h"
@@ -56,7 +57,7 @@ GSourceFuncs g_wayland_source_funcs = {WaylandSourcePrepare, WaylandSourceCheck,
 
 }  // namespace
 
-class ChromeBrowserMainExtraPartsExo::WaylandWatcher {
+class ExoParts::WaylandWatcher {
  public:
   explicit WaylandWatcher(exo::wayland::Server* server)
       : wayland_poll_(new GPollFD),
@@ -87,8 +88,7 @@ class ChromeBrowserMainExtraPartsExo::WaylandWatcher {
   DISALLOW_COPY_AND_ASSIGN(WaylandWatcher);
 };
 #else
-class ChromeBrowserMainExtraPartsExo::WaylandWatcher
-    : public base::MessagePumpLibevent::Watcher {
+class ExoParts::WaylandWatcher : public base::MessagePumpLibevent::Watcher {
  public:
   explicit WaylandWatcher(exo::wayland::Server* server)
       : controller_(FROM_HERE), server_(server) {
@@ -113,15 +113,24 @@ class ChromeBrowserMainExtraPartsExo::WaylandWatcher
 };
 #endif
 
-ChromeBrowserMainExtraPartsExo::ChromeBrowserMainExtraPartsExo() {}
+// static
+std::unique_ptr<ExoParts> ExoParts::CreateIfNecessary() {
+  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableWaylandServer)) {
+    return nullptr;
+  }
 
-ChromeBrowserMainExtraPartsExo::~ChromeBrowserMainExtraPartsExo() {}
+  return base::WrapUnique(new ExoParts());
+}
 
-void ChromeBrowserMainExtraPartsExo::PreProfileInit() {
-  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnableWaylandServer))
-    return;
+ExoParts::~ExoParts() {
+  wayland_watcher_.reset();
+  wayland_server_.reset();
+  exo::WMHelper::SetInstance(nullptr);
+  wm_helper_.reset();
+}
 
+ExoParts::ExoParts() {
   arc_notification_surface_manager_ =
       base::MakeUnique<arc::ArcNotificationSurfaceManagerImpl>();
   if (ash_util::IsRunningInMash())
@@ -136,12 +145,3 @@ void ChromeBrowserMainExtraPartsExo::PreProfileInit() {
   if (wayland_server_)
     wayland_watcher_ = base::MakeUnique<WaylandWatcher>(wayland_server_.get());
 }
-
-void ChromeBrowserMainExtraPartsExo::PostMainMessageLoopRun() {
-  wayland_watcher_.reset();
-  wayland_server_.reset();
-  if (wm_helper_) {
-    exo::WMHelper::SetInstance(nullptr);
-    wm_helper_.reset();
-  }
-}

chrome/browser/chrome_browser_main_extra_parts_exo.h → chrome/browser/exo_parts.h

@@ -2,13 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_CHROME_BROWSER_MAIN_EXTRA_PARTS_EXO_H_
-#define CHROME_BROWSER_CHROME_BROWSER_MAIN_EXTRA_PARTS_EXO_H_
+#ifndef CHROME_BROWSER_EXO_PARTS_H_
+#define CHROME_BROWSER_EXO_PARTS_H_
 
 #include <memory>
 
 #include "base/macros.h"
-#include "chrome/browser/chrome_browser_main_extra_parts.h"
 
 namespace exo {
 class Display;
@@ -16,22 +15,22 @@ class WMHelper;
 namespace wayland {
 class Server;
 }
-}
+}  // namespace exo
 
 namespace arc {
 class ArcNotificationSurfaceManagerImpl;
 }
 
-class ChromeBrowserMainExtraPartsExo : public ChromeBrowserMainExtraParts {
+class ExoParts {
  public:
-  ChromeBrowserMainExtraPartsExo();
-  ~ChromeBrowserMainExtraPartsExo() override;
+  // Creates ExoParts. Returns null if exo should not be created.
+  static std::unique_ptr<ExoParts> CreateIfNecessary();
 
-  // Overridden from ChromeBrowserMainExtraParts:
-  void PreProfileInit() override;
-  void PostMainMessageLoopRun() override;
+  ~ExoParts();
 
  private:
+  ExoParts();
+
   std::unique_ptr<arc::ArcNotificationSurfaceManagerImpl>
       arc_notification_surface_manager_;
   std::unique_ptr<exo::WMHelper> wm_helper_;
@@ -40,7 +39,7 @@ class ChromeBrowserMainExtraPartsExo : public ChromeBrowserMainExtraParts {
   class WaylandWatcher;
   std::unique_ptr<WaylandWatcher> wayland_watcher_;
 
-  DISALLOW_COPY_AND_ASSIGN(ChromeBrowserMainExtraPartsExo);
+  DISALLOW_COPY_AND_ASSIGN(ExoParts);
 };
 
-#endif  // CHROME_BROWSER_CHROME_BROWSER_MAIN_EXTRA_PARTS_EXO_H_
+#endif  // CHROME_BROWSER_EXO_PARTS_H_

chrome/browser/ui/views/ash/chrome_browser_main_extra_parts_ash.cc

@@ -44,6 +44,10 @@
 #include "ui/keyboard/keyboard_controller.h"
 #include "ui/views/mus/mus_client.h"
 
+#if BUILDFLAG(ENABLE_WAYLAND_SERVER)
+#include "chrome/browser/exo_parts.h"
+#endif
+
 ChromeBrowserMainExtraPartsAsh::ChromeBrowserMainExtraPartsAsh() {}
 
 ChromeBrowserMainExtraPartsAsh::~ChromeBrowserMainExtraPartsAsh() {}
@@ -117,9 +121,19 @@ void ChromeBrowserMainExtraPartsAsh::PreProfileInit() {
   keyboard::InitializeKeyboard();
 
   ui::SelectFileDialog::SetFactory(new SelectFileDialogExtensionFactory);
+
+#if BUILDFLAG(ENABLE_WAYLAND_SERVER)
+  exo_parts_ = ExoParts::CreateIfNecessary();
+#endif
 }
 
 void ChromeBrowserMainExtraPartsAsh::PostProfileInit() {
+#if BUILDFLAG(ENABLE_WAYLAND_SERVER)
+  // ExoParts uses state from ash, delete it before ash so that exo can
+  // uninstall correctly.
+  exo_parts_.reset();
+#endif
+
   if (ash_util::IsRunningInMash()) {
     DCHECK(!ash::Shell::HasInstance());
     DCHECK(!ChromeLauncherController::instance());

chrome/browser/ui/views/ash/chrome_browser_main_extra_parts_ash.h

@@ -9,6 +9,7 @@
 
 #include "base/macros.h"
 #include "chrome/browser/chrome_browser_main_extra_parts.h"
+#include "chrome/common/features.h"
 
 namespace ash {
 class ShelfModel;
@@ -37,6 +38,10 @@ class SystemTrayClient;
 class VolumeController;
 class VpnListForwarder;
 
+#if BUILDFLAG(ENABLE_WAYLAND_SERVER)
+class ExoParts;
+#endif
+
 // Browser initialization for Ash. Only runs on Chrome OS.
 // TODO(jamescook): Fold this into ChromeBrowserMainPartsChromeOS.
 class ChromeBrowserMainExtraPartsAsh : public ChromeBrowserMainExtraParts {
@@ -75,6 +80,10 @@ class ChromeBrowserMainExtraPartsAsh : public ChromeBrowserMainExtraParts {
   std::unique_ptr<ui::UserActivityDetector> user_activity_detector_;
   std::unique_ptr<aura::UserActivityForwarder> user_activity_forwarder_;
 
+#if BUILDFLAG(ENABLE_WAYLAND_SERVER)
+  std::unique_ptr<ExoParts> exo_parts_;
+#endif
+
   DISALLOW_COPY_AND_ASSIGN(ChromeBrowserMainExtraPartsAsh);
 };