Revert of Update AuraInit to handle failed initialization (patchset #2...

Revert of Update AuraInit to handle failed initialization (patchset #2 id:40001 of https://codereview.chromium.org/2967943002/ )

Reason for revert:
Causing chrome --mash to fail to start

[118236:118236:0706/142624.101546:3509128143561:ERROR:chrome_main_delegate.cc(1187)] Main process exiting because service ash quit unexpectedly.

Original issue's description:
> It is possible for AuraInit to fail initialization. If the remote ServiceManager 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}
> Committed: https://chromium.googlesource.com/chromium/src/+/14402447b7df16d16126dc49a3a44cf66c318c88

TBR=sadrul@chromium.org,sky@chromium.org,jonross@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=678687

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

ash/autoclick/mus/autoclick_application.cc

@@ -90,13 +90,9 @@ AutoclickApplication::AutoclickApplication()
 AutoclickApplication::~AutoclickApplication() {}
 
 void AutoclickApplication::OnStart() {
-  aura_init_ = views::AuraInit::Create(
+  aura_init_ = base::MakeUnique<views::AuraInit>(
       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,14 +129,10 @@ void WindowManagerApplication::OnStart() {
   mojo_interface_factory::RegisterInterfaces(
       &registry_, base::ThreadTaskRunnerHandle::Get());
 
-  aura_init_ = views::AuraInit::Create(
+  aura_init_ = base::MakeUnique<views::AuraInit>(
       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,11 +69,9 @@ TouchHudApplication::TouchHudApplication() : binding_(this) {
 TouchHudApplication::~TouchHudApplication() {}
 
 void TouchHudApplication::OnStart() {
-  aura_init_ = views::AuraInit::Create(
+  aura_init_ = base::MakeUnique<views::AuraInit>(
       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,11 +222,9 @@ void CatalogViewer::RemoveWindow(views::Widget* window) {
 }
 
 void CatalogViewer::OnStart() {
-  aura_init_ = views::AuraInit::Create(
+  aura_init_ = base::MakeUnique<views::AuraInit>(
       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,12 +31,10 @@ class ViewsExamples : public service_manager::Service,
  private:
   // service_manager::Service:
   void OnStart() override {
-    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();
+    aura_init_ = base::MakeUnique<views::AuraInit>(
+        context()->connector(), context()->identity(),
+        "views_mus_resources.pak", std::string(), nullptr,
+        views::AuraInit::Mode::AURA_MUS);
   }
   void OnBindInterface(const service_manager::BindSourceInfo& source_info,
                        const std::string& interface_name,

mash/example/window_type_launcher/window_type_launcher.cc

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

mash/simple_wm/simple_wm.cc

@@ -364,13 +364,9 @@ void SimpleWM::OnStart() {
   started_ = true;
   screen_ = base::MakeUnique<display::ScreenBase>();
   display::Screen::SetScreenInstance(screen_.get());
-  aura_init_ = views::AuraInit::Create(
+  aura_init_ = base::MakeUnique<views::AuraInit>(
       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,11 +294,9 @@ void TaskViewer::RemoveWindow(views::Widget* widget) {
 }
 
 void TaskViewer::OnStart() {
-  aura_init_ = views::AuraInit::Create(
+  aura_init_ = base::MakeUnique<views::AuraInit>(
       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,56 +52,27 @@ class MusViewsDelegate : public ViewsDelegate {
 
 }  // namespace
 
-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,
+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) {
-  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)
+    : 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);
-
+              : aura::Env::Mode::LOCAL)) {
+  if (!ViewsDelegate::GetInstance())
+    views_delegate_ = base::MakeUnique<MusViewsDelegate>();
   if (mode == Mode::AURA_MUS) {
     mus_client_ =
         base::WrapUnique(new MusClient(connector, identity, io_task_runner));
   }
   ui::MaterialDesignController::Initialize();
-  if (!InitializeResources(connector, resource_file, resource_file_200))
-    return false;
+  if (!InitializeResources(connector))
+    return;
 
 // Initialize the skia font code to go ask fontconfig underneath.
 #if defined(OS_LINUX)
@@ -114,20 +85,30 @@ bool AuraInit::Init(service_manager::Connector* connector,
   gfx::Font();
 
   ui::InitializeInputMethodForTesting();
-  return true;
+  initialized_ = true;
 }
 
-bool AuraInit::InitializeResources(service_manager::Connector* connector,
-                                   const std::string& resource_file,
-                                   const std::string& resource_file_200) {
+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
+}
+
+bool AuraInit::InitializeResources(service_manager::Connector* connector) {
   // 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;
@@ -142,15 +123,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,51 +51,42 @@ 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.
-  static std::unique_ptr<AuraInit> Create(
-      service_manager::Connector* connector,
+  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();
 
   // 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 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 initialized() { return initialized_; }
 
-  bool InitializeResources(service_manager::Connector* connector,
-                           const std::string& resource_file,
-                           const std::string& resource_file_200);
+ private:
+  bool InitializeResources(service_manager::Connector* connector);
 
 #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);
 };