Revert "Move LayoutMenuList::DidUpdateActiveOption() to HTMLSelectElement"

This reverts commit 3c4c13ac.

Reason for revert: Made ChromeVoxBackgroundTest.SelectSingleBasic flaky

Original change's description:
> Move LayoutMenuList::DidUpdateActiveOption() to HTMLSelectElement
> 
> Also,
>  - Remove LayoutMenuList::DidSelectOption().
>  - Add HTMLFormControlElement::UpdateFromElement() to add
>   DidUpdateMenuListActiveOption() to each of
>   LayoutMenuList::UpdateFromElement() calls.
> 
> This CL is a preparation to build a LayoutNG counterpart of LayoutMenuList.
> This CL has no behavior changes.
> 
> Bug: 1040828
> Change-Id: If7917af101119a830e899a50711a6c1491e22eb3
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2003164
> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
> Commit-Queue: Kent Tamura <tkent@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#732313}

TBR=yosin@chromium.org,tkent@chromium.org,kojii@chromium.org

Change-Id: I4f571b72f01f0494fc855c1ceb3841e7ff02a963
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1040828
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2006268Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732571}

third_party/blink/renderer/core/html/forms/html_form_control_element.cc

@@ -205,15 +205,13 @@ const AtomicString& HTMLFormControlElement::autocapitalize() const {
 void HTMLFormControlElement::AttachLayoutTree(AttachContext& context) {
   HTMLElement::AttachLayoutTree(context);
 
-  // The call to UpdateFromElement() needs to go after the call through
-  // to the base class's attachLayoutTree() because that can sometimes do a
-  // close on the LayoutObject.
-  UpdateFromElement();
-}
+  if (!GetLayoutObject())
+    return;
 
-void HTMLFormControlElement::UpdateFromElement() {
-  if (auto* layout_object = GetLayoutObject())
-    layout_object->UpdateFromElement();
+  // The call to updateFromElement() needs to go after the call through
+  // to the base class's attachLayoutTree() because that can sometimes do a
+  // close on the layoutObject.
+  GetLayoutObject()->UpdateFromElement();
 }
 
 void HTMLFormControlElement::DidMoveToNewDocument(Document& old_document) {

third_party/blink/renderer/core/html/forms/html_form_control_element.h

@@ -150,10 +150,6 @@ class CORE_EXPORT HTMLFormControlElement : public HTMLElement,
 
   virtual void ResetImpl() {}
 
-  // This is called just after attaching a LayoutObject. However,
-  // GetLayoutObject() can be nullptr.
-  virtual void UpdateFromElement();
-
  private:
   bool IsFormControlElement() const final { return true; }
   bool AlwaysCreateUserAgentShadowRoot() const override { return true; }

third_party/blink/renderer/core/html/forms/html_select_element.cc

@@ -357,7 +357,7 @@ void HTMLSelectElement::OptionElementChildrenChanged(
 
   if (GetLayoutObject()) {
     if (option.Selected() && UsesMenuList())
-      UpdateFromElement();
+      GetLayoutObject()->UpdateFromElement();
     if (AXObjectCache* cache =
             GetLayoutObject()->GetDocument().ExistingAXObjectCache())
       cache->ChildrenChanged(this);
@@ -903,8 +903,8 @@ void HTMLSelectElement::SetSuggestedOption(HTMLOptionElement* option) {
     return;
   suggested_option_ = option;
 
-  if (GetLayoutObject()) {
-    UpdateFromElement();
+  if (LayoutObject* layout_object = GetLayoutObject()) {
+    layout_object->UpdateFromElement();
     ScrollToOption(option);
   }
   if (PopupIsVisible())
@@ -1078,7 +1078,8 @@ void HTMLSelectElement::SelectOption(HTMLOptionElement* element,
   }
 
   // For the menu list case, this is what makes the selected element appear.
-  UpdateFromElement();
+  if (LayoutObject* layout_object = GetLayoutObject())
+    layout_object->UpdateFromElement();
   // PopupMenu::UpdateFromElement() posts an O(N) task.
   if (PopupIsVisible() && should_update_popup)
     popup_->UpdateFromElement(PopupMenu::kBySelectionChange);
@@ -1095,9 +1096,8 @@ void HTMLSelectElement::SelectOption(HTMLOptionElement* element,
       // Need to check UsesMenuList() again because event handlers might
       // change the status.
       if (UsesMenuList()) {
-        // DidUpdateMenuListActiveOption() is O(N) because of
-        // HTMLOptionElement::index().
-        DidUpdateMenuListActiveOption(element);
+        // DidSelectOption() is O(N) because of HTMLOptionElement::index().
+        ToLayoutMenuList(layout_object)->DidSelectOption(element);
       }
     }
   }
@@ -1510,27 +1510,6 @@ void HTMLSelectElement::UpdateSelectedState(HTMLOptionElement* clicked_option,
   UpdateListBoxSelection(!multi_select);
 }
 
-void HTMLSelectElement::DidUpdateMenuListActiveOption(
-    HTMLOptionElement* option) {
-  if (!GetDocument().ExistingAXObjectCache())
-    return;
-
-  int option_index = option ? option->index() : -1;
-  if (ax_menulist_last_active_index_ == option_index)
-    return;
-  ax_menulist_last_active_index_ = option_index;
-
-  // We skip sending accessiblity notifications for the very first option,
-  // otherwise we get extra focus and select events that are undesired.
-  if (!has_updated_menulist_active_option_) {
-    has_updated_menulist_active_option_ = true;
-    return;
-  }
-
-  GetDocument().ExistingAXObjectCache()->HandleUpdateActiveMenuOption(
-      ToLayoutMenuList(GetLayoutObject()), option_index);
-}
-
 HTMLOptionElement* HTMLSelectElement::EventTargetOption(const Event& event) {
   return DynamicTo<HTMLOptionElement>(event.target()->ToNode());
 }
@@ -2023,7 +2002,8 @@ void HTMLSelectElement::PopupDidHide() {
 
 void HTMLSelectElement::SetIndexToSelectOnCancel(int list_index) {
   index_to_select_on_cancel_ = list_index;
-  UpdateFromElement();
+  if (GetLayoutObject())
+    GetLayoutObject()->UpdateFromElement();
 }
 
 HTMLOptionElement* HTMLSelectElement::OptionToBeShown() const {
@@ -2223,20 +2203,6 @@ void HTMLSelectElement::ChangeRendering() {
   DetachLayoutTree();
   SetNeedsStyleRecalc(kLocalStyleChange, StyleChangeReasonForTracing::Create(
                                              style_change_reason::kControl));
-
-  if (UsesMenuList()) {
-    ax_menulist_last_active_index_ = -1;
-    has_updated_menulist_active_option_ = false;
-  }
-}
-
-void HTMLSelectElement::UpdateFromElement() {
-  auto* layout_object = GetLayoutObject();
-  if (!layout_object)
-    return;
-  layout_object->UpdateFromElement();
-  if (UsesMenuList())
-    DidUpdateMenuListActiveOption(OptionToBeShown());
 }
 
 }  // namespace blink

third_party/blink/renderer/core/html/forms/html_select_element.h

@@ -179,7 +179,6 @@ class CORE_EXPORT HTMLSelectElement final
 
  private:
   const AtomicString& FormControlType() const override;
-  void UpdateFromElement() override;
 
   bool MayTriggerVirtualKeyboard() const override;
 
@@ -243,7 +242,6 @@ class CORE_EXPORT HTMLSelectElement final
   void ParseMultipleAttribute(const AtomicString&);
   HTMLOptionElement* LastSelectedOption() const;
   void UpdateSelectedState(HTMLOptionElement*, bool multi, bool shift);
-  void DidUpdateMenuListActiveOption(HTMLOptionElement*);
   void MenuListDefaultEventHandler(Event&);
   void HandlePopupOpenKeyboardEvent(Event&);
   bool ShouldOpenPopupForKeyDownEvent(const KeyboardEvent&);
@@ -305,8 +303,6 @@ class CORE_EXPORT HTMLSelectElement final
   Member<HTMLOptionElement> active_selection_end_;
   Member<HTMLOptionElement> option_to_scroll_to_;
   Member<HTMLOptionElement> suggested_option_;
-  int ax_menulist_last_active_index_ = -1;
-  bool has_updated_menulist_active_option_ = false;
   bool is_multiple_;
   bool is_in_non_contiguous_selection_;
   bool active_selection_state_;

third_party/blink/renderer/core/layout/layout_menu_list.cc

@@ -46,8 +46,10 @@ LayoutMenuList::LayoutMenuList(Element* element)
       button_text_(nullptr),
       inner_block_(nullptr),
       is_empty_(false),
+      has_updated_active_option_(false),
       inner_block_height_(LayoutUnit()),
-      options_width_(0) {
+      options_width_(0),
+      last_active_index_(-1) {
   DCHECK(IsA<HTMLSelectElement>(element));
 }
 
@@ -264,6 +266,8 @@ void LayoutMenuList::UpdateFromElement() {
 
   SetText(text.StripWhiteSpace());
 
+  DidUpdateActiveOption(option);
+
   DCHECK(inner_block_);
   if (HasOptionStyleChanged(inner_block_->StyleRef()))
     UpdateInnerStyle();
@@ -335,6 +339,30 @@ void LayoutMenuList::ComputeLogicalHeight(
   LayoutBox::ComputeLogicalHeight(logical_height, logical_top, computed_values);
 }
 
+void LayoutMenuList::DidSelectOption(HTMLOptionElement* option) {
+  DidUpdateActiveOption(option);
+}
+
+void LayoutMenuList::DidUpdateActiveOption(HTMLOptionElement* option) {
+  if (!GetDocument().ExistingAXObjectCache())
+    return;
+
+  int option_index = option ? option->index() : -1;
+  if (last_active_index_ == option_index)
+    return;
+  last_active_index_ = option_index;
+
+  // We skip sending accessiblity notifications for the very first option,
+  // otherwise we get extra focus and select events that are undesired.
+  if (!has_updated_active_option_) {
+    has_updated_active_option_ = true;
+    return;
+  }
+
+  GetDocument().ExistingAXObjectCache()->HandleUpdateActiveMenuOption(
+      this, option_index);
+}
+
 LayoutUnit LayoutMenuList::ClientPaddingLeft() const {
   return PaddingLeft() + inner_block_->PaddingLeft();
 }

third_party/blink/renderer/core/layout/layout_menu_list.h

@@ -31,6 +31,7 @@
 
 namespace blink {
 
+class HTMLOptionElement;
 class HTMLSelectElement;
 class LayoutText;
 
@@ -40,6 +41,7 @@ class CORE_EXPORT LayoutMenuList final : public LayoutFlexibleBox {
   ~LayoutMenuList() override;
 
   HTMLSelectElement* SelectElement() const;
+  void DidSelectOption(HTMLOptionElement*);
   String GetText() const;
 
   const char* GetName() const override { return "LayoutMenuList"; }
@@ -101,15 +103,20 @@ class CORE_EXPORT LayoutMenuList final : public LayoutFlexibleBox {
   void UpdateOptionsWidth() const;
   void SetIndexToSelectOnCancel(int list_index);
 
+  void DidUpdateActiveOption(HTMLOptionElement*);
+
   LayoutText* button_text_;
   LayoutBlock* inner_block_;
 
   bool is_empty_ : 1;
+  bool has_updated_active_option_ : 1;
   LayoutUnit inner_block_height_;
   // m_optionsWidth is calculated and cached on demand.
   // updateOptionsWidth() should be called before reading them.
   mutable int options_width_;
 
+  int last_active_index_;
+
   scoped_refptr<const ComputedStyle> option_style_;
 };