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..0e2c076c783919ce206c75636ff7b76a41c9d7be 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,27 +19,75 @@
 #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|.
+    node_data->role = ui::AX_ROLE_MENU_ITEM;
+    node_data->SetName(controller_->GetElidedValueAt(index_));
+  }
+
+  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) {
+  for (size_t i = 0; i < controller_->GetLineCount(); ++i) {
+    AutofillPopupChildView* child = new AutofillPopupChildView(controller, i);

[Evan Stade, 2017/03/03 04:41:29]

You can repro this crash by filling in a form, then clicking in the form again
(popup shows again) then pressing space. AutofillPopupControllerImpl::Show() is
called with new suggestions. It reuses the previous AutofillPopupView.

Here in the ctor you are assuming that the suggestions won't change over the
lifetime of the AutofillPopupView. Unfortunately this is an incorrect
assumption. The newly modified suggestions don't match the view hierarchy you've
created here.

[csashi, 2017/03/03 04:48:19]

Thanks! This was Mathieu's hypothesis as well but I did not know how to force
this scenario. Will try this and fix accordingly.

+    AddChildView(child);
+  }
+  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() {
@@ -51,6 +102,7 @@ void AutofillPopupViewViews::OnPaint(gfx::Canvas* canvas) {
       ui::NativeTheme::kColorId_ResultsTableNormalBackground));
   OnPaintBorder(canvas);
 
+  CHECK_EQ(controller_->GetLineCount(), (size_t)child_count());
   for (size_t i = 0; i < controller_->GetLineCount(); ++i) {
     gfx::Rect line_rect = controller_->layout_model().GetRowBounds(i);
 
@@ -70,6 +122,18 @@ void AutofillPopupViewViews::InvalidateRow(size_t row) {
   SchedulePaintInRect(controller_->layout_model().GetRowBounds(row));
 }
 
+void AutofillPopupViewViews::NotifyAccessibilityEventForRow(
+    ui::AXEvent event_type,
+    size_t row) {
+  CHECK_LT(row, (size_t)child_count());

[sky, 2017/03/03 03:35:46]

Use C++ style casts, .e.g static_cast<size_t>(child_count());

[csashi, 2017/03/03 04:39:42]

Done.

+  views::View* child = child_at(row);
+  CHECK(child);
+  CHECK_EQ(child->GetClassName(), AutofillPopupChildView::kViewClassName);
+  AutofillPopupChildView* child_view =
+      static_cast<AutofillPopupChildView*>(child);
+  child_view->NotifyAccessibilityEvent(event_type, true);
+}
+
 /**
 * Autofill entries in ltr.
 *
@@ -189,6 +253,12 @@ 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));
+}
+
 AutofillPopupView* AutofillPopupView::Create(
     AutofillPopupController* controller) {
   views::Widget* observing_widget =