exo: Fix dragging edge cases

components/exo/shell_surface.h

 // 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.
 
 #ifndef COMPONENTS_EXO_SHELL_SURFACE_H_
 #define COMPONENTS_EXO_SHELL_SURFACE_H_
 
 #include <deque>
 #include <memory>
 #include <string>
@@ skipping 16 matching lines @@
 class WindowResizer;
 }
 
 namespace base {
 namespace trace_event {
 class TracedValue;
 }
 }
 
 namespace exo {
+class Display;
 class Surface;
 
 // This class provides functions for treating a surfaces like toplevel,
 // fullscreen or popup widgets, move, resize or maximize them, associate
 // metadata like title and class, etc.
 class ShellSurface : public SurfaceDelegate,
                      public SurfaceObserver,
                      public views::WidgetDelegate,
                      public views::View,
                      public ash::AccessibilityObserver,
                      public ash::wm::WindowStateObserver,
                      public aura::WindowObserver,
-                     public WMHelper::ActivationObserver {
+                     public WMHelper::ActivationObserver,
+                     public WMHelper::ShellObserver {
  public:
-  ShellSurface(Surface* surface,
+  ShellSurface(const Display& display,
+               Surface* surface,
                ShellSurface* parent,
                const gfx::Rect& initial_bounds,
                bool activatable,
                int container);
-  explicit ShellSurface(Surface* surface);
+  ShellSurface(const Display& display, Surface* surface);
   ~ShellSurface() override;
 
   // Set the callback to run when the user wants the shell surface to be closed.
   // The receiver can chose to not close the window on this signal.
   void set_close_callback(const base::Closure& close_callback) {
     close_callback_ = close_callback;
   }
 
   // Set the callback to run when the surface is destroyed.
   void set_surface_destroyed_callback(
@@ skipping 37 matching lines @@
 
   // Maximizes the shell surface.
   void Maximize();
 
   // Minimize the shell surface.
   void Minimize();
 
   // Restore the shell surface.
   void Restore();
 
+  // Start and stop a client-driven move of the shell surface.
+  void SetMoving();
+  void UnsetMoving(bool revert);
+
   // Set fullscreen state for shell surface.
   void SetFullscreen(bool fullscreen);
 
   // Pins the shell surface. |trusted| flag is ignored when |pinned| is false.
   void SetPinned(bool pinned, bool trusted);
 
   // Set title for surface.
   void SetTitle(const base::string16& title);
 
   // Sets the system modality.
@@ skipping 86 matching lines @@
   void OnWindowBoundsChanged(aura::Window* window,
                              const gfx::Rect& old_bounds,
                              const gfx::Rect& new_bounds) override;
   void OnWindowDestroying(aura::Window* window) override;
 
   // Overridden from WMHelper::ActivationObserver:
   void OnWindowActivated(
       aura::Window* gained_active,
       aura::Window* lost_active) override;
 
+  // Overridden from WMHelper::ShellObserver:
+  void OnOverviewModeStarted() override;
+  void OnOverviewModeEnded() override;
+
   // Overridden from ui::EventHandler:
   void OnKeyEvent(ui::KeyEvent* event) override;
   void OnMouseEvent(ui::MouseEvent* event) override;
 
   // Overridden from ui::AcceleratorTarget:
   bool AcceleratorPressed(const ui::Accelerator& accelerator) override;
 
   aura::Window* shadow_underlay() { return shadow_underlay_; }
 
  private:
@@ skipping 14 matching lines @@
   // Asks the client to configure its surface.
   void Configure();
 
   // Attempt to start a drag operation. The type of drag operation to start is
   // determined by |component|.
   void AttemptToStartDrag(int component);
 
   // End current drag operation.
   void EndDrag(bool revert);
 
+  // End current drag or client-driven move.
+  void EndDragOrMove(bool revert);
+
   // Returns true if surface is currently being resized.
   bool IsResizing() const;
 
   // Returns the "visible bounds" for the surface from the user's perspective.
   gfx::Rect GetVisibleBounds() const;
 
   // Returns the origin for the surface taking visible bounds and current
   // resize direction into account.
   gfx::Point GetSurfaceOrigin() const;
 
   // Updates the bounds of widget to match the current surface bounds.
   void UpdateWidgetBounds();
 
+  // Updates the bounds of surface to match the current widget bounds.
+  void UpdateSurfaceBounds();
+
   // Creates, deletes and update the shadow bounds based on
   // |pending_shadow_content_bounds_|.
   void UpdateShadow();
 
+  // In the coordinate system of the parent root window.
+  gfx::Point GetMouseLocation() const;
+
+  const Display& display_;

[oshima, 2016/10/12 01:56:54]

can you use const Display* ?

[Dominik Laskowski, 2016/10/13 03:21:17]

It should never be null, and the reference makes that clear.

[oshima, 2016/10/17 17:22:58]

It doesn't prevent from passing null reference. assertion (DCHECK/DCHECK) is
better choice IMO, but I'll leave this to the owner.

[Dominik Laskowski, 2016/10/18 21:22:56]

The caller would have to dereference a null pointer though. I don't mind
changing it to a pointer, but I feel this is self-documenting.

reveman: let me know what style you prefer.

   views::Widget* widget_ = nullptr;
   Surface* surface_;
   aura::Window* parent_;
   const gfx::Rect initial_bounds_;
   const bool activatable_;
   // Container Window Id (see ash/common/shell_window_ids.h)
   const int container_;
   bool pending_show_widget_ = false;
   base::string16 title_;
   std::string application_id_;
   gfx::Rect geometry_;
   gfx::Rect pending_geometry_;
   double scale_ = 1.0;
   double pending_scale_ = 1.0;
   base::Closure close_callback_;
   base::Closure surface_destroyed_callback_;
   StateChangedCallback state_changed_callback_;
   ConfigureCallback configure_callback_;
   ScopedConfigure* scoped_configure_ = nullptr;
+  bool ignore_widget_bounds_changes_ = false;
   bool ignore_window_bounds_changes_ = false;
   gfx::Point origin_;
   gfx::Vector2d pending_origin_offset_;
   gfx::Vector2d pending_origin_config_offset_;
   int resize_component_ = HTCAPTION;  // HT constant (see ui/base/hit_test.h)
   int pending_resize_component_ = HTCAPTION;
   aura::Window* shadow_overlay_ = nullptr;
   aura::Window* shadow_underlay_ = nullptr;
   std::unique_ptr<ui::EventHandler> shadow_underlay_event_handler_;
   gfx::Rect shadow_content_bounds_;
   std::deque<Config> pending_configs_;
   std::unique_ptr<ash::WindowResizer> resizer_;
+  std::unique_ptr<ash::WindowResizer> move_resizer_;

[oshima, 2016/10/12 01:56:54]

the original resizer_ is also used to drag. How about

phantom_window_resizer?

I also wonder if we really need two resizer. If we just want to create a phantom
window, you may not need resizer.

[Dominik Laskowski, 2016/10/13 03:21:17]

The pointer is also used to check whether a client-driven move is in progress,
so that name would be confusing.

We need all the logic in DragWindowResizer except resizer chaining. We could
factor it out into another class, but chaining a dummy resizer is simpler.

[oshima, 2016/10/17 17:22:58]

Hmm, then I may be reading this wrong. Can you explain why we need two resizer?

[Dominik Laskowski, 2016/10/18 21:22:56]

There are three states we want to detect:

         No drag: !resizer_ && !move_resizer_
Client-side drag: !resizer_ &&  move_resizer_
Server-side drag:  resizer_ && !move_resizer_

[Dominik Laskowski, 2016/10/21 03:05:19]

After an offline chat with oshima, the suggestion was to use a single resizer,
and detect the above states using other member variables. However, this would
complicate the code, because the only way to distinguish a remote shell surface
is:

!initial_bounds_.IsEmpty() && !parent_

reveman: do you prefer the two resizers? If not, could we introduce explicit
state to tell shell surfaces apart, e.g. an enum for regular/pop-up/remote? That
would be contradictory with the idea of moving concepts specific to the remote
shell (e.g. virtual coordinates) out of ShellSurface. On the other hand, it
would simplify a lot of conditions that implicitly depend on otherwise unused
initial state, e.g. gfx::Size(1, 1).

   std::unique_ptr<ScopedAnimationsDisabled> scoped_animations_disabled_;
   int top_inset_height_ = 0;
   int pending_top_inset_height_ = 0;
   float rectangular_shadow_background_opacity_ = 1.0;
 
   DISALLOW_COPY_AND_ASSIGN(ShellSurface);
 };
 
 }  // namespace exo
 
 #endif  // COMPONENTS_EXO_SHELL_SURFACE_H_