Commit 7c332249 authored by varkha's avatar varkha Committed by Commit bot

Revert of Revert of Adjusts dragging logic to be less likely to trigger false...

Revert of Revert of Adjusts dragging logic to be less likely to trigger false positive switch from snapping to docking (patchset #1 id:1 of https://codereview.chromium.org/1128933005/)

Reason for revert:
This revert probably did not fix the flakiness. I suspect that a revert that actually ended up fixing the bots is this: https://codereview.chromium.org/1139943002.

I will re-land my original change.

Original issue's description:
> Revert of Adjusts dragging logic to be less likely to trigger false positive switch from snapping to docking (patchset #5 id:90001 of https://codereview.chromium.org/1127133003/)
>
> Reason for revert:
> I suspect this is causing a browser_tests failure on the bots:
>
> http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29/builds/2069
> https://build.chromium.org/p/chromium.webkit/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29/builds/1913
>
> Sorry!
>
> Original issue's description:
> > Adjusts dragging logic to be less likely to trigger false positive switch from
> > snapping to docking.
> >
> > This CL adjusts the logic which selects whether a window will be snapped to
> > half of the workspace width or will be docked at the end of the drag when the
> > window is dragged to the edge of the workspace in the special case of the dock
> > being visible. The CL makes it easier to snap a window to half the workspace
> > width in this special case (and harder to inadvertently dock the window)
> >
> > BUG=484877
> > TEST=None
> >
> > Committed: https://crrev.com/988c697aff35b3fd347dec35f2f2ac159c8328a9
> > Cr-Commit-Position: refs/heads/master@{#329268}
>
> TBR=pkotwicz@chromium.org,varkha@chromium.org
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=484877
>
> Committed: https://crrev.com/0d4f7d597774f7415f1aff374db498c8fbe88a9a
> Cr-Commit-Position: refs/heads/master@{#329301}

TBR=pkotwicz@chromium.org,dpranke@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=484877

Review URL: https://codereview.chromium.org/1140733002

Cr-Commit-Position: refs/heads/master@{#329413}
parent 2c6ef293
...@@ -12,20 +12,26 @@ namespace { ...@@ -12,20 +12,26 @@ namespace {
// We cycle to the second mode if any of the following happens while the mouse // We cycle to the second mode if any of the following happens while the mouse
// is on the edge of the workspace: // is on the edge of the workspace:
// . The user stops moving the mouse for |kMaxDelay| and then moves the mouse // . The user stops moving the mouse for |kMaxDelay| and then moves the mouse
// again. // again in the preferred direction from the last paused location for at least
// . The mouse moves |kMaxPixels| horizontal pixels. // |kMaxPixelsAfterPause| horizontal pixels.
// . The mouse is moved |kMaxMoves| times. // . The mouse moves |kMaxPixels| horizontal pixels in the preferred direction.
const int kMaxDelay = 500; // . The mouse is moved |kMaxMoves| times since the last pause.
const int kMaxDelay = 400;
const int kMaxPixels = 100; const int kMaxPixels = 100;
const int kMaxPixelsAfterPause = 10;
const int kMaxMoves = 25; const int kMaxMoves = 25;
} // namespace } // namespace
TwoStepEdgeCycler::TwoStepEdgeCycler(const gfx::Point& start) TwoStepEdgeCycler::TwoStepEdgeCycler(const gfx::Point& start,
TwoStepEdgeCycler::Direction direction)
: second_mode_(false), : second_mode_(false),
time_last_move_(base::TimeTicks::Now()), time_last_move_(base::TimeTicks::Now()),
num_moves_(0), num_moves_(0),
start_x_(start.x()) { start_x_(start.x()),
paused_x_(start.x()),
paused_(false),
direction_(direction) {
} }
TwoStepEdgeCycler::~TwoStepEdgeCycler() { TwoStepEdgeCycler::~TwoStepEdgeCycler() {
...@@ -35,13 +41,25 @@ void TwoStepEdgeCycler::OnMove(const gfx::Point& location) { ...@@ -35,13 +41,25 @@ void TwoStepEdgeCycler::OnMove(const gfx::Point& location) {
if (second_mode_) if (second_mode_)
return; return;
++num_moves_; if ((base::TimeTicks::Now() - time_last_move_).InMilliseconds() > kMaxDelay) {
second_mode_ = paused_ = true;
(base::TimeTicks::Now() - time_last_move_).InMilliseconds() > paused_x_ = location.x();
kMaxDelay || num_moves_ = 0;
std::abs(location.x() - start_x_) >= kMaxPixels || }
num_moves_ >= kMaxMoves;
time_last_move_ = base::TimeTicks::Now(); time_last_move_ = base::TimeTicks::Now();
int compare_x = paused_ ? paused_x_ : start_x_;
if (location.x() != compare_x &&
(location.x() < compare_x) != (direction_ == DIRECTION_LEFT)) {
return;
}
++num_moves_;
bool moved_in_the_same_direction_after_pause =
paused_ && std::abs(location.x() - paused_x_) >= kMaxPixelsAfterPause;
second_mode_ = moved_in_the_same_direction_after_pause ||
std::abs(location.x() - start_x_) >= kMaxPixels ||
num_moves_ >= kMaxMoves;
} }
} // namespace ash } // namespace ash
...@@ -19,7 +19,10 @@ namespace ash { ...@@ -19,7 +19,10 @@ namespace ash {
// the workspace. // the workspace.
class ASH_EXPORT TwoStepEdgeCycler { class ASH_EXPORT TwoStepEdgeCycler {
public: public:
explicit TwoStepEdgeCycler(const gfx::Point& start); // The direction in which a mouse should travel to switch mode.
enum Direction { DIRECTION_LEFT, DIRECTION_RIGHT };
explicit TwoStepEdgeCycler(const gfx::Point& start, Direction direction);
~TwoStepEdgeCycler(); ~TwoStepEdgeCycler();
// Update which mode should be used as a result of a mouse / touch move. // Update which mode should be used as a result of a mouse / touch move.
...@@ -41,6 +44,15 @@ class ASH_EXPORT TwoStepEdgeCycler { ...@@ -41,6 +44,15 @@ class ASH_EXPORT TwoStepEdgeCycler {
// Initial x-coordinate. // Initial x-coordinate.
int start_x_; int start_x_;
// x-coordinate when paused.
int paused_x_;
// Whether the movement was paused.
bool paused_;
// Determines a preferred movement direction that we are watching.
Direction direction_;
DISALLOW_COPY_AND_ASSIGN(TwoStepEdgeCycler); DISALLOW_COPY_AND_ASSIGN(TwoStepEdgeCycler);
}; };
......
...@@ -941,10 +941,14 @@ void WorkspaceWindowResizer::UpdateSnapPhantomWindow(const gfx::Point& location, ...@@ -941,10 +941,14 @@ void WorkspaceWindowResizer::UpdateSnapPhantomWindow(const gfx::Point& location,
edge_cycler_.reset(); edge_cycler_.reset();
return; return;
} }
if (!edge_cycler_) if (!edge_cycler_) {
edge_cycler_.reset(new TwoStepEdgeCycler(location)); edge_cycler_.reset(new TwoStepEdgeCycler(
else location, snap_type_ == SNAP_LEFT
? TwoStepEdgeCycler::DIRECTION_LEFT
: TwoStepEdgeCycler::DIRECTION_RIGHT));
} else {
edge_cycler_->OnMove(location); edge_cycler_->OnMove(location);
}
// Update phantom window with snapped or docked guide bounds. // Update phantom window with snapped or docked guide bounds.
// Windows that cannot be snapped or are less wide than kMaxDockWidth can get // Windows that cannot be snapped or are less wide than kMaxDockWidth can get
......
...@@ -60,19 +60,14 @@ class ASH_EXPORT WorkspaceWindowResizer : public WindowResizer { ...@@ -60,19 +60,14 @@ class ASH_EXPORT WorkspaceWindowResizer : public WindowResizer {
void CompleteDrag() override; void CompleteDrag() override;
void RevertDrag() override; void RevertDrag() override;
private:
WorkspaceWindowResizer(wm::WindowState* window_state,
const std::vector<aura::Window*>& attached_windows);
private: private:
friend class WorkspaceWindowResizerTest; friend class WorkspaceWindowResizerTest;
// The edge to which the window should be snapped at the end of the drag. // The edge to which the window should be snapped at the end of the drag.
enum SnapType { enum SnapType { SNAP_LEFT, SNAP_RIGHT, SNAP_NONE };
SNAP_LEFT,
SNAP_RIGHT, WorkspaceWindowResizer(wm::WindowState* window_state,
SNAP_NONE const std::vector<aura::Window*>& attached_windows);
};
// Lays out the attached windows. |bounds| is the bounds of the main window. // Lays out the attached windows. |bounds| is the bounds of the main window.
void LayoutAttachedWindows(gfx::Rect* bounds); void LayoutAttachedWindows(gfx::Rect* bounds);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment