Basic display management for mus

components/mus/ws/platform_screen_impl_ozone.cc

+// Copyright 2016 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 "components/mus/ws/platform_screen_impl_ozone.h"
+
+#include "base/bind.h"
+#include "base/location.h"
+#include "base/memory/ptr_util.h"
+#include "base/sys_info.h"
+#include "base/thread_task_runner_handle.h"
+#include "ui/display/types/display_snapshot.h"
+#include "ui/display/types/native_display_delegate.h"
+#include "ui/ozone/public/ozone_platform.h"
+
+namespace mus {
+namespace ws {
+
+// static
+std::unique_ptr<PlatformScreen> PlatformScreen::Create() {
+  return base::WrapUnique(new PlatformScreenImplOzone);
+}
+
+PlatformScreenImplOzone::PlatformScreenImplOzone()
+    : is_configuring_(false),
+      should_configure_(false),
+      weak_ptr_factory_(this) {}
+
+PlatformScreenImplOzone::~PlatformScreenImplOzone() {}
+
+void PlatformScreenImplOzone::Init() {
+  native_display_delegate_ =
+      ui::OzonePlatform::GetInstance()->CreateNativeDisplayDelegate();
+  native_display_delegate_->AddObserver(this);
+  native_display_delegate_->Initialize();
+}
+
+void PlatformScreenImplOzone::ConfigurePhysicalDisplay(
+    const PlatformScreen::ConfiguredDisplayCallback& callback) {
+  callback_ = callback;
+
+  if (base::SysInfo::IsRunningOnChromeOS()) {
+    // Kick off the configuration of the physical displays comprising the
+    // |PlatformScreenImplOzone|
+    OnConfigurationChanged();
+  } else {
+    // PostTask()ed to maximize control flow similarity with the ChromeOS case.
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE,
+        base::Bind(&PlatformScreenImplOzone::FixedSizeScreenConfiguration,

[sky, 2016/05/04 17:05:45]

Again, you shouldn't need a member function and the weak ptr for this.

[rjkroege, 2016/05/04 22:50:13]

Done.

+                   weak_ptr_factory_.GetWeakPtr(), callback));
+  }
+}
+
+// TODO(rjkroege): Support multiple displays.
+// The display subsystem tells us that the display configuration has changed on
+// the first and subsequent change in display parameters. We have to kick this
+// off manually the first time.
+void PlatformScreenImplOzone::OnConfigurationChanged() {
+  DCHECK(native_display_delegate_) << "DefaultDisplayManager::"
+                                      "OnConfigurationChanged requires a "
+                                      "native_display_delegate_ to work.";
+  if (callback_.is_null())

[sky, 2016/05/04 17:05:45]

AFAICT you never reset the callback, so it will never be null?

[rjkroege, 2016/05/04 22:50:13]

No longer applicable.

+    return;
+  if (is_configuring_)

[sky, 2016/05/04 17:05:45]

How come you don't early out if is_configuring_ is true? If is_configuring_ is
true won't you end up back here shortly by way of the PostTask on 110?

[rjkroege, 2016/05/04 22:50:13]

Doh. Thanks for catching this. I had deleted that by accident since an earlier
version of this CL.

+    should_configure_ = true;
+  is_configuring_ = true;
+  native_display_delegate_->GrabServer();
+  native_display_delegate_->GetDisplays(
+      base::Bind(&PlatformScreenImplOzone::OnDisplaysAquired,
+                 weak_ptr_factory_.GetWeakPtr(), callback_));
+}
+
+// TODO(rjkroege): Remove this code once ozone oxygen has the same
+// display creation semantics as ozone drm.
+void PlatformScreenImplOzone::FixedSizeScreenConfiguration(
+    ConfiguredDisplayCallback callback) {
+  callback.Run(1, gfx::Rect(1024, 768));
+}
+
+// The display subsystem calls |OnDisplaysAquired| to deliver |displays|
+// describing the attached displays.
+void PlatformScreenImplOzone::OnDisplaysAquired(
+    const ConfiguredDisplayCallback& callback,
+    const std::vector<ui::DisplaySnapshot*>& displays) {
+  DCHECK(native_display_delegate_) << "DefaultDisplayManager::"
+                                      "OnConfigurationChanged requires a "
+                                      "native_display_delegate_ to work.";
+  CHECK(displays.size() == 1) << "Mus only supports one 1 display\n";
+  gfx::Point origin;
+  for (auto display : displays) {
+    if (!display->native_mode()) {
+      LOG(ERROR) << "Display " << display->display_id()
+                 << " doesn't have a native mode";
+      continue;
+    }
+    // Setup each native display. This places a task on the DRM thread's
+    // runqueue that configures the window size correctly before the call to
+    // Configure.
+    native_display_delegate_->Configure(
+        *display, display->native_mode(), origin,
+        base::Bind(&PlatformScreenImplOzone::OnDisplayConfigured,
+                   weak_ptr_factory_.GetWeakPtr(), callback,
+                   display->display_id(),
+                   gfx::Rect(origin, display->native_mode()->size())));
+    origin.Offset(display->native_mode()->size().width(), 0);
+  }
+  native_display_delegate_->UngrabServer();
+  is_configuring_ = false;
+  if (should_configure_) {
+    should_configure_ = false;
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::Bind(&PlatformScreenImplOzone::OnConfigurationChanged,
+                              weak_ptr_factory_.GetWeakPtr()));
+  }
+}
+
+// The display subsystem calls |OnDisplayConfigured| for each display that has
+// been successfully configured. This in turn calls |callback_| with the
+// identity and bounds of each physical display.
+void PlatformScreenImplOzone::OnDisplayConfigured(
+    const ConfiguredDisplayCallback& callback,

[sky, 2016/05/04 17:05:45]

Why do you pass callback here and have callback as a member?

[rjkroege, 2016/05/04 22:50:13]

Seemed like an easy way to ensure that I only called the callback once for each
call to ConfigurePhysicalDisplay without particularly modifying
OnConfigurationChanged. Refactored differently.

+    int64_t id,
+    const gfx::Rect& bounds,
+    bool success) {
+  if (success)
+    callback.Run(id, bounds);
+  else
+    LOG(FATAL) << "Failed to configure display at " << bounds.ToString();
+}
+
+}  // namespace ws
+}  // namespace mus