Skip to content

Commit 4fb1ba0

Browse files
ayeteadoegmta
authored andcommitted
LibCore: Remove unused NotifierActivationEvent fd() and type() methods
In 11b8bbe one thing that was claimed was that we now properly set the Notifier's actual fd on the NotifierActivationEvent. It turns out that claim was false because a crucial step was forgotten: actually set the m_notifier_fd when registering. Despite that mistake, it ultimately was irrelevant as the methods on NotifierActivationEvent are currently unused code. We were posting the event to the correct Notifier receiver so the on_activation was still getting invoked. Given they are unused, NotifierActivationEvent can be defined the same way as TimerEvent is, where we just pass the event type enum to the Event base class. Additionally, NotificationType can be moved to the Notifier header as this enum is now always used in the context of creating or using a Notifier instance.
1 parent c34b5a5 commit 4fb1ba0

File tree

7 files changed

+21
-34
lines changed

7 files changed

+21
-34
lines changed

Libraries/LibCore/Event.h

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -71,32 +71,14 @@ class TimerEvent final : public Event {
7171
~TimerEvent() = default;
7272
};
7373

74-
enum class NotificationType : u8 {
75-
None = 0,
76-
Read = 1,
77-
Write = 2,
78-
HangUp = 4,
79-
Error = 8,
80-
};
81-
82-
AK_ENUM_BITWISE_OPERATORS(NotificationType);
83-
8474
class NotifierActivationEvent final : public Event {
8575
public:
86-
explicit NotifierActivationEvent(int fd, NotificationType type)
76+
explicit NotifierActivationEvent()
8777
: Event(Event::NotifierActivation)
88-
, m_fd(fd)
89-
, m_type(type)
9078
{
9179
}
92-
~NotifierActivationEvent() = default;
9380

94-
int fd() const { return m_fd; }
95-
NotificationType type() const { return m_type; }
96-
97-
private:
98-
int m_fd;
99-
NotificationType m_type;
81+
~NotifierActivationEvent() = default;
10082
};
10183

10284
}

Libraries/LibCore/EventLoopImplementationUnix.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,7 @@ void EventLoopManagerUnix::wait_for_events(EventLoopImplementation::PumpMode mod
405405

406406
#ifdef AK_OS_ANDROID
407407
// FIXME: Make the check work under Android, perhaps use ALooper.
408-
ThreadEventQueue::current().post_event(notifier, make<NotifierActivationEvent>(notifier.fd(), notifier.type()));
408+
ThreadEventQueue::current().post_event(notifier, make<NotifierActivationEvent>());
409409
#else
410410
auto revents = thread_data.poll_fds[i].revents;
411411

@@ -422,7 +422,7 @@ void EventLoopManagerUnix::wait_for_events(EventLoopImplementation::PumpMode mod
422422
type &= notifier.type();
423423

424424
if (type != NotificationType::None)
425-
ThreadEventQueue::current().post_event(notifier, make<NotifierActivationEvent>(notifier.fd(), type));
425+
ThreadEventQueue::current().post_event(notifier, make<NotifierActivationEvent>());
426426
#endif
427427
}
428428
}

Libraries/LibCore/EventLoopImplementationWindows.cpp

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -96,13 +96,7 @@ struct EventLoopNotifier final : CompletionPacket {
9696
{
9797
}
9898

99-
Notifier::Type notifier_type() const { return m_notifier_type; }
100-
int notifier_fd() const { return m_notifier_fd; }
101-
102-
// These are a space tradeoff for avoiding a double indirection through the notifier*.
10399
Notifier* notifier;
104-
Notifier::Type m_notifier_type;
105-
int m_notifier_fd { -1 };
106100
OwnHandle wait_packet;
107101
OwnHandle wait_event;
108102
};
@@ -211,7 +205,7 @@ size_t EventLoopImplementationWindows::pump(PumpMode pump_mode)
211205
}
212206
if (packet->type == CompletionType::Notifer) {
213207
auto* notifier_data = static_cast<EventLoopNotifier*>(packet);
214-
event_queue.post_event(*notifier_data->notifier, make<NotifierActivationEvent>(notifier_data->notifier_fd(), notifier_data->notifier_type()));
208+
event_queue.post_event(*notifier_data->notifier, make<NotifierActivationEvent>());
215209
NTSTATUS status = g_system.NtAssociateWaitCompletionPacket(notifier_data->wait_packet.handle, thread_data->iocp.handle, notifier_data->wait_event.handle, notifier_data, NULL, 0, 0, NULL);
216210
VERIFY(NT_SUCCESS(status));
217211
continue;
@@ -279,7 +273,6 @@ void EventLoopManagerWindows::register_notifier(Notifier& notifier)
279273
auto notifier_data = make<EventLoopNotifier>();
280274
notifier_data->type = CompletionType::Notifer;
281275
notifier_data->notifier = &notifier;
282-
notifier_data->m_notifier_type = notifier.type();
283276
notifier_data->wait_event.handle = event;
284277
NTSTATUS status = g_system.NtCreateWaitCompletionPacket(&notifier_data->wait_packet.handle, GENERIC_READ | GENERIC_WRITE, NULL);
285278
VERIFY(NT_SUCCESS(status));

Libraries/LibCore/Notifier.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,16 @@
1313

1414
namespace Core {
1515

16+
enum class NotificationType : u8 {
17+
None = 0,
18+
Read = 1,
19+
Write = 2,
20+
HangUp = 4,
21+
Error = 8,
22+
};
23+
24+
AK_ENUM_BITWISE_OPERATORS(NotificationType);
25+
1626
class Notifier final : public EventReceiver {
1727
C_OBJECT(Notifier);
1828

Libraries/LibWebView/EventLoop/EventLoopImplementationMacOS.mm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ static void socket_notifier(CFSocketRef socket, CFSocketCallBackType notificatio
331331
// before dispatching the event, which allows it to be triggered again.
332332
CFSocketEnableCallBacks(socket, notification_type);
333333

334-
Core::NotifierActivationEvent event(notifier.fd(), notifier.type());
334+
Core::NotifierActivationEvent event;
335335
notifier.dispatch_event(event);
336336

337337
// This manual process of enabling the callbacks also seems to require waking the event loop,

Libraries/LibWebView/EventLoop/EventLoopImplementationQt.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ void EventLoopManagerQt::unregister_timer(intptr_t timer_id)
297297

298298
static void qt_notifier_activated(Core::Notifier& notifier)
299299
{
300-
Core::NotifierActivationEvent event(notifier.fd(), notifier.type());
300+
Core::NotifierActivationEvent event;
301301
notifier.dispatch_event(event);
302302
}
303303

UI/Android/src/main/cpp/ALooperEventLoopImplementation.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,8 +221,10 @@ static int notifier_callback(int fd, int events, void* data)
221221
if (events & ALOOPER_EVENT_ERROR)
222222
type |= Core::NotificationType::Error;
223223

224-
Core::NotifierActivationEvent event(notifier.fd(), type);
225-
notifier.dispatch_event(event);
224+
if (type != Core::NotificationType::None) {
225+
Core::NotifierActivationEvent event;
226+
notifier.dispatch_event(event);
227+
}
226228

227229
// Wake up from ALooper_pollAll, and service this event on the event queue
228230
current_impl().wake();

0 commit comments

Comments
 (0)