ronantakizawa/codereview-bench huggingface dataset:
Repository: LadybirdBrowser/ladybird
File: Libraries/LibGfx/Font/FontVariationSettings.h (C/C++)

Code before:
/*
 * Copyright (c) 2025, Norbiros <me@norbiros.dev>
 *
 * SPDX-License-Identifier: BSD-2-Clause
 */

#include <AK/HashMap.h>
#include <AK/QuickSort.h>
#include <AK/Vector.h>

#pragma once

namespace Gfx {

using FourByteTag = uint32_t;

constexpr FourByteTag MakeFourByteTag(char a, char b, char c, char d)
{
    return (static_cast<uint32_t>(a) << 24) | (static_cast<uint32_t>(b) << 16) | (static_cast<uint32_t>(c) << 8) | static_cast<uint32_t>(d);
}

struct FontVariationAxis {
    FourByteTag tag;
    float value;

    FontVariationAxis(FourByteTag t, float v)
        : tag(t)
        , value(v)
    {
    }

    bool operator==(FontVariationAxis const& other) const
    {
        return tag == other.tag && value == other.value;
    }
};

// FIXME: Support other named axes like 'slnt', 'ital', 'opsz', 'GRAD', etc.
struct FontVariationSettings {
    HashMap<FourByteTag, float> axes;

    FontVariationSettings() = default;

    // https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_fonts/Variable_fonts_guide#weight
    void weight(float value)

Review comment (suggestion):
LibGfx actually has a type for this already: `FourCC`.

Code after:
/*
 * Copyright (c) 2025, Norbiros <me@norbiros.dev>
 *
 * SPDX-License-Identifier: BSD-2-Clause
 */

#include <AK/HashMap.h>
#include <AK/QuickSort.h>
#include <AK/Vector.h>
#include <LibGfx/FourCC.h>

#pragma once

namespace Gfx {

struct FontVariationAxis {
    FourCC tag;
    float value;

    FontVariationAxis(FourCC t, float v)
        : tag(t)
        , value(v)
    {
    }

    bool operator==(FontVariationAxis const& other) const
    {
        return tag == other.tag && value == other.value;
    }
};

// FIXME: Support other named axes like 'slnt', 'ital', 'opsz', 'GRAD', etc.
struct FontVariationSettings {
    HashMap<FourCC, float> axes;

    FontVariationSettings() = default;

    // https://learn.microsoft.com/en-us/typography/opentype/spec/dvaraxistag_wght
    void set_weight(float value)
    {
        axes.set(FourCC("wght"), value);
    }

    // https://learn.microsoft.com/en-us/typography/opentype/spec/dvaraxistag_wdth
    void set_width(float value)
Repository: LadybirdBrowser/ladybird
File: Libraries/LibGfx/Font/FontVariationSettings.h (C/C++)

Code before:
    return (static_cast<uint32_t>(a) << 24) | (static_cast<uint32_t>(b) << 16) | (static_cast<uint32_t>(c) << 8) | static_cast<uint32_t>(d);
}

struct FontVariationAxis {
    FourByteTag tag;
    float value;

    FontVariationAxis(FourByteTag t, float v)
        : tag(t)
        , value(v)
    {
    }

    bool operator==(FontVariationAxis const& other) const
    {
        return tag == other.tag && value == other.value;
    }
};

// FIXME: Support other named axes like 'slnt', 'ital', 'opsz', 'GRAD', etc.
struct FontVariationSettings {
    HashMap<FourByteTag, float> axes;

    FontVariationSettings() = default;

    // https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_fonts/Variable_fonts_guide#weight
    void weight(float value)
    {
        axes.set(MakeFourByteTag('w', 'g', 'h', 't'), value);
    }

    // https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_fonts/Variable_fonts_guide#width
    void width(float value)
    {
        axes.set(MakeFourByteTag('w', 'd', 't', 'h'), value);
    }

    bool is_empty() const { return axes.is_empty(); }

    Vector<FontVariationAxis> to_sorted_list() const
    {
        Vector<FontVariationAxis> list;
        for (auto const& entry : axes)
            list.append(FontVariationAxis(entry.key, entry.value));

        quick_sort(list, [](auto const& a, auto const& b) {
            return a.tag < b.tag;
        });

        return list;
    }

Review comment (suggestion):
Link to a spec where possible. (MDN is a guide but not a spec.) From that page I found [this](https://learn.microsoft.com/en-us/typography/opentype/spec/dvaraxisreg#registered-axis-tags) which has individual pages on some registered axes, so you could link to those.

Code after:

    FontVariationAxis(FourCC t, float v)
        : tag(t)
        , value(v)
    {
    }

    bool operator==(FontVariationAxis const& other) const
    {
        return tag == other.tag && value == other.value;
    }
};

// FIXME: Support other named axes like 'slnt', 'ital', 'opsz', 'GRAD', etc.
struct FontVariationSettings {
    HashMap<FourCC, float> axes;

