It is possible for AuraInit to fail initialization. If the remote Ser…

…viceManager closes while a Service is starting up, then the synchronous calls to Directory will fail before error states are received.

This change follows up a previous change to AuraInit and QuickLauncher which was addressing these crashes in unittests. It refactors AuraInit to have an explicit initialization method. All callsites are updated to check the success of init, and to close cleanly upon failures.

TBR=sky@chromium.org
TEST=mash_browser_tests
BUG=678687

Review-Url: https://codereview.chromium.org/2967943002
Cr-Commit-Position: refs/heads/master@{#484583}

ash/autoclick/mus/autoclick_application.cc

@@ -90,9 +90,13 @@ AutoclickApplication::AutoclickApplication()
 AutoclickApplication::~AutoclickApplication() {}
 
 void AutoclickApplication::OnStart() {
-  aura_init_ = base::MakeUnique<views::AuraInit>(
+  aura_init_ = views::AuraInit::Create(
       context()->connector(), context()->identity(), "views_mus_resources.pak",
       std::string(), nullptr, views::AuraInit::Mode::AURA_MUS);
+  if (!aura_init_) {
+    context()->QuitNow();
+    return;
+  }
   autoclick_controller_common_.reset(new AutoclickControllerCommon(
       base::TimeDelta::FromMilliseconds(kDefaultAutoclickDelayMs), this));
 }

ash/mus/window_manager_application.cc

@@ -129,10 +129,14 @@ void WindowManagerApplication::OnStart() {
   mojo_interface_factory::RegisterInterfaces(
       &registry_, base::ThreadTaskRunnerHandle::Get());
 
-  aura_init_ = base::MakeUnique<views::AuraInit>(
+  aura_init_ = views::AuraInit::Create(
       context()->connector(), context()->identity(), "ash_mus_resources.pak",
       "ash_mus_resources_200.pak", nullptr,
       views::AuraInit::Mode::AURA_MUS_WINDOW_MANAGER);
+  if (!aura_init_) {
+    context()->QuitNow();
+    return;
+  }
   window_manager_ = base::MakeUnique<WindowManager>(
       context()->connector(), ash_config_, show_primary_host_on_connect_);
 

ash/touch_hud/mus/touch_hud_application.cc

@@ -69,9 +69,11 @@ TouchHudApplication::TouchHudApplication() : binding_(this) {
 TouchHudApplication::~TouchHudApplication() {}
 
 void TouchHudApplication::OnStart() {
-  aura_init_ = base::MakeUnique<views::AuraInit>(
+  aura_init_ = views::AuraInit::Create(
       context()->connector(), context()->identity(), "views_mus_resources.pak",
       std::string(), nullptr, views::AuraInit::Mode::AURA_MUS);
+  if (!aura_init_)
+    context()->QuitNow();
 }
 
 void TouchHudApplication::OnBindInterface(

mash/catalog_viewer/catalog_viewer.cc

@@ -222,9 +222,11 @@ void CatalogViewer::RemoveWindow(views::Widget* window) {
 }
 
 void CatalogViewer::OnStart() {
-  aura_init_ = base::MakeUnique<views::AuraInit>(
+  aura_init_ = views::AuraInit::Create(
       context()->connector(), context()->identity(), "views_mus_resources.pak",
       std::string(), nullptr, views::AuraInit::Mode::AURA_MUS);
+  if (!aura_init_)
+    context()->QuitNow();
 }
 
 void CatalogViewer::OnBindInterface(

mash/example/views_examples/views_examples.cc

@@ -31,10 +31,12 @@ class ViewsExamples : public service_manager::Service,
  private:
   // service_manager::Service:
   void OnStart() override {
-    aura_init_ = base::MakeUnique<views::AuraInit>(
-        context()->connector(), context()->identity(),
-        "views_mus_resources.pak", std::string(), nullptr,
-        views::AuraInit::Mode::AURA_MUS);
+    aura_init_ =
+        views::AuraInit::Create(context()->connector(), context()->identity(),
+                                "views_mus_resources.pak", std::string(),
+                                nullptr, views::AuraInit::Mode::AURA_MUS);
+    if (!aura_init_)
+      context()->QuitNow();
   }
   void OnBindInterface(const service_manager::BindSourceInfo& source_info,
                        const std::string& interface_name,

mash/example/window_type_launcher/window_type_launcher.cc

@@ -453,9 +453,11 @@ void WindowTypeLauncher::RemoveWindow(views::Widget* window) {
 }
 
 void WindowTypeLauncher::OnStart() {
-  aura_init_ = base::MakeUnique<views::AuraInit>(
+  aura_init_ = views::AuraInit::Create(
       context()->connector(), context()->identity(), "views_mus_resources.pak",
       std::string(), nullptr, views::AuraInit::Mode::AURA_MUS);
+  if (!aura_init_)
+    context()->QuitNow();
 }
 
 void WindowTypeLauncher::OnBindInterface(

mash/quick_launch/quick_launch.cc

@@ -171,14 +171,13 @@ void QuickLaunch::RemoveWindow(views::Widget* window) {
 }
 
 void QuickLaunch::OnStart() {
-  aura_init_ = base::MakeUnique<views::AuraInit>(
-      context()->connector(), context()->identity(), "views_mus_resources.pak",
-      std::string(), nullptr, views::AuraInit::Mode::AURA_MUS);
-
   // If AuraInit was unable to initialize there is no longer a peer connection.
   // The ServiceManager is in the process of shutting down, however we haven't
   // been notified yet. Close our ServiceContext and shutdown.
-  if (!aura_init_->initialized()) {
+  aura_init_ = views::AuraInit::Create(
+      context()->connector(), context()->identity(), "views_mus_resources.pak",
+      std::string(), nullptr, views::AuraInit::Mode::AURA_MUS);
+  if (!aura_init_) {
     context()->QuitNow();
     return;
   }

mash/simple_wm/simple_wm.cc

@@ -364,9 +364,13 @@ void SimpleWM::OnStart() {
   started_ = true;
   screen_ = base::MakeUnique<display::ScreenBase>();
   display::Screen::SetScreenInstance(screen_.get());
-  aura_init_ = base::MakeUnique<views::AuraInit>(
+  aura_init_ = views::AuraInit::Create(
       context()->connector(), context()->identity(), "views_mus_resources.pak",
       std::string(), nullptr, views::AuraInit::Mode::AURA_MUS_WINDOW_MANAGER);
+  if (!aura_init_) {
+    context()->QuitNow();
+    return;
+  }
   window_tree_client_ = base::MakeUnique<aura::WindowTreeClient>(
       context()->connector(), this, this);
   aura::Env::GetInstance()->SetWindowTreeClient(window_tree_client_.get());

mash/task_viewer/task_viewer.cc

@@ -294,9 +294,11 @@ void TaskViewer::RemoveWindow(views::Widget* widget) {
 }
 
 void TaskViewer::OnStart() {
-  aura_init_ = base::MakeUnique<views::AuraInit>(
+  aura_init_ = views::AuraInit::Create(
       context()->connector(), context()->identity(), "views_mus_resources.pak",
       std::string(), nullptr, views::AuraInit::Mode::AURA_MUS);
+  if (!aura_init_)
+    context()->QuitNow();
 }
 
 void TaskViewer::OnBindInterface(

ui/views/mus/aura_init.cc

@@ -52,27 +52,56 @@ class MusViewsDelegate : public ViewsDelegate {
 
 }  // namespace
 
-AuraInit::AuraInit(service_manager::Connector* connector,
-                   const service_manager::Identity& identity,
-                   const std::string& resource_file,
-                   const std::string& resource_file_200,
-                   scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
-                   Mode mode)
-    : resource_file_(resource_file),
-      resource_file_200_(resource_file_200),
-      env_(aura::Env::CreateInstance(
-          (mode == Mode::AURA_MUS || mode == Mode::AURA_MUS_WINDOW_MANAGER)
-              ? aura::Env::Mode::MUS
-              : aura::Env::Mode::LOCAL)) {
+AuraInit::AuraInit() {
   if (!ViewsDelegate::GetInstance())
     views_delegate_ = base::MakeUnique<MusViewsDelegate>();
+}
+
+AuraInit::~AuraInit() {
+#if defined(OS_LINUX)
+  if (font_loader_.get()) {
+    SkFontConfigInterface::SetGlobal(nullptr);
+    // FontLoader is ref counted. We need to explicitly shutdown the background
+    // thread, otherwise the background thread may be shutdown after the app is
+    // torn down, when we're in a bad state.
+    font_loader_->Shutdown();
+  }
+#endif
+}
+
+std::unique_ptr<AuraInit> AuraInit::Create(
+    service_manager::Connector* connector,
+    const service_manager::Identity& identity,
+    const std::string& resource_file,
+    const std::string& resource_file_200,
+    scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
+    Mode mode) {
+  std::unique_ptr<AuraInit> aura_init = base::WrapUnique(new AuraInit());
+  if (!aura_init->Init(connector, identity, resource_file, resource_file_200,
+                       io_task_runner, mode)) {
+    aura_init.reset();
+  }
+  return aura_init;
+}
+
+bool AuraInit::Init(service_manager::Connector* connector,
+                    const service_manager::Identity& identity,
+                    const std::string& resource_file,
+                    const std::string& resource_file_200,
+                    scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
+                    Mode mode) {
+  env_ = aura::Env::CreateInstance(
+      (mode == Mode::AURA_MUS || mode == Mode::AURA_MUS_WINDOW_MANAGER)
+          ? aura::Env::Mode::MUS
+          : aura::Env::Mode::LOCAL);
+
   if (mode == Mode::AURA_MUS) {
     mus_client_ =
         base::WrapUnique(new MusClient(connector, identity, io_task_runner));
   }
   ui::MaterialDesignController::Initialize();
-  if (!InitializeResources(connector))
-    return;
+  if (!InitializeResources(connector, resource_file, resource_file_200))
+    return false;
 
 // Initialize the skia font code to go ask fontconfig underneath.
 #if defined(OS_LINUX)
@@ -85,30 +114,20 @@ AuraInit::AuraInit(service_manager::Connector* connector,
   gfx::Font();
 
   ui::InitializeInputMethodForTesting();
-  initialized_ = true;
-}
-
-AuraInit::~AuraInit() {
-#if defined(OS_LINUX)
-  if (font_loader_.get()) {
-    SkFontConfigInterface::SetGlobal(nullptr);
-    // FontLoader is ref counted. We need to explicitly shutdown the background
-    // thread, otherwise the background thread may be shutdown after the app is
-    // torn down, when we're in a bad state.
-    font_loader_->Shutdown();
-  }
-#endif
+  return true;
 }
 
-bool AuraInit::InitializeResources(service_manager::Connector* connector) {
+bool AuraInit::InitializeResources(service_manager::Connector* connector,
+                                   const std::string& resource_file,
+                                   const std::string& resource_file_200) {
   // Resources may have already been initialized (e.g. when 'chrome --mash' is
   // used to launch the current app).
   if (ui::ResourceBundle::HasSharedInstance())
     return false;
 
-  std::set<std::string> resource_paths({resource_file_});
-  if (!resource_file_200_.empty())
-    resource_paths.insert(resource_file_200_);
+  std::set<std::string> resource_paths({resource_file});
+  if (!resource_file_200.empty())
+    resource_paths.insert(resource_file_200);
 
   catalog::ResourceLoader loader;
   filesystem::mojom::DirectoryPtr directory;
@@ -123,15 +142,15 @@ bool AuraInit::InitializeResources(service_manager::Connector* connector) {
   if (!loader.OpenFiles(std::move(directory), resource_paths))
     return false;
   ui::RegisterPathProvider();
-  base::File pak_file = loader.TakeFile(resource_file_);
+  base::File pak_file = loader.TakeFile(resource_file);
   base::File pak_file_2 = pak_file.Duplicate();
   ui::ResourceBundle::InitSharedInstanceWithPakFileRegion(
       std::move(pak_file), base::MemoryMappedFile::Region::kWholeFile);
   ui::ResourceBundle::GetSharedInstance().AddDataPackFromFile(
       std::move(pak_file_2), ui::SCALE_FACTOR_100P);
-  if (!resource_file_200_.empty())
+  if (!resource_file_200.empty())
     ui::ResourceBundle::GetSharedInstance().AddDataPackFromFile(
-        loader.TakeFile(resource_file_200_), ui::SCALE_FACTOR_200P);
+        loader.TakeFile(resource_file_200), ui::SCALE_FACTOR_200P);
   return true;
 }
 

ui/views/mus/aura_init.h

@@ -51,42 +51,51 @@ class VIEWS_MUS_EXPORT AuraInit {
     UI
   };
 
+  ~AuraInit();
+
+  // Returns an AuraInit if initialization can be completed successfully,
+  // otherwise a nullptr is returned. If initialization fails then Aura is in an
+  // unusable state, and calling services should shutdown.
   // |resource_file| is the file to load strings and 1x icons from.
   // |resource_file_200| can be an empty string, otherwise it is the file to
   // load 2x icons from.
-  AuraInit(service_manager::Connector* connector,
-           const service_manager::Identity& identity,
-           const std::string& resource_file,
-           const std::string& resource_file_200 = std::string(),
-           scoped_refptr<base::SingleThreadTaskRunner> io_task_runner = nullptr,
-           Mode mode = Mode::UI);
-  ~AuraInit();
+  static std::unique_ptr<AuraInit> Create(
+      service_manager::Connector* connector,
+      const service_manager::Identity& identity,
+      const std::string& resource_file,
+      const std::string& resource_file_200 = std::string(),
+      scoped_refptr<base::SingleThreadTaskRunner> io_task_runner = nullptr,
+      Mode mode = Mode::UI);
 
   // Only valid if Mode::AURA_MUS was passed to constructor.
   MusClient* mus_client() { return mus_client_.get(); }
 
+ private:
+  AuraInit();
+
   // Returns true if AuraInit was able to successfully complete initialization.
   // If this returns false, then Aura is in an unusable state, and calling
   // services should shutdown.
-  bool initialized() { return initialized_; }
-
- private:
-  bool InitializeResources(service_manager::Connector* connector);
+  bool Init(
+      service_manager::Connector* connector,
+      const service_manager::Identity& identity,
+      const std::string& resource_file,
+      const std::string& resource_file_200 = std::string(),
+      scoped_refptr<base::SingleThreadTaskRunner> io_task_runner = nullptr,
+      Mode mode = Mode::UI);
+
+  bool InitializeResources(service_manager::Connector* connector,
+                           const std::string& resource_file,
+                           const std::string& resource_file_200);
 
 #if defined(OS_LINUX)
   sk_sp<font_service::FontLoader> font_loader_;
 #endif
 
-  const std::string resource_file_;
-  const std::string resource_file_200_;
-
   std::unique_ptr<aura::Env> env_;
   std::unique_ptr<MusClient> mus_client_;
   std::unique_ptr<ViewsDelegate> views_delegate_;
 
-  // Whether or not initialization succeeds.
-  bool initialized_ = false;
-
   DISALLOW_COPY_AND_ASSIGN(AuraInit);
 };