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

This is a reland of 3c4c13ac

The difference from the original CL:
 - Replace LayoutObject::UpdateFromElement() call with
   HTMLFormControlElement::UpdateFromElement() in
   HTMLFormControlElement::DidRecalcStyle() too.

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}

Bug: 1040828
Change-Id: Ibba19df1c4f9a402c057bbf4548b18f1fdeb44f4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2006271Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732771}

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

@@ -205,13 +205,15 @@ const AtomicString& HTMLFormControlElement::autocapitalize() const {
 void HTMLFormControlElement::AttachLayoutTree(AttachContext& context) {
   HTMLElement::AttachLayoutTree(context);
 
-  if (!GetLayoutObject())
-    return;
-
-  // The call to updateFromElement() needs to go after the call through
+  // 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();
+  // close on the LayoutObject.
+  UpdateFromElement();
+}
+
+void HTMLFormControlElement::UpdateFromElement() {
+  if (auto* layout_object = GetLayoutObject())
+    layout_object->UpdateFromElement();
 }
 
 void HTMLFormControlElement::DidMoveToNewDocument(Document& old_document) {
@@ -270,10 +272,8 @@ String HTMLFormControlElement::ResultForDialogSubmit() {
 }
 
 void HTMLFormControlElement::DidRecalcStyle(const StyleRecalcChange change) {
-  if (change.ReattachLayoutTree())
-    return;
-  if (LayoutObject* layout_object = GetLayoutObject())
-    layout_object->UpdateFromElement();
+  if (!change.ReattachLayoutTree())
+    UpdateFromElement();
 }
 
 bool HTMLFormControlElement::SupportsFocus() const {

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

@@ -150,6 +150,10 @@ 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())
-      GetLayoutObject()->UpdateFromElement();
+      UpdateFromElement();
     if (AXObjectCache* cache =
             GetLayoutObject()->GetDocument().ExistingAXObjectCache())
       cache->ChildrenChanged(this);
@@ -903,8 +903,8 @@ void HTMLSelectElement::SetSuggestedOption(HTMLOptionElement* option) {
     return;
   suggested_option_ = option;
 
-  if (LayoutObject* layout_object = GetLayoutObject()) {
-    layout_object->UpdateFromElement();
+  if (GetLayoutObject()) {
+    UpdateFromElement();
     ScrollToOption(option);
   }
   if (PopupIsVisible())
@@ -1078,8 +1078,7 @@ void HTMLSelectElement::SelectOption(HTMLOptionElement* element,
   }
 
   // For the menu list case, this is what makes the selected element appear.
-  if (LayoutObject* layout_object = GetLayoutObject())
-    layout_object->UpdateFromElement();
+  UpdateFromElement();
   // PopupMenu::UpdateFromElement() posts an O(N) task.
   if (PopupIsVisible() && should_update_popup)
     popup_->UpdateFromElement(PopupMenu::kBySelectionChange);
@@ -1096,8 +1095,9 @@ void HTMLSelectElement::SelectOption(HTMLOptionElement* element,
       // Need to check UsesMenuList() again because event handlers might
       // change the status.
       if (UsesMenuList()) {
-        // DidSelectOption() is O(N) because of HTMLOptionElement::index().
-        ToLayoutMenuList(layout_object)->DidSelectOption(element);
+        // DidUpdateMenuListActiveOption() is O(N) because of
+        // HTMLOptionElement::index().
+        DidUpdateMenuListActiveOption(element);
       }
     }
   }
@@ -1510,6 +1510,27 @@ 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());
 }
@@ -2002,8 +2023,7 @@ void HTMLSelectElement::PopupDidHide() {
 
 void HTMLSelectElement::SetIndexToSelectOnCancel(int list_index) {
   index_to_select_on_cancel_ = list_index;
-  if (GetLayoutObject())
-    GetLayoutObject()->UpdateFromElement();
+  UpdateFromElement();
 }
 
 HTMLOptionElement* HTMLSelectElement::OptionToBeShown() const {
@@ -2203,6 +2223,20 @@ 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,6 +179,7 @@ class CORE_EXPORT HTMLSelectElement final
 
  private:
   const AtomicString& FormControlType() const override;
+  void UpdateFromElement() override;
 
   bool MayTriggerVirtualKeyboard() const override;
 
@@ -242,6 +243,7 @@ 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&);
@@ -303,6 +305,8 @@ 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,10 +46,8 @@ 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),
-      last_active_index_(-1) {
+      options_width_(0) {
   DCHECK(IsA<HTMLSelectElement>(element));
 }
 
@@ -266,8 +264,6 @@ void LayoutMenuList::UpdateFromElement() {
 
   SetText(text.StripWhiteSpace());
 
-  DidUpdateActiveOption(option);
-
   DCHECK(inner_block_);
   if (HasOptionStyleChanged(inner_block_->StyleRef()))
     UpdateInnerStyle();
@@ -339,30 +335,6 @@ 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,7 +31,6 @@
 
 namespace blink {
 
-class HTMLOptionElement;
 class HTMLSelectElement;
 class LayoutText;
 
@@ -41,7 +40,6 @@ 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"; }
@@ -103,20 +101,15 @@ 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_;
 };