REGRESSION(257219@main): [GTK] Cursor blinks oddly in text fields, ev…

…en if blinking disabled

https://bugs.webkit.org/show_bug.cgi?id=252687

Reviewed by Carlos Garcia Campos.

Before 257219@main if RenderTheme::caretBlinkInterval() returned zero
then the timer controlling the blink animation was never started. When
introducing CaretAnimator + SimpleCaretAnimator the check was not
carried over, resulting in the animator trying to paint the caret
on-and-off as fast as possible when blinking should have been disabled.

The fix reintroduces the check, reusing setBlinkingSuspended() to enable
(or disable) blinking depending on RenderTheme::caretBlinkInterval(),
which in turn is changed to return std::optional<Seconds>, with
std::nullopt signalling that blinking is disables. The improved typing
ensures that the code explicitly handles the case of blinking being
disabled.

While at it, remove the RenderThemeGtk.{h,cpp} files, because they only
contained an implementation of caretBlinkInterval() that was exactly the
same code guarded with PLATFORM(GTK) in RenderThemeAdwaita.

* Source/WebCore/SourcesGTK.txt: Remove RenderThemeGtk.cpp from the list.
* Source/WebCore/platform/CaretAnimator.h:
(WebCore::CaretAnimator::didStart): Adapt to use std::optional<Seconds>.
* Source/WebCore/platform/SimpleCaretAnimator.cpp:
(WebCore::SimpleCaretAnimator::updateAnimationProperties): Arrange to
enable or disable caret blinking, adapting to use std::optional<Seconds>.
* Source/WebCore/rendering/RenderTheme.h:
(WebCore::RenderTheme::caretBlinkInterval const): Change the return type
to std::optional<Seconds>.
* Source/WebCore/rendering/RenderThemeAdwaita.cpp:
(WebCore::RenderTheme::singleton): Remove PLATFORM(GTK) guard.
(WebCore::RenderThemeAdwaita::caretBlinkInterval const): Adapt to use
std::optional<Seconds>.
* Source/WebCore/rendering/RenderThemeAdwaita.h: Ditto.
* Source/WebCore/rendering/RenderThemeGtk.cpp: Removed.
* Source/WebCore/rendering/RenderThemeGtk.h: Removed.

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

Source/WebCore/SourcesGTK.txt

@@ -127,5 +127,3 @@ platform/text/hyphen/HyphenationLibHyphen.cpp
 platform/unix/LoggingUnix.cpp
 
 platform/xdg/MIMETypeRegistryXdg.cpp
-
-rendering/RenderThemeGtk.cpp

Source/WebCore/platform/CaretAnimator.h

@@ -101,11 +101,13 @@ class CaretAnimator {
 
     virtual void updateAnimationProperties(ReducedResolutionSeconds) = 0;
 
-    void didStart(ReducedResolutionSeconds currentTime, Seconds interval)
+    void didStart(ReducedResolutionSeconds currentTime, std::optional<Seconds> interval)
     {
         m_startTime = currentTime;
         m_isActive = true;
-        m_blinkTimer.startOneShot(interval);
+        setBlinkingSuspended(!interval);
+        if (interval)
+            m_blinkTimer.startOneShot(*interval);
     }
 
     void didEnd()

Source/WebCore/platform/SimpleCaretAnimator.cpp

@@ -39,17 +39,23 @@ void SimpleCaretAnimator::updateAnimationProperties(ReducedResolutionSeconds cur
 {
     auto caretBlinkInterval = RenderTheme::singleton().caretBlinkInterval();
 
+    setBlinkingSuspended(!caretBlinkInterval);
+
     // Ensure the caret is always visible when blinking is suspended.
     if (isBlinkingSuspended() && m_presentationProperties.blinkState == PresentationProperties::BlinkState::On) {
-        m_blinkTimer.startOneShot(caretBlinkInterval);
+        m_blinkTimer.startOneShot(caretBlinkInterval.value_or(0_ms));
         return;
     }
 
+    // If blinking is disabled, set isBlinkingSuspended() would have made the
+    // previous check return early and at this point there must be an interval.
+    ASSERT(caretBlinkInterval.has_value());
+
     if (currentTime - m_lastTimeCaretPaintWasToggled >= caretBlinkInterval) {
         setBlinkState(!m_presentationProperties.blinkState);
         m_lastTimeCaretPaintWasToggled = currentTime;
 
-        m_blinkTimer.startOneShot(caretBlinkInterval);
+        m_blinkTimer.startOneShot(*caretBlinkInterval);
     }
 }
 

Source/WebCore/rendering/RenderTheme.h

@@ -197,7 +197,7 @@ class RenderTheme {
 #endif
     virtual void platformColorsDidChange();
 
-    virtual Seconds caretBlinkInterval() const { return 500_ms; }
+    virtual std::optional<Seconds> caretBlinkInterval() const { return { 500_ms }; }
 
     // System fonts and colors for CSS.
     virtual Color systemColor(CSSValueID, OptionSet<StyleColorOptions>) const;

Source/WebCore/rendering/RenderThemeAdwaita.cpp

@@ -115,13 +115,11 @@ static inline Color getAccentColor(const RenderObject& renderObject)
     return getSystemAccentColor();
 }
 
-#if !PLATFORM(GTK)
 RenderTheme& RenderTheme::singleton()
 {
     static MainThreadNeverDestroyed<RenderThemeAdwaita> theme;
     return theme;
 }
-#endif
 
 bool RenderThemeAdwaita::supportsFocusRing(const RenderStyle& style) const
 {
@@ -695,12 +693,14 @@ void RenderThemeAdwaita::adjustListButtonStyle(RenderStyle& style, const Element
 #endif // ENABLE(DATALIST_ELEMENT)
 
 #if PLATFORM(GTK)
-Seconds RenderThemeAdwaita::caretBlinkInterval() const
+std::optional<Seconds> RenderThemeAdwaita::caretBlinkInterval() const
 {
     gboolean shouldBlink;
     gint time;
     g_object_get(gtk_settings_get_default(), "gtk-cursor-blink", &shouldBlink, "gtk-cursor-blink-time", &time, nullptr);
-    return shouldBlink ? 500_us * time : 0_s;
+    if (shouldBlink)
+        return { 500_us * time };
+    return { };
 }
 #endif
 

Source/WebCore/rendering/RenderThemeAdwaita.h

@@ -103,7 +103,7 @@ class RenderThemeAdwaita : public RenderTheme {
 #endif
 
 #if PLATFORM(GTK)
-    Seconds caretBlinkInterval() const override;
+    std::optional<Seconds> caretBlinkInterval() const override;
 #endif
 };