Uses child views in Autofill Popup so we can trigger

chrome/browser/ui/views/autofill/autofill_popup_view_views.cc

Index: chrome/browser/ui/views/autofill/autofill_popup_view_views.cc
diff --git a/chrome/browser/ui/views/autofill/autofill_popup_view_views.cc b/chrome/browser/ui/views/autofill/autofill_popup_view_views.cc
index 89925cee23efa839e82e4105a6ff2f6b6bc68806..29d374d85052fba318288f2f520c4d06b1677906 100644
--- a/chrome/browser/ui/views/autofill/autofill_popup_view_views.cc
+++ b/chrome/browser/ui/views/autofill/autofill_popup_view_views.cc
@@ -6,8 +6,11 @@
 
 #include "chrome/browser/ui/autofill/autofill_popup_controller.h"
 #include "chrome/browser/ui/autofill/autofill_popup_layout_model.h"
+#include "chrome/grit/generated_resources.h"
 #include "components/autofill/core/browser/popup_item_ids.h"
 #include "components/autofill/core/browser/suggestion.h"
+#include "ui/accessibility/ax_node_data.h"
+#include "ui/base/l10n/l10n_util.h"
 #include "ui/events/keycodes/keyboard_codes.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/geometry/point.h"
@@ -16,30 +19,87 @@
 #include "ui/gfx/native_widget_types.h"
 #include "ui/gfx/text_utils.h"
 #include "ui/views/border.h"
+#include "ui/views/view.h"
 #include "ui/views/widget/widget.h"
 
 namespace autofill {
 
+namespace {
+
+// Child view only for triggering accessibility events. Rendering is handled
+// by |AutofillPopupViewViews|.
+class AutofillPopupChildView : public views::View {
+ public:
+  // Internal class name.
+  static const char kViewClassName[];
+
+  // |controller| should not be NULL.
+  AutofillPopupChildView(AutofillPopupController* controller, size_t index)
+      : controller_(controller), index_(index) {
+    SetFocusBehavior(FocusBehavior::ALWAYS);
+  }
+
+ private:
+  ~AutofillPopupChildView() override {}
+
+  // views::Views implementation
+  const char* GetClassName() const override { return kViewClassName; }
+
+  void GetAccessibleNodeData(ui::AXNodeData* node_data) override {
+    // We do not special case |POPUP_ITEM_ID_SEPARATOR| suggestion because we
+    // skip that suggestion when the user navigates through the popup
+    // suggestions. See | AutofillPopupControllerImpl::CanAccept|.

[Evan Stade, 2017/03/09 03:21:15]

nit: I'm not sure this comment is necessary, simply because it would be unwieldy
to add a comment to the code for every thing we /don't/ do.

[csashi, 2017/03/09 05:16:33]

Done.

+    node_data->role = ui::AX_ROLE_MENU_ITEM;
+    node_data->SetName(controller_->GetElidedValueAt(index_));

[Evan Stade, 2017/03/09 03:21:16]

it seems like we shouldn't be using the elided version but the full value

[csashi, 2017/03/09 05:16:33]

Done.

+  }
+
+  AutofillPopupController* controller_;  // Weak reference.
+  const size_t index_;
+
+  DISALLOW_COPY_AND_ASSIGN(AutofillPopupChildView);
+};
+
+// static
+const char AutofillPopupChildView::kViewClassName[] = "AutofillPopupChildView";
+
+}  // namespace
+
 AutofillPopupViewViews::AutofillPopupViewViews(
     AutofillPopupController* controller,
     views::Widget* parent_widget)
     : AutofillPopupBaseView(controller, parent_widget),
-      controller_(controller) {}
+      controller_(controller) {
+  CreateChildViews();
+  SetFocusBehavior(FocusBehavior::ALWAYS);
+}
 
 AutofillPopupViewViews::~AutofillPopupViewViews() {}
 
 void AutofillPopupViewViews::Show() {
   DoShow();
+  NotifyAccessibilityEvent(ui::AX_EVENT_MENU_START, true);
 }
 
 void AutofillPopupViewViews::Hide() {
   // The controller is no longer valid after it hides us.
   controller_ = NULL;
-
   DoHide();
+  NotifyAccessibilityEvent(ui::AX_EVENT_MENU_END, true);
 }
 
-void AutofillPopupViewViews::UpdateBoundsAndRedrawPopup() {
+void AutofillPopupViewViews::OnSuggestionsChanged() {
+  // We recreate the child views so we can be sure the |controller_|'s
+  // |GetLineCount()| will match the number of child views. Otherwise,
+  // the number of suggestions i.e. |GetLineCount()| may not match 1x1 with the
+  // child views. See crbug.com/697466.
+  // Currently, child views get the data from 'controller_'. If the child views
+  // save additional state corresponding to the suggestion, we must recreate the
+  // child views even when |GetLineCount()| == |child_count()| because the
+  // suggestion content may change.
+  if (controller_->GetLineCount() != static_cast<size_t>(child_count())) {
+    RemoveAllChildViews(true /* delete_children */);
+    CreateChildViews();
+  }
   DoUpdateBoundsAndRedrawPopup();
 }
 
@@ -51,6 +111,7 @@ void AutofillPopupViewViews::OnPaint(gfx::Canvas* canvas) {
       ui::NativeTheme::kColorId_ResultsTableNormalBackground));
   OnPaintBorder(canvas);
 
+  CHECK_EQ(controller_->GetLineCount(), static_cast<size_t>(child_count()));

[Evan Stade, 2017/03/09 03:21:16]

nit: s/check/dcheck

[csashi, 2017/03/09 05:16:33]

Done.

[csashi, 2017/03/09 05:16:33]

Done.

   for (size_t i = 0; i < controller_->GetLineCount(); ++i) {
     gfx::Rect line_rect = controller_->layout_model().GetRowBounds(i);
 
@@ -66,8 +127,17 @@ void AutofillPopupViewViews::OnPaint(gfx::Canvas* canvas) {
   }
 }
 
-void AutofillPopupViewViews::InvalidateRow(size_t row) {
+void AutofillPopupViewViews::InvalidateRow(size_t row, bool is_selected) {

[Evan Stade, 2017/03/09 03:21:16]

I think this is reasonable, but you should change the name of the function to
reflect what it's actually doing, and you can probably just call it once instead
of twice.

SelectedRowChanged(size_t previous_row_selection, size_t current_row_selection)
{
   ...
}

[csashi, 2017/03/09 05:16:33]

Done.

   SchedulePaintInRect(controller_->layout_model().GetRowBounds(row));
+  if (is_selected) {
+    CHECK_LT(row, static_cast<size_t>(child_count()));
+    views::View* child = child_at(row);
+    CHECK(child);
+    CHECK_EQ(child->GetClassName(), AutofillPopupChildView::kViewClassName);
+    AutofillPopupChildView* child_view =
+        static_cast<AutofillPopupChildView*>(child);
+    child_view->NotifyAccessibilityEvent(ui::AX_EVENT_SELECTION, true);
+  }
 }
 
 /**
@@ -189,6 +259,19 @@ void AutofillPopupViewViews::DrawAutofillEntry(gfx::Canvas* canvas,
   }
 }
 
+void AutofillPopupViewViews::GetAccessibleNodeData(ui::AXNodeData* node_data) {
+  node_data->role = ui::AX_ROLE_MENU;
+  node_data->SetName(
+      l10n_util::GetStringUTF16(IDS_AUTOFILL_POPUP_ACCESSIBLE_NODE_DATA));
+}
+
+void AutofillPopupViewViews::CreateChildViews() {
+  for (size_t i = 0; i < controller_->GetLineCount(); ++i) {
+    AutofillPopupChildView* child = new AutofillPopupChildView(controller_, i);
+    AddChildView(child);
+  }
+}
+
 AutofillPopupView* AutofillPopupView::Create(
     AutofillPopupController* controller) {
   views::Widget* observing_widget =