Relnad Update AuraInit to handle failed initialization

This change relands AuraInit changes, with a fix to the shared resources logic which was causing a crash on chrome --mash Revert "Revert of Update AuraInit to handle failed initialization (patchset chromium#2 id:40001 of https://codereview.chromium.org/2967943002/ )" This reverts commit e3f9dbb. TBR=sky@chromium.org TEST=ran chrome --mash Bug: 678687 Change-Id: I6d64b04c520564a022b65e2e2f6c322803a9db64 Reviewed-on: https://chromium-review.googlesource.com/562497 Commit-Queue: Jonathan Ross <jonross@chromium.org> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> Cr-Commit-Position: refs/heads/master@{#485258}
marcosholgado · Jul 10, 2017 · 1ab1bcf · 1ab1bcf
1 parent 3eb6363
commit 1ab1bcf
Show file tree

Hide file tree

Showing 11 changed files with 117 additions and 68 deletions.
diff --git a/ash/autoclick/mus/autoclick_application.cc b/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));
 }

diff --git a/ash/mus/window_manager_application.cc b/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_);
 

diff --git a/ash/touch_hud/mus/touch_hud_application.cc b/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(

diff --git a/mash/catalog_viewer/catalog_viewer.cc b/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(

diff --git a/mash/example/views_examples/views_examples.cc b/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,

diff --git a/mash/example/window_type_launcher/window_type_launcher.cc b/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(

diff --git a/mash/quick_launch/quick_launch.cc b/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;
   }

diff --git a/mash/simple_wm/simple_wm.cc b/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());

diff --git a/mash/task_viewer/task_viewer.cc b/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(

diff --git a/ui/views/mus/aura_init.cc b/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;
+    return true;
 
-  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;
 }
 

diff --git a/ui/views/mus/aura_init.h b/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);
 };