[webkit-reviews] review granted: [Bug 226692] Reduce use of legacy EventLoopEventQueue and EventLoopTaskQueue : [Attachment 430674] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Jun 5 22:30:29 PDT 2021

Darin Adler <darin at apple.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 226692: Reduce use of legacy EventLoopEventQueue and EventLoopTaskQueue

Attachment 430674: Patch


--- Comment #5 from Darin Adler <darin at apple.com> ---
Comment on attachment 430674
  --> https://bugs.webkit.org/attachment.cgi?id=430674

View in context: https://bugs.webkit.org/attachment.cgi?id=430674&action=review

> Source/WebCore/Modules/remoteplayback/RemotePlayback.cpp:293
> +    queueTaskToDispatchEvent(*this, TaskSource::MediaElement,

The old code would silently do nothing if m_state had a bad value. I think this
one will crash. Not that a bad value is really possible, but it is a change in

Also, if we are going to refactor to share more code like this, why not have
the switch statement just put the event name into an AtomString or const
AtomString*, and share the call to Event::create too?

> Source/WebCore/dom/FullscreenManager.cpp:68
> +		   weakThis->dispatchFullscreenChangeEvents();

I’d think that generally we’d want to do something more like:


I will guess that there is no practical real issue in this case, but our object
lifetime strategy is normally to say the caller should be holding a reference
to objects before calling functions on them. I’d like these rules to be easier
to understand and less situational, but this is what I’ve gleaned from talking
to others about our lifetime strategy.

More information about the webkit-reviews mailing list