    FontVariationSettings() = default;

    // https://learn.microsoft.com/en-us/typography/opentype/spec/dvaraxistag_wght
    void set_weight(float value)
    {
        axes.set(FourCC("wght"), value);
    }

    // https://learn.microsoft.com/en-us/typography/opentype/spec/dvaraxistag_wdth
    void set_width(float value)
    {
        axes.set(FourCC("wdth"), value);
    }

    bool is_empty() const { return axes.is_empty(); }

    Vector<FontVariationAxis> to_sorted_list() const
    {
        Vector<FontVariationAxis> list;
        list.ensure_capacity(axes.size());

        for (auto const& entry : axes)
            list.unchecked_append(FontVariationAxis(entry.key, entry.value));

        quick_sort(list, [](auto const& a, auto const& b) {
            return a.tag < b.tag;
        });

        return list;
    }
};

}
Repository: LadybirdBrowser/ladybird
File: Libraries/LibGfx/Font/TypefaceSkia.h (C/C++)

Code before:
/*
 * Copyright (c) 2024, Aliaksandr Kalenik <kalenik.aliaksandr@gmail.com>
 *
 * SPDX-License-Identifier: BSD-2-Clause
 */

#pragma once

#include <LibGfx/Font/Typeface.h>
#include <core/SkStream.h>
#include <include/core/SkTypes.h>

namespace Gfx {

class TypefaceSkia : public Gfx::Typeface {
    AK_MAKE_NONCOPYABLE(TypefaceSkia);

public:
    static ErrorOr<NonnullRefPtr<TypefaceSkia>> load_from_buffer(ReadonlyBytes, int index = 0);
    RefPtr<TypefaceSkia> clone_with_variations(Vector<FontVariationAxis> const& axes) const;

    virtual u32 glyph_count() const override;
    virtual u16 units_per_em() const override;
    virtual u32 glyph_id_for_code_point(u32 code_point) const override;
    virtual FlyString const& family() const override;
    virtual u16 weight() const override;
    virtual u16 width() const override;
    virtual u8 slope() const override;

    virtual ReadonlyBytes buffer() const LIFETIME_BOUND override { return m_buffer; }
    virtual unsigned ttc_index() const override { return m_ttc_index; }

    SkTypeface const* sk_typeface() const;

private:
    struct Impl;

Review comment (suggestion):
could we rearrange things so that Skia headers are included only from .cpp files? otherwise, we end up leaking Skia definitions, which is something we want to avoid.

Code after:
/*
 * Copyright (c) 2024, Aliaksandr Kalenik <kalenik.aliaksandr@gmail.com>
 *
 * SPDX-License-Identifier: BSD-2-Clause
 */

#pragma once

#include <LibGfx/Font/Typeface.h>

namespace Gfx {

class TypefaceSkia : public Gfx::Typeface {
    AK_MAKE_NONCOPYABLE(TypefaceSkia);

public:
    static ErrorOr<NonnullRefPtr<TypefaceSkia>> load_from_buffer(ReadonlyBytes, int index = 0);
    RefPtr<TypefaceSkia const> clone_with_variations(Vector<FontVariationAxis> const& axes) const;

    virtual u32 glyph_count() const override;
    virtual u16 units_per_em() const override;
    virtual u32 glyph_id_for_code_point(u32 code_point) const override;
    virtual FlyString const& family() const override;
    virtual u16 weight() const override;
    virtual u16 width() const override;
    virtual u8 slope() const override;

    virtual ReadonlyBytes buffer() const LIFETIME_BOUND override { return m_buffer; }
    virtual unsigned ttc_index() const override { return m_ttc_index; }

    SkTypeface const* sk_typeface() const;

private:
    struct Impl;
    Impl& impl() const { return *m_impl; }
    NonnullOwnPtr<Impl> m_impl;
Repository: LadybirdBrowser/ladybird
File: Libraries/LibGfx/FourCC.h (C/C++)

Code before:
        result.cc[1] = static_cast<char>(value >> 16);
        result.cc[2] = static_cast<char>(value >> 8);
        result.cc[3] = static_cast<char>(value);
        return result;
    }

    bool operator==(FourCC const&) const = default;
    bool operator!=(FourCC const&) const = default;

    u32 to_u32() const
    {
        return (static_cast<u8>(cc[0]) << 24)
            | (static_cast<u8>(cc[1]) << 16)
            | (static_cast<u8>(cc[2]) << 8)
            | static_cast<u8>(cc[3]);
    }

    bool operator<(FourCC const& other) const
    {
        for (int i = 0; i < 4; ++i) {
            unsigned char a = static_cast<unsigned char>(cc[i]);
            unsigned char b = static_cast<unsigned char>(other.cc[i]);
            if (cc[i] != other.cc[i])
                return a < b;
        }
        return false;
    }

    char cc[4];
};

}

template<>
struct AK::Traits<Gfx::FourCC> : public AK::DefaultTraits<Gfx::FourCC> {
    static unsigned hash(Gfx::FourCC const& key)
    {
        return key.to_u32();
    }
};


Review comment (suggestion):
might be a silly question: would it work if you compared it like so?

```suggestion
		return this.to_u32() < other.to_u32();
```

Code after:
        result.cc[1] = static_cast<char>(value >> 16);
        result.cc[2] = static_cast<char>(value >> 8);
        result.cc[3] = static_cast<char>(value);
        return result;
    }

    bool operator==(FourCC const&) const = default;
    bool operator!=(FourCC const&) const = default;

    u32 to_u32() const
    {
        return (static_cast<u8>(cc[0]) << 24)
            | (static_cast<u8>(cc[1]) << 16)
            | (static_cast<u8>(cc[2]) << 8)
            | static_cast<u8>(cc[3]);
    }

    bool operator<(FourCC const& other) const
    {
        return this->to_u32() < other.to_u32();
    }

    char cc[4];
};

}

template<>
struct AK::Traits<Gfx::FourCC> : public AK::DefaultTraits<Gfx::FourCC> {
    static unsigned hash(Gfx::FourCC const& key)
    {
        return key.to_u32();
    }
};
Repository: LadybirdBrowser/ladybird
File: Libraries/LibGfx/Font/FontVariationSettings.h (C/C++)

Code before:
    }
};

// FIXME: Support other named axes like 'slnt', 'ital', 'opsz', 'GRAD', etc.
struct FontVariationSettings {
    HashMap<FourCC, float> axes;

    FontVariationSettings() = default;

    // https://learn.microsoft.com/en-us/typography/opentype/spec/dvaraxistag_wght
    void set_weight(float value)
    {
        axes.set(FourCC("wght"), value);
    }

    // https://learn.microsoft.com/en-us/typography/opentype/spec/dvaraxistag_wdth
    void set_width(float value)
    {
        axes.set(FourCC("wdth"), value);
    }

    bool is_empty() const { return axes.is_empty(); }

    Vector<FontVariationAxis> to_sorted_list() const
    {
        Vector<FontVariationAxis> list;
        for (auto const& entry : axes)
            list.append(FontVariationAxis(entry.key, entry.value));

        quick_sort(list, [](auto const& a, auto const& b) {
            return a.tag < b.tag;
        });

        return list;
    }
};

}


Review comment (suggestion):
```cpp
list.ensure_capacity(axes.size());
//...
list.unchecked_append(..);
```

Code after:
    }
};

// FIXME: Support other named axes like 'slnt', 'ital', 'opsz', 'GRAD', etc.
struct FontVariationSettings {
    HashMap<FourCC, float> axes;

    FontVariationSettings() = default;

    // https://learn.microsoft.com/en-us/typography/opentype/spec/dvaraxistag_wght
    void set_weight(float value)
    {
        axes.set(FourCC("wght"), value);
    }

    // https://learn.microsoft.com/en-us/typography/opentype/spec/dvaraxistag_wdth
    void set_width(float value)
    {
        axes.set(FourCC("wdth"), value);
    }

    bool is_empty() const { return axes.is_empty(); }

    Vector<FontVariationAxis> to_sorted_list() const
    {
        Vector<FontVariationAxis> list;
        list.ensure_capacity(axes.size());

        for (auto const& entry : axes)
            list.unchecked_append(FontVariationAxis(entry.key, entry.value));

        quick_sort(list, [](auto const& a, auto const& b) {
            return a.tag < b.tag;
        });

        return list;
    }
};

}
Repository: LadybirdBrowser/ladybird
File: AK/Random.h (C/C++)

Code before:
inline T crypto_random()
{
    T t;
    crypto_randombytes_buf({ &t, sizeof(T) });
    return t;
}

u32 crypto_random_uniform(u32 max_bounds);
u64 crypto_random_uniform_64(u64 max_bounds);

template<typename Collection>
inline void shuffle(Collection& collection)
{
    // Fisher-Yates shuffle
    for (size_t i = collection.size() - 1; i >= 1; --i) {
        size_t j = crypto_random_uniform(i + 1);
        AK::swap(collection[i], collection[j]);
    }
}

}

#if USING_AK_GLOBALLY
using AK::crypto_random;
using AK::crypto_random_uniform;
using AK::crypto_randombytes_buf;
using AK::shuffle;
#endif


Review comment (performance):
Imo, the old names were much better. And if our default random is secure (which it should be - no need to use a less secure RNG for anything), we don't necessarily need to reflect that in the name. A higher performance RNG could be named as such to make its intention clear.

Code after:
inline T get_random()
{
    T t;
    fill_with_random({ &t, sizeof(T) });
    return t;
}

u32 get_random_uniform(u32 max_bounds);
u64 get_random_uniform_64(u64 max_bounds);

// http://vigna.di.unimi.it/ftp/papers/xorshiftplus.pdf
class XorShift128PlusRNG {
public:
    XorShift128PlusRNG();
    double get();

private:
    u64 splitmix64(u64& state);
    u64 advance();
    u64 m_low { 0 };
    u64 m_high { 0 };
};

template<typename Collection>
inline void shuffle(Collection& collection)
{
    // Fisher-Yates shuffle
    for (size_t i = collection.size() - 1; i >= 1; --i) {
        size_t j = get_random_uniform(i + 1);
        AK::swap(collection[i], collection[j]);
    }
}

}

#if USING_AK_GLOBALLY
using AK::fill_with_random;
using AK::get_random;
using AK::get_random_uniform;
using AK::shuffle;
using AK::XorShift128PlusRNG;
#endif
Repository: LadybirdBrowser/ladybird
File: Tests/AK/TestRandom.cpp (C++)

Code before:
 *
 * SPDX-License-Identifier: BSD-2-Clause
 */

#include <LibTest/TestCase.h>

#include <AK/ByteBuffer.h>
#include <AK/Random.h>

TEST_CASE(csprng_generates_unique_values)
{
    ByteBuffer first_buffer = MUST(ByteBuffer::create_zeroed(100));
    ByteBuffer second_buffer = MUST(ByteBuffer::create_zeroed(100));

    AK::fill_with_random(first_buffer);
    AK::fill_with_random(second_buffer);

    u32 first_u32 = get_random_uniform(100);
    u32 second_u32 = get_random_uniform(100);

    u64 first_u64 = AK::get_random_uniform_64(100);
    u64 second_u64 = AK::get_random_uniform_64(100);

    EXPECT_NE(first_buffer, second_buffer);
    EXPECT_NE(first_u32, second_u32);
    EXPECT_NE(first_u64, second_u64);
}


Review comment (bug):
This seems very flaky - isn't there an approx. 2% chance that either of these will fail during an average run? You're expecting two random numbers from 0 to 99 to not be equal to each other.

You could increase the range by a lot _and_ allow for accidental failures up to a certain threshold before definitively failing the test.

Code after:
 *
 * SPDX-License-Identifier: BSD-2-Clause
 */

#include <LibTest/TestCase.h>

#include <AK/ByteBuffer.h>
#include <AK/Random.h>

TEST_CASE(csprng_generates_unique_values)
{
    constexpr size_t size = 800;
    constexpr size_t max_failures = 3;
    size_t failures = 0;

    for (size_t i = 0; i < 3; i++) {
        ByteBuffer first_buffer = MUST(ByteBuffer::create_zeroed(size));
        ByteBuffer second_buffer = MUST(ByteBuffer::create_zeroed(size));

        AK::fill_with_random(first_buffer);
        AK::fill_with_random(second_buffer);

        u32 first_u32 = AK::get_random_uniform(size);
        u32 second_u32 = AK::get_random_uniform(size);

        u64 first_u64 = AK::get_random_uniform_64(size);
        u64 second_u64 = AK::get_random_uniform_64(size);

        if (first_buffer == second_buffer)
            failures++;

        if (first_u32 == second_u32)
            failures++;

        if (first_u64 == second_u64)
            failures++;
    }

    EXPECT(failures < max_failures);
}
Repository: LadybirdBrowser/ladybird
File: Services/RequestServer/RequestPipe.h (C/C++)

Code before:
/*
 * Copyright (c) 2025, ayeteadoe <ayeteadoe@gmail.com>
 *
 * SPDX-License-Identifier: BSD-2-Clause
 */

#pragma once

#include <AK/RefCounted.h>

namespace RequestServer {

class RequestPipe : public RefCounted<RequestPipe> {
public:
    static ErrorOr<NonnullRefPtr<RequestPipe>> try_create();

    ~RequestPipe();

    int reader_fd() const { return m_reader_fd; }
    int writer_fd() const { return m_writer_fd; }

    ErrorOr<ssize_t> write(Vector<u8> const& bytes);

private:
    explicit RequestPipe(int reader_fd, int writer_fd)
        : m_reader_fd(reader_fd)
        , m_writer_fd(writer_fd)
    {
        VERIFY(m_reader_fd >= 0);
        VERIFY(m_writer_fd >= 0);
    }

    int m_reader_fd { -1 };
    int m_writer_fd { -1 };
};

}


Review comment (suggestion):
Let's take this parameter as:
```c++
ErrorOr<ssize_t> write(ReadonlyBytes bytes);
```

Code after:
/*
 * Copyright (c) 2025, ayeteadoe <ayeteadoe@gmail.com>
 *
 * SPDX-License-Identifier: BSD-2-Clause
 */

#pragma once

#include <AK/Span.h>

namespace RequestServer {

class RequestPipe {
    AK_MAKE_NONCOPYABLE(RequestPipe);

public:
    RequestPipe(RequestPipe&& other);
    RequestPipe& operator=(RequestPipe&& other);
    ~RequestPipe();

    static ErrorOr<RequestPipe> create();

    int reader_fd() const { return m_reader_fd; }
    int writer_fd() const { return m_writer_fd; }

    ErrorOr<ssize_t> write(ReadonlyBytes bytes);

private:
    RequestPipe(int reader_fd, int writer_fd);

    int m_reader_fd { -1 };
    int m_writer_fd { -1 };
};

}
Repository: LadybirdBrowser/ladybird
File: UI/AppKit/Interface/LadybirdWebView.mm (Objective-C++)

Code before:
- (BOOL)performKeyEquivalent:(NSEvent*)event
{
    if ([event window] != [self window]) {
        return NO;
    }
    if ([[self window] firstResponder] != self) {
        return NO;
    }
    if (self.event_being_redispatched == event) {
        return NO;
    }

    [self keyDown:event];
    return YES;
}

- (void)keyDown:(NSEvent*)event
{
    if (self.event_being_redispatched == event) {
        return;
    }

    [self interpretKeyEvents:@[ event ]];
}

- (void)insertText:(id)string replacementRange:(NSRange)replacementRange
{
    NSEvent* event = [NSApp currentEvent];
    if (event && event.type == NSEventTypeKeyDown) {
        auto key_event = Ladybird::ns_event_to_key_event(Web::KeyEvent::Type::KeyDown, event);
        m_web_view_bridge->enqueue_input_event(move(key_event));
    }
}

- (void)doCommandBySelector:(SEL)selector
{
    NSEvent* event = [NSApp currentEvent];
    if (event && event.type == NSEventTypeKeyDown) {
        auto key_event = Ladybird::ns_event_to_key_event(Web::KeyEvent::Type::KeyDown, event);
        m_web_view_bridge->enqueue_input_event(move(key_event));
    }
}

- (BOOL)acceptsFirstResponder
{
    return YES;
}

