[Cleanup] Screen cleanup

ash/shell.cc

Index: ash/shell.cc
diff --git a/ash/shell.cc b/ash/shell.cc
index be5661516138f8434135232c6519f6cf54039851..997d59e49f8d8427dcf296eaa46ad9f36160f545 100644
--- a/ash/shell.cc
+++ b/ash/shell.cc
@@ -35,7 +35,6 @@
 #include "ash/media_delegate.h"
 #include "ash/new_window_delegate.h"
 #include "ash/root_window_controller.h"
-#include "ash/screen_ash.h"
 #include "ash/session_state_delegate.h"
 #include "ash/shelf/app_list_shelf_item_delegate.h"
 #include "ash/shelf/shelf_delegate.h"
@@ -549,8 +548,7 @@ void Shell::DoInitialWorkspaceAnimation() {
 // Shell, private:
 
 Shell::Shell(ShellDelegate* delegate)
-    : screen_(new ScreenAsh),
-      target_root_window_(NULL),
+    : target_root_window_(NULL),
       scoped_target_root_window_(NULL),
       delegate_(delegate),
       window_positioner_(new WindowPositioner),
@@ -566,10 +564,12 @@ Shell::Shell(ShellDelegate* delegate)
   DCHECK(delegate_.get());
   display_manager_.reset(new internal::DisplayManager);
 
-  ANNOTATE_LEAKING_OBJECT_PTR(screen_);  // see crbug.com/156466
-  gfx::Screen::SetScreenInstance(gfx::SCREEN_TYPE_ALTERNATE, screen_);
-  if (!gfx::Screen::GetScreenByType(gfx::SCREEN_TYPE_NATIVE))
-    gfx::Screen::SetScreenInstance(gfx::SCREEN_TYPE_NATIVE, screen_);
+  gfx::Screen::SetScreenInstance(gfx::SCREEN_TYPE_ALTERNATE,
+                                 display_manager_->screen());
+  if (!gfx::Screen::GetScreenByType(gfx::SCREEN_TYPE_NATIVE)) {
+    gfx::Screen::SetScreenInstance(gfx::SCREEN_TYPE_NATIVE,
+                                   display_manager_->screen());
+  }

[Jun Mukai, 2014/01/15 19:20:15]

If the screen object is owned by DisplayManager, I prefer to put the
initialization / shutdown code to ctor/dtor of DisplayManager.

Did you try that?  At least the initialization code can be in DisplayManager I
guess, but the shutdown code seems a bit tricky?

[oshima, 2014/01/15 19:42:51]

That's good point, although I can't move the shutdown code to dtor due to
dependency. I moved initialization to ctor, and moved impl to
CreateScreenForShutdown .

   display_controller_.reset(new DisplayController);
 #if defined(OS_CHROMEOS) && defined(USE_X11)
   bool is_panel_fitting_disabled =
@@ -691,6 +691,16 @@ Shell::~Shell() {
 
   resolution_notification_controller_.reset();
   desktop_background_controller_.reset();
+  bool native_is_alternative =
+      gfx::Screen::GetScreenByType(gfx::SCREEN_TYPE_NATIVE) ==
+      gfx::Screen::GetScreenByType(gfx::SCREEN_TYPE_ALTERNATE);
+  gfx::Screen* for_shutdown =
+      display_manager_->CreateScreenForShutdown();
+  // This object should be alive until the end.
+  ANNOTATE_LEAKING_OBJECT_PTR(for_shutdown);
+  gfx::Screen::SetScreenInstance(gfx::SCREEN_TYPE_ALTERNATE, for_shutdown);
+  if (native_is_alternative)
+    gfx::Screen::SetScreenInstance(gfx::SCREEN_TYPE_NATIVE, for_shutdown);
 
   // This also deletes all RootWindows. Note that we invoke Shutdown() on
   // DisplayController before resetting |display_controller_|, since destruction
@@ -794,7 +804,7 @@ void Shell::Init() {
   resolution_notification_controller_.reset(
       new internal::ResolutionNotificationController);
 
-  cursor_manager_.SetDisplay(DisplayController::GetPrimaryDisplay());
+  cursor_manager_.SetDisplay(GetScreen()->GetPrimaryDisplay());
 
   nested_dispatcher_controller_.reset(new NestedDispatcherController);
   accelerator_controller_.reset(new AcceleratorController);