Fix crashers in GetPrimaryDisplay

ash/display/display_controller.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef ASH_DISPLAY_DISPLAY_CONTROLLER_H_
 #define ASH_DISPLAY_DISPLAY_CONTROLLER_H_
 
 #include <map>
 #include <vector>
 
@@ skipping 65 matching lines @@
     // but before the change is applied to aura/ash.
     virtual void OnDisplayConfigurationChanging() = 0;
 
    protected:
     virtual ~Observer() {}
   };
 
   DisplayController();
   virtual ~DisplayController();
 
+  // Gets primary display. This information is stored in global
+  // object as this can be accessed after Shell is closed.
+  static const gfx::Display& GetPrimaryDisplay();

[Daniel Erat, 2012/09/19 20:47:33]

nit: returning a reference to an internal pointer feels a bit strange.  could
you make this return const gfx::Display* or just a gfx::Display (since it looks
like this gets copied downstream) instead?

[oshima, 2012/09/19 21:21:28]

This pattern isn't uncommon in chrome (quick search of "  const\s[^\s]*&.*(" in
header shows a lot, although it doesn't necessarily mean it's ok just because
it's common :) 
and this won't cause real issue as this is global object which should outlive
UI.
Or are you worrying about because it may be deleted DisplayController's
constructor?

I'm happy to make this change as gfx::Display is treated as POD, but want to
understand
your concern before making the change.

[Daniel Erat, 2012/09/19 21:31:47]

Accessors that return const references are super-common for members that are
embedded within the class (since you know that the reference will be valid for
the lifetime of the object), but seem very rare for pointer members -- I only
see six files when I grep for 'const\s[^\s]*&.*(.*return \*'.  It makes me
nervous since it's possible for the reference to become invalid while the object
is still alive, if the object's pointer gets updated to point somewhere else.

+
   // Initializes primary display.
   void InitPrimaryDisplay();
 
   // Initialize secondary displays.
   void InitSecondaryDisplays();
 
   // Add/Remove observers.
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
 
   // Returns the root window for primary display.
   aura::RootWindow* GetPrimaryRootWindow();
 
   // Returns the root window for |display_id|.
   aura::RootWindow* GetRootWindowForDisplayId(int64 id);
 
-  // Sets/Gets primary display.
-  const gfx::Display& primary_display() const {
-    return primary_display_;
-  }
+  // Sets primary display. This re-assigns the current root
+  // window to given |display|.
   void SetPrimaryDisplay(const gfx::Display& display);
 
   // Returns the secondary display.
   gfx::Display* GetSecondaryDisplay();
 
   // Closes all child windows in the all root windows.
   void CloseChildWindows();
 
   // Returns all root windows. In non extended desktop mode, this
   // returns the primary root window only.
@@ skipping 37 matching lines @@
   std::map<int64, aura::RootWindow*> root_windows_;
 
   // The default display layout.
   DisplayLayout default_display_layout_;
 
   // Per-device display layout.
   std::map<std::string, DisplayLayout> secondary_layouts_;
 
   ObserverList<Observer> observers_;
 
-  gfx::Display primary_display_;
-
   DISALLOW_COPY_AND_ASSIGN(DisplayController);
 };
 
 }  // namespace ash
 
 #endif  // ASH_DISPLAY_DISPLAY_CONTROLLER_H_