Cherry-pick 274586@main (35318b4). https://bugs.webkit.org/show_bug.c…

…gi?id=269322

    Crash under ~RenderMenuList due to CheckedPtr usage
    https://bugs.webkit.org/show_bug.cgi?id=269322
    rdar://119790256

    Reviewed by Alan Baradlay.

    From the crash trace, we can see that HTMLSelectElement::defaultEventHandler()
    holds a CheckedPtr to its RenderMenuList renderer and calls showPopup() on
    the renderer. This ends up running JS, which removes the select element from
    the DOM and in turns destroys the renderer. The usage is currently safe since
    nothing is using the renderer after the JS has run. However, it was tripping
    the CheckedPtr assertion.

    To address the issue, switch to using WeakPtr for now and add comments to
    clarify lifetime. We should consider refactoring this in a follow up though.

    * Source/WebCore/html/HTMLSelectElement.cpp:
    (WebCore::HTMLSelectElement::platformHandleKeydownEvent):
    (WebCore::HTMLSelectElement::menuListDefaultEventHandler):
    (WebCore::HTMLSelectElement::showPicker):
    * Source/WebCore/rendering/RenderMenuList.cpp:
    (RenderMenuList::showPopup):

    Canonical link: https://commits.webkit.org/274586@main

Source/WebCore/html/HTMLSelectElement.cpp

@@ -1145,7 +1145,7 @@ bool HTMLSelectElement::platformHandleKeydownEvent(KeyboardEvent* event)
             // Calling focus() may cause us to lose our renderer. Return true so
             // that our caller doesn't process the event further, but don't set
             // the event as handled.
-            CheckedPtr renderer = dynamicDowncast<RenderMenuList>(this->renderer());
+            WeakPtr renderer = dynamicDowncast<RenderMenuList>(this->renderer());
             if (!renderer)
                 return true;
 
@@ -1154,7 +1154,7 @@ bool HTMLSelectElement::platformHandleKeydownEvent(KeyboardEvent* event)
             // gets called from RenderMenuList::valueChanged, which gets called
             // after the user makes a selection from the menu.
             saveLastSelection();
-            renderer->showPopup();
+            renderer->showPopup(); // showPopup() may run JS and cause the renderer to get destroyed.
             event->setDefaultHandled();
         }
         return true;
@@ -1244,7 +1244,7 @@ void HTMLSelectElement::menuListDefaultEventHandler(Event& event)
                 protectedDocument()->updateStyleIfNeeded();
 
                 // Calling focus() may remove the renderer or change the renderer type.
-                CheckedPtr renderer = dynamicDowncast<RenderMenuList>(this->renderer());
+                WeakPtr renderer = dynamicDowncast<RenderMenuList>(this->renderer());
                 if (!renderer)
                     return;
 
@@ -1253,7 +1253,7 @@ void HTMLSelectElement::menuListDefaultEventHandler(Event& event)
                 // gets called from RenderMenuList::valueChanged, which gets called
                 // after the user makes a selection from the menu.
                 saveLastSelection();
-                renderer->showPopup();
+                renderer->showPopup(); // showPopup() may run JS and cause the renderer to get destroyed.
                 handled = true;
             }
         } else if (RenderTheme::singleton().popsMenuByArrowKeys()) {
@@ -1262,7 +1262,7 @@ void HTMLSelectElement::menuListDefaultEventHandler(Event& event)
                 protectedDocument()->updateStyleIfNeeded();
 
                 // Calling focus() may remove the renderer or change the renderer type.
-                CheckedPtr renderer = dynamicDowncast<RenderMenuList>(this->renderer());
+                WeakPtr renderer = dynamicDowncast<RenderMenuList>(this->renderer());
                 if (!renderer)
                     return;
 
@@ -1271,7 +1271,7 @@ void HTMLSelectElement::menuListDefaultEventHandler(Event& event)
                 // gets called from RenderMenuList::valueChanged, which gets called
                 // after the user makes a selection from the menu.
                 saveLastSelection();
-                renderer->showPopup();
+                renderer->showPopup(); // showPopup() may run JS and cause the renderer to get destroyed.
                 handled = true;
             } else if (keyCode == '\r') {
                 if (RefPtr form = this->form())
@@ -1290,15 +1290,15 @@ void HTMLSelectElement::menuListDefaultEventHandler(Event& event)
 #if !PLATFORM(IOS_FAMILY)
         protectedDocument()->updateStyleIfNeeded();
 
-        if (CheckedPtr menuList = dynamicDowncast<RenderMenuList>(renderer())) {
+        if (WeakPtr menuList = dynamicDowncast<RenderMenuList>(renderer())) {
             ASSERT(!menuList->popupIsVisible());
             // Save the selection so it can be compared to the new
             // selection when we call onChange during selectOption,
             // which gets called from RenderMenuList::valueChanged,
             // which gets called after the user makes a selection from
             // the menu.
             saveLastSelection();
-            menuList->showPopup();
+            menuList->showPopup(); // showPopup() may run JS and cause the renderer to get destroyed.
         }
 #endif
         event.setDefaultHandled();
@@ -1696,8 +1696,8 @@ ExceptionOr<void> HTMLSelectElement::showPicker()
         return Exception { ExceptionCode::NotAllowedError, "Select showPicker() requires a user gesture."_s };
 
 #if !PLATFORM(IOS_FAMILY)
-    if (CheckedPtr renderMenuList = dynamicDowncast<RenderMenuList>(renderer()))
-        renderMenuList->showPopup();
+    if (WeakPtr renderMenuList = dynamicDowncast<RenderMenuList>(renderer()))
+        renderMenuList->showPopup(); // showPopup() may run JS and cause the renderer to get destroyed.
 #endif
 
     return { };

Source/WebCore/rendering/RenderMenuList.cpp

@@ -374,7 +374,7 @@ void RenderMenuList::showPopup()
     FloatPoint absTopLeft = localToAbsolute(FloatPoint(), UseTransforms);
     IntRect absBounds = absoluteBoundingBoxRectIgnoringTransforms();
     absBounds.setLocation(roundedIntPoint(absTopLeft));
-    m_popup->show(absBounds, &view().frameView(), selectElement().optionToListIndex(selectElement().selectedIndex()));
+    m_popup->show(absBounds, &view().frameView(), selectElement().optionToListIndex(selectElement().selectedIndex())); // May destroy `this`.
 }
 #endif