Reland "chromeos: fix DCHECK in ~WindowTreeHostManager"

This is a reland of b650c38e
Original change's description:
> 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}

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

chrome/browser/BUILD.gn

@@ -3912,8 +3912,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

@@ -389,10 +389,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)
@@ -907,10 +903,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,6 +121,10 @@ void ChromeBrowserMainExtraPartsAsh::PreProfileInit() {
   keyboard::InitializeKeyboard();
 
   ui::SelectFileDialog::SetFactory(new SelectFileDialogExtensionFactory);
+
+#if BUILDFLAG(ENABLE_WAYLAND_SERVER)
+  exo_parts_ = ExoParts::CreateIfNecessary();
+#endif
 }
 
 void ChromeBrowserMainExtraPartsAsh::PostProfileInit() {
@@ -147,6 +155,12 @@ void ChromeBrowserMainExtraPartsAsh::PostProfileInit() {
 }
 
 void ChromeBrowserMainExtraPartsAsh::PostMainMessageLoopRun() {
+#if BUILDFLAG(ENABLE_WAYLAND_SERVER)
+  // ExoParts uses state from ash, delete it before ash so that exo can
+  // uninstall correctly.
+  exo_parts_.reset();
+#endif
+
   chrome_launcher_controller_.reset();
   chrome_shelf_model_.reset();
   vpn_list_forwarder_.reset();

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);
 };