exo: Refactor ShellSurface and WaylandRemoteShell

components/exo/wayland/server.cc

 // Copyright 2015 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/exo/wayland/server.h"
 
 #include <alpha-compositing-unstable-v1-server-protocol.h>
 #include <gaming-input-unstable-v1-server-protocol.h>
 #include <grp.h>
 #include <keyboard-configuration-unstable-v1-server-protocol.h>
@@ skipping 11 matching lines @@
 #include <wayland-server-protocol-core.h>
 #include <xdg-shell-unstable-v5-server-protocol.h>
 #include <xdg-shell-unstable-v6-server-protocol.h>
 
 #include <algorithm>
 #include <cstdlib>
 #include <iterator>
 #include <string>
 #include <utility>
 
-#include "ash/common/shell_observer.h"
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/shell.h"
 #include "base/bind.h"
 #include "base/cancelable_callback.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/memory/free_deleter.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/weak_ptr.h"
 #include "base/strings/stringprintf.h"
@@ skipping 1039 matching lines @@
   WaylandPrimaryDisplayObserver(wl_resource* output_resource)
       : output_resource_(output_resource) {
     display::Screen::GetScreen()->AddObserver(this);
     SendDisplayMetrics();
   }
   ~WaylandPrimaryDisplayObserver() override {
     display::Screen::GetScreen()->RemoveObserver(this);
   }
 
   // Overridden from display::DisplayObserver:
-  void OnDisplayAdded(const display::Display& new_display) override {}
-  void OnDisplayRemoved(const display::Display& new_display) override {}
   void OnDisplayMetricsChanged(const display::Display& display,
                                uint32_t changed_metrics) override {
     if (display::Screen::GetScreen()->GetPrimaryDisplay().id() != display.id())
       return;
 
     // There is no need to check DISPLAY_METRIC_PRIMARY because when primary
     // changes, bounds always changes. (new primary should have had non
     // 0,0 origin).
     // Only exception is when switching to newly connected primary with
     // the same bounds. This happens whenyou're in docked mode, suspend,
@@ skipping 935 matching lines @@
         weak_ptr_factory_(this) {
     auto* helper = WMHelper::GetInstance();
     helper->AddMaximizeModeObserver(this);
     helper->AddActivationObserver(this);
     display::Screen::GetScreen()->AddObserver(this);
 
     layout_mode_ = helper->IsMaximizeModeWindowManagerEnabled()
                        ? ZCR_REMOTE_SHELL_V1_LAYOUT_MODE_TABLET
                        : ZCR_REMOTE_SHELL_V1_LAYOUT_MODE_WINDOWED;
 
-    SendPrimaryDisplayMetrics();
+    SendDisplayMetrics();
     SendActivated(helper->GetActiveWindow(), nullptr);
   }
   ~WaylandRemoteShell() override {
     auto* helper = WMHelper::GetInstance();
     helper->RemoveMaximizeModeObserver(this);
     helper->RemoveActivationObserver(this);
     display::Screen::GetScreen()->RemoveObserver(this);
   }
 
   std::unique_ptr<ShellSurface> CreateShellSurface(Surface* surface,
                                                    int container) {
     return display_->CreateRemoteShellSurface(surface, container);
   }
 
   std::unique_ptr<NotificationSurface> CreateNotificationSurface(
       Surface* surface,
       const std::string& notification_id) {
     return display_->CreateNotificationSurface(surface, notification_id);
   }
 
   // Overridden from display::DisplayObserver:
-  void OnDisplayAdded(const display::Display& new_display) override {}
-  void OnDisplayRemoved(const display::Display& new_display) override {}
   void OnDisplayMetricsChanged(const display::Display& display,
                                uint32_t changed_metrics) override {
     if (display::Screen::GetScreen()->GetPrimaryDisplay().id() != display.id())
       return;
 
-    // No need to update when a primary dislpay has changed without bounds
+    // No need to update when a primary display has changed without bounds
     // change. See WaylandPrimaryDisplayObserver::OnDisplayMetricsChanged
     // for more details.
     if (changed_metrics &
         (DISPLAY_METRIC_BOUNDS | DISPLAY_METRIC_DEVICE_SCALE_FACTOR |
          DISPLAY_METRIC_ROTATION | DISPLAY_METRIC_WORK_AREA)) {
-      SendDisplayMetrics(display);
+      ScheduleSendDisplayMetrics();
     }
   }
 
   // Overridden from WMHelper::MaximizeModeObserver:
   void OnMaximizeModeStarted() override {
     layout_mode_ = ZCR_REMOTE_SHELL_V1_LAYOUT_MODE_TABLET;
-
-    send_configure_after_layout_change_ = true;
-    base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-        FROM_HERE, base::Bind(&WaylandRemoteShell::MaybeSendConfigure,
-                              weak_ptr_factory_.GetWeakPtr()),
-        base::TimeDelta::FromMilliseconds(kConfigureDelayAfterLayoutSwitchMs));
+    ScheduleSendDisplayMetrics(kConfigureDelayAfterLayoutSwitchMs);
   }
   void OnMaximizeModeEnded() override {
     layout_mode_ = ZCR_REMOTE_SHELL_V1_LAYOUT_MODE_WINDOWED;
-    send_configure_after_layout_change_ = true;
-    base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-        FROM_HERE, base::Bind(&WaylandRemoteShell::MaybeSendConfigure,
-                              weak_ptr_factory_.GetWeakPtr()),
-        base::TimeDelta::FromMilliseconds(kConfigureDelayAfterLayoutSwitchMs));
+    ScheduleSendDisplayMetrics(kConfigureDelayAfterLayoutSwitchMs);
   }
 
   // Overridden from WMHelper::ActivationObserver:
   void OnWindowActivated(aura::Window* gained_active,
                          aura::Window* lost_active) override {
     SendActivated(gained_active, lost_active);
   }
 
  private:
-  void SendPrimaryDisplayMetrics() {
-    const display::Display primary =
-        display::Screen::GetScreen()->GetPrimaryDisplay();
-
-    SendDisplayMetrics(primary);
+  void ScheduleSendDisplayMetrics(int delay_ms = 0) {

[reveman, 2017/02/10 02:10:53]

nit: remove default argument and explicitly pass 0

[Dominik Laskowski, 2017/02/10 03:05:19]

Done.

+    needs_send_display_metrics_ = true;
+    base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+        FROM_HERE, base::Bind(&WaylandRemoteShell::SendDisplayMetrics,
+                              weak_ptr_factory_.GetWeakPtr()),
+        base::TimeDelta::FromMilliseconds(delay_ms));
   }
 
-  void MaybeSendConfigure() {
-    if (send_configure_after_layout_change_)
-      SendPrimaryDisplayMetrics();
-  }
+  void SendDisplayMetrics() {
+    if (!needs_send_display_metrics_)
+      return;
+    needs_send_display_metrics_ = false;
 
-  void SendDisplayMetrics(const display::Display& display) {
-    send_configure_after_layout_change_ = false;
+    const display::Screen* screen = display::Screen::GetScreen();
+    const display::Display primary = screen->GetPrimaryDisplay();

[reveman, 2017/02/10 02:10:54]

nit: s/primary/primary_display/

[Dominik Laskowski, 2017/02/10 03:05:19]

Done.

 
-    const gfx::Insets& work_area_insets = display.GetWorkAreaInsets();
+    gfx::Size size = primary.size();

[reveman, 2017/02/10 02:10:53]

where is this used? is the temporary variable needed? maybe primary_size in that
case

[Dominik Laskowski, 2017/02/10 03:05:19]

In the dependent CL, where it might be the size of the primary or virtual
display.

[reveman, 2017/02/10 05:51:54]

Save that for the follow up CL. Changes are hard to understand when they include
things like this that will only make sense in a follow up patch.

[Dominik Laskowski, 2017/02/10 18:47:17]

Done.

+    const gfx::Insets& work_area_insets = primary.GetWorkAreaInsets();
 
     zcr_remote_shell_v1_send_configuration_changed(
-        remote_shell_resource_, display.size().width(), display.size().height(),
-        OutputTransform(display.rotation()),
-        wl_fixed_from_double(display.device_scale_factor()),
+        remote_shell_resource_, size.width(), size.height(),
+        OutputTransform(primary.rotation()),
+        wl_fixed_from_double(primary.device_scale_factor()),
         work_area_insets.left(), work_area_insets.top(),
         work_area_insets.right(), work_area_insets.bottom(), layout_mode_);
     wl_client_flush(wl_resource_get_client(remote_shell_resource_));
   }
 
   void SendActivated(aura::Window* gained_active, aura::Window* lost_active) {
     Surface* gained_active_surface =
         gained_active ? ShellSurface::GetMainSurface(gained_active) : nullptr;
     Surface* lost_active_surface =
         lost_active ? ShellSurface::GetMainSurface(lost_active) : nullptr;
@@ skipping 24 matching lines @@
                                        lost_active_surface_resource);
     wl_client_flush(client);
   }
 
   // The exo display instance. Not owned.
   Display* const display_;
 
   // The remote shell resource associated with observer.
   wl_resource* const remote_shell_resource_;
 
-  bool send_configure_after_layout_change_ = false;
+  bool needs_send_display_metrics_ = true;
 
   int layout_mode_ = ZCR_REMOTE_SHELL_V1_LAYOUT_MODE_WINDOWED;
 
   base::WeakPtrFactory<WaylandRemoteShell> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(WaylandRemoteShell);
 };
 
 void remote_shell_destroy(wl_client* client, wl_resource* resource) {
   // Nothing to do here.
@@ skipping 1559 matching lines @@
   DCHECK(event_loop);
   wl_event_loop_dispatch(event_loop, timeout.InMilliseconds());
 }
 
 void Server::Flush() {
   wl_display_flush_clients(wl_display_.get());
 }
 
 }  // namespace wayland
 }  // namespace exo