[Mullet m2] Open a sub-popup functionality.

This is far from its final look (the styling will be addressed in
subsequent CLs), but already visualizable:
https://screenshot.googleplex.com/5k2eiUVrgEm2XiP


Bug: 1466116
Low-Coverage-Reason: NOTIMPLEMENTED() impl is not tested
Change-Id: If6107d1df6bfdd96d5867d34411df776e1b6fd09
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4737983
Reviewed-by: Christoph Schwering <schwering@google.com>
Commit-Queue: Dmitry Vykochko <vykochko@google.com>
Reviewed-by: Bruno Braga <brunobraga@google.com>
Reviewed-by: Jan Keitel <jkeitel@google.com>
Cr-Commit-Position: refs/heads/main@{#1192148}

chrome/browser/autofill/mock_autofill_popup_controller.h

@@ -98,7 +98,7 @@ class MockAutofillPopupController
               (const gfx::RectF& anchor_bounds,
                std::vector<Suggestion> suggestions),
               (override));
-
+  MOCK_METHOD(void, HideSubPopup, (), (override));
   void set_suggestions(const std::vector<PopupItemId>& ids) {
     suggestions_.clear();
 

chrome/browser/ui/autofill/autofill_keyboard_accessory_adapter.cc

@@ -200,6 +200,10 @@ AutofillKeyboardAccessoryAdapter::OpenSubPopup(
   return nullptr;
 }
 
+void AutofillKeyboardAccessoryAdapter::HideSubPopup() {
+  NOTIMPLEMENTED() << "No sub-popups on Keyboard Accessory";
+}
+
 bool AutofillKeyboardAccessoryAdapter::GetRemovalConfirmationText(
     int index,
     std::u16string* title,

chrome/browser/ui/autofill/autofill_keyboard_accessory_adapter.h

@@ -112,7 +112,7 @@ class AutofillKeyboardAccessoryAdapter : public AutofillPopupView,
   base::WeakPtr<AutofillPopupController> OpenSubPopup(
       const gfx::RectF& anchor_bounds,
       std::vector<Suggestion> suggestions) override;
-
+  void HideSubPopup() override;
   void Hide(PopupHidingReason reason) override;
   void ViewDestroyed() override;
   gfx::NativeView container_view() const override;

chrome/browser/ui/autofill/autofill_popup_controller.h

@@ -98,6 +98,9 @@ class AutofillPopupController : public AutofillPopupViewDelegate {
       const gfx::RectF& anchor_bounds,
       std::vector<Suggestion> suggestions) = 0;
 
+  // Hides open by `OpenSubPopup()` popup, noop if there is no open sub-popup.
+  virtual void HideSubPopup() = 0;
+
  protected:
   ~AutofillPopupController() override = default;
 };

chrome/browser/ui/autofill/autofill_popup_controller_impl.cc

@@ -125,7 +125,7 @@ AutofillPopupControllerImpl::AutofillPopupControllerImpl(
       delegate_(delegate),
       show_pwd_migration_warning_callback_(
           std::move(show_pwd_migration_warning_callback)),
-      parent_(parent) {
+      parent_controller_(parent) {
   ClearState();
   delegate->RegisterDeletionCallback(base::BindOnce(
       &AutofillPopupControllerImpl::HideViewAndDie, GetWeakPtr()));
@@ -182,9 +182,10 @@ void AutofillPopupControllerImpl::Show(
   if (view_) {
     OnSuggestionsChanged();
   } else {
-    bool has_parent = parent_ && parent_->get();
-    view_ = has_parent ? parent_->get()->CreateSubPopupView(GetWeakPtr())
-                       : AutofillPopupView::Create(GetWeakPtr());
+    bool has_parent = parent_controller_ && parent_controller_->get();
+    view_ = has_parent
+                ? parent_controller_->get()->CreateSubPopupView(GetWeakPtr())
+                : AutofillPopupView::Create(GetWeakPtr());
 
     // It is possible to fail to create the popup, in this case
     // treat the popup as hiding right away.
@@ -479,9 +480,17 @@ AutofillPopupControllerImpl::OpenSubPopup(const gfx::RectF& anchor_bounds,
 
   // Show() can fail and cause controller deletion. Therefore store the weak
   // pointer before, so that this method returns null when that happens.
-  base::WeakPtr<AutofillPopupController> result = controller->GetWeakPtr();
+  sub_popup_controller_ = controller->GetWeakPtr();
   controller->Show(std::move(suggestions), trigger_source_);
-  return result;
+  return sub_popup_controller_;
+}
+
+void AutofillPopupControllerImpl::HideSubPopup() {
+  if (sub_popup_controller_) {
+    sub_popup_controller_->Hide(
+        PopupHidingReason::kExpandedSuggestionCollapsedSubPopup);
+    sub_popup_controller_ = nullptr;
+  }
 }
 
 void AutofillPopupControllerImpl::OnEnterPictureInPicture() {
@@ -615,6 +624,8 @@ void AutofillPopupControllerImpl::ClearState() {
 }
 
 void AutofillPopupControllerImpl::HideViewAndDie() {
+  HideSubPopup();
+
   // Invalidates in particular ChromeAutofillClient's WeakPtr to |this|, which
   // prevents recursive calls triggered by `view_->Hide()`
   // (crbug.com/1267047).

chrome/browser/ui/autofill/autofill_popup_controller_impl.h

@@ -125,6 +125,7 @@ class AutofillPopupControllerImpl
   base::WeakPtr<AutofillPopupController> OpenSubPopup(
       const gfx::RectF& anchor_bounds,
       std::vector<Suggestion> suggestions) override;
+  void HideSubPopup() override;
 
   // Disables show thresholds. See the documentation of the member for details.
   void DisableThresholdForTesting(bool disable_threshold) {
@@ -322,7 +323,10 @@ class AutofillPopupControllerImpl
   // Parent's popup controller. The root popup doesn't have a parent, but in
   // sub-popups it must be present.
   const absl::optional<base::WeakPtr<ExpandablePopupParentControllerImpl>>
-      parent_;
+      parent_controller_;
+
+  // The open sub-popup controller if any, `nullptr` otherwise.
+  base::WeakPtr<AutofillPopupController> sub_popup_controller_;
 
   // AutofillPopupControllerImpl deletes itself. To simplify memory management,
   // we delete the object asynchronously.

chrome/browser/ui/views/autofill/popup/popup_cell_utils.cc

@@ -104,13 +104,6 @@ std::u16string GetIconAccessibleName(const std::string& icon_text) {
   return std::u16string();
 }
 
-std::unique_ptr<views::ImageView> ImageViewFromVectorIcon(
-    const gfx::VectorIcon& vector_icon,
-    int icon_size = kIconSize) {
-  return std::make_unique<views::ImageView>(
-      ui::ImageModel::FromVectorIcon(vector_icon, ui::kColorIcon, icon_size));
-}
-
 std::unique_ptr<views::ImageView> ImageViewFromImageSkia(
     const gfx::ImageSkia& image_skia) {
   if (image_skia.isNull()) {
@@ -560,4 +553,11 @@ void AddSuggestionStrategyContentCellChildren(
   AddCallbacksToContentView(controller, line_number, *view);
 }
 
+std::unique_ptr<views::ImageView> ImageViewFromVectorIcon(
+    const gfx::VectorIcon& vector_icon,
+    int icon_size = kIconSize) {
+  return std::make_unique<views::ImageView>(
+      ui::ImageModel::FromVectorIcon(vector_icon, ui::kColorIcon, icon_size));
+}
+
 }  // namespace autofill::popup_cell_utils

chrome/browser/ui/views/autofill/popup/popup_cell_view.cc

@@ -67,8 +67,12 @@ void PopupCellView::SetSelected(bool selected) {
 }
 
 void PopupCellView::SetPermanentlyHighlighted(bool permanently_highlighted) {
-  permanently_highlighted_ = permanently_highlighted;
-  RefreshStyle();
+  if (permanently_highlighted_ != permanently_highlighted) {
+    permanently_highlighted_ = permanently_highlighted;
+    RefreshStyle();
+    NotifyAccessibilityEvent(ax::mojom::Event::kCheckedStateChanged,
+                             /*send_native_event=*/true);
+  }
 }
 
 bool PopupCellView::IsHighlighted() const {
@@ -217,7 +221,8 @@ bool PopupCellView::HandleAccessibleAction(
 
 void PopupCellView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
   if (a11y_delegate_) {
-    a11y_delegate_->GetAccessibleNodeData(GetSelected(), node_data);
+    a11y_delegate_->GetAccessibleNodeData(GetSelected(),
+                                          permanently_highlighted_, node_data);
   }
 }
 

chrome/browser/ui/views/autofill/popup/popup_cell_view.h

@@ -44,8 +44,9 @@ class PopupCellView : public views::View {
     virtual ~AccessibilityDelegate() = default;
 
     // Sets the a11y information in `node_data` based on whether the cell in
-    // question `is_selected` or not.
+    // question `is_selected` or not, or `is_permanently_highlighted` or not.
     virtual void GetAccessibleNodeData(bool is_selected,
+                                       bool is_permanently_highlighted,
                                        ui::AXNodeData* node_data) const = 0;
   };
 

chrome/browser/ui/views/autofill/popup/popup_row_strategy.cc

@@ -4,6 +4,8 @@
 
 #include "chrome/browser/ui/views/autofill/popup/popup_row_strategy.h"
 
+#include <memory>
+
 #include "base/feature_list.h"
 #include "chrome/app/vector_icons/vector_icons.h"
 #include "chrome/browser/ui/autofill/autofill_popup_controller.h"
@@ -12,10 +14,13 @@
 #include "chrome/browser/ui/views/autofill/popup/popup_cell_utils.h"
 #include "chrome/browser/ui/views/autofill/popup/popup_view_utils.h"
 #include "components/autofill/core/browser/metrics/autofill_metrics.h"
+#include "components/autofill/core/browser/ui/suggestion.h"
 #include "components/autofill/core/common/autofill_features.h"
 #include "components/strings/grit/components_strings.h"
+#include "components/vector_icons/vector_icons.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/views/controls/image_view.h"
+#include "ui/views/controls/label.h"
 #include "ui/views/controls/menu/menu_config.h"
 #include "ui/views/controls/throbber.h"
 #include "ui/views/layout/box_layout_view.h"
@@ -42,6 +47,9 @@ constexpr PopupItemId kItemTypesUsingLeadingIcons[] = {
     PopupItemId::kPasswordAccountStorageReSignin,
     PopupItemId::kPasswordAccountStorageOptInAndGenerate};
 
+constexpr int kExpandableControlCellInsetPadding = 16;
+constexpr int kExpandableControlCellIconSize = 8;
+
 // ********************* AccessibilityDelegate implementations *****************
 
 // ********************* ContentItemAccessibilityDelegate  *********************
@@ -56,6 +64,7 @@ class ContentItemAccessibilityDelegate
   ~ContentItemAccessibilityDelegate() override = default;
 
   void GetAccessibleNodeData(bool is_selected,
+                             bool is_permanently_highlighted,
                              ui::AXNodeData* node_data) const override;
 
  private:
@@ -91,6 +100,7 @@ ContentItemAccessibilityDelegate::ContentItemAccessibilityDelegate(
 
 void ContentItemAccessibilityDelegate::GetAccessibleNodeData(
     bool is_selected,
+    bool is_permanently_highlighted,
     ui::AXNodeData* node_data) const {
   DCHECK(node_data);
   // Options are selectable.
@@ -102,6 +112,33 @@ void ContentItemAccessibilityDelegate::GetAccessibleNodeData(
   node_data->AddIntAttribute(ax::mojom::IntAttribute::kSetSize, set_size_);
 }
 
+// ************** ExpandableControlCellAccessibilityDelegate  ******************
+class ExpandableControlCellAccessibilityDelegate
+    : public PopupCellView::AccessibilityDelegate {
+ public:
+  ExpandableControlCellAccessibilityDelegate() = default;
+  ~ExpandableControlCellAccessibilityDelegate() override = default;
+
+  void GetAccessibleNodeData(bool is_selected,
+                             bool is_permanently_highlighted,
+                             ui::AXNodeData* node_data) const override;
+};
+
+// Sets the checked state according to `is_permanently_highlighted`,
+// `is_selected` is ignored as the first one is more important and updating
+// two states within hundreds of milliseconds can be confusing.
+void ExpandableControlCellAccessibilityDelegate::GetAccessibleNodeData(
+    bool is_selected,
+    bool is_permanently_highlighted,
+    ui::AXNodeData* node_data) const {
+  node_data->role = ax::mojom::Role::kToggleButton;
+  node_data->SetNameChecked(l10n_util::GetStringUTF16(
+      IDS_AUTOFILL_EXPANDABLE_SUGGESTION_CONTROLL_A11Y_NAME));
+  node_data->SetCheckedState(is_permanently_highlighted
+                                 ? ax::mojom::CheckedState::kTrue
+                                 : ax::mojom::CheckedState::kFalse);
+}
+
 }  // namespace
 
 /**************************** PopupRowBaseStrategy ****************************/
@@ -190,10 +227,27 @@ PopupSuggestionStrategy::CreateAutocompleteRow() {
   return view;
 }
 
-// This method is currently not implemented but we will re-evaluate it (probably
-// implement it) when granular filling starts its implementation phase.
 std::unique_ptr<PopupCellView> PopupSuggestionStrategy::CreateControl() {
-  return nullptr;
+  const Suggestion& kSuggestion =
+      GetController()->GetSuggestionAt(GetLineNumber());
+  if (kSuggestion.children.empty()) {
+    return nullptr;
+  }
+
+  std::unique_ptr<PopupCellView> view =
+      views::Builder<PopupCellView>(
+          std::make_unique<PopupCellView>(
+              GetController()
+                  ->ShouldIgnoreMouseObservedOutsideItemBoundsCheck()))
+          .SetAccessibilityDelegate(
+              std::make_unique<ExpandableControlCellAccessibilityDelegate>())
+          .Build();
+  view->SetLayoutManager(std::make_unique<views::BoxLayout>(
+      views::BoxLayout::Orientation::kHorizontal,
+      gfx::Insets(kExpandableControlCellInsetPadding)));
+  view->AddChildView(popup_cell_utils::ImageViewFromVectorIcon(
+      vector_icons::kSubmenuArrowIcon, kExpandableControlCellIconSize));
+  return view;
 }
 
 /************************ PopupPasswordSuggestionStrategy *******************/

chrome/browser/ui/views/autofill/popup/popup_row_view.cc

@@ -143,6 +143,12 @@ void PopupRowView::SetCellPermanentlyHighlighted(CellType type,
   }
 }
 
+gfx::RectF PopupRowView::GetCellBounds(CellType cell) const {
+  const PopupCellView* view = GetCellView(cell);
+  // The view is expected to be present.
+  return gfx::RectF(view->GetBoundsInScreen());
+}
+
 bool PopupRowView::HandleKeyPressEvent(
     const content::NativeWebKeyboardEvent& event) {
   // Some cells may want to define their own behavior.
@@ -193,7 +199,7 @@ void PopupRowView::SelectPreviousCell() {
   }
 }
 
-PopupCellView* PopupRowView::GetCellView(CellType type) {
+const PopupCellView* PopupRowView::GetCellView(CellType type) const {
   switch (type) {
     case CellType::kContent:
       return content_view_.get();
@@ -202,6 +208,10 @@ PopupCellView* PopupRowView::GetCellView(CellType type) {
   }
 }
 
+PopupCellView* PopupRowView::GetCellView(CellType type) {
+  return const_cast<PopupCellView*>(std::as_const(*this).GetCellView(type));
+}
+
 BEGIN_METADATA(PopupRowView, views::View)
 ADD_PROPERTY_METADATA(absl::optional<PopupRowView::CellType>, SelectedCell)
 END_METADATA

chrome/browser/ui/views/autofill/popup/popup_row_view.h

@@ -14,6 +14,7 @@
 #include "content/public/common/input/native_web_keyboard_event.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
 #include "ui/base/metadata/metadata_header_macros.h"
+#include "ui/gfx/geometry/rect.h"
 #include "ui/views/view.h"
 
 namespace content {
@@ -85,6 +86,9 @@ class PopupRowView : public views::View {
   // Sets the highlighted state on the cell of specified type.
   void SetCellPermanentlyHighlighted(CellType cell, bool highlighted);
 
+  // Returns the cell's bounds, the cell of the requested type must be present.
+  gfx::RectF GetCellBounds(CellType cell) const;
+
   // Attempts to process a key press `event`. Returns true if it did (and the
   // parent no longer needs to handle it).
   bool HandleKeyPressEvent(const content::NativeWebKeyboardEvent& event);
@@ -101,6 +105,7 @@ class PopupRowView : public views::View {
   void SelectPreviousCell();
 
   // Returns the cell view or `nullptr` if it was not created.
+  const PopupCellView* GetCellView(CellType type) const;
   PopupCellView* GetCellView(CellType type);
 
   AccessibilitySelectionDelegate& GetA11ySelectionDelegate() {

chrome/browser/ui/views/autofill/popup/popup_view_views.cc

@@ -208,13 +208,15 @@ void PopupViewViews::SetSelectedCell(absl::optional<CellIndex> cell_index) {
     new_row.SetSelectedCell(cell_index->second);
     new_row.ScrollViewToVisible();
 
-    if (cell_index->second == PopupRowView::CellType::kControl &&
-        !controller_->GetSuggestionAt(cell_index->first).children.empty()) {
-      open_sub_popup_timer_.Start(
-          FROM_HERE, kOpenSubPopupDelay,
-          base::BindOnce(&PopupViewViews::SetCellWithOpenSubPopup,
-                         weak_ptr_factory_.GetWeakPtr(), cell_index));
-    }
+    bool can_open_sub_popup =
+        cell_index->second == PopupRowView::CellType::kControl &&
+        !controller_->GetSuggestionAt(cell_index->first).children.empty();
+    absl::optional<CellIndex> open_sub_popup_cell =
+        can_open_sub_popup ? cell_index : absl::nullopt;
+    open_sub_popup_timer_.Start(
+        FROM_HERE, kOpenSubPopupDelay,
+        base::BindOnce(&PopupViewViews::SetCellWithOpenSubPopup,
+                       weak_ptr_factory_.GetWeakPtr(), open_sub_popup_cell));
 
   } else {
     row_with_selected_cell_ = absl::nullopt;
@@ -703,6 +705,7 @@ void PopupViewViews::SetCellWithOpenSubPopup(
 
   // Close previously open sub-popup if any.
   if (open_sub_popup_cell_ && HasPopupRowViewAt(open_sub_popup_cell_->first)) {
+    controller_->HideSubPopup();
     GetPopupRowViewAt(open_sub_popup_cell_->first)
         .SetCellPermanentlyHighlighted(open_sub_popup_cell_->second, false);
     open_sub_popup_cell_ = absl::nullopt;
@@ -710,9 +713,19 @@ void PopupViewViews::SetCellWithOpenSubPopup(
 
   // Open a sub-popup on the new cell if provided.
   if (cell_index && HasPopupRowViewAt(cell_index->first)) {
-    GetPopupRowViewAt(cell_index->first)
-        .SetCellPermanentlyHighlighted(cell_index->second, true);
-    open_sub_popup_cell_ = cell_index;
+    const Suggestion& suggestion =
+        controller_->GetSuggestionAt(cell_index->first);
+
+    CHECK(!suggestion.children.empty());
+    CHECK(cell_index->second == PopupRowView::CellType::kControl);
+
+    PopupRowView& row = GetPopupRowViewAt(cell_index->first);
+
+    if (controller_->OpenSubPopup(row.GetCellBounds(cell_index->second),
+                                  suggestion.children)) {
+      row.SetCellPermanentlyHighlighted(cell_index->second, true);
+      open_sub_popup_cell_ = cell_index;
+    }
   }
 }
 

chrome/browser/ui/views/autofill/popup/popup_view_views_unittest.cc

@@ -113,6 +113,9 @@ class PopupViewViewsTest : public ChromeViewsTestBase {
     web_contents_->Resize({0, 0, 1024, 768});
     ON_CALL(autofill_popup_controller_, GetWebContents())
         .WillByDefault(Return(web_contents_.get()));
+    ON_CALL(autofill_popup_controller_, OpenSubPopup)
+        .WillByDefault(Return(autofill_popup_sub_controller_.GetWeakPtr()));
+
     widget_ = CreateTestWidget();
     generator_ = std::make_unique<ui::test::EventGenerator>(
         GetRootWindow(widget_.get()));
@@ -223,6 +226,7 @@ class PopupViewViewsTest : public ChromeViewsTestBase {
   std::unique_ptr<ui::test::EventGenerator> generator_;
   std::unique_ptr<PopupViewViews> view_;
   NiceMock<MockAutofillPopupController> autofill_popup_controller_;
+  NiceMock<MockAutofillPopupController> autofill_popup_sub_controller_;
 };
 
 class PopupViewViewsTestWithAnyPopupItemId

chrome/browser/ui/views/autofill/popup/test_popup_row_strategy.cc

@@ -16,6 +16,7 @@ namespace autofill {
 
 void TestAccessibilityDelegate::GetAccessibleNodeData(
     bool is_selected,
+    bool is_permanently_highlighted,
     ui::AXNodeData* node_data) const {
   node_data->role = ax::mojom::Role::kListBoxOption;
   node_data->SetNameChecked(kVoiceOverName);

chrome/browser/ui/views/autofill/popup/test_popup_row_strategy.h

@@ -21,6 +21,7 @@ class TestAccessibilityDelegate : public PopupCellView::AccessibilityDelegate {
   ~TestAccessibilityDelegate() override = default;
 
   void GetAccessibleNodeData(bool is_selected,
+                             bool is_permanently_highlighted,
                              ui::AXNodeData* node_data) const override;
 };
 

components/autofill/core/browser/ui/popup_types.h

@@ -80,7 +80,10 @@ enum class PopupHidingReason {
   // No frame currently has focus. This case is caught for safety because it
   // might be reachable due to race conditions.
   kNoFrameHasFocus = 23,
-  kMaxValue = kNoFrameHasFocus
+  // Sub-popup related reason, used when closing a sub-popup (e.g. by moving
+  // the mouse out of the suggestion control or by the keyboard navigation).
+  kExpandedSuggestionCollapsedSubPopup = 24,
+  kMaxValue = kExpandedSuggestionCollapsedSubPopup
 };
 
 }  // namespace autofill

components/autofill_strings.grdp

@@ -578,5 +578,8 @@
     <message name="IDS_AUTOFILL_EDIT_ADDRESS_DIALOG_CANCEL_BUTTON_LABEL" desc="Label of the Cancel button in the dialog that edits an address before saving it.">
       Cancel
     </message>
+    <message name="IDS_AUTOFILL_EXPANDABLE_SUGGESTION_CONTROLL_A11Y_NAME" desc="Autofill desktop popup: the a11y name for the expandable suggestion controll." translateable="false">
+      Expand suggestion
+    </message>
   </if>
 </grit-part>