Reland "Support key up/down to move through query history."

This is a reland of 2a5578c2

Fix the build failure in
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/linux-chromeos-dbg/9034

Add ASH_EXPORT on AssistantQueryHistory because it is included in ash_unittests,
or else build with is_debug = true fails.

Original change's description:
> Support key up/down to move through query history.
>
> We follow the behavior of linux shell:
> (1) Whenever there is a new query, the iterator is reset to the new
>     query.
> (2) Prev of the iterator moves till the first query and stick there.
> (3) Next of the iterator can move beyond last query and get an empty
>     query.
>
>
> Bug: b:119504796
> Change-Id: Iee1247359df78b8e2189fa86c3bd1b8b6ebfe612
> Reviewed-on: https://chromium-review.googlesource.com/c/1334271
> Commit-Queue: Yilei Li <yileili@google.com>
> Reviewed-by: Tao Wu <wutao@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#609888}

Bug: b:119504796
Change-Id: I445b022caef9e82272a07c3a8949e1decc4aaeff
Reviewed-on: https://chromium-review.googlesource.com/c/1345811Reviewed-by: Tao Wu <wutao@chromium.org>
Commit-Queue: Yilei Li <yileili@google.com>
Cr-Commit-Position: refs/heads/master@{#610951}

ash/BUILD.gn

@@ -184,6 +184,8 @@ component("ash") {
     "assistant/model/assistant_interaction_model_observer.h",
     "assistant/model/assistant_query.cc",
     "assistant/model/assistant_query.h",
+    "assistant/model/assistant_query_history.cc",
+    "assistant/model/assistant_query_history.h",
     "assistant/model/assistant_response.cc",
     "assistant/model/assistant_response.h",
     "assistant/model/assistant_screen_context_model.cc",
@@ -1628,6 +1630,7 @@ test("ash_unittests") {
     "ash_service_unittest.cc",
     "assistant/assistant_controller_unittest.cc",
     "assistant/assistant_screen_context_controller_unittest.cc",
+    "assistant/model/assistant_query_history_unittest.cc",
     "assistant/ui/assistant_container_view_unittest.cc",
     "assistant/util/deep_link_util_unittest.cc",
     "autoclick/autoclick_drag_event_rewriter_unittest.cc",

ash/assistant/assistant_interaction_controller.cc

@@ -203,6 +203,28 @@ void AssistantInteractionController::OnHighlighterSelectionRecognized(
   StartMetalayerInteraction(/*region=*/rect);
 }
 
+void AssistantInteractionController::OnCommittedQueryChanged(
+    const AssistantQuery& assistant_query) {
+  std::string query;
+  switch (assistant_query.type()) {
+    case AssistantQueryType::kText: {
+      const auto* assistant_text_query =
+          static_cast<const AssistantTextQuery*>(&assistant_query);
+      query = assistant_text_query->text();
+      break;
+    }
+    case AssistantQueryType::kVoice: {
+      const auto* assistant_voice_query =
+          static_cast<const AssistantVoiceQuery*>(&assistant_query);
+      query = assistant_voice_query->high_confidence_speech();
+      break;
+    }
+    case AssistantQueryType::kNull:
+      break;
+  }
+  model_.query_history().Add(query);
+}
+
 void AssistantInteractionController::OnInteractionStateChanged(
     InteractionState interaction_state) {
   if (interaction_state != InteractionState::kActive)

ash/assistant/assistant_interaction_controller.h

@@ -67,6 +67,7 @@ class AssistantInteractionController
   void OnInteractionStateChanged(InteractionState interaction_state) override;
   void OnInputModalityChanged(InputModality input_modality) override;
   void OnMicStateChanged(MicState mic_state) override;
+  void OnCommittedQueryChanged(const AssistantQuery& query) override;
   void OnResponseChanged(
       const std::shared_ptr<AssistantResponse>& response) override;
 

ash/assistant/model/assistant_interaction_model.h

@@ -9,6 +9,7 @@
 #include <string>
 #include <vector>
 
+#include "ash/assistant/model/assistant_query_history.h"
 #include "base/macros.h"
 #include "base/observer_list.h"
 
@@ -119,6 +120,12 @@ class AssistantInteractionModel {
   // Updates the speech level in dB.
   void SetSpeechLevel(float speech_level_db);
 
+  // Returns the reference to query history.
+  AssistantQueryHistory& query_history() { return query_history_; }
+
+  // Returns the const reference to query history.
+  const AssistantQueryHistory& query_history() const { return query_history_; }
+
  private:
   void NotifyInteractionStateChanged();
   void NotifyInputModalityChanged();
@@ -134,6 +141,7 @@ class AssistantInteractionModel {
   InteractionState interaction_state_ = InteractionState::kInactive;
   InputModality input_modality_ = InputModality::kKeyboard;
   MicState mic_state_ = MicState::kClosed;
+  AssistantQueryHistory query_history_;
   std::unique_ptr<AssistantQuery> committed_query_;
   std::unique_ptr<AssistantQuery> pending_query_;
   std::unique_ptr<AssistantResponse> pending_response_;

ash/assistant/model/assistant_query_history.cc

+// Copyright 2018 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/assistant/model/assistant_query_history.h"
+
+namespace ash {
+
+AssistantQueryHistory::AssistantQueryHistory(int capacity)
+    : capacity_(capacity) {
+  queries_.reserve(capacity);
+}
+
+AssistantQueryHistory::~AssistantQueryHistory() = default;
+
+std::unique_ptr<AssistantQueryHistory::Iterator>
+AssistantQueryHistory::GetIterator() const {
+  return std::make_unique<AssistantQueryHistory::Iterator>(queries_);
+}
+
+void AssistantQueryHistory::Add(const std::string& query) {
+  if (query.empty())
+    return;
+
+  if (static_cast<int>(queries_.size()) == capacity_)
+    queries_.pop_front();
+  queries_.push_back(query);
+}
+
+AssistantQueryHistory::Iterator::Iterator(
+    const base::circular_deque<std::string>& queries)
+    : queries_(queries), cur_pos_(queries_.size()) {}
+
+AssistantQueryHistory::Iterator::~Iterator() = default;
+
+base::Optional<std::string> AssistantQueryHistory::Iterator::Next() {
+  // queries_.size() is of type unsigned int and queries_.size() -1 will
+  // overflow if it is 0.
+  if (cur_pos_ + 1 >= queries_.size()) {
+    cur_pos_ = queries_.size();
+    return base::nullopt;
+  }
+  cur_pos_++;
+  return base::make_optional<std::string>(queries_[cur_pos_]);
+}
+
+base::Optional<std::string> AssistantQueryHistory::Iterator::Prev() {
+  if (queries_.size() == 0)
+    return base::nullopt;
+
+  if (cur_pos_ != 0)
+    cur_pos_--;
+
+  return base::make_optional<std::string>(queries_[cur_pos_]);
+}
+
+void AssistantQueryHistory::Iterator::ResetToLast() {
+  cur_pos_ = queries_.size();
+}
+
+}  // namespace ash

ash/assistant/model/assistant_query_history.h

+// Copyright 2018 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_ASSISTANT_MODEL_ASSISTANT_QUERY_HISTORY_H_
+#define ASH_ASSISTANT_MODEL_ASSISTANT_QUERY_HISTORY_H_
+
+#include <memory>
+#include <string>
+
+#include "ash/ash_export.h"
+#include "base/containers/circular_deque.h"
+#include "base/macros.h"
+#include "base/optional.h"
+
+namespace ash {
+
+// Caches user query history.
+class ASH_EXPORT AssistantQueryHistory {
+ public:
+  class Iterator {
+   public:
+    Iterator(const base::circular_deque<std::string>& queries);
+    ~Iterator();
+
+    // Fetches the next query. If current is already the last query, or there is
+    // no query in history, returns nullopt.
+    base::Optional<std::string> Next();
+
+    // Fetches the previous query. If current is already the first query, return
+    // the first query. If there is no query in history, returns nullopt.
+    base::Optional<std::string> Prev();
+
+    // Resets to the last query. It also makes current iterator valid again if
+    // new queries are added to the underlying AssistantQueryHistory.
+    void ResetToLast();
+
+   private:
+    const base::circular_deque<std::string>& queries_;
+    size_t cur_pos_;
+
+    DISALLOW_COPY_AND_ASSIGN(Iterator);
+  };
+
+  AssistantQueryHistory(int capacity = 100);
+  ~AssistantQueryHistory();
+
+  // Gets the iterator of query history.
+  std::unique_ptr<Iterator> GetIterator() const;
+
+  // Adds a query to history. If it is empty, ignore it.
+  void Add(const std::string& query);
+
+ private:
+  const int capacity_;
+  base::circular_deque<std::string> queries_;
+
+  DISALLOW_COPY_AND_ASSIGN(AssistantQueryHistory);
+};
+
+}  // namespace ash
+
+#endif  // ASH_ASSISTANT_MODEL_ASSISTANT_QUERY_HISTORY_H_

ash/assistant/model/assistant_query_history_unittest.cc

+// Copyright 2018 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/assistant/model/assistant_query_history.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace ash {
+
+// Assert that iterator Prev() or Next() does not crash on an empty history.
+TEST(AssistantQueryHistory, Empty) {
+  AssistantQueryHistory history(10);
+  auto it = history.GetIterator();
+  EXPECT_EQ(base::nullopt, it->Next());
+  EXPECT_EQ(base::nullopt, it->Prev());
+  EXPECT_EQ(base::nullopt, it->Next());
+  EXPECT_EQ(base::nullopt, it->Next());
+  EXPECT_EQ(base::nullopt, it->Next());
+  EXPECT_EQ(base::nullopt, it->Prev());
+}
+
+TEST(AssistantQueryHistory, Full) {
+  int size = 3;
+  AssistantQueryHistory history(size);
+  // Make more queries than history limit.
+  for (int i = 0; i <= size; i++)
+    history.Add(std::to_string(i));
+  auto it = history.GetIterator();
+  // Assert history only contains last 3 queries.
+  for (int i = size; i > 0; i--)
+    EXPECT_EQ(std::to_string(i), it->Prev().value());
+  EXPECT_EQ(std::to_string(1), it->Prev().value());
+  // Assert that iterate does not pass first query.
+  EXPECT_EQ(std::to_string(1), it->Prev().value());
+
+  // Make more queries than history limit again.
+  for (int i = 0; i <= size; i++)
+    history.Add(std::to_string(i + 4));
+  it->ResetToLast();
+  // Assert that history only contains last 3 queries.
+  for (int i = size; i > 0; i--)
+    EXPECT_EQ(std::to_string(i + 4), it->Prev());
+  EXPECT_EQ(std::to_string(5), it->Prev().value());
+  // Assert that iterate does not pass first query.
+  EXPECT_EQ(std::to_string(5), it->Prev().value());
+}
+
+TEST(AssistantQueryHistory, Add) {
+  AssistantQueryHistory history(10);
+  auto it = history.GetIterator();
+  EXPECT_EQ(base::nullopt, it->Next());
+  EXPECT_EQ(base::nullopt, it->Next());
+  EXPECT_EQ(base::nullopt, it->Next());
+  EXPECT_EQ(base::nullopt, it->Next());
+  history.Add("Query01");
+  history.Add("Query02");
+  it = history.GetIterator();
+  EXPECT_EQ(base::nullopt, it->Next());
+  EXPECT_EQ("Query02", it->Prev().value());
+  EXPECT_EQ("Query01", it->Prev().value());
+  EXPECT_EQ("Query01", it->Prev().value());
+  EXPECT_EQ("Query02", it->Next().value());
+  EXPECT_EQ(base::nullopt, it->Next());
+  EXPECT_EQ(base::nullopt, it->Next());
+}
+
+}  // namespace ash

ash/assistant/ui/dialog_plate/dialog_plate.cc

@@ -69,7 +69,11 @@ DialogPlate::DialogPlate(AssistantController* assistant_controller)
               base::Unretained(this)),
           /*end_animation_callback=*/base::BindRepeating(
               &DialogPlate::OnAnimationEnded,
-              base::Unretained(this)))) {
+              base::Unretained(this)))),
+      query_history_iterator_(assistant_controller_->interaction_controller()
+                                  ->model()
+                                  ->query_history()
+                                  .GetIterator()) {
   InitLayout();
 
   // The Assistant controller indirectly owns the view hierarchy to which
@@ -109,30 +113,43 @@ void DialogPlate::ButtonPressed(views::Button* sender, const ui::Event& event) {
 
 bool DialogPlate::HandleKeyEvent(views::Textfield* textfield,
                                  const ui::KeyEvent& key_event) {
-  if (key_event.key_code() != ui::KeyboardCode::VKEY_RETURN)
-    return false;
-
   if (key_event.type() != ui::EventType::ET_KEY_PRESSED)
     return false;
 
-  // In tablet mode the virtual keyboard should not be sticky, so we hide it
-  // when committing a query.
-  if (IsTabletMode())
-    textfield_->GetFocusManager()->ClearFocus();
+  switch (key_event.key_code()) {
+    case ui::KeyboardCode::VKEY_RETURN: {
+      // In tablet mode the virtual keyboard should not be sticky, so we hide it
+      // when committing a query.
+      if (IsTabletMode())
+        textfield_->GetFocusManager()->ClearFocus();
+
+      const base::StringPiece16& trimmed_text = base::TrimWhitespace(
+          textfield_->text(), base::TrimPositions::TRIM_ALL);
+
+      // 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));
+      }
 
-  const base::StringPiece16& trimmed_text =
-      base::TrimWhitespace(textfield_->text(), base::TrimPositions::TRIM_ALL);
+      textfield_->SetText(base::string16());
 
-  // 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));
+      return true;
+    }
+    case ui::KeyboardCode::VKEY_UP:
+    case ui::KeyboardCode::VKEY_DOWN: {
+      DCHECK(query_history_iterator_);
+      auto opt_query = key_event.key_code() == ui::KeyboardCode::VKEY_UP
+                           ? query_history_iterator_->Prev()
+                           : query_history_iterator_->Next();
+      textfield_->SetText(base::UTF8ToUTF16(opt_query.value_or("")));
+      return true;
+    }
+    default:
+      return false;
   }
-
-  textfield_->SetText(base::string16());
-
-  return true;
 }
 
 void DialogPlate::OnInputModalityChanged(InputModality input_modality) {
@@ -219,6 +236,12 @@ void DialogPlate::OnInputModalityChanged(InputModality input_modality) {
   }
 }
 
+void DialogPlate::OnCommittedQueryChanged(
+    const AssistantQuery& committed_query) {
+  DCHECK(query_history_iterator_);
+  query_history_iterator_->ResetToLast();
+}
+
 void DialogPlate::OnUiVisibilityChanged(AssistantVisibility new_visibility,
                                         AssistantVisibility old_visibility,
                                         AssistantSource source) {

ash/assistant/ui/dialog_plate/dialog_plate.h

@@ -9,6 +9,7 @@
 #include <string>
 
 #include "ash/assistant/model/assistant_interaction_model_observer.h"
+#include "ash/assistant/model/assistant_query_history.h"
 #include "ash/assistant/model/assistant_ui_model_observer.h"
 #include "ash/assistant/ui/dialog_plate/action_view.h"
 #include "base/macros.h"
@@ -87,6 +88,7 @@ class DialogPlate : public views::View,
 
   // AssistantInteractionModelObserver:
   void OnInputModalityChanged(InputModality input_modality) override;
+  void OnCommittedQueryChanged(const AssistantQuery& committed_query) override;
 
   // AssistantUiModelObserver:
   void OnUiVisibilityChanged(AssistantVisibility new_visibility,
@@ -120,6 +122,7 @@ class DialogPlate : public views::View,
   views::Textfield* textfield_;                      // Owned by view hierarchy.
 
   std::unique_ptr<ui::CallbackLayerAnimationObserver> animation_observer_;
+  std::unique_ptr<AssistantQueryHistory::Iterator> query_history_iterator_;
 
   base::ObserverList<DialogPlateObserver>::Unchecked observers_;