Fix window/display bounds with multiple windows in mus.

services/ui/display/platform_screen_impl_ozone.cc

Index: services/ui/display/platform_screen_impl_ozone.cc
diff --git a/services/ui/display/platform_screen_impl_ozone.cc b/services/ui/display/platform_screen_impl_ozone.cc
index 9ab9d726325e82c8458d8f1395834ca55d671581..48fdf1711b3cb199bf36157d7d821b2390cfa1d7 100644
--- a/services/ui/display/platform_screen_impl_ozone.cc
+++ b/services/ui/display/platform_screen_impl_ozone.cc
@@ -4,6 +4,8 @@
 
 #include "services/ui/display/platform_screen_impl_ozone.h"
 
+#include <memory>
+
 #include "base/command_line.h"
 #include "base/memory/ptr_util.h"
 #include "base/sys_info.h"
@@ -18,24 +20,6 @@
 namespace display {
 namespace {
 
-// TODO(rjkroege): Remove this code once ozone headless has the same
-// display creation semantics as ozone drm.
-// Some ozone platforms do not configure physical displays and so do not
-// callback into this class via the implementation of NativeDisplayObserver.
-// FixedSizeScreenConfiguration() short-circuits the implementation of display
-// configuration in this case by calling the |callback| provided to
-// ConfigurePhysicalDisplay() with a hard-coded |id| and |bounds|.
-void FixedSizeScreenConfiguration(
-    const PlatformScreen::ConfiguredDisplayCallback& callback) {
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch("multi-display")) {
-    // This really doesn't work properly. Use at your own risk.
-    callback.Run(100, gfx::Rect(800, 800));
-    callback.Run(200, gfx::Rect(800, 0, 800, 800));
-  } else {
-    callback.Run(100, gfx::Rect(0, 0, 1024, 768));
-  }
-}
-
 // Needed for DisplayConfigurator::ForceInitialConfigure.
 const SkColor kChromeOsBootColor = SkColorSetRGB(0xfe, 0xfe, 0xfe);
 
@@ -53,6 +37,9 @@ PlatformScreenImplOzone::~PlatformScreenImplOzone() {
 }
 
 void PlatformScreenImplOzone::Init() {
+  // We want display configuration to happen even off device to keep the control
+  // flow similar.
+  display_configurator_.set_configure_display(true);
   display_configurator_.AddObserver(this);
   display_configurator_.Init(
       ui::OzonePlatform::GetInstance()->CreateNativeDisplayDelegate(), false);
@@ -60,45 +47,65 @@ void PlatformScreenImplOzone::Init() {
 
 void PlatformScreenImplOzone::ConfigurePhysicalDisplay(
     const PlatformScreen::ConfiguredDisplayCallback& callback) {
+  callback_ = callback;
+
   if (base::SysInfo::IsRunningOnChromeOS()) {
-    callback_ = callback;
     display_configurator_.ForceInitialConfigure(kChromeOsBootColor);
   } else {
-    // PostTask()ed to maximize control flow similarity with the ChromeOS case.
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::Bind(&FixedSizeScreenConfiguration, callback));
+    if (base::CommandLine::ForCurrentProcess()->HasSwitch("multi-display")) {
+      // This really doesn't work properly. Use at your own risk.
+      display_configurator_.AddVirtualDisplay(gfx::Size(800, 800));
+      display_configurator_.AddVirtualDisplay(gfx::Size(800, 800));
+    } else {
+      display_configurator_.AddVirtualDisplay(gfx::Size(1024, 768));
+    }
   }
 }
 
+int64_t PlatformScreenImplOzone::GetPrimaryDisplayId() const {
+  return primary_display_id_;
+}
+
 void PlatformScreenImplOzone::OnDisplayModeChanged(
     const ui::DisplayConfigurator::DisplayStateList& displays) {
-  // TODO(kylechar): Remove check when adding/removing displays is supported.
-  CHECK(!callback_.is_null());
-
   if (displays.size() > 1) {
     LOG(ERROR)
         << "Mus doesn't really support multiple displays, expect it to crash";
   }
 
-  gfx::Point origin;
-  for (auto* display : displays) {
-    const ui::DisplayMode* current_mode = display->current_mode();
-    gfx::Rect bounds(origin, current_mode->size());
+  // TODO(kylechar): Use DisplayLayout/DisplayLayoutStore here when possible.
+  std::set<uint64_t> all_displays_;

[sky, 2016/08/10 22:15:59]

'all_displays_' -> 'all_displays'

[kylechar, 2016/08/11 15:00:36]

Done.

+  for (ui::DisplaySnapshot* display : displays) {
+    const int64_t id = display->display_id();
+
+    all_displays_.insert(id);
+
+    if (displays_.find(id) != displays_.end())

[sky, 2016/08/10 22:15:59]

Can this function be called if something changes related to the display, such as
the size?

[kylechar, 2016/08/11 15:00:36]

If the display size changes because of a display configuration request (also
things like adding/removing monitors) then yes OnDisplayModeChanged would be
called by DisplayConfigurator. I'm not handling that yet because we're missing a
bunch of things to make it happen, but it will need to be handled from here.

There is also the display size changing when running under Oxygen because the
user resized the XWindow. That is a totally different code path and I'm tempted
to just make the window a fixed size and force the user to resize via Display
Settings even under Oxygen. That would help to unify the on device and off
device code paths.

+      continue;
 
-    callback_.Run(display->display_id(), bounds);
+    const ui::DisplayMode* current_mode = display->current_mode();
+    gfx::Rect bounds(next_display_origin_, current_mode->size());
 
     // Move the origin so that next display is to the right of current display.
-    origin.Offset(current_mode->size().width(), 0);
+    next_display_origin_.Offset(current_mode->size().width(), 0);
+
+    // The first display added will be our primary display.
+    if (displays_.empty())
+      primary_display_id_ = id;
+
+    // Keep track of what displays have already been added.
+    displays_.insert(display->display_id());
+
+    callback_.Run(id, bounds);
   }
 
-  callback_.Reset();
+  DCHECK(displays_ == all_displays_) << "Removing displays is not supported.";
 }
 
 void PlatformScreenImplOzone::OnDisplayModeChangeFailed(
     const ui::DisplayConfigurator::DisplayStateList& displays,
     ui::MultipleDisplayState failed_new_state) {
   LOG(ERROR) << "OnDisplayModeChangeFailed from DisplayConfigurator";
-  callback_.Reset();
 }
 
 }  // namespace display