LibWeb: Fix scroll state refresh in cached display list for iframes

6507d23 introduced a bug when snapshot for iframe is saved in
`PaintNestedDisplayList` and, since display lists are immutable, it's
not possible to update before the next repaint.

This change fixes the issue by moving `ScrollStateSnapshot` for
nested display lists from `PaintNestedDisplayList` to
`HashMap<NonnullRefPtr<DisplayList>, ScrollStateSnapshot>` that is
placed into pending rendering task, making it possible to update
snapshots for all display lists before the next repaint.

This change doesn't have a test because it's really hard to make a ref
test that will specifically check scenario when scroll offset of an
iframe is advanced after display list is cached. We already have
`Tests/LibWeb/Ref/input/scroll-iframe.html` but unfortunately it did
not catch this bug.

Fixes https://github.com/LadybirdBrowser/ladybird/issues/5486

Libraries/LibWeb/CSS/StyleValues/CursorStyleValue.cpp

@@ -94,8 +94,7 @@ Optional<Gfx::ImageCursor> CursorStyleValue::make_image_cursor(Layout::NodeWithS
         case DisplayListPlayerType::SkiaCPU: {
             auto painting_surface = Gfx::PaintingSurface::wrap_bitmap(bitmap);
             Painting::DisplayListPlayerSkia display_list_player;
-            Painting::ScrollStateSnapshot scroll_state_snapshot;
-            display_list_player.execute(*display_list, scroll_state_snapshot, painting_surface);
+            display_list_player.execute(*display_list, {}, painting_surface);
             break;
         }
         }

Libraries/LibWeb/DOM/Document.cpp

@@ -6405,6 +6405,11 @@ void Document::invalidate_display_list()
     }
 }
 
+RefPtr<Painting::DisplayList> Document::cached_display_list() const
+{
+    return m_cached_display_list;
+}
+
 RefPtr<Painting::DisplayList> Document::record_display_list(HTML::PaintConfig config)
 {
     if (m_cached_display_list && m_cached_display_list_paint_config == config) {

Libraries/LibWeb/DOM/Document.h

@@ -831,6 +831,7 @@ public:
     void set_needs_display(InvalidateDisplayList = InvalidateDisplayList::Yes);
     void set_needs_display(CSSPixelRect const&, InvalidateDisplayList = InvalidateDisplayList::Yes);
 
+    RefPtr<Painting::DisplayList> cached_display_list() const;
     RefPtr<Painting::DisplayList> record_display_list(HTML::PaintConfig);
 
     void invalidate_display_list();

Libraries/LibWeb/Forward.h

@@ -40,7 +40,9 @@ class DisplayList;
 class DisplayListPlayerSkia;
 class DisplayListRecorder;
 class SVGGradientPaintStyle;
+class ScrollStateSnapshot;
 using PaintStyle = RefPtr<SVGGradientPaintStyle>;
+using ScrollStateSnapshotByDisplayList = HashMap<NonnullRefPtr<DisplayList>, ScrollStateSnapshot>;
 
 }
 

Libraries/LibWeb/HTML/Navigable.cpp

@@ -46,6 +46,7 @@
 #include <LibWeb/Loader/GeneratedPagesLoader.h>
 #include <LibWeb/Page/Page.h>
 #include <LibWeb/Painting/DisplayListPlayerSkia.h>
+#include <LibWeb/Painting/NavigableContainerViewportPaintable.h>
 #include <LibWeb/Painting/Paintable.h>
 #include <LibWeb/Painting/ViewportPaintable.h>
 #include <LibWeb/Platform/EventLoopPlugin.h>
@@ -2612,14 +2613,35 @@ void Navigable::start_display_list_rendering(Gfx::PaintingSurface& painting_surf
         callback();
         return;
     }
-    document->paintable()->refresh_scroll_state();
     auto display_list = document->record_display_list(paint_config);
     if (!display_list) {
         callback();
         return;
     }
-    auto scroll_state_snapshot = document->paintable()->scroll_state().snapshot();
-    m_rendering_thread.enqueue_rendering_task(*display_list, move(scroll_state_snapshot), painting_surface, move(callback));
+
+    auto& document_paintable = *document->paintable();
+    Painting::ScrollStateSnapshotByDisplayList scroll_state_snapshot_by_display_list;
+    document_paintable.refresh_scroll_state();
+    auto scroll_state_snapshot = document_paintable.scroll_state().snapshot();
+    scroll_state_snapshot_by_display_list.set(*display_list, move(scroll_state_snapshot));
+    // Collect scroll state snapshots for each nested navigable
+    document_paintable.for_each_in_inclusive_subtree_of_type<Painting::NavigableContainerViewportPaintable>([&scroll_state_snapshot_by_display_list](auto& navigable_container_paintable) {
+        auto const* hosted_document = navigable_container_paintable.layout_box().dom_node().content_document_without_origin_check();
+        if (!hosted_document || !hosted_document->paintable())
+            return TraversalDecision::Continue;
+        // We are only interested in collecting scroll state snapshots for visible nested navigables, which is
+        // detectable by checking if they have a cached display list that should've been populated by
+        // record_display_list() on top-level document.
+        auto navigable_display_list = hosted_document->cached_display_list();
+        if (!navigable_display_list)
+            return TraversalDecision::Continue;
+        const_cast<DOM::Document&>(*hosted_document).paintable()->refresh_scroll_state();
+        auto navigable_scroll_state_snapshot = hosted_document->paintable()->scroll_state().snapshot();
+        scroll_state_snapshot_by_display_list.set(*navigable_display_list, move(navigable_scroll_state_snapshot));
+        return TraversalDecision::Continue;
+    });
+
+    m_rendering_thread.enqueue_rendering_task(*display_list, move(scroll_state_snapshot_by_display_list), painting_surface, move(callback));
 }
 
 RefPtr<Gfx::SkiaBackendContext> Navigable::skia_backend_context() const

Libraries/LibWeb/HTML/RenderingThread.cpp

@@ -67,7 +67,7 @@ void RenderingThread::rendering_thread_loop()
             break;
         }
 
-        m_skia_player->execute(*task->display_list, task->scroll_state_snapshot, task->painting_surface);
+        m_skia_player->execute(*task->display_list, move(task->scroll_state_snapshot_by_display_list), task->painting_surface);
         if (m_exit)
             break;
         m_main_thread_event_loop.deferred_invoke([callback = move(task->callback)] {
@@ -76,10 +76,10 @@ void RenderingThread::rendering_thread_loop()
     }
 }
 
-void RenderingThread::enqueue_rendering_task(NonnullRefPtr<Painting::DisplayList> display_list, Painting::ScrollStateSnapshot&& scroll_state_snapshot, NonnullRefPtr<Gfx::PaintingSurface> painting_surface, Function<void()>&& callback)
+void RenderingThread::enqueue_rendering_task(NonnullRefPtr<Painting::DisplayList> display_list, Painting::ScrollStateSnapshotByDisplayList&& scroll_state_snapshot_by_display_list, NonnullRefPtr<Gfx::PaintingSurface> painting_surface, Function<void()>&& callback)
 {
     Threading::MutexLocker const locker { m_rendering_task_mutex };
-    m_rendering_tasks.enqueue(Task { move(display_list), move(scroll_state_snapshot), move(painting_surface), move(callback) });
+    m_rendering_tasks.enqueue(Task { move(display_list), move(scroll_state_snapshot_by_display_list), move(painting_surface), move(callback) });
     m_rendering_task_ready_wake_condition.signal();
 }
 

Libraries/LibWeb/HTML/RenderingThread.h

@@ -14,7 +14,6 @@
 #include <LibThreading/Thread.h>
 #include <LibWeb/Forward.h>
 #include <LibWeb/Page/Page.h>
-#include <LibWeb/Painting/ScrollState.h>
 
 namespace Web::HTML {
 
@@ -28,7 +27,7 @@ public:
 
     void start(DisplayListPlayerType);
     void set_skia_player(OwnPtr<Painting::DisplayListPlayerSkia>&& player);
-    void enqueue_rendering_task(NonnullRefPtr<Painting::DisplayList>, Painting::ScrollStateSnapshot&&, NonnullRefPtr<Gfx::PaintingSurface>, Function<void()>&& callback);
+    void enqueue_rendering_task(NonnullRefPtr<Painting::DisplayList>, Painting::ScrollStateSnapshotByDisplayList&&, NonnullRefPtr<Gfx::PaintingSurface>, Function<void()>&& callback);
 
 private:
     void rendering_thread_loop();
@@ -44,7 +43,7 @@ private:
 
     struct Task {
         NonnullRefPtr<Painting::DisplayList> display_list;
-        Painting::ScrollStateSnapshot scroll_state_snapshot;
+        Painting::ScrollStateSnapshotByDisplayList scroll_state_snapshot_by_display_list;
         NonnullRefPtr<Gfx::PaintingSurface> painting_surface;
         Function<void()> callback;
     };

Libraries/LibWeb/Painting/Command.h

@@ -416,7 +416,6 @@ struct AddMask {
 
 struct PaintNestedDisplayList {
     RefPtr<DisplayList> display_list;
-    ScrollStateSnapshot scroll_state_snapshot;
     Gfx::IntRect rect;
 
     [[nodiscard]] Gfx::IntRect bounding_rect() const { return rect; }

Libraries/LibWeb/Painting/DisplayList.cpp

@@ -4,6 +4,7 @@
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
+#include <AK/TemporaryChange.h>
 #include <LibWeb/Painting/DevicePixelConverter.h>
 #include <LibWeb/Painting/DisplayList.h>
 
@@ -46,12 +47,14 @@ static bool command_is_clip_or_mask(Command const& command)
         });
 }
 
-void DisplayListPlayer::execute(DisplayList& display_list, ScrollStateSnapshot const& scroll_state, RefPtr<Gfx::PaintingSurface> surface)
+void DisplayListPlayer::execute(DisplayList& display_list, ScrollStateSnapshotByDisplayList&& scroll_state_snapshot_by_display_list, RefPtr<Gfx::PaintingSurface> surface)
 {
+    TemporaryChange change { m_scroll_state_snapshots_by_display_list, move(scroll_state_snapshot_by_display_list) };
     if (surface) {
         surface->lock_context();
     }
-    execute_impl(display_list, scroll_state, surface);
+    auto scroll_state_snapshot = m_scroll_state_snapshots_by_display_list.get(display_list).value_or({});
+    execute_impl(display_list, scroll_state_snapshot, surface);
     if (surface) {
         surface->unlock_context();
     }

Libraries/LibWeb/Painting/DisplayList.h

@@ -15,24 +15,25 @@
 #include <LibGfx/ImmutableBitmap.h>
 #include <LibGfx/PaintStyle.h>
 #include <LibWeb/CSS/Enums.h>
+#include <LibWeb/Forward.h>
 #include <LibWeb/Painting/ClipFrame.h>
 #include <LibWeb/Painting/Command.h>
 #include <LibWeb/Painting/ScrollState.h>
 
 namespace Web::Painting {
 
-class DisplayList;
-
 class DisplayListPlayer {
 public:
     virtual ~DisplayListPlayer() = default;
 
-    void execute(DisplayList&, ScrollStateSnapshot const&, RefPtr<Gfx::PaintingSurface>);
+    void execute(DisplayList&, ScrollStateSnapshotByDisplayList&&, RefPtr<Gfx::PaintingSurface>);
 
 protected:
     Gfx::PaintingSurface& surface() const { return m_surfaces.last(); }
     void execute_impl(DisplayList&, ScrollStateSnapshot const& scroll_state, RefPtr<Gfx::PaintingSurface>);
 
+    ScrollStateSnapshotByDisplayList m_scroll_state_snapshots_by_display_list;
+
 private:
     virtual void flush() = 0;
     virtual void draw_glyph_run(DrawGlyphRun const&) = 0;

Libraries/LibWeb/Painting/DisplayListPlayerSkia.cpp

@@ -991,7 +991,8 @@ void DisplayListPlayerSkia::paint_nested_display_list(PaintNestedDisplayList con
 {
     auto& canvas = surface().canvas();
     canvas.translate(command.rect.x(), command.rect.y());
-    execute_impl(*command.display_list, command.scroll_state_snapshot, {});
+    ScrollStateSnapshot scroll_state_snapshot = m_scroll_state_snapshots_by_display_list.get(*command.display_list).value_or({});
+    execute_impl(*command.display_list, scroll_state_snapshot, {});
 }
 
 void DisplayListPlayerSkia::paint_scrollbar(PaintScrollBar const& command)

Libraries/LibWeb/Painting/DisplayListRecorder.cpp

@@ -29,9 +29,9 @@ DisplayListRecorder::~DisplayListRecorder() = default;
         m_command_list.append(__VA_ARGS__, _scroll_frame_id, _clip_frame); \
     } while (false)
 
-void DisplayListRecorder::paint_nested_display_list(RefPtr<DisplayList> display_list, ScrollStateSnapshot&& scroll_state_snapshot, Gfx::IntRect rect)
+void DisplayListRecorder::paint_nested_display_list(RefPtr<DisplayList> display_list, Gfx::IntRect rect)
 {
-    APPEND(PaintNestedDisplayList { move(display_list), move(scroll_state_snapshot), rect });
+    APPEND(PaintNestedDisplayList { move(display_list), rect });
 }
 
 void DisplayListRecorder::add_rounded_rect_clip(CornerRadii corner_radii, Gfx::IntRect border_rect, CornerClip corner_clip)

Libraries/LibWeb/Painting/DisplayListRecorder.h

@@ -130,7 +130,7 @@ public:
     void push_stacking_context(PushStackingContextParams params);
     void pop_stacking_context();
 
-    void paint_nested_display_list(RefPtr<DisplayList> display_list, ScrollStateSnapshot&&, Gfx::IntRect rect);
+    void paint_nested_display_list(RefPtr<DisplayList> display_list, Gfx::IntRect rect);
 
     void add_rounded_rect_clip(CornerRadii corner_radii, Gfx::IntRect border_rect, CornerClip corner_clip);
     void add_mask(RefPtr<DisplayList> display_list, Gfx::IntRect rect);
@@ -158,7 +158,7 @@ public:
     DisplayListRecorder(DisplayList&);
     ~DisplayListRecorder();
 
-    DisplayList& display_list() { return m_command_list; }
+    DisplayList const& display_list() const { return m_command_list; }
 
     int m_save_nesting_level { 0 };
 

Libraries/LibWeb/Painting/NavigableContainerViewportPaintable.cpp

@@ -60,8 +60,7 @@ void NavigableContainerViewportPaintable::paint(PaintContext& context, PaintPhas
         paint_config.paint_overlay = context.should_paint_overlay();
         paint_config.should_show_line_box_borders = context.should_show_line_box_borders();
         auto display_list = const_cast<DOM::Document*>(hosted_document)->record_display_list(paint_config);
-        auto scroll_state_snapshot = hosted_document->paintable()->scroll_state().snapshot();
-        context.display_list_recorder().paint_nested_display_list(display_list, move(scroll_state_snapshot), context.enclosing_device_rect(absolute_rect).to_type<int>());
+        context.display_list_recorder().paint_nested_display_list(display_list, context.enclosing_device_rect(absolute_rect).to_type<int>());
 
         context.display_list_recorder().restore();
 

Libraries/LibWeb/Painting/NavigableContainerViewportPaintable.h

@@ -16,6 +16,8 @@ class NavigableContainerViewportPaintable final : public PaintableBox {
     GC_DECLARE_ALLOCATOR(NavigableContainerViewportPaintable);
 
 public:
+    virtual bool is_navigable_container_viewport_paintable() const override { return true; }
+
     static GC::Ref<NavigableContainerViewportPaintable> create(Layout::NavigableContainerViewport const&);
 
     virtual void paint(PaintContext&, PaintPhase) const override;
@@ -26,4 +28,7 @@ private:
     NavigableContainerViewportPaintable(Layout::NavigableContainerViewport const&);
 };
 
+template<>
+inline bool Paintable::fast_is<NavigableContainerViewportPaintable>() const { return is_navigable_container_viewport_paintable(); }
+
 }

Libraries/LibWeb/Painting/Paintable.h

@@ -112,6 +112,7 @@ public:
     template<typename T>
     bool fast_is() const = delete;
 
+    [[nodiscard]] virtual bool is_navigable_container_viewport_paintable() const { return false; }
     [[nodiscard]] virtual bool is_paintable_box() const { return false; }
     [[nodiscard]] virtual bool is_paintable_with_lines() const { return false; }
     [[nodiscard]] virtual bool is_svg_paintable() const { return false; }

Libraries/LibWeb/Painting/SVGMaskable.cpp

@@ -100,8 +100,7 @@ RefPtr<Gfx::ImmutableBitmap> SVGMaskable::calculate_mask_of_svg(PaintContext& co
         StackingContext::paint_svg(paint_context, paintable, PaintPhase::Foreground);
         auto painting_surface = Gfx::PaintingSurface::wrap_bitmap(*mask_bitmap);
         DisplayListPlayerSkia display_list_player;
-        ScrollStateSnapshot scroll_state_snapshot;
-        display_list_player.execute(display_list, scroll_state_snapshot, painting_surface);
+        display_list_player.execute(display_list, {}, painting_surface);
         return mask_bitmap;
     };
     RefPtr<Gfx::Bitmap> mask_bitmap = {};

Libraries/LibWeb/SVG/SVGDecodedImageData.cpp

@@ -114,8 +114,7 @@ RefPtr<Gfx::Bitmap> SVGDecodedImageData::render(Gfx::IntSize size) const
     case DisplayListPlayerType::SkiaCPU: {
         auto painting_surface = Gfx::PaintingSurface::wrap_bitmap(*bitmap);
         Painting::DisplayListPlayerSkia display_list_player;
-        Painting::ScrollStateSnapshot scroll_state_snapshot;
-        display_list_player.execute(*display_list, scroll_state_snapshot, painting_surface);
+        display_list_player.execute(*display_list, {}, painting_surface);
         break;
     }
     default: