Handles the failures of output state change.

chromeos/display/output_configurator.cc

 // 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.
 
 #include "chromeos/display/output_configurator.h"
 
 #include <X11/Xlib.h>
 #include <X11/extensions/dpms.h>
 #include <X11/extensions/Xrandr.h>
 
@@ skipping 643 matching lines @@
                  connected_output_count_)) {
     did_change = true;
   }
   // We have seen cases where the XRandR data can get out of sync with our own
   // cache so over-write it with whatever we detected, even if we didn't think
   // anything changed.
   output_state_ = next_state;
 
   XRRFreeScreenResources(screen);
   XUngrabServer(display);
+
+  if (!did_change)
+    FOR_EACH_OBSERVER(Observer, observers_, OnDisplayModeChangeFailed());
   return did_change;
 }
 
 bool OutputConfigurator::ScreenPowerSet(bool power_on, bool all_displays) {
   VLOG(1) << "OutputConfigurator::SetScreensOn " << power_on
           << " all displays " << all_displays;
   if (!is_running_on_chrome_os_)
     return false;
 
   bool success = false;
@@ skipping 54 matching lines @@
   XRRFreeScreenResources(screen);
   XUngrabServer(display);
 
   return success;
 }
 
 bool OutputConfigurator::SetDisplayMode(OutputState new_state) {
   if (output_state_ == STATE_INVALID ||
       output_state_ == STATE_HEADLESS ||
       output_state_ == STATE_SINGLE)
     return false;

[oshima, 2012/10/01 21:44:02]

is it possible to return output_state_ == new_state,
and simply test if output_state_ has changed later?

[Jun Mukai, 2012/10/01 21:49:35]

The header said it "returns false if it was called in a single-head or headless
mode."  Did you intend to change this too?
I think changing this would be fine since the existing callers don't care its
return value anyway for now.

[oshima, 2012/10/01 22:16:19]

I was thinking to add

if (output_state_ == new_state)
   return true;   // switching to the same state is considered successful.

and this shouldn't require changes to header. Do you think there is a risk?

[Jun Mukai, 2012/10/01 22:36:56]

That would be safe.  Added.

 
   Display* display = base::MessagePumpAuraX11::GetDefaultXDisplay();
   CHECK(display != NULL);
   XGrabServer(display);
   Window window = DefaultRootWindow(display);
   XRRScreenResources* screen = GetScreenResourcesAndRecordUMA(display, window);
   CHECK(screen != NULL);
 
+  bool did_change = false;
   OutputSnapshot outputs[2] = { {0}, {0} };
   connected_output_count_ =
       GetDualOutputs(display, screen, &outputs[0], &outputs[1]);
   if (EnterState(display,
                  screen,
                  window,
                  new_state,
                  outputs,
                  connected_output_count_)) {
     output_state_ = new_state;
+    did_change = true;
   }
 
   XRRFreeScreenResources(screen);
   XUngrabServer(display);
+
+  if (!did_change)
+    FOR_EACH_OBSERVER(Observer, observers_, OnDisplayModeChangeFailed());
   return true;
 }
 
 bool OutputConfigurator::Dispatch(const base::NativeEvent& event) {
   // Ignore this event if the Xrandr extension isn't supported.
   if (!is_running_on_chrome_os_ ||
       (event->type - xrandr_event_base_ != RRNotify)) {
     return true;
   }
   XEvent* xevent = static_cast<XEvent*>(event);
@@ skipping 74 matching lines @@
   // distinguish internal display.
   return false;
 }
 
 void OutputConfigurator::NotifyOnDisplayChanged() {
   notification_timer_.reset();
   FOR_EACH_OBSERVER(Observer, observers_, OnDisplayModeChanged());
 }
 
 }  // namespace chromeos