- (BOOL)canBecomeKeyView
{
    return YES;

Review comment (suggestion):
Organizationally, instead of putting these methods in between the `keyDown` and `keyUp` overrides, let's move them to their own section below prefixed with a mark:

```c++
#pragma mark - NSTextInputClient
```

The mark just makes it clearer which methods belong to which interface, and marks show up in code navigation in some IDEs

Code after:
- (BOOL)performKeyEquivalent:(NSEvent*)event
{
    if ([event window] != [self window]) {
        return NO;
    }
    if ([[self window] firstResponder] != self) {
        return NO;
    }
    if (self.event_being_redispatched == event) {
        return NO;
    }

    [self keyDown:event];
    return YES;
}

- (void)keyDown:(NSEvent*)event
{
    if (self.event_being_redispatched == event) {
        return;
    }

    [self interpretKeyEvents:@[ event ]];
}

- (void)keyUp:(NSEvent*)event
{
    if (self.event_being_redispatched == event) {
        return;
    }

    auto key_event = Ladybird::ns_event_to_key_event(Web::KeyEvent::Type::KeyUp, event);
    m_web_view_bridge->enqueue_input_event(move(key_event));
}

- (void)flagsChanged:(NSEvent*)event
{
    if (self.event_being_redispatched == event) {
        return;
    }

    auto enqueue_event_if_needed = [&](auto flag) {
        auto is_flag_set = [&](auto flags) { return (flags & flag) != 0; };
        Web::KeyEvent::Type type;

        if (is_flag_set(event.modifierFlags) && !is_flag_set(m_modifier_flags)) {
            type = Web::KeyEvent::Type::KeyDown;
        } else if (!is_flag_set(event.modifierFlags) && is_flag_set(m_modifier_flags)) {
            type = Web::KeyEvent::Type::KeyUp;
        } else {
            return;
Repository: LadybirdBrowser/ladybird
File: UI/AppKit/Interface/LadybirdWebView.mm (Objective-C++)

Code before:
    if (self.event_being_redispatched == event) {
        return;
    }

    [self interpretKeyEvents:@[ event ]];
}

- (void)insertText:(id)string replacementRange:(NSRange)replacementRange
{
    NSEvent* event = [NSApp currentEvent];
    if (event && event.type == NSEventTypeKeyDown) {
        auto key_event = Ladybird::ns_event_to_key_event(Web::KeyEvent::Type::KeyDown, event);
        m_web_view_bridge->enqueue_input_event(move(key_event));
    }
}

- (void)doCommandBySelector:(SEL)selector
{
    NSEvent* event = [NSApp currentEvent];
    if (event && event.type == NSEventTypeKeyDown) {
        auto key_event = Ladybird::ns_event_to_key_event(Web::KeyEvent::Type::KeyDown, event);
        m_web_view_bridge->enqueue_input_event(move(key_event));
    }
}

- (BOOL)acceptsFirstResponder
{
    return YES;
}

- (BOOL)canBecomeKeyView
{
    return YES;
}

- (BOOL)hasMarkedText
{
    return NO;
}

- (NSRange)markedRange
{
    return NSMakeRange(NSNotFound, 0);
}

- (NSRange)selectedRange
{
    return NSMakeRange(NSNotFound, 0);
}

- (void)setMarkedText:(id)string selectedRange:(NSRange)selectedRange replacementRange:(NSRange)replacementRange

Review comment (suggestion):
(Though some of these don't seem to be `NSTextInputClient` thus should be in their own mark - looks like `NSResponder` here)

Code after:
    if (self.event_being_redispatched == event) {
        return;
    }

    [self interpretKeyEvents:@[ event ]];
}

- (void)keyUp:(NSEvent*)event
{
    if (self.event_being_redispatched == event) {
        return;
    }

    auto key_event = Ladybird::ns_event_to_key_event(Web::KeyEvent::Type::KeyUp, event);
    m_web_view_bridge->enqueue_input_event(move(key_event));
}

- (void)flagsChanged:(NSEvent*)event
{
    if (self.event_being_redispatched == event) {
        return;
    }

    auto enqueue_event_if_needed = [&](auto flag) {
        auto is_flag_set = [&](auto flags) { return (flags & flag) != 0; };
        Web::KeyEvent::Type type;

        if (is_flag_set(event.modifierFlags) && !is_flag_set(m_modifier_flags)) {
            type = Web::KeyEvent::Type::KeyDown;
        } else if (!is_flag_set(event.modifierFlags) && is_flag_set(m_modifier_flags)) {
            type = Web::KeyEvent::Type::KeyUp;
        } else {
            return;
        }

        auto key_event = Ladybird::ns_event_to_key_event(type, event);
        m_web_view_bridge->enqueue_input_event(move(key_event));
    };

    enqueue_event_if_needed(NSEventModifierFlagShift);
    enqueue_event_if_needed(NSEventModifierFlagControl);
    enqueue_event_if_needed(NSEventModifierFlagOption);
    enqueue_event_if_needed(NSEventModifierFlagCommand);

    m_modifier_flags = event.modifierFlags;
}

- (void)cursorUpdate:(NSEvent*)event
{
    // The NSApp will override the custom cursor set from on_cursor_change when the view hierarchy changes in some way
    // (such as when we show self.status_label on link hover). Overriding this method with an empty implementation will