Change DialogPlateDelegate to DialogPlateObserver.

DialogPlate will now notify a pool of observers as opposed to a single
delegate.

Bug: b:80542452
Change-Id: I776ad59eac9c684ae8719eb1a43dc017064b4473
Reviewed-on: https://chromium-review.googlesource.com/1112061
Commit-Queue: David Black <dmblack@google.com>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570240}

ash/assistant/assistant_controller.cc

@@ -145,21 +145,6 @@ void AssistantController::DownloadImage(
   assistant_image_downloader_->Download(account_id, url, std::move(callback));
 }
 
-// TODO(dmblack): Update DialogPlate to accept multiple listeners and then
-// remove this code from AssistantController. Use observer pattern.
-void AssistantController::OnDialogPlateButtonPressed(DialogPlateButtonId id) {
-  assistant_interaction_controller_->OnDialogPlateButtonPressed(id);
-  assistant_ui_controller_->OnDialogPlateButtonPressed(id);
-}
-
-// TODO(dmblack): Update DialogPlate to accept multiple listeners and then
-// remove this code from AssistantController. Use observer pattern.
-void AssistantController::OnDialogPlateContentsCommitted(
-    const std::string& text) {
-  assistant_interaction_controller_->OnDialogPlateContentsCommitted(text);
-  assistant_ui_controller_->OnDialogPlateContentsCommitted(text);
-}
-
 void AssistantController::OnHighlighterSelectionRecognized(
     const gfx::Rect& rect) {
   // TODO(muyuanli): Request screen context for |rect|.

ash/assistant/assistant_controller.h

@@ -9,7 +9,6 @@
 #include <string>
 #include <vector>
 
-#include "ash/assistant/ui/dialog_plate/dialog_plate.h"
 #include "ash/highlighter/highlighter_controller.h"
 #include "ash/public/interfaces/assistant_controller.mojom.h"
 #include "ash/public/interfaces/assistant_image_downloader.mojom.h"
@@ -30,7 +29,6 @@ class AssistantInteractionController;
 class AssistantUiController;
 
 class AssistantController : public mojom::AssistantController,
-                            public DialogPlateDelegate,
                             public HighlighterController::Observer {
  public:
   AssistantController();
@@ -74,10 +72,6 @@ class AssistantController : public mojom::AssistantController,
   void RequestScreenshot(const gfx::Rect& rect,
                          RequestScreenshotCallback callback) override;
 
-  // DialogPlateDelegate:
-  void OnDialogPlateButtonPressed(DialogPlateButtonId id) override;
-  void OnDialogPlateContentsCommitted(const std::string& text) override;
-
   // HighlighterController::Observer:
   void OnHighlighterSelectionRecognized(const gfx::Rect& rect) override;
 

ash/assistant/assistant_interaction_controller.cc

@@ -238,11 +238,7 @@ void AssistantInteractionController::OnDialogPlateButtonPressed(
 
 void AssistantInteractionController::OnDialogPlateContentsCommitted(
     const std::string& text) {
-  // TODO(dmblack): This case no longer makes sense now that the DialogPlate has
-  // been rebuilt. Remove the ability to commit empty DialogPlate contents.
-  if (text.empty())
-    return;
-
+  DCHECK(!text.empty());
   StartTextInteraction(text);
 }
 

ash/assistant/assistant_interaction_controller.h

@@ -29,7 +29,7 @@ class AssistantInteractionController
       public AssistantInteractionModelObserver,
       public AssistantUiModelObserver,
       public HighlighterController::Observer,
-      public DialogPlateDelegate,
+      public DialogPlateObserver,
       public mojom::ManagedWebContentsOpenUrlDelegate {
  public:
   using AssistantSuggestion = chromeos::assistant::mojom::AssistantSuggestion;
@@ -90,7 +90,7 @@ class AssistantInteractionController
   // mojom::ManagedWebContentsOpenUrlDelegate:
   void OnOpenUrlFromTab(const GURL& url) override;
 
-  // DialogPlateDelegate:
+  // DialogPlateObserver:
   void OnDialogPlateButtonPressed(DialogPlateButtonId id) override;
   void OnDialogPlateContentsCommitted(const std::string& text) override;
 

ash/assistant/assistant_ui_controller.h

@@ -40,7 +40,7 @@ class ASH_EXPORT AssistantUiController
     : public views::WidgetObserver,
       public AssistantInteractionModelObserver,
       public CaptionBarDelegate,
-      public DialogPlateDelegate,
+      public DialogPlateObserver,
       public HighlighterController::Observer {
  public:
   explicit AssistantUiController(AssistantController* assistant_controller);
@@ -77,7 +77,7 @@ class ASH_EXPORT AssistantUiController
   // CaptionBarDelegate:
   bool OnCaptionButtonPressed(CaptionButtonId id) override;
 
-  // DialogPlateDelegate:
+  // DialogPlateObserver:
   void OnDialogPlateButtonPressed(DialogPlateButtonId id) override;
 
   // HighlighterController::Observer:

ash/assistant/ui/assistant_main_view.cc

@@ -7,6 +7,7 @@
 #include <memory>
 
 #include "ash/assistant/assistant_controller.h"
+#include "ash/assistant/assistant_interaction_controller.h"
 #include "ash/assistant/assistant_ui_controller.h"
 #include "ash/assistant/model/assistant_interaction_model.h"
 #include "ash/assistant/ui/assistant_ui_constants.h"
@@ -29,9 +30,10 @@ AssistantMainView::AssistantMainView(AssistantController* assistant_controller)
       min_height_dip_(kMinHeightDip) {
   InitLayout();
 
-  // Set delegates.
+  // Set delegate/observers.
   caption_bar_->set_delegate(assistant_controller_->ui_controller());
-  dialog_plate_->set_delegate(assistant_controller_);
+  dialog_plate_->AddObserver(assistant_controller_->interaction_controller());
+  dialog_plate_->AddObserver(assistant_controller_->ui_controller());
 
   // The AssistantController indirectly owns the view hierarchy to which
   // AssistantMainView belongs so is guaranteed to outlive it.
@@ -39,6 +41,10 @@ AssistantMainView::AssistantMainView(AssistantController* assistant_controller)
 }
 
 AssistantMainView::~AssistantMainView() {
+  dialog_plate_->RemoveObserver(assistant_controller_->ui_controller());
+  dialog_plate_->RemoveObserver(
+      assistant_controller_->interaction_controller());
+
   assistant_controller_->ui_controller()->RemoveModelObserver(this);
 }
 

ash/assistant/ui/dialog_plate/dialog_plate.cc

@@ -66,6 +66,14 @@ DialogPlate::~DialogPlate() {
   assistant_controller_->interaction_controller()->RemoveModelObserver(this);
 }
 
+void DialogPlate::AddObserver(DialogPlateObserver* observer) {
+  observers_.AddObserver(observer);
+}
+
+void DialogPlate::RemoveObserver(DialogPlateObserver* observer) {
+  observers_.RemoveObserver(observer);
+}
+
 gfx::Size DialogPlate::CalculatePreferredSize() const {
   return gfx::Size(INT_MAX, GetHeightForWidth(INT_MAX));
 }
@@ -198,8 +206,8 @@ void DialogPlate::ButtonPressed(views::Button* sender, const ui::Event& event) {
 }
 
 void DialogPlate::OnButtonPressed(DialogPlateButtonId id) {
-  if (delegate_)
-    delegate_->OnDialogPlateButtonPressed(id);
+  for (DialogPlateObserver& observer : observers_)
+    observer.OnDialogPlateButtonPressed(id);
 
   textfield_->SetText(base::string16());
 }
@@ -238,20 +246,15 @@ bool DialogPlate::HandleKeyEvent(views::Textfield* textfield,
   if (key_event.type() != ui::EventType::ET_KEY_PRESSED)
     return false;
 
-  const base::string16& text = textfield_->text();
-
-  // We filter out committing an empty string here but do allow committing a
-  // whitespace only string. If the user commits a whitespace only string, we
-  // want to be able to show a helpful message. This is taken care of in
-  // AssistantController's handling of the commit event.
-  if (text.empty())
-    return false;
-
   const base::StringPiece16& trimmed_text =
-      base::TrimWhitespace(text, base::TrimPositions::TRIM_ALL);
+      base::TrimWhitespace(textfield_->text(), base::TrimPositions::TRIM_ALL);
 
-  if (delegate_)
-    delegate_->OnDialogPlateContentsCommitted(base::UTF16ToUTF8(trimmed_text));
+  // Only non-empty trimmed text is consider a valid contents commit. Anything
+  // else will simply result in the DialogPlate being cleared.
+  if (!trimmed_text.empty()) {
+    for (DialogPlateObserver& observer : observers_)
+      observer.OnDialogPlateContentsCommitted(base::UTF16ToUTF8(trimmed_text));
+  }
 
   textfield_->SetText(base::string16());
 

ash/assistant/ui/dialog_plate/dialog_plate.h

@@ -11,6 +11,7 @@
 #include "ash/assistant/model/assistant_ui_model_observer.h"
 #include "ash/assistant/ui/dialog_plate/action_view.h"
 #include "base/macros.h"
+#include "base/observer_list.h"
 #include "ui/views/controls/button/button.h"
 #include "ui/views/controls/textfield/textfield_controller.h"
 #include "ui/views/view.h"
@@ -31,9 +32,9 @@ enum class DialogPlateButtonId {
   kSettings,
 };
 
-// DialogPlateDelegate ---------------------------------------------------------
+// DialogPlateObserver ---------------------------------------------------------
 
-class DialogPlateDelegate {
+class DialogPlateObserver {
  public:
   // Invoked when the dialog plate button identified by |id| is pressed.
   virtual void OnDialogPlateButtonPressed(DialogPlateButtonId id) {}
@@ -42,7 +43,7 @@ class DialogPlateDelegate {
   virtual void OnDialogPlateContentsCommitted(const std::string& text) {}
 
  protected:
-  virtual ~DialogPlateDelegate() = default;
+  virtual ~DialogPlateObserver() = default;
 };
 
 // DialogPlate -----------------------------------------------------------------
@@ -62,6 +63,10 @@ class DialogPlate : public views::View,
   explicit DialogPlate(AssistantController* assistant_controller);
   ~DialogPlate() override;
 
+  // Adds/removes the specified |observer|.
+  void AddObserver(DialogPlateObserver* observer);
+  void RemoveObserver(DialogPlateObserver* observer);
+
   // views::View:
   gfx::Size CalculatePreferredSize() const override;
   int GetHeightForWidth(int width) const override;
@@ -85,8 +90,6 @@ class DialogPlate : public views::View,
   // AssistantUiModelObserver:
   void OnUiVisibilityChanged(bool visible, AssistantSource source) override;
 
-  void set_delegate(DialogPlateDelegate* delegate) { delegate_ = delegate; }
-
  private:
   void InitLayout();
   void InitKeyboardLayoutContainer();
@@ -102,7 +105,7 @@ class DialogPlate : public views::View,
   views::ImageButton* voice_input_toggle_;           // Owned by view hierarchy.
   views::View* voice_layout_container_;              // Owned by view hierarchy.
 
-  DialogPlateDelegate* delegate_ = nullptr;
+  base::ObserverList<DialogPlateObserver> observers_;
 
   DISALLOW_COPY_AND_ASSIGN(DialogPlate);
 };