Fix the unwanted splitview drag indicator in portrait mode.

In portrait mode, when dragging a window from top or from bottom, we
should only show the bottom or top drag indicator. This CL ensures
that the other drag indicator stays invisible when only one drag indicator
is supposed to show up.

Bug: 987031, 997885
Change-Id: I8644080741e0dfff8ae96c7f3888f55288ec18e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1829643
Commit-Queue: Xiaoqian Dai <xdai@chromium.org>
Reviewed-by: Avery Musbach <amusbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703957}

ash/wm/splitview/split_view_constants.h

@@ -81,6 +81,12 @@ constexpr int kSplitviewWhiteBarSpawnLongSideLength = 2;
 // The distance from the divider to where its handler spawns.
 constexpr int kSplitviewWhiteBarSpawnUnsignedOffset = 2;
 
+// The opacity of the drag-to-snap or cannot-snap drag indicator.
+constexpr float kHighlightOpacity = 0.3f;
+
+// The opacity of the split view snap preview area.
+constexpr float kPreviewAreaHighlightOpacity = 0.18f;
+
 }  // namespace ash
 
 #endif  // ASH_WM_SPLITVIEW_SPLIT_VIEW_CONSTANTS_H_

ash/wm/splitview/split_view_highlight_view.cc

@@ -217,6 +217,16 @@ void SplitViewHighlightView::OnIndicatorTypeChanged(
     return;
   }
 
+  // Having ruled out kNone and the "preview area" states, we know that
+  // |indicator_state| is either a "drag area" state or a "cannot snap" state.
+  // If there is an indicator on only one side, and if this, in the sense of the
+  // C++ keyword this, is the indicator on the opposite side, then bail out.
+  if (is_right_or_bottom_
+          ? SplitViewDragIndicators::IsLeftIndicatorState(indicator_state)
+          : SplitViewDragIndicators::IsRightIndicatorState(indicator_state)) {
+    return;
+  }
+
   if (SplitViewDragIndicators::IsPreviewAreaState(previous_indicator_state)) {
     const bool was_this_the_preview =
         is_right_or_bottom_ !=
@@ -231,15 +241,6 @@ void SplitViewHighlightView::OnIndicatorTypeChanged(
     return;
   }
 
-  // Having ruled out kNone and the "preview area" states, we know that
-  // |indicator_state| is either a "drag area" state or a "cannot snap" state.
-  // If there is an indicator on only one side, and if this, in the sense of the
-  // C++ keyword this, is the indicator on the opposite side, then bail out.
-  if (is_right_or_bottom_
-          ? SplitViewDragIndicators::IsLeftIndicatorState(indicator_state)
-          : SplitViewDragIndicators::IsRightIndicatorState(indicator_state)) {
-    return;
-  }
   SetColor(SplitViewDragIndicators::IsCannotSnapState(indicator_state)
                ? SK_ColorBLACK
                : SK_ColorWHITE);

ash/wm/splitview/split_view_highlight_view_unittest.cc

@@ -5,6 +5,7 @@
 #include "ash/wm/splitview/split_view_highlight_view.h"
 
 #include "ash/test/ash_test_base.h"
+#include "ash/wm/splitview/split_view_constants.h"
 #include "ash/wm/splitview/split_view_highlight_view_test_api.h"
 #include "base/test/icu_test_util.h"
 #include "ui/gfx/transform.h"
@@ -206,4 +207,40 @@ TEST_F(SplitViewHighlightViewTest, RightBounds) {
   EXPECT_TRUE(GetTransform(test_api.GetRightBottomView()).IsIdentity());
 }
 
+// Test when there is only one indicator is visible, the other indicator should
+// always have 0 opacity.
+TEST_F(SplitViewHighlightViewTest, SingleIndicatorOpacityTest) {
+  const gfx::Rect bounds(100, 0, 100, 100);
+  left_highlight()->SetBounds(bounds, /*landscape=*/false,
+                              /*animation_type=*/base::nullopt);
+  left_highlight()->SetPaintToLayer();
+  left_highlight()->layer()->SetOpacity(0.f);
+  right_highlight()->SetBounds(bounds, /*landscape=*/false,
+                               /*animation_type=*/base::nullopt);
+  right_highlight()->SetPaintToLayer();
+  right_highlight()->layer()->SetOpacity(0.f);
+
+  left_highlight()->OnIndicatorTypeChanged(IndicatorState::kDragAreaRight,
+                                           IndicatorState::kNone);
+  EXPECT_EQ(0.f, left_highlight()->layer()->opacity());
+  right_highlight()->OnIndicatorTypeChanged(IndicatorState::kDragAreaRight,
+                                            IndicatorState::kNone);
+  EXPECT_EQ(kHighlightOpacity, right_highlight()->layer()->opacity());
+
+  left_highlight()->OnIndicatorTypeChanged(IndicatorState::kPreviewAreaRight,
+                                           IndicatorState::kDragAreaRight);
+  EXPECT_EQ(0.f, left_highlight()->layer()->opacity());
+  right_highlight()->OnIndicatorTypeChanged(IndicatorState::kPreviewAreaRight,
+                                            IndicatorState::kDragAreaRight);
+  EXPECT_EQ(kPreviewAreaHighlightOpacity,
+            right_highlight()->layer()->opacity());
+
+  left_highlight()->OnIndicatorTypeChanged(IndicatorState::kDragAreaRight,
+                                           IndicatorState::kPreviewAreaRight);
+  EXPECT_EQ(0.f, left_highlight()->layer()->opacity());
+  right_highlight()->OnIndicatorTypeChanged(IndicatorState::kDragAreaRight,
+                                            IndicatorState::kPreviewAreaRight);
+  EXPECT_EQ(kHighlightOpacity, right_highlight()->layer()->opacity());
+}
+
 }  // namespace ash

ash/wm/splitview/split_view_utils.cc

@@ -52,9 +52,6 @@ constexpr base::TimeDelta kLabelAnimation =
 constexpr base::TimeDelta kLabelAnimationDelay =
     base::TimeDelta::FromMilliseconds(167);
 
-constexpr float kHighlightOpacity = 0.3f;
-constexpr float kPreviewAreaHighlightOpacity = 0.18f;
-
 // Toast data.
 constexpr char kAppCannotSnapToastId[] = "split_view_app_cannot_snap";
 constexpr int kAppCannotSnapToastDurationMs = 2500;