Move note taking related methods out of PaletteDelegate

Create ash::NoteTakingController and have chromeos::NoteTakingHelper communicate with it over mojo. Bug: 773698 Test: ash_unittests --gtest_filter=CreateNoteTest.* Change-Id: Ib41e9adc0d95624c450097c48d1c0779ed8ee763 Reviewed-on: https://chromium-review.googlesource.com/713195 Reviewed-by: James Cook <jamescook@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Jacob Dufault <jdufault@chromium.org> Commit-Queue: Vladislav Kaznacheev <kaznacheev@chromium.org> Cr-Commit-Position: refs/heads/master@{#508837}
Denger-Network · Oct 13, 2017 · 92edf62 · 92edf62
1 parent 63ef7ef
commit 92edf62
Show file tree

Hide file tree

Showing 23 changed files with 359 additions and 61 deletions.
diff --git a/ash/BUILD.gn b/ash/BUILD.gn
@@ -306,6 +306,8 @@ component("ash") {
     "multi_profile_uma.h",
     "new_window_controller.cc",
     "new_window_controller.h",
+    "note_taking_controller.cc",
+    "note_taking_controller.h",
     "palette_delegate.h",
     "pointer_watcher_adapter_classic.cc",
     "pointer_watcher_adapter_classic.h",

diff --git a/ash/mojo_interface_factory.cc b/ash/mojo_interface_factory.cc
@@ -14,6 +14,7 @@
 #include "ash/login/lock_screen_controller.h"
 #include "ash/media_controller.h"
 #include "ash/new_window_controller.h"
+#include "ash/note_taking_controller.h"
 #include "ash/public/cpp/ash_switches.h"
 #include "ash/session/session_controller.h"
 #include "ash/shelf/shelf_controller.h"
@@ -92,6 +93,11 @@ void BindNightLightControllerRequestOnMainThread(
   Shell::Get()->night_light_controller()->BindRequest(std::move(request));
 }
 
+void BindNoteTakingControllerRequestOnMainThread(
+    mojom::NoteTakingControllerRequest request) {
+  Shell::Get()->note_taking_controller()->BindRequest(std::move(request));
+}
+
 void BindSessionControllerRequestOnMainThread(
     mojom::SessionControllerRequest request) {
   Shell::Get()->session_controller()->BindRequest(std::move(request));
@@ -163,6 +169,9 @@ void RegisterInterfaces(
         base::Bind(&BindNightLightControllerRequestOnMainThread),
         main_thread_task_runner);
   }
+  registry->AddInterface(
+      base::Bind(&BindNoteTakingControllerRequestOnMainThread),
+      main_thread_task_runner);
   registry->AddInterface(base::Bind(&BindSessionControllerRequestOnMainThread),
                          main_thread_task_runner);
   registry->AddInterface(base::Bind(&BindShelfRequestOnMainThread),

diff --git a/ash/mus/manifest.json b/ash/mus/manifest.json
@@ -18,6 +18,7 @@
           "ash::mojom::MediaController",
           "ash::mojom::NewWindowController",
           "ash::mojom::NightLightController",
+          "ash::mojom::NoteTakingController",
           "ash::mojom::SessionController",
           "ash::mojom::ShelfController",
           "ash::mojom::ShutdownController",

diff --git a/ash/mus/standalone/manifest.json b/ash/mus/standalone/manifest.json
@@ -13,6 +13,7 @@
           "ash::mojom::LocaleNotificationController",
           "ash::mojom::MediaController",
           "ash::mojom::NewWindowController",
+          "ash::mojom::NoteTakingController",
           "ash::mojom::SessionController",
           "ash::mojom::ShelfController",
           "ash::mojom::ShutdownController",

diff --git a/ash/note_taking_controller.cc b/ash/note_taking_controller.cc
@@ -0,0 +1,45 @@
+// 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.
+
+#include "ash/note_taking_controller.h"
+
+namespace ash {
+
+NoteTakingController::NoteTakingController() : binding_(this) {}
+
+NoteTakingController::~NoteTakingController() = default;
+
+void NoteTakingController::BindRequest(
+    mojom::NoteTakingControllerRequest request) {
+  binding_.Bind(std::move(request));
+}
+
+void NoteTakingController::SetClient(
+    mojom::NoteTakingControllerClientPtr client) {
+  DCHECK(!client_);
+  client_ = std::move(client);
+  client_.set_connection_error_handler(base::Bind(
+      &NoteTakingController::OnClientConnectionLost, base::Unretained(this)));
+}
+
+bool NoteTakingController::CanCreateNote() const {
+  return static_cast<bool>(client_);
+}
+
+void NoteTakingController::CreateNote() {
+  DCHECK(client_);
+  client_->CreateNote();
+}
+
+void NoteTakingController::OnClientConnectionLost() {
+  client_.reset();
+  binding_.Close();
+}
+
+void NoteTakingController::FlushMojoForTesting() {
+  if (client_)
+    client_.FlushForTesting();
+}
+
+}  // namespace ash
diff --git a/ash/note_taking_controller.h b/ash/note_taking_controller.h
@@ -0,0 +1,49 @@
+// 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.
+
+#ifndef ASH_NOTETAKING_CONTROLLER_H_
+#define ASH_NOTETAKING_CONTROLLER_H_
+
+#include "ash/ash_export.h"
+#include "ash/public/interfaces/note_taking_controller.mojom.h"
+#include "mojo/public/cpp/bindings/binding.h"
+
+namespace ash {
+
+// Controller for the note taking functionality.
+class ASH_EXPORT NoteTakingController : public mojom::NoteTakingController {
+ public:
+  NoteTakingController();
+  ~NoteTakingController() override;
+
+  void BindRequest(mojom::NoteTakingControllerRequest request);
+
+  // mojom::NoteTakingController:
+  void SetClient(mojom::NoteTakingControllerClientPtr client) override;
+
+  // Returns true if the client is attached.
+  bool CanCreateNote() const;
+
+  // Calls the method of the same name on |client_|.
+  void CreateNote();
+
+ private:
+  friend class TestNoteTakingControllerClient;
+
+  void OnClientConnectionLost();
+
+  void FlushMojoForTesting();
+
+  // Binding for mojom::NoteTakingController interface.
+  mojo::Binding<ash::mojom::NoteTakingController> binding_;
+
+  // Interface to NoteTaking controller client (chrome).
+  mojom::NoteTakingControllerClientPtr client_;
+
+  DISALLOW_COPY_AND_ASSIGN(NoteTakingController);
+};
+
+}  // namespace ash
+
+#endif  // ASH_NOTETAKING_CONTROLLER_H_
diff --git a/ash/palette_delegate.h b/ash/palette_delegate.h
@@ -30,12 +30,6 @@ class PaletteDelegate {
   virtual std::unique_ptr<EnableListenerSubscription> AddPaletteEnableListener(
       const EnableListener& on_state_changed) = 0;
 
-  // Create a new note.
-  virtual void CreateNote() = 0;
-
-  // Returns true if there is a note-taking application available.
-  virtual bool HasNoteApp() = 0;
-
   // Returns true if the palette should be automatically opened on an eject
   // event.
   virtual bool ShouldAutoOpenPalette() = 0;

diff --git a/ash/public/interfaces/BUILD.gn b/ash/public/interfaces/BUILD.gn
@@ -26,6 +26,7 @@ mojom("interfaces_internal") {
     "media.mojom",
     "new_window.mojom",
     "night_light_controller.mojom",
+    "note_taking_controller.mojom",
     "pref_connector.mojom",
     "session_controller.mojom",
     "shelf.mojom",

diff --git a/ash/public/interfaces/note_taking_controller.mojom b/ash/public/interfaces/note_taking_controller.mojom
@@ -0,0 +1,19 @@
+// 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;
+
+// Interface for ash client (e.g. Chrome) to connect to the note taking
+// controller.
+interface NoteTakingController {
+  // Sets the client interface.
+  SetClient(NoteTakingControllerClient client);
+};
+
+// Interface for ash to notify the client (e.g. Chrome) about the new note
+// creation.
+interface NoteTakingControllerClient {
+  // Called when the controller needs to create a new note.
+  CreateNote();
+};
diff --git a/ash/shell.cc b/ash/shell.cc
@@ -52,6 +52,7 @@
 #include "ash/media_controller.h"
 #include "ash/message_center/message_center_controller.h"
 #include "ash/new_window_controller.h"
+#include "ash/note_taking_controller.h"
 #include "ash/palette_delegate.h"
 #include "ash/public/cpp/ash_switches.h"
 #include "ash/public/cpp/config.h"
@@ -616,6 +617,7 @@ Shell::Shell(std::unique_ptr<ShellDelegate> shell_delegate,
       new_window_controller_(std::make_unique<NewWindowController>()),
       session_controller_(std::make_unique<SessionController>(
           shell_delegate->GetShellConnector())),
+      note_taking_controller_(std::make_unique<NoteTakingController>()),
       shell_delegate_(std::move(shell_delegate)),
       shutdown_controller_(std::make_unique<ShutdownController>()),
       system_tray_controller_(std::make_unique<SystemTrayController>()),

diff --git a/ash/shell.h b/ash/shell.h
@@ -122,6 +122,7 @@ class MouseCursorEventFilter;
 class MruWindowTracker;
 class NewWindowController;
 class NightLightController;
+class NoteTakingController;
 class OverlayEventFilter;
 class PaletteDelegate;
 class PartialMagnificationController;
@@ -381,6 +382,9 @@ class ASH_EXPORT Shell : public SessionObserver,
     return new_window_controller_.get();
   }
   NightLightController* night_light_controller();
+  NoteTakingController* note_taking_controller() {
+    return note_taking_controller_.get();
+  }
   OverlayEventFilter* overlay_filter() { return overlay_filter_.get(); }
   PaletteDelegate* palette_delegate() { return palette_delegate_.get(); }
   PartialMagnificationController* partial_magnification_controller() {
@@ -679,6 +683,7 @@ class ASH_EXPORT Shell : public SessionObserver,
   std::unique_ptr<ResizeShadowController> resize_shadow_controller_;
   std::unique_ptr<SessionController> session_controller_;
   std::unique_ptr<NightLightController> night_light_controller_;
+  std::unique_ptr<NoteTakingController> note_taking_controller_;
   std::unique_ptr<ShelfController> shelf_controller_;
   std::unique_ptr<ShelfWindowWatcher> shelf_window_watcher_;
   std::unique_ptr<ShellDelegate> shell_delegate_;

diff --git a/ash/shell/shell_delegate_impl.cc b/ash/shell/shell_delegate_impl.cc
@@ -39,8 +39,6 @@ class PaletteDelegateImpl : public PaletteDelegate {
     on_state_changed.Run(false);
     return nullptr;
   }
-  void CreateNote() override {}
-  bool HasNoteApp() override { return false; }
   bool ShouldAutoOpenPalette() override { return false; }
   bool ShouldShowPalette() override { return false; }
 

diff --git a/ash/system/palette/test_palette_delegate.cc b/ash/system/palette/test_palette_delegate.cc
@@ -18,15 +18,6 @@ TestPaletteDelegate::AddPaletteEnableListener(
   return nullptr;
 }
 
-void TestPaletteDelegate::CreateNote() {
-  ++create_note_count_;
-}
-
-bool TestPaletteDelegate::HasNoteApp() {
-  ++has_note_app_count_;
-  return has_note_app_;
-}
-
 bool TestPaletteDelegate::ShouldAutoOpenPalette() {
   return should_auto_open_palette_;
 }

diff --git a/ash/system/palette/test_palette_delegate.h b/ash/system/palette/test_palette_delegate.h
@@ -16,12 +16,6 @@ class TestPaletteDelegate : public PaletteDelegate {
   TestPaletteDelegate();
   ~TestPaletteDelegate() override;
 
-  int create_note_count() const { return create_note_count_; }
-
-  int has_note_app_count() const { return has_note_app_count_; }
-
-  void set_has_note_app(bool has_note_app) { has_note_app_ = has_note_app; }
-
   void set_should_auto_open_palette(bool should_auto_open_palette) {
     should_auto_open_palette_ = should_auto_open_palette;
   }
@@ -34,14 +28,9 @@ class TestPaletteDelegate : public PaletteDelegate {
   // PaletteDelegate:
   std::unique_ptr<EnableListenerSubscription> AddPaletteEnableListener(
       const EnableListener& on_state_changed) override;
-  void CreateNote() override;
-  bool HasNoteApp() override;
   bool ShouldAutoOpenPalette() override;
   bool ShouldShowPalette() override;
 
-  int create_note_count_ = 0;
-  int has_note_app_count_ = 0;
-  bool has_note_app_ = false;
   bool should_auto_open_palette_ = false;
   bool should_show_palette_ = false;
 

diff --git a/ash/system/palette/tools/create_note_action.cc b/ash/system/palette/tools/create_note_action.cc
@@ -4,7 +4,7 @@
 
 #include "ash/system/palette/tools/create_note_action.h"
 
-#include "ash/palette_delegate.h"
+#include "ash/note_taking_controller.h"
 #include "ash/resources/vector_icons/vector_icons.h"
 #include "ash/shell.h"
 #include "ash/strings/grit/ash_strings.h"
@@ -29,14 +29,14 @@ PaletteToolId CreateNoteAction::GetToolId() const {
 void CreateNoteAction::OnEnable() {
   CommonPaletteTool::OnEnable();
 
-  Shell::Get()->palette_delegate()->CreateNote();
+  Shell::Get()->note_taking_controller()->CreateNote();
 
   delegate()->DisableTool(GetToolId());
   delegate()->HidePalette();
 }
 
 views::View* CreateNoteAction::CreateView() {
-  if (!Shell::Get()->palette_delegate()->HasNoteApp())
+  if (!Shell::Get()->note_taking_controller()->CanCreateNote())
     return nullptr;
 
   return CreateDefaultView(