Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LibWeb: Implement some viewport proximity features #2603

Merged
merged 1 commit into from
Jan 4, 2025

Conversation

Psychpsyo
Copy link
Contributor

This gets rid of some FIXMEs related to viewport proximity and relevancy to the user in the update the rendering algorithm.

I know there is no tests, but I tried some of the relevant WPT tests for the new sections and none of them are passing yet so I assume there is other code missing elsewhere that would need to actually make use of the element's proximity to the viewport. (beyond just setting it in this one place.)

So I don't think there is any sensible tests that can be written for these two functions on their own.

@ladybird-bot
Copy link
Collaborator

Hello!

One or more of the commit messages in this PR do not match the Ladybird code submission policy, please check the lint_commits CI job for more details on which commits were flagged and why.
Please do not close this PR and open another, instead modify your commit message(s) with git commit --amend and force push those changes to update this PR.

@Psychpsyo Psychpsyo force-pushed the fix-event-loop-fixmes branch from 6cb3c6e to 92abe2a Compare November 26, 2024 23:10
@Psychpsyo Psychpsyo changed the title Implement some viewport proximity features LibWeb: Implement some viewport proximity features Nov 26, 2024
@Psychpsyo Psychpsyo force-pushed the fix-event-loop-fixmes branch from 92abe2a to 5b72427 Compare November 27, 2024 09:54
Libraries/LibWeb/DOM/Element.cpp Show resolved Hide resolved
Libraries/LibWeb/DOM/Element.cpp Outdated Show resolved Hide resolved
Libraries/LibWeb/DOM/Element.cpp Show resolved Hide resolved
Libraries/LibWeb/DOM/Element.cpp Outdated Show resolved Hide resolved
@Psychpsyo Psychpsyo force-pushed the fix-event-loop-fixmes branch from 5b72427 to 5edba43 Compare November 27, 2024 11:08
@Psychpsyo Psychpsyo requested a review from AtkinsSJ November 27, 2024 11:09
@Psychpsyo Psychpsyo force-pushed the fix-event-loop-fixmes branch 2 times, most recently from 6c09f7a to e9f00d6 Compare November 29, 2024 22:38
@AtkinsSJ
Copy link
Member

AtkinsSJ commented Dec 4, 2024

There are a couple of tests crashing on CI. They both look like flakes but I'm re-running CI just in case.

Otherwise this looks good.

@Psychpsyo
Copy link
Contributor Author

Psychpsyo commented Dec 4, 2024

There are a couple of tests crashing on CI. They both look like flakes but I'm re-running CI just in case.

Otherwise this looks good.

I am not quite sure how much of it is flaky and how much is a race condition.
I haven't had the time to properly look into this yet, but my current assumption is that, by the time "update the rendering" runs, something (the document element?) is still null.

But I am not sure.

@AtkinsSJ
Copy link
Member

AtkinsSJ commented Dec 4, 2024

Pasting it here so it's easier to find. I would have thought that document->update_layout(); a few lines above would ensure that there's a document element though, this is very odd.

/Users/runner/work/ladybird/ladybird/Libraries/LibWeb/HTML/EventLoop/EventLoop.cpp:344:43: runtime error: member call on null pointer of type 'Web::DOM::Node'
    #0 0x1109a84d8 in Web::HTML::EventLoop::update_the_rendering() EventLoop.cpp:344
    #1 0x10f97fa84 in AK::Function<void ()>::operator()() const Function.h:120
    #2 0x1109a3798 in Web::HTML::EventLoop::process() EventLoop.cpp:179
    #3 0x10f97fa84 in AK::Function<void ()>::operator()() const Function.h:120
    #4 0x104729f58 in AK::Function<void ()>::operator()() const Function.h:120
    #5 0x10472612c in Core::EventReceiver::dispatch_event(Core::Event&, Core::EventReceiver*) EventReceiver.cpp:162
    #6 0x10477bae4 in Core::ThreadEventQueue::process() ThreadEventQueue.cpp:121
    #7 0x10471a0f4 in Core::EventLoop::spin_until(AK::Function<bool ()>) EventLoop.cpp:95
    #8 0x1116147f4 in Web::Platform::EventLoopPluginSerenity::spin_until(GC::Root<GC::Function<bool ()>>) EventLoopPluginSerenity.cpp:19
    #9 0x1109a16ac in Web::HTML::EventLoop::spin_until(GC::Ref<GC::Function<bool ()>>) EventLoop.cpp:97
    #10 0x110fd8f9c in Web::HTML::TraversableNavigable::apply_the_history_step(int, bool, AK::Optional<Web::HTML::SourceSnapshotParams>, GC::Ptr<Web::HTML::Navigable>, AK::Optional<Web::HTML::UserNavigationInvolvement>, AK::Optional<Web::Bindings::NavigationType>, Web::HTML::TraversableNavigable::SynchronousNavigation) TraversableNavigable.cpp:672
    #11 0x110fe7830 in Web::HTML::TraversableNavigable::apply_the_push_or_replace_history_step(int, Web::HTML::HistoryHandlingBehavior, Web::HTML::TraversableNavigable::SynchronousNavigation) TraversableNavigable.cpp:1197
    #12 0x110d2b1a4 in Web::HTML::finalize_a_cross_document_navigation(GC::Ref<Web::HTML::Navigable>, Web::HTML::HistoryHandlingBehavior, GC::Ref<Web::HTML::SessionHistoryEntry>) Navigable.cpp:1933
    #13 0x110d500e4 in AK::Function<void ()>::CallableWrapper<Web::HTML::Navigable::navigate(Web::HTML::Navigable::NavigateParams)::$_2::operator()() const::'lambda0'()::operator()() const::'lambda'()>::call() Function.h:187
    #14 0x10f97fa84 in AK::Function<void ()>::operator()() const Function.h:120
    #15 0x110f3899c in AK::Function<void ()>::CallableWrapper<Web::HTML::SessionHistoryTraversalQueue::SessionHistoryTraversalQueue()::$_0>::call() Function.h:187
    #16 0x104729f58 in AK::Function<void ()>::operator()() const Function.h:120
    #17 0x10472612c in Core::EventReceiver::dispatch_event(Core::Event&, Core::EventReceiver*) EventReceiver.cpp:162
    #18 0x10477bae4 in Core::ThreadEventQueue::process() ThreadEventQueue.cpp:121
    #19 0x10471a0f4 in Core::EventLoop::spin_until(AK::Function<bool ()>) EventLoop.cpp:95
    #20 0x1116147f4 in Web::Platform::EventLoopPluginSerenity::spin_until(GC::Root<GC::Function<bool ()>>) EventLoopPluginSerenity.cpp:19
    #21 0x1109a16ac in Web::HTML::EventLoop::spin_until(GC::Ref<GC::Function<bool ()>>) EventLoop.cpp:97
    #22 0x110fdf0d0 in Web::HTML::TraversableNavigable::check_if_unloading_is_canceled(AK::Vector<GC::Root<Web::HTML::Navigable>, 0ul>, GC::Ptr<Web::HTML::TraversableNavigable>, AK::Optional<int>, AK::Optional<Web::HTML::UserNavigationInvolvement>) TraversableNavigable.cpp:992
    #23 0x110fe4004 in Web::HTML::TraversableNavigable::check_if_unloading_is_canceled(AK::Vector<GC::Root<Web::HTML::Navigable>, 0ul>) TraversableNavigable.cpp:1002
    #24 0x110d4c354 in AK::Function<void ()>::CallableWrapper<Web::HTML::Navigable::navigate(Web::HTML::Navigable::NavigateParams)::$_2>::call() Function.h:187
    #25 0x10f97fa84 in AK::Function<void ()>::operator()() const Function.h:120
    #26 0x104729f58 in AK::Function<void ()>::operator()() const Function.h:120
    #27 0x10477b9e0 in Core::ThreadEventQueue::process() ThreadEventQueue.cpp:118
    #28 0x10471a0f4 in Core::EventLoop::spin_until(AK::Function<bool ()>) EventLoop.cpp:95
    #29 0x1116147f4 in Web::Platform::EventLoopPluginSerenity::spin_until(GC::Root<GC::Function<bool ()>>) EventLoopPluginSerenity.cpp:19
    #30 0x1109a16ac in Web::HTML::EventLoop::spin_until(GC::Ref<GC::Function<bool ()>>) EventLoop.cpp:97
    #31 0x110d0ff08 in Web::HTML::create_navigation_params_by_fetching(GC::Ptr<Web::HTML::SessionHistoryEntry>, GC::Ptr<Web::HTML::Navigable>, Web::HTML::SourceSnapshotParams const&, Web::HTML::TargetSnapshotParams const&, Web::HTML::CSPNavigationType, AK::Optional<AK::String>) Navigable.cpp:892
    #32 0x110d069d8 in Web::HTML::Navigable::populate_session_history_entry_document(GC::Ptr<Web::HTML::SessionHistoryEntry>, Web::HTML::SourceSnapshotParams const&, Web::HTML::TargetSnapshotParams const&, AK::Optional<AK::String>, AK::Variant<AK::Optional<AK::StringView>, GC::Ref<Web::HTML::NavigationParams>, GC::Ref<Web::HTML::NonFetchSchemeNavigationParams>>, Web::HTML::CSPNavigationType, bool, GC::Ptr<GC::Function<void ()>>) Navigable.cpp:1110
    #33 0x110d4d624 in AK::Function<void ()>::CallableWrapper<Web::HTML::Navigable::navigate(Web::HTML::Navigable::NavigateParams)::$_2>::call() Function.h:187
    #34 0x10f97fa84 in AK::Function<void ()>::operator()() const Function.h:120
    #35 0x104729f58 in AK::Function<void ()>::operator()() const Function.h:120
    #36 0x10477b9e0 in Core::ThreadEventQueue::process() ThreadEventQueue.cpp:118
    #37 0x10471a0f4 in Core::EventLoop::spin_until(AK::Function<bool ()>) EventLoop.cpp:95
    #38 0x1116147f4 in Web::Platform::EventLoopPluginSerenity::spin_until(GC::Root<GC::Function<bool ()>>) EventLoopPluginSerenity.cpp:19
    #39 0x1109a16ac in Web::HTML::EventLoop::spin_until(GC::Ref<GC::Function<bool ()>>) EventLoop.cpp:97
    #40 0x110d0ff08 in Web::HTML::create_navigation_params_by_fetching(GC::Ptr<Web::HTML::SessionHistoryEntry>, GC::Ptr<Web::HTML::Navigable>, Web::HTML::SourceSnapshotParams const&, Web::HTML::TargetSnapshotParams const&, Web::HTML::CSPNavigationType, AK::Optional<AK::String>) Navigable.cpp:892
    #41 0x110d069d8 in Web::HTML::Navigable::populate_session_history_entry_document(GC::Ptr<Web::HTML::SessionHistoryEntry>, Web::HTML::SourceSnapshotParams const&, Web::HTML::TargetSnapshotParams const&, AK::Optional<AK::String>, AK::Variant<AK::Optional<AK::StringView>, GC::Ref<Web::HTML::NavigationParams>, GC::Ref<Web::HTML::NonFetchSchemeNavigationParams>>, Web::HTML::CSPNavigationType, bool, GC::Ptr<GC::Function<void ()>>) Navigable.cpp:1110
    #42 0x110d4d624 in AK::Function<void ()>::CallableWrapper<Web::HTML::Navigable::navigate(Web::HTML::Navigable::NavigateParams)::$_2>::call() Function.h:187
    #43 0x10f97fa84 in AK::Function<void ()>::operator()() const Function.h:120
    #44 0x104729f58 in AK::Function<void ()>::operator()() const Function.h:120
    #45 0x10477b9e0 in Core::ThreadEventQueue::process() ThreadEventQueue.cpp:118
    #46 0x10471a0f4 in Core::EventLoop::spin_until(AK::Function<bool ()>) EventLoop.cpp:95
    #47 0x1116147f4 in Web::Platform::EventLoopPluginSerenity::spin_until(GC::Root<GC::Function<bool ()>>) EventLoopPluginSerenity.cpp:19
    #48 0x1109a16ac in Web::HTML::EventLoop::spin_until(GC::Ref<GC::Function<bool ()>>) EventLoop.cpp:97
    #49 0x110fdf0d0 in Web::HTML::TraversableNavigable::check_if_unloading_is_canceled(AK::Vector<GC::Root<Web::HTML::Navigable>, 0ul>, GC::Ptr<Web::HTML::TraversableNavigable>, AK::Optional<int>, AK::Optional<Web::HTML::UserNavigationInvolvement>) TraversableNavigable.cpp:992
    #50 0x110fe4004 in Web::HTML::TraversableNavigable::check_if_unloading_is_canceled(AK::Vector<GC::Root<Web::HTML::Navigable>, 0ul>) TraversableNavigable.cpp:1002
    #51 0x110d4c354 in AK::Function<void ()>::CallableWrapper<Web::HTML::Navigable::navigate(Web::HTML::Navigable::NavigateParams)::$_2>::call() Function.h:187
    #52 0x10f97fa84 in AK::Function<void ()>::operator()() const Function.h:120
    #53 0x104729f58 in AK::Function<void ()>::operator()() const Function.h:120
    #54 0x10477b9e0 in Core::ThreadEventQueue::process() ThreadEventQueue.cpp:118
    #55 0x10471a0f4 in Core::EventLoop::spin_until(AK::Function<bool ()>) EventLoop.cpp:95
    #56 0x1116147f4 in Web::Platform::EventLoopPluginSerenity::spin_until(GC::Root<GC::Function<bool ()>>) EventLoopPluginSerenity.cpp:19
    #57 0x1109a16ac in Web::HTML::EventLoop::spin_until(GC::Ref<GC::Function<bool ()>>) EventLoop.cpp:97
    #58 0x110fdf0d0 in Web::HTML::TraversableNavigable::check_if_unloading_is_canceled(AK::Vector<GC::Root<Web::HTML::Navigable>, 0ul>, GC::Ptr<Web::HTML::TraversableNavigable>, AK::Optional<int>, AK::Optional<Web::HTML::UserNavigationInvolvement>) TraversableNavigable.cpp:992
    #59 0x110fe4004 in Web::HTML::TraversableNavigable::check_if_unloading_is_canceled(AK::Vector<GC::Root<Web::HTML::Navigable>, 0ul>) TraversableNavigable.cpp:1002
    #60 0x110d4c354 in AK::Function<void ()>::CallableWrapper<Web::HTML::Navigable::navigate(Web::HTML::Navigable::NavigateParams)::$_2>::call() Function.h:187
    #61 0x10f97fa84 in AK::Function<void ()>::operator()() const Function.h:120
    #62 0x104729f58 in AK::Function<void ()>::operator()() const Function.h:120
    #63 0x10477b9e0 in Core::ThreadEventQueue::process() ThreadEventQueue.cpp:118
    #64 0x10471a0f4 in Core::EventLoop::spin_until(AK::Function<bool ()>) EventLoop.cpp:95
    #65 0x1116147f4 in Web::Platform::EventLoopPluginSerenity::spin_until(GC::Root<GC::Function<bool ()>>) EventLoopPluginSerenity.cpp:19
    #66 0x1109a16ac in Web::HTML::EventLoop::spin_until(GC::Ref<GC::Function<bool ()>>) EventLoop.cpp:97
    #67 0x110fdf0d0 in Web::HTML::TraversableNavigable::check_if_unloading_is_canceled(AK::Vector<GC::Root<Web::HTML::Navigable>, 0ul>, GC::Ptr<Web::HTML::TraversableNavigable>, AK::Optional<int>, AK::Optional<Web::HTML::UserNavigationInvolvement>) TraversableNavigable.cpp:992
    #68 0x110fe4004 in Web::HTML::TraversableNavigable::check_if_unloading_is_canceled(AK::Vector<GC::Root<Web::HTML::Navigable>, 0ul>) TraversableNavigable.cpp:1002
    #69 0x110d4c354 in AK::Function<void ()>::CallableWrapper<Web::HTML::Navigable::navigate(Web::HTML::Navigable::NavigateParams)::$_2>::call() Function.h:187
    #70 0x10f97fa84 in AK::Function<void ()>::operator()() const Function.h:120
    #71 0x104729f58 in AK::Function<void ()>::operator()() const Function.h:120
    #72 0x10477b9e0 in Core::ThreadEventQueue::process() ThreadEventQueue.cpp:118
    #73 0x104789f04 in Core::EventLoopImplementationUnix::exec() EventLoopImplementationUnix.cpp:316
    #74 0x104719de4 in Core::EventLoop::exec() EventLoop.cpp:88
    #75 0x102432a58 in serenity_main(Main::Arguments) main.cpp:208
    #76 0x10289420c in main Main.cpp:39
    #77 0x193ad0270  (<unknown module>)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /Users/runner/work/ladybird/ladybird/Libraries/LibWeb/HTML/EventLoop/EventLoop.cpp:344:43 in 
WebContent process crashed! Last page loaded: file:///Users/runner/work/ladybird/ladybird/Tests/LibWeb/Text/input/wpt-import/html/syntax/parsing/html5lib_tests10.html

Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!

@github-actions github-actions bot added stale conflicts Pull request has merge conflicts that need resolution labels Dec 26, 2024
Copy link

github-actions bot commented Jan 1, 2025

Your pull request has conflicts that need to be resolved before it can be reviewed and merged. Make sure to rebase your branch on top of the latest master.

@github-actions github-actions bot removed the stale label Jan 2, 2025
@Psychpsyo Psychpsyo force-pushed the fix-event-loop-fixmes branch from e9f00d6 to fe4cbb9 Compare January 3, 2025 16:06
@github-actions github-actions bot removed the conflicts Pull request has merge conflicts that need resolution label Jan 3, 2025
@Psychpsyo Psychpsyo force-pushed the fix-event-loop-fixmes branch from fe4cbb9 to 141b5b4 Compare January 3, 2025 17:53
@Psychpsyo
Copy link
Contributor Author

Pasting it here so it's easier to find. I would have thought that document->update_layout(); a few lines above would ensure that there's a document element though, this is very odd.

I've now just made it check if there is a document element and only iterate over the children if there is.
No document element should mean no elements to go through.

@AtkinsSJ AtkinsSJ merged commit 331b1b2 into LadybirdBrowser:master Jan 4, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants