remove highlighter_controller.mojom

TBR=tsepez@chromium.org

Bug: 958201
Test: trybot
Change-Id: Ic3150a3c42b4956b64f5de62833d80c2237b23cf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1623587
Commit-Queue: Jun Mukai <mukai@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#663993}

ash/highlighter/highlighter_controller.cc

@@ -59,8 +59,7 @@ float GetScreenshotScale(aura::Window* window) {
 
 }  // namespace
 
-HighlighterController::HighlighterController()
-    : binding_(this), weak_factory_(this) {
+HighlighterController::HighlighterController() : weak_factory_(this) {
   Shell::Get()->AddPreTargetHandler(this);
 }
 
@@ -100,19 +99,6 @@ void HighlighterController::AbortSession() {
     UpdateEnabledState(HighlighterEnabledState::kDisabledBySessionAbort);
 }
 
-void HighlighterController::BindRequest(
-    mojom::HighlighterControllerRequest request) {
-  binding_.Bind(std::move(request));
-}
-
-void HighlighterController::SetClient(
-    mojom::HighlighterControllerClientPtr client) {
-  client_ = std::move(client);
-  client_.set_connection_error_handler(
-      base::BindOnce(&HighlighterController::OnClientConnectionLost,
-                     weak_factory_.GetWeakPtr()));
-}
-
 void HighlighterController::SetEnabled(bool enabled) {
   FastInkPointerController::SetEnabled(enabled);
   if (enabled) {
@@ -132,13 +118,6 @@ void HighlighterController::SetEnabled(bool enabled) {
     if (highlighter_view_ && !highlighter_view_->animating())
       DestroyPointerView();
   }
-
-  if (client_)
-    client_->HandleEnabledStateChange(enabled);
-}
-
-void HighlighterController::ExitHighlighterMode() {
-  CallExitCallback();
 }
 
 views::View* HighlighterController::GetPointerView() const {
@@ -241,9 +220,6 @@ void HighlighterController::RecognizeGesture() {
             ? gfx::ToEnclosingRect(box)
             : gfx::ToEnclosingRect(
                   gfx::ScaleRect(box, GetScreenshotScale(current_window)));
-    if (client_)
-      client_->HandleSelection(selection_rect);
-
     for (auto& observer : observers_)
       observer.OnHighlighterSelectionRecognized(selection_rect);
 
@@ -302,21 +278,9 @@ void HighlighterController::DestroyResultView() {
   result_view_.reset();
 }
 
-void HighlighterController::OnClientConnectionLost() {
-  client_.reset();
-  binding_.Close();
-  // The client has detached, force-exit the highlighter mode.
-  CallExitCallback();
-}
-
 void HighlighterController::CallExitCallback() {
   if (!exit_callback_.is_null())
     std::move(exit_callback_).Run();
 }
 
-void HighlighterController::FlushMojoForTesting() {
-  if (client_)
-    client_.FlushForTesting();
-}
-
 }  // namespace ash

ash/highlighter/highlighter_controller.h

@@ -9,16 +9,18 @@
 
 #include "ash/ash_export.h"
 #include "ash/components/fast_ink/fast_ink_pointer_controller.h"
-#include "ash/public/interfaces/highlighter_controller.mojom.h"
 #include "base/callback.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
-#include "mojo/public/cpp/bindings/binding.h"
 
 namespace base {
 class OneShotTimer;
 }
 
+namespace gfx {
+class Rect;
+}
+
 namespace ash {
 
 class HighlighterResultView;
@@ -42,8 +44,7 @@ enum class HighlighterEnabledState {
 // Enables/disables highlighter as well as receives points
 // and passes them off to be rendered.
 class ASH_EXPORT HighlighterController
-    : public fast_ink::FastInkPointerController,
-      public mojom::HighlighterController {
+    : public fast_ink::FastInkPointerController {
  public:
   // Interface for classes that wish to be notified with highlighter status.
   class Observer {
@@ -79,12 +80,6 @@ class ASH_EXPORT HighlighterController
   // calling this method is a no-op.
   void AbortSession();
 
-  void BindRequest(mojom::HighlighterControllerRequest request);
-
-  // mojom::HighlighterController:
-  void SetClient(mojom::HighlighterControllerClientPtr client) override;
-  void ExitHighlighterMode() override;
-
  private:
   friend class HighlighterControllerTestApi;
 
@@ -107,14 +102,9 @@ class ASH_EXPORT HighlighterController
   // Destroys |result_view_|, if it exists.
   void DestroyResultView();
 
-  // Called when the mojo connection with the client is closed.
-  void OnClientConnectionLost();
-
   // Calls and clears the mode exit callback, if it is set.
   void CallExitCallback();
 
-  void FlushMojoForTesting();
-
   // Caches the highlighter enabled state.
   HighlighterEnabledState enabled_state_ =
       HighlighterEnabledState::kDisabledByUser;
@@ -151,12 +141,6 @@ class ASH_EXPORT HighlighterController
   // If true, the mode is not exited until a valid selection is made.
   bool require_success_ = true;
 
-  // Binding for mojom::HighlighterController interface.
-  mojo::Binding<ash::mojom::HighlighterController> binding_;
-
-  // Interface to highlighter controller client (chrome).
-  mojom::HighlighterControllerClientPtr client_;
-
   base::ObserverList<Observer>::Unchecked observers_;
 
   base::WeakPtrFactory<HighlighterController> weak_factory_;

ash/highlighter/highlighter_controller_test_api.cc

@@ -13,12 +13,12 @@ namespace ash {
 
 HighlighterControllerTestApi::HighlighterControllerTestApi(
     HighlighterController* instance)
-    : binding_(this), instance_(instance) {
+    : instance_(instance) {
   AttachClient();
 }
 
 HighlighterControllerTestApi::~HighlighterControllerTestApi() {
-  if (binding_.is_bound())
+  if (scoped_observer_)
     DetachClient();
   if (instance_->enabled())
     instance_->SetEnabled(false);
@@ -26,25 +26,19 @@ HighlighterControllerTestApi::~HighlighterControllerTestApi() {
 }
 
 void HighlighterControllerTestApi::AttachClient() {
-  DCHECK(!binding_.is_bound());
-  DCHECK(!highlighter_controller_);
-  instance_->BindRequest(mojo::MakeRequest(&highlighter_controller_));
-  ash::mojom::HighlighterControllerClientPtr client;
-  binding_.Bind(mojo::MakeRequest(&client));
-  highlighter_controller_->SetClient(std::move(client));
-  highlighter_controller_.FlushForTesting();
+  scoped_observer_ = std::make_unique<ScopedObserver>(this);
+  scoped_observer_->Add(instance_);
 }
 
 void HighlighterControllerTestApi::DetachClient() {
-  DCHECK(binding_.is_bound());
-  DCHECK(highlighter_controller_);
-  highlighter_controller_ = nullptr;
-  binding_.Close();
-  instance_->FlushMojoForTesting();
+  scoped_observer_.reset();
+  instance_->CallExitCallback();
 }
 
 void HighlighterControllerTestApi::SetEnabled(bool enabled) {
-  instance_->SetEnabled(enabled);
+  instance_->UpdateEnabledState(
+      enabled ? HighlighterEnabledState::kEnabled
+              : HighlighterEnabledState::kDisabledBySessionComplete);
 }
 
 void HighlighterControllerTestApi::DestroyPointerView() {
@@ -85,21 +79,22 @@ const fast_ink::FastInkPoints& HighlighterControllerTestApi::predicted_points()
 }
 
 bool HighlighterControllerTestApi::HandleEnabledStateChangedCalled() {
-  instance_->FlushMojoForTesting();
   return handle_enabled_state_changed_called_;
 }
 
 bool HighlighterControllerTestApi::HandleSelectionCalled() {
-  instance_->FlushMojoForTesting();
   return handle_selection_called_;
 }
 
-void HighlighterControllerTestApi::HandleSelection(const gfx::Rect& rect) {
+void HighlighterControllerTestApi::OnHighlighterSelectionRecognized(
+    const gfx::Rect& rect) {
   handle_selection_called_ = true;
   selection_ = rect;
 }
 
-void HighlighterControllerTestApi::HandleEnabledStateChange(bool enabled) {
+void HighlighterControllerTestApi::OnHighlighterEnabledChanged(
+    HighlighterEnabledState state) {
+  const bool enabled = (state == HighlighterEnabledState::kEnabled);
   handle_enabled_state_changed_called_ = true;
   enabled_ = enabled;
 }

ash/highlighter/highlighter_controller_test_api.h

@@ -5,9 +5,9 @@
 #ifndef ASH_HIGHLIGHTER_HIGHLIGHTER_CONTROLLER_TEST_API_H_
 #define ASH_HIGHLIGHTER_HIGHLIGHTER_CONTROLLER_TEST_API_H_
 
-#include "ash/public/interfaces/highlighter_controller.mojom.h"
+#include "ash/highlighter/highlighter_controller.h"
 #include "base/macros.h"
-#include "mojo/public/cpp/bindings/binding.h"
+#include "base/scoped_observer.h"
 #include "ui/gfx/geometry/rect.h"
 
 namespace fast_ink {
@@ -16,13 +16,11 @@ class FastInkPoints;
 
 namespace ash {
 
-class HighlighterController;
-
 // An api for testing the HighlighterController class.
 // Implements ash::mojom::HighlighterControllerClient and binds itself as the
 // client to provide the tests with access to gesture recognition results.
 class HighlighterControllerTestApi
-    : public ash::mojom::HighlighterControllerClient {
+    : public ash::HighlighterController::Observer {
  public:
   explicit HighlighterControllerTestApi(HighlighterController* instance);
   ~HighlighterControllerTestApi() override;
@@ -46,28 +44,22 @@ class HighlighterControllerTestApi
   const fast_ink::FastInkPoints& predicted_points() const;
 
   void ResetEnabledState() { handle_enabled_state_changed_called_ = false; }
-  // Flushes the mojo connection, then checks whether HandleEnabledStateChange
-  // has been called on the client since the last call to ResetEnabledState.
   bool HandleEnabledStateChangedCalled();
   bool enabled() const { return enabled_; }
 
   void ResetSelection() { handle_selection_called_ = false; }
-  // Flushes the mojo connection, then checks whether HandleSelection
-  // has been called on the client since the last call to ResetSelection.
   bool HandleSelectionCalled();
   const gfx::Rect& selection() const { return selection_; }
 
  private:
-  // HighlighterSelectionObserver:
-  void HandleSelection(const gfx::Rect& rect) override;
-  void HandleEnabledStateChange(bool enabled) override;
+  using ScopedObserver =
+      ScopedObserver<HighlighterController, HighlighterController::Observer>;
 
-  // Binds to the client interface.
-  mojo::Binding<ash::mojom::HighlighterControllerClient> binding_;
-
-  // HighlighterController interface.
-  ash::mojom::HighlighterControllerPtr highlighter_controller_;
+  // HighlighterSelectionObserver:
+  void OnHighlighterSelectionRecognized(const gfx::Rect& rect) override;
+  void OnHighlighterEnabledChanged(HighlighterEnabledState state) override;
 
+  std::unique_ptr<ScopedObserver> scoped_observer_;
   HighlighterController* instance_;
 
   bool handle_selection_called_ = false;

ash/mojo_interface_factory.cc

@@ -17,7 +17,6 @@
 #include "ash/autotest/shelf_integration_test_api.h"
 #include "ash/display/cros_display_config.h"
 #include "ash/events/event_rewriter_controller.h"
-#include "ash/highlighter/highlighter_controller.h"
 #include "ash/ime/ime_controller.h"
 #include "ash/keyboard/ash_keyboard_controller.h"
 #include "ash/keyboard/ui/keyboard_controller.h"
@@ -111,11 +110,6 @@ void BindEventRewriterControllerRequestOnMainThread(
   Shell::Get()->event_rewriter_controller()->BindRequest(std::move(request));
 }
 
-void BindHighlighterControllerRequestOnMainThread(
-    mojom::HighlighterControllerRequest request) {
-  Shell::Get()->highlighter_controller()->BindRequest(std::move(request));
-}
-
 void BindImeControllerRequestOnMainThread(mojom::ImeControllerRequest request) {
   Shell::Get()->ime_controller()->BindRequest(std::move(request));
 }
@@ -226,9 +220,6 @@ void RegisterInterfaces(
   registry->AddInterface(
       base::BindRepeating(&BindEventRewriterControllerRequestOnMainThread),
       main_thread_task_runner);
-  registry->AddInterface(
-      base::BindRepeating(&BindHighlighterControllerRequestOnMainThread),
-      main_thread_task_runner);
   registry->AddInterface(
       base::BindRepeating(&BindImeControllerRequestOnMainThread),
       main_thread_task_runner);

ash/public/cpp/manifest.cc

@@ -12,7 +12,6 @@
 #include "ash/public/interfaces/constants.mojom.h"
 #include "ash/public/interfaces/cros_display_config.mojom.h"
 #include "ash/public/interfaces/event_rewriter_controller.mojom.h"
-#include "ash/public/interfaces/highlighter_controller.mojom.h"
 #include "ash/public/interfaces/ime_controller.mojom.h"
 #include "ash/public/interfaces/keyboard_controller.mojom.h"
 #include "ash/public/interfaces/kiosk_next_shell.mojom.h"
@@ -68,13 +67,12 @@ const service_manager::Manifest& GetManifest() {
                   mojom::AssistantVolumeControl,
                   mojom::KioskNextShellController,
                   mojom::CrosDisplayConfigController,
-                  mojom::EventRewriterController, mojom::HighlighterController,
-                  mojom::ImeController, mojom::KeyboardController,
-                  mojom::LocaleUpdateController, mojom::LoginScreen,
-                  mojom::MediaController, mojom::NightLightController,
-                  mojom::ShutdownController, mojom::TabletModeController,
-                  mojom::TrayAction, mojom::VoiceInteractionController,
-                  mojom::VpnList>())
+                  mojom::EventRewriterController, mojom::ImeController,
+                  mojom::KeyboardController, mojom::LocaleUpdateController,
+                  mojom::LoginScreen, mojom::MediaController,
+                  mojom::NightLightController, mojom::ShutdownController,
+                  mojom::TabletModeController, mojom::TrayAction,
+                  mojom::VoiceInteractionController, mojom::VpnList>())
           .ExposeCapability("test", service_manager::Manifest::InterfaceList<
                                         mojom::ShelfIntegrationTestApi>())
           .RequireCapability("*", "accessibility")

ash/public/interfaces/BUILD.gn

@@ -27,7 +27,6 @@ mojom("interfaces_internal") {
     "constants.mojom",
     "cros_display_config.mojom",
     "event_rewriter_controller.mojom",
-    "highlighter_controller.mojom",
     "ime_controller.mojom",
     "ime_info.mojom",
     "keyboard_config.mojom",

ash/public/interfaces/highlighter_controller.mojom

-// Copyright 2017 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.
-
-module ash.mojom;
-
-import "ui/gfx/geometry/mojo/geometry.mojom";
-
-// Interface for ash client (e.g. Chrome) to connect to the highlighter
-// controller, the component implementing on-screen content selection
-// with a stylus.
-interface HighlighterController {
-  // Sets the client interface.
-  SetClient(HighlighterControllerClient client);
-
-  // Exits the highlighter mode if it is currently enabled.
-  ExitHighlighterMode();
-};
-
-// Interface for ash to notify the client (e.g. Chrome) about the highlighter
-// selection and state.
-interface HighlighterControllerClient {
-  // Called when when a valid selection is made. Selected rectangle is in
-  // screen coordinates, clipped to screen bounds if necessary.
-  HandleSelection(gfx.mojom.Rect rect);
-
-  // Called when the highlighter tool becomes enabled or disabled.
-  HandleEnabledStateChange(bool enabled);
-};