[Read Anything] Refactor trees from the app controller to the app model.

Bug: 1266555
Change-Id: Ia133bdf27713ef4cb574b67bfd821113f1be92a8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4289271
Commit-Queue: Lauren Winston <lwinston@google.com>
Reviewed-by: Mark Schillaci <mschillaci@google.com>
Cr-Commit-Position: refs/heads/main@{#1112419}

chrome/renderer/accessibility/read_anything_app_controller.cc

@@ -311,12 +311,12 @@ void ReadAnythingAppController::AccessibilityEventReceived(
     const std::vector<ui::AXEvent>& events) {
   DCHECK_NE(tree_id, ui::AXTreeIDUnknown());
   // Create a new tree if an event is received for a tree that is not yet in
-  // |trees_|.
-  if (!base::Contains(trees_, tree_id)) {
+  // the tree list.
+  if (!model_.ContainsTree(tree_id)) {
     std::unique_ptr<ui::AXSerializableTree> new_tree =
         std::make_unique<ui::AXSerializableTree>();
     new_tree->AddObserver(this);
-    trees_[tree_id] = std::move(new_tree);
+    model_.AddTree(tree_id, std::move(new_tree));
   }
   // If a tree update on the active tree is received while distillation is in
   // progress, cache updates that are received but do not yet unserialize them.
@@ -343,9 +343,7 @@ void ReadAnythingAppController::UnserializeUpdates(
   if (updates.empty()) {
     return;
   }
-  DCHECK_NE(tree_id, ui::AXTreeIDUnknown());
-  DCHECK(base::Contains(trees_, tree_id));
-  ui::AXSerializableTree* tree = trees_[tree_id].get();
+  ui::AXSerializableTree* tree = model_.GetTreeFromId(tree_id).get();
   DCHECK(tree);
   // Try to merge updates. If the updates are mergeable, MergeAXTreeUpdates will
   // return true and merge_updates_out will contain the updates. Otherwise, if
@@ -384,10 +382,10 @@ void ReadAnythingAppController::OnActiveAXTreeIDChanged(
 #endif
 
   // When the UI first constructs, this function may be called before tree_id
-  // has been added to trees_ in AccessibilityEventReceived. In that case, do
-  // not distill.
+  // has been added to the tree list in AccessibilityEventReceived. In that
+  // case, do not distill.
   if (model_.active_tree_id() != ui::AXTreeIDUnknown() &&
-      base::Contains(trees_, model_.active_tree_id())) {
+      model_.ContainsTree(model_.active_tree_id())) {
     Distill();
   }
   OnAXTreeDestroyed(previous_active_tree_id);
@@ -406,15 +404,16 @@ void ReadAnythingAppController::OnAXTreeDestroyed(const ui::AXTreeID& tree_id) {
   // an accessibility tree. This means that it would never call
   // AccessibilityEventsReceived(), meaning its RFH's AXTreeID would not be in
   // trees. When that tab was destroyed, this function will be called with a
-  // tree_id not in trees_, so we return early.
-  if (!base::Contains(trees_, tree_id)) {
+  // tree_id not in the tree list, so we return early.
+  if (!model_.ContainsTree(tree_id)) {
     return;
   }
-  auto child_tree_ids = trees_[tree_id]->GetAllChildTreeIds();
+  std::set<ui::AXTreeID> child_tree_ids =
+      model_.GetTreeFromId(tree_id)->GetAllChildTreeIds();
   for (const auto& child_tree_id : child_tree_ids) {
     OnAXTreeDestroyed(child_tree_id);
   }
-  trees_.erase(tree_id);
+  model_.EraseTree(tree_id);
 }
 
 void ReadAnythingAppController::OnAtomicUpdateFinished(
@@ -455,9 +454,8 @@ void ReadAnythingAppController::OnAtomicUpdateFinished(
 }
 
 void ReadAnythingAppController::Distill() {
-  DCHECK_NE(model_.active_tree_id(), ui::AXTreeIDUnknown());
-  DCHECK(base::Contains(trees_, model_.active_tree_id()));
-  ui::AXSerializableTree* tree = trees_[model_.active_tree_id()].get();
+  ui::AXSerializableTree* tree =
+      model_.GetTreeFromId(model_.active_tree_id()).get();
   std::unique_ptr<ui::AXTreeSource<const ui::AXNode*>> tree_source(
       tree->CreateTreeSource());
   ui::AXTreeSerializer<const ui::AXNode*> serializer(tree_source.get());
@@ -481,16 +479,15 @@ void ReadAnythingAppController::OnAXTreeDistilled(
   // 2. model_.active_tree_id() == ui::AXTreeIDUnknown(): The active tree was
   // change to
   //    an unknown tree id.
-  // 3. !base::Contains(trees_, tree_id): The distilled tree was destroyed.
+  // 3. !model_.ContainsTree(tree_id): The distilled tree was destroyed.
   // 4. tree_id == ui::AXTreeIDUnknown(): The distiller sent back an unknown
   //    tree id which occurs when there was an error.
   if (tree_id != model_.active_tree_id() ||
       model_.active_tree_id() == ui::AXTreeIDUnknown() ||
-      !base::Contains(trees_, tree_id) || tree_id == ui::AXTreeIDUnknown()) {
+      !model_.ContainsTree(tree_id) || tree_id == ui::AXTreeIDUnknown()) {
     return;
   }
-  model_.ResetSelection(
-      trees_[model_.active_tree_id()]->GetUnignoredSelection());
+  model_.ResetSelection();
   if (!content_node_ids_.empty()) {
     // If there are content_node_ids, this means the AXTree was successfully
     // distilled. Post-process in preparation to display the distilled content.
@@ -527,7 +524,7 @@ void ReadAnythingAppController::Draw() {
 void ReadAnythingAppController::PostProcessAXTreeWithSelection() {
   DCHECK(model_.has_selection());
   DCHECK_NE(model_.active_tree_id(), ui::AXTreeIDUnknown());
-  DCHECK(base::Contains(trees_, model_.active_tree_id()));
+  DCHECK(model_.ContainsTree(model_.active_tree_id()));
 
   // TODO(crbug.com/1266555): Refactor selection updates into the model once
   //  trees have been moved to the model.
@@ -680,9 +677,8 @@ gin::ObjectTemplateBuilder ReadAnythingAppController::GetObjectTemplateBuilder(
 }
 
 ui::AXNodeID ReadAnythingAppController::RootId() const {
-  DCHECK_NE(model_.active_tree_id(), ui::AXTreeIDUnknown());
-  DCHECK(base::Contains(trees_, model_.active_tree_id()));
-  ui::AXSerializableTree* tree = trees_.at(model_.active_tree_id()).get();
+  ui::AXSerializableTree* tree =
+      model_.GetTreeFromId(model_.active_tree_id()).get();
   return tree->root()->id();
 }
 
@@ -916,9 +912,8 @@ void ReadAnythingAppController::SetPageHandlerForTesting(
 // moved into the model.
 ui::AXNode* ReadAnythingAppController::GetAXNode(
     ui::AXNodeID ax_node_id) const {
-  DCHECK_NE(model_.active_tree_id(), ui::AXTreeIDUnknown());
-  DCHECK(base::Contains(trees_, model_.active_tree_id()));
-  ui::AXSerializableTree* tree = trees_.at(model_.active_tree_id()).get();
+  ui::AXSerializableTree* tree =
+      model_.GetTreeFromId(model_.active_tree_id()).get();
   return tree->GetFromId(ax_node_id);
 }
 

chrome/renderer/accessibility/read_anything_app_controller.h

@@ -192,9 +192,6 @@ class ReadAnythingAppController
 
   // State:
 
-  // AXTrees of web contents in the browser’s tab strip.
-  std::map<ui::AXTreeID, std::unique_ptr<ui::AXSerializableTree>> trees_;
-
   // A queue of pending updates on the active AXTree, which will be
   // unserialized once distillation completes.
   std::vector<ui::AXTreeUpdate> pending_updates_;

chrome/renderer/accessibility/read_anything_app_controller_browsertest.cc

@@ -12,6 +12,7 @@
 #include "content/public/renderer/render_frame.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "ui/accessibility/ax_node.h"
+#include "ui/accessibility/ax_serializable_tree.h"
 
 class MockAXTreeDistiller : public AXTreeDistiller {
  public:
@@ -211,10 +212,19 @@ class ReadAnythingAppControllerTest : public ChromeRenderViewTest {
     return controller_->IsNodeIgnoredForReadAnything(ax_node_id);
   }
 
-  size_t GetNumTrees() { return controller_->trees_.size(); }
+  size_t GetNumTrees() { return controller_->model_.NumTreesForTesting(); }
 
   bool HasTree(ui::AXTreeID tree_id) {
-    return base::Contains(controller_->trees_, tree_id);
+    return controller_->model_.ContainsTree(tree_id);
+  }
+
+  void EraseTree(ui::AXTreeID tree_id) {
+    controller_->model_.EraseTree(tree_id);
+  }
+
+  void AddTree(ui::AXTreeID tree_id,
+               std::unique_ptr<ui::AXSerializableTree> tree) {
+    controller_->model_.AddTree(tree_id, std::move(tree));
   }
 
   size_t GetNumPendingUpdates() { return controller_->pending_updates_.size(); }
@@ -671,6 +681,41 @@ TEST_F(ReadAnythingAppControllerTest,
   ASSERT_EQ(0u, GetNumTrees());
 }
 
+TEST_F(ReadAnythingAppControllerTest, ModelUpdatesTreeState) {
+  // Set up trees.
+  ui::AXTreeID tree_id_2 = ui::AXTreeID::CreateNewAXTreeID();
+  ui::AXTreeID tree_id_3 = ui::AXTreeID::CreateNewAXTreeID();
+
+  AddTree(tree_id_2, std::make_unique<ui::AXSerializableTree>());
+  AddTree(tree_id_3, std::make_unique<ui::AXSerializableTree>());
+
+  ASSERT_EQ(3u, GetNumTrees());
+  ASSERT_TRUE(HasTree(tree_id_2));
+  ASSERT_TRUE(HasTree(tree_id_3));
+  ASSERT_TRUE(HasTree(tree_id_));
+
+  // Remove one tree.
+  EraseTree(tree_id_2);
+  ASSERT_EQ(2u, GetNumTrees());
+  ASSERT_TRUE(HasTree(tree_id_3));
+  ASSERT_FALSE(HasTree(tree_id_2));
+  ASSERT_TRUE(HasTree(tree_id_));
+
+  // Remove the second tree.
+  EraseTree(tree_id_);
+  ASSERT_EQ(1u, GetNumTrees());
+  ASSERT_TRUE(HasTree(tree_id_3));
+  ASSERT_FALSE(HasTree(tree_id_2));
+  ASSERT_FALSE(HasTree(tree_id_));
+
+  // Remove the last tree.
+  EraseTree(tree_id_3);
+  ASSERT_EQ(0u, GetNumTrees());
+  ASSERT_FALSE(HasTree(tree_id_3));
+  ASSERT_FALSE(HasTree(tree_id_2));
+  ASSERT_FALSE(HasTree(tree_id_));
+}
+
 TEST_F(ReadAnythingAppControllerTest, DoesNotCrashIfActiveAXTreeIDUnknown) {
   EXPECT_CALL(*distiller_, Distill).Times(0);
   ui::AXTreeID tree_id = ui::AXTreeIDUnknown();

chrome/renderer/accessibility/read_anything_app_model.cc

@@ -4,6 +4,10 @@
 
 #include "chrome/renderer/accessibility/read_anything_app_model.h"
 
+#include "base/containers/contains.h"
+
+#include "ui/accessibility/ax_serializable_tree.h"
+
 ReadAnythingAppModel::ReadAnythingAppModel() = default;
 ReadAnythingAppModel::~ReadAnythingAppModel() = default;
 
@@ -26,7 +30,9 @@ void ReadAnythingAppModel::Reset() {
   distillation_in_progress_ = false;
 }
 
-void ReadAnythingAppModel::ResetSelection(ui::AXSelection selection) {
+void ReadAnythingAppModel::ResetSelection() {
+  ui::AXSelection selection =
+      GetTreeFromId(active_tree_id_)->GetUnignoredSelection();
   has_selection_ = selection.anchor_object_id != ui::kInvalidAXNodeID &&
                    selection.focus_object_id != ui::kInvalidAXNodeID;
 
@@ -53,6 +59,32 @@ void ReadAnythingAppModel::SetEnd(ui::AXNodeID end_node_id,
   end_offset_ = end_offset;
 }
 
+const std::unique_ptr<ui::AXSerializableTree>&
+ReadAnythingAppModel::GetTreeFromId(ui::AXTreeID tree_id) const {
+  DCHECK_NE(tree_id, ui::AXTreeIDUnknown());
+  DCHECK(ContainsTree(tree_id));
+  return trees_.at(tree_id);
+}
+
+bool ReadAnythingAppModel::ContainsTree(ui::AXTreeID tree_id) const {
+  return base::Contains(trees_, tree_id);
+}
+
+void ReadAnythingAppModel::AddTree(
+    ui::AXTreeID tree_id,
+    std::unique_ptr<ui::AXSerializableTree> tree) {
+  DCHECK(!ContainsTree(tree_id));
+  trees_[tree_id] = std::move(tree);
+}
+
+void ReadAnythingAppModel::EraseTree(ui::AXTreeID tree_id) {
+  trees_.erase(tree_id);
+}
+
+size_t ReadAnythingAppModel::NumTreesForTesting() const {
+  return trees_.size();
+}
+
 double ReadAnythingAppModel::GetLetterSpacingValue(
     read_anything::mojom::LetterSpacing letter_spacing) const {
   switch (letter_spacing) {

chrome/renderer/accessibility/read_anything_app_model.h

@@ -12,8 +12,10 @@
 
 namespace ui {
 class AXNode;
+class AXSerializableTree;
 }  // namespace ui
 
+class ReadAnythingAppControllerTest;
 // A class that holds state for the ReadAnythingAppController for the Read
 // Anything WebUI app.
 class ReadAnythingAppModel {
@@ -67,13 +69,28 @@ class ReadAnythingAppModel {
   void OnThemeChanged(read_anything::mojom::ReadAnythingThemePtr new_theme);
 
   void Reset();
-  void ResetSelection(ui::AXSelection selection);
+  void ResetSelection();
+
+  const std::unique_ptr<ui::AXSerializableTree>& GetTreeFromId(
+      ui::AXTreeID tree_id) const;
+  void AddTree(ui::AXTreeID tree_id,
+               std::unique_ptr<ui::AXSerializableTree> tree);
+
+  bool ContainsTree(ui::AXTreeID tree_id) const;
+
+  void EraseTree(ui::AXTreeID tree_id);
 
  private:
+  friend ReadAnythingAppControllerTest;
   double GetLetterSpacingValue(
       read_anything::mojom::LetterSpacing letter_spacing) const;
   double GetLineSpacingValue(
       read_anything::mojom::LineSpacing line_spacing) const;
+  size_t NumTreesForTesting() const;
+
+  // State.
+  // AXTrees of web contents in the browser’s tab strip.
+  std::map<ui::AXTreeID, std::unique_ptr<ui::AXSerializableTree>> trees_;
 
   // The AXTreeID of the currently active web contents.
   ui::AXTreeID active_tree_id_ = ui::AXTreeIDUnknown();