Basic display management for mus

components/mus/ws/platform_screen.h

+// 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.
+
+#ifndef COMPONENTS_MUS_WS_PLATFORM_SCREEN_H_
+#define COMPONENTS_MUS_WS_PLATFORM_SCREEN_H_
+
+#include <stdint.h>
+
+#include <set>
+#include <vector>
+
+#include "base/callback.h"
+#include "base/memory/weak_ptr.h"
+#include "ui/display/types/native_display_observer.h"
+#include "ui/gfx/geometry/rect.h"
+
+namespace ui {
+class NativeDisplayDelegate;
+class DisplaySnapshot;
+}
+
+namespace mus {
+namespace ws {
+
+// In Chrome, a "Screen" is the aggregate of all attached physical displays.

[sky, 2016/05/03 02:39:51]

nit: remove this first sentence as it's confusing in this context.

[rjkroege, 2016/05/03 21:03:03]

Done.

+// A |PlatformScreen| provides the necessary functionality to configure all
+// attached physical displays.
+class PlatformScreen : public ui::NativeDisplayObserver {
+ public:
+  PlatformScreen();
+  ~PlatformScreen() override;
+
+  // When using Ozone, PlatformScreen must be initialized after the Ozone

[sky, 2016/05/03 02:39:51]

This is confusing. I think you want something like Initializes platform specific
screen resources.

[rjkroege, 2016/05/03 21:03:04]

Re-worded to try to make the point that I was trying to make.

+  // platform is initialized.
+  void Init();
+
+  // The PhysicalScreen will invoke this callback when a previously requested
+  // display configuration has been confirmed.
+  typedef base::Callback<void(int64_t, gfx::Rect)> ConfiguredDisplayCallback;

[sky, 2016/05/03 02:39:51]

Use 'using' instead. It is way easier to read.

[rjkroege, 2016/05/03 21:03:03]

Done.

+
+  // |ConfigurePhysicalDisplay| configures a single physical display and returns

[sky, 2016/05/03 02:39:51]

nit: generally |s are for references to parameters and members, () is for
functions.

[rjkroege, 2016/05/03 21:03:04]

Done.

+  // pending bounds for it via callback.
+  void ConfigurePhysicalDisplay(ConfiguredDisplayCallback callback);

[sky, 2016/05/03 02:39:51]

const&

[rjkroege, 2016/05/03 21:03:03]

Done.

+
+ private:
+  void FixedSizeScreenConfiguration(ConfiguredDisplayCallback callback);

[sky, 2016/05/03 02:39:51]

Add a description as this isn't obvious.

[rjkroege, 2016/05/03 21:03:03]

Done.

+
+  // TODO(rjkroege): NativeDisplayObserver is misnamed as it tracks changes in
+  // the physical "Screen".  Consider renaming it to NativeScreenObserver.
+  // ui::NativeDisplayObserver:
+  void OnConfigurationChanged() override;
+
+  // Additional display management callbacks.
+  void OnDisplaysAquired(ConfiguredDisplayCallback callback,

[sky, 2016/05/03 02:39:51]

const&

[rjkroege, 2016/05/03 21:03:03]

Done.

+                         const std::vector<ui::DisplaySnapshot*>& displays);
+  void OnDisplayConfigured(ConfiguredDisplayCallback callback,

[sky, 2016/05/03 02:39:51]

const&

[rjkroege, 2016/05/03 21:03:04]

Done.

+                           int64_t id,
+                           const gfx::Rect& bounds,
+                           bool success);
+
+  // Flags used to keep track of the current state of display configuration.
+  // |is_configuring| is true if configuring the Screen and starts false.
+  bool is_configuring_;

[sky, 2016/05/03 02:39:51]

Initialize here (or update platform_screen.cc to initialize this and
should_configure_).

[rjkroege, 2016/05/03 21:03:03]

Done.

+
+  // If |is_configuring_| is true and another display configuration event
+  // happens, the event is deferred. |should_configure_| is set to true and a
+  // display configuration will be scheduled after the current one finishes.
+  // NB: events are handled once per vblank.
+  bool should_configure_;
+
+  std::unique_ptr<ui::NativeDisplayDelegate> native_display_delegate_;
+  ConfiguredDisplayCallback callback_;
+
+  base::WeakPtrFactory<PlatformScreen> weak_ptr_factory_;
+  DISALLOW_COPY_AND_ASSIGN(PlatformScreen);

[sky, 2016/05/03 02:39:51]

nit: newline between 75/76.

[rjkroege, 2016/05/03 21:03:03]

Done.

+};
+
+}  // namespace ws
+}  // namespace mus
+
+#endif  // COMPONENTS_MUS_WS_PLATFORM_SCREEN_H_