Reland: Update DisplayConfigurator to use the asynchronous tasks

Original review: https://codereview.chromium.org/801493002/

BUG=429746
TESTS=display_unittests with ASAN
TBR=derat@chromium.org

Review URL: https://codereview.chromium.org/800143002

Cr-Commit-Position: refs/heads/master@{#308228}

ui/display/chromeos/configure_displays_task.cc

@@ -65,17 +65,19 @@ void ConfigureDisplaysTask::Run() {
   if (is_configuring_)
     return;
 
-  base::AutoReset<bool> recursivity_guard(&is_configuring_, true);
-  while (!pending_request_indexes_.empty()) {
-    size_t index = pending_request_indexes_.front();
-    DisplayConfigureRequest* request = &requests_[index];
-    pending_request_indexes_.pop();
-    delegate_->Configure(*request->display, request->mode, request->origin,
-                         base::Bind(&ConfigureDisplaysTask::OnConfigured,
-                                    weak_ptr_factory_.GetWeakPtr(), index));
+  {
+    base::AutoReset<bool> recursivity_guard(&is_configuring_, true);
+    while (!pending_request_indexes_.empty()) {
+      size_t index = pending_request_indexes_.front();
+      DisplayConfigureRequest* request = &requests_[index];
+      pending_request_indexes_.pop();
+      delegate_->Configure(*request->display, request->mode, request->origin,
+                           base::Bind(&ConfigureDisplaysTask::OnConfigured,
+                                      weak_ptr_factory_.GetWeakPtr(), index));
+    }
   }
 
-  // Nothing should be modified after the |callback_| is called since thie
+  // Nothing should be modified after the |callback_| is called since the
   // task may be deleted in the callback.
   if (num_displays_configured_ == requests_.size())
     callback_.Run(task_status_);

ui/display/chromeos/display_configurator.h

@@ -13,6 +13,7 @@
 
 #include "base/event_types.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "base/timer/timer.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
@@ -31,6 +32,7 @@ struct DisplayConfigureRequest;
 class DisplayMode;
 class DisplaySnapshot;
 class NativeDisplayDelegate;
+class UpdateDisplayConfigurationTask;
 
 // This class interacts directly with the system display configurator.
 class DISPLAY_EXPORT DisplayConfigurator : public NativeDisplayObserver {
@@ -172,7 +174,7 @@ class DISPLAY_EXPORT DisplayConfigurator : public NativeDisplayObserver {
   DisplayConfigurator();
   virtual ~DisplayConfigurator();
 
-  MultipleDisplayState display_state() const { return display_state_; }
+  MultipleDisplayState display_state() const { return current_display_state_; }
   chromeos::DisplayPowerState requested_power_state() const {
     return requested_power_state_;
   }
@@ -222,12 +224,12 @@ class DISPLAY_EXPORT DisplayConfigurator : public NativeDisplayObserver {
   // the display(s) in question so that the low power state is engaged.
   // |flags| contains bitwise-or-ed kSetDisplayPower* values. Returns true if
   // the system successfully enters (or was already in) |power_state|.
-  bool SetDisplayPower(chromeos::DisplayPowerState power_state, int flags);
+  void SetDisplayPower(chromeos::DisplayPowerState power_state, int flags);
 
   // Force switching the display mode to |new_state|. Returns false if
   // switching failed (possibly because |new_state| is invalid for the
   // current set of connected displays).
-  bool SetDisplayMode(MultipleDisplayState new_state);
+  void SetDisplayMode(MultipleDisplayState new_state);
 
   // NativeDisplayDelegate::Observer overrides:
   virtual void OnConfigurationChanged() override;
@@ -279,6 +281,8 @@ class DISPLAY_EXPORT DisplayConfigurator : public NativeDisplayObserver {
                                   ui::ColorCalibrationProfile new_profile);
 
  private:
+  class DisplayLayoutManagerImpl;
+
   // Mapping a display_id to a protection request bitmask.
   typedef std::map<int64_t, uint32_t> ContentProtections;
   // Mapping a client to its protection request.
@@ -288,29 +292,6 @@ class DISPLAY_EXPORT DisplayConfigurator : public NativeDisplayObserver {
   // Performs platform specific delegate initialization.
   scoped_ptr<NativeDisplayDelegate> CreatePlatformNativeDisplayDelegate();
 
-  // Updates |cached_displays_| to contain currently-connected displays. Calls
-  // |delegate_->GetDisplays()| and then does additional work, like finding the
-  // mirror mode and setting user-preferred modes. Note that the server must be
-  // grabbed via |delegate_->GrabServer()| first.
-  void UpdateCachedDisplays();
-
-  // Helper method for UpdateCachedDisplays() that initializes the passed-in
-  // displays' |mirror_mode| fields by looking for a mode in |internal_display|
-  // and |external_display| having the same resolution. Returns false if a
-  // shared
-  // mode wasn't found or created.
-  //
-  // |try_panel_fitting| allows creating a panel-fitting mode for
-  // |internal_display| instead of only searching for a matching mode (note that
-  // it may lead to a crash if |internal_info| is not capable of panel fitting).
-  //
-  // |preserve_aspect| limits the search/creation only to the modes having the
-  // native aspect ratio of |external_display|.
-  bool FindMirrorMode(DisplayState* internal_display,
-                      DisplayState* external_display,
-                      bool try_panel_fitting,
-                      bool preserve_aspect);
-
   // Configures displays. Invoked by |configure_timer_|.
   void ConfigureDisplays();
 
@@ -321,22 +302,6 @@ class DISPLAY_EXPORT DisplayConfigurator : public NativeDisplayObserver {
   // Notifies observers about an attempted state change.
   void NotifyObservers(bool success, MultipleDisplayState attempted_state);
 
-  // Switches to the state specified in |display_state| and |power_state|.
-  // If the hardware mirroring failed and |mirroring_controller_| is set,
-  // it switches to |STATE_DUAL_EXTENDED| and calls |SetSoftwareMirroring()|
-  // to enable software based mirroring.
-  // On success, updates |display_state_|, |power_state_|, and
-  // |cached_displays_| and returns true.
-  bool EnterStateOrFallBackToSoftwareMirroring(
-      MultipleDisplayState display_state,
-      chromeos::DisplayPowerState power_state);
-
-  // Switches to the state specified in |display_state| and |power_state|.
-  // On success, updates |display_state_|, |power_state_|, and
-  // |cached_displays_| and returns true.
-  bool EnterState(MultipleDisplayState display_state,
-                  chromeos::DisplayPowerState power_state);
-
   // Returns the display state that should be used with |cached_displays_| while
   // in |power_state|.
   MultipleDisplayState ChooseDisplayState(
@@ -352,6 +317,23 @@ class DISPLAY_EXPORT DisplayConfigurator : public NativeDisplayObserver {
   // Applies display protections according to requests.
   bool ApplyProtections(const ContentProtections& requests);
 
+  // If |configuration_task_| isn't initialized, initializes it and starts the
+  // configuration task.
+  void RunPendingConfiguration();
+
+  // Callback for |configuration_taks_|. When the configuration process finishes
+  // this is called with the result (|success|) and the updated display state.
+  void OnConfigured(bool success,
+                    const std::vector<DisplayState>& displays,
+                    const gfx::Size& framebuffer_size,
+                    MultipleDisplayState new_display_state,
+                    chromeos::DisplayPowerState new_power_state);
+
+  // Helps in identifying if a configuration task needs to be scheduled.
+  // Return true if any of the |requested_*| parameters have been updated. False
+  // otherwise.
+  bool ShouldRunConfigurationTask() const;
+
   StateController* state_controller_;
   SoftwareMirroringController* mirroring_controller_;
   scoped_ptr<NativeDisplayDelegate> native_display_delegate_;
@@ -366,21 +348,35 @@ class DISPLAY_EXPORT DisplayConfigurator : public NativeDisplayObserver {
   // configuration to immediately fail without changing the state.
   bool configure_display_;
 
-  // The current display state.
-  MultipleDisplayState display_state_;
+  // Current configuration state.
+  MultipleDisplayState current_display_state_;
+  chromeos::DisplayPowerState current_power_state_;
 
-  gfx::Size framebuffer_size_;
+  // Pending requests. These values are used when triggering the next display
+  // configuration.
+  //
+  // Stores the user requested state or INVALID if nothing was requested.
+  MultipleDisplayState requested_display_state_;
 
-  // The last-requested and current power state. These may differ if
-  // configuration fails: SetDisplayMode() needs the last-requested state while
-  // SetDisplayPower() needs the current state.
+  // Stores the requested power state.
   chromeos::DisplayPowerState requested_power_state_;
-  chromeos::DisplayPowerState current_power_state_;
+
+  // True if |requested_power_state_| has been changed due to a user request.
+  bool requested_power_state_change_;
+
+  // Bitwise-or value of the |kSetDisplayPower*| flags defined above.
+  int requested_power_flags_;
+
+  // True if the caller wants to force the display configuration process.
+  bool force_configure_;
 
   // Most-recently-used display configuration. Note that the actual
   // configuration changes asynchronously.
   DisplayStateList cached_displays_;
 
+  // Most-recently-used framebuffer size.
+  gfx::Size framebuffer_size_;
+
   ObserverList<Observer> observers_;
 
   // The timer to delay configuring displays. This is used to aggregate multiple
@@ -397,6 +393,13 @@ class DISPLAY_EXPORT DisplayConfigurator : public NativeDisplayObserver {
   // Display controlled by an external entity.
   bool display_externally_controlled_;
 
+  scoped_ptr<DisplayLayoutManager> layout_manager_;
+
+  scoped_ptr<UpdateDisplayConfigurationTask> configuration_task_;
+
+  // This must be the last variable.
+  base::WeakPtrFactory<DisplayConfigurator> weak_ptr_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(DisplayConfigurator);
 };