Re-enable tests in HighlighterControllerTest

The two tests (SelectionInsideScreen, HighlighterGesturesScaled)
were disabled because they stopped working in Mash).

SelectionInsideScreen is now passing reliably after Mash removal.

HighlighterGesturesScaled required some changes to accommodate
the new contract of highlighter selection callback (it is now
passing a rectangle in dip instead of px).

Bug: 917113
Test: automated
Change-Id: I43a2dd9a28ce0efc2748cf208e9c04c470d89026
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1695657Reviewed-by: Vladislav Kaznacheev <kaznacheev@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Commit-Queue: Vladislav Kaznacheev <kaznacheev@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676120}

ash/highlighter/highlighter_controller.cc

@@ -10,7 +10,6 @@
 #include "ash/highlighter/highlighter_gesture_util.h"
 #include "ash/highlighter/highlighter_result_view.h"
 #include "ash/highlighter/highlighter_view.h"
-#include "ash/public/cpp/scale_utility.h"
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/shell.h"
 #include "ash/shell_state.h"
@@ -43,20 +42,6 @@ gfx::RectF AdjustHorizontalStroke(const gfx::RectF& box,
                     box.width() + pen_tip_size.width(), pen_tip_size.height());
 }
 
-// This method computes the scale required to convert window-relative DIP
-// coordinates to the coordinate space of the screenshot taken from that window.
-// The transform returned by WindowTreeHost::GetRootTransform translates points
-// from DIP to physical screen pixels (by taking into account not only the
-// scale but also the rotation and the offset).
-// However, the screenshot bitmap is always oriented the same way as the window
-// from which it was taken, and has zero offset.
-// The code below deduces the scale from the transform by applying it to a pair
-// of points separated by the distance of 1, and measuring the distance between
-// the transformed points.
-float GetScreenshotScale(aura::Window* window) {
-  return GetScaleFactorForTransform(window->GetHost()->GetRootTransform());
-}
-
 }  // namespace
 
 HighlighterController::HighlighterController() : weak_factory_(this) {
@@ -209,17 +194,7 @@ void HighlighterController::RecognizeGesture() {
     Shell::Get()->shell_state()->SetRootWindowForNewWindows(
         current_window->GetRootWindow());
 
-    // TODO(muyuanli): Delete the check when native assistant is default on.
-    // This is a temporary workaround to support both ARC-based assistant
-    // and native assistant. In ARC-based assistant, we send the rect in pixels
-    // to ARC side, where the app will crop the screenshot. In native assistant,
-    // we pass the rect directly to UI snapshot API, which assumes coordinates
-    // in DP.
-    const gfx::Rect selection_rect =
-        chromeos::switches::IsAssistantEnabled()
-            ? gfx::ToEnclosingRect(box)
-            : gfx::ToEnclosingRect(
-                  gfx::ScaleRect(box, GetScreenshotScale(current_window)));
+    const gfx::Rect selection_rect = gfx::ToEnclosingRect(box);
     for (auto& observer : observers_)
       observer.OnHighlighterSelectionRecognized(selection_rect);
 

ash/highlighter/highlighter_controller_unittest.cc

@@ -294,17 +294,12 @@ TEST_F(HighlighterControllerTest, HighlighterGestures) {
   controller_->RemoveObserver(&observer);
 }
 
-// Disabled due to https://crbug.com/917113.
-TEST_F(HighlighterControllerTest, DISABLED_HighlighterGesturesScaled) {
+TEST_F(HighlighterControllerTest, HighlighterGesturesScaled) {
   controller_test_api_->SetEnabled(true);
   ui::test::EventGenerator* event_generator = GetEventGenerator();
   event_generator->EnterPenPointerMode();
 
-  const gfx::Rect original_rect(200, 100, 400, 300);
-
-  // Allow for rounding errors.
-  gfx::Rect inflated(original_rect);
-  inflated.Inset(-1, -1);
+  const gfx::Rect original_px(200, 100, 400, 300);
 
   constexpr float display_scales[] = {1.f, 1.5f, 2.0f};
   constexpr float ui_scales[] = {0.5f,  0.67f, 1.0f,  1.25f,
@@ -321,12 +316,21 @@ TEST_F(HighlighterControllerTest, DISABLED_HighlighterGesturesScaled) {
       UpdateDisplayAndWaitForCompositingEnded(display_spec);
 
       controller_test_api_->ResetSelection();
-      TraceRect(original_rect);
+      TraceRect(original_px);
       EXPECT_TRUE(controller_test_api_->HandleSelectionCalled());
 
-      const gfx::Rect selection = controller_test_api_->selection();
-      EXPECT_TRUE(inflated.Contains(selection));
-      EXPECT_TRUE(selection.Contains(original_rect));
+      const float combined_scale = display_scale * ui_scale;
+
+      const gfx::Rect selection_dp = controller_test_api_->selection();
+      const gfx::Rect selection_px = gfx::ToEnclosingRect(
+          gfx::ScaleRect(gfx::RectF(selection_dp), combined_scale));
+      EXPECT_TRUE(selection_px.Contains(original_px));
+
+      gfx::Rect inflated_px(original_px);
+      // Allow for rounding errors within 1dp.
+      const int error_margin = static_cast<int>(std::ceil(combined_scale));
+      inflated_px.Inset(-error_margin, -error_margin);
+      EXPECT_TRUE(inflated_px.Contains(selection_px));
     }
   }
 }
@@ -414,9 +418,7 @@ TEST_F(HighlighterControllerTest, InterruptedStroke) {
 }
 
 // Test that the selection is never crossing the screen bounds.
-//
-// Disabled due to https://crbug.com/917113.
-TEST_F(HighlighterControllerTest, DISABLED_SelectionInsideScreen) {
+TEST_F(HighlighterControllerTest, SelectionInsideScreen) {
   controller_test_api_->SetEnabled(true);
   ui::test::EventGenerator* event_generator = GetEventGenerator();
   event_generator->EnterPenPointerMode();