Bug 138755 – [CSS Font Loading] Switch to dispatching events asynchronously

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Bear Travis]

The FontLoader code should be refactored to take advantage of EventSender rather than maintaining its own event dispatch timing logic.

[Bear Travis]

Created attachment 241625 [details]
Initial Patch

[Bear Travis]

I was originally going to use EventSender, but it was not really designed for this use case. Would ideally be able to reuse something like the GenericEventQueue, but I need to fire the callbacks at the appropriate time as well, so I'm bundling the timer logic into FontLoader.

[Bear Travis]

Created attachment 241734 [details]
Modified Patch

[Bear Travis]

Created attachment 241740 [details]
Updated Patch

[Bear Travis]

Created attachment 241802 [details]
Updated Patch

[Bear Travis]

Created attachment 241913 [details]
Modifying timer

[Simon Fraser (smfr)]

Comment on attachment 241913 [details]
Modifying timer

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

> Source/WebCore/css/FontLoader.cpp:172
>      Vector<RefPtr<Event> > pendingEvents;

No need for the space between > and >

> Source/WebCore/css/FontLoader.cpp:185
> +        Vector<RefPtr<VoidCallback> > callbacks;

No need for space between > and >

> Source/WebCore/css/FontLoader.h:98
> +    void timerFired(Timer&) { firePendingEvents(); }

Please rename to pendingEventsTimerFired().

> Source/WebCore/css/FontLoader.h:110
> +    std::unique_ptr<Timer> m_pendingEventTimer;

We normally just store Timers by value.

[Bear Travis]

Created attachment 241970 [details]
testing DRT fix

[Bear Travis]

Comment on attachment 241970 [details]
testing DRT fix

This probably needs to be broken up for the DRT fix on Windows, but I want to make sure it passes the bots first. So not yet ready for review.

[Bear Travis]

Created attachment 241971 [details]
Incorporating feedback

[Simon Fraser (smfr)]

Comment on attachment 241971 [details]
Incorporating feedback

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

> Source/WebCore/ChangeLog:10
> +        to EventSender or the GenericEventQueue. Several bugs have popped
> +        up in the past because FontLoader has been sending events during

Would be nice to add a testcase for that specific behavior (since it's very bad if it happens).

[Bear Travis]

Created attachment 241998 [details]
Updated Patch, including test

Adding one of the test cases that caused the need for this refactoring.

[WebKit Commit Bot]

Comment on attachment 241998 [details]
Updated Patch, including test

Clearing flags on attachment: 241998

Committed r176453: <http://trac.webkit.org/changeset/176453>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.