Bug 214340 - There should be only one RunLoop Timer class
Summary: There should be only one RunLoop Timer class
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Geoffrey Garen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-07-14 20:48 PDT by Geoffrey Garen
Modified: 2020-07-19 18:23 PDT (History)
14 users (show)

See Also:


Attachments
WIP (7.49 KB, patch)
2020-07-14 20:50 PDT, Geoffrey Garen
no flags Details | Formatted Diff | Diff
WIP (12.18 KB, patch)
2020-07-15 16:31 PDT, Geoffrey Garen
no flags Details | Formatted Diff | Diff
WIP (18.74 KB, patch)
2020-07-16 09:49 PDT, Geoffrey Garen
no flags Details | Formatted Diff | Diff
WIP (18.94 KB, patch)
2020-07-16 10:48 PDT, Geoffrey Garen
no flags Details | Formatted Diff | Diff
WIP (19.02 KB, patch)
2020-07-16 13:00 PDT, Geoffrey Garen
no flags Details | Formatted Diff | Diff
WIP (34.45 KB, patch)
2020-07-17 11:06 PDT, Geoffrey Garen
no flags Details | Formatted Diff | Diff
Patch (37.76 KB, patch)
2020-07-17 20:37 PDT, Geoffrey Garen
no flags Details | Formatted Diff | Diff
Patch (37.76 KB, patch)
2020-07-17 20:42 PDT, Geoffrey Garen
darin: review+
ggaren: commit-queue?
Details | Formatted Diff | Diff
Patch for landing (37.75 KB, patch)
2020-07-19 17:29 PDT, Geoffrey Garen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Geoffrey Garen 2020-07-14 20:48:10 PDT
There should only be one RunLoop Timer class
Comment 1 Geoffrey Garen 2020-07-14 20:50:08 PDT
Created attachment 404318 [details]
WIP
Comment 2 Geoffrey Garen 2020-07-15 16:31:17 PDT
Created attachment 404404 [details]
WIP
Comment 3 Geoffrey Garen 2020-07-16 09:49:36 PDT
Created attachment 404456 [details]
WIP
Comment 4 Geoffrey Garen 2020-07-16 10:48:47 PDT
Created attachment 404463 [details]
WIP
Comment 5 Geoffrey Garen 2020-07-16 13:00:35 PDT
Created attachment 404477 [details]
WIP
Comment 6 Geoffrey Garen 2020-07-17 11:06:51 PDT
Created attachment 404575 [details]
WIP
Comment 7 Geoffrey Garen 2020-07-17 20:37:13 PDT
Created attachment 404632 [details]
Patch
Comment 8 Geoffrey Garen 2020-07-17 20:42:03 PDT
Created attachment 404633 [details]
Patch
Comment 9 Darin Adler 2020-07-19 11:20:04 PDT
Comment on attachment 404633 [details]
Patch

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

Wish there was slightly more abstraction so we didn’t have to say COCOA_EVENT_LOOP all the time.

> Source/WTF/wtf/RunLoop.h:53
> +typedef HashSet<RefPtr<SchedulePair>, SchedulePairHash> SchedulePairHashSet;

In new code we should use "using" instead of typedef.

> Source/WTF/wtf/cf/RunLoopCF.cpp:94
> +        Function<void()> function(static_cast<Function<void()>::Impl*>(context));

Kind of wish this function constructor was named since it takes ownership. A sort of asymmetry with leakImpl.

> Source/WTF/wtf/cf/RunLoopCF.cpp:122
> +        TimerBase* timer = static_cast<TimerBase*>(context);

auto?
Comment 10 Geoffrey Garen 2020-07-19 17:22:03 PDT
> Wish there was slightly more abstraction so we didn’t have to say
> COCOA_EVENT_LOOP all the time.

Yeah, me too. I'm going to think about this more.

> > Source/WTF/wtf/RunLoop.h:53
> > +typedef HashSet<RefPtr<SchedulePair>, SchedulePairHash> SchedulePairHashSet;
> 
> In new code we should use "using" instead of typedef.

Fixed.

> > Source/WTF/wtf/cf/RunLoopCF.cpp:94
> > +        Function<void()> function(static_cast<Function<void()>::Impl*>(context));
> 
> Kind of wish this function constructor was named since it takes ownership. A
> sort of asymmetry with leakImpl.

I'll take a look at making the constructor private and adding a friend adoptImpl function.

> > Source/WTF/wtf/cf/RunLoopCF.cpp:122
> > +        TimerBase* timer = static_cast<TimerBase*>(context);
> 
> auto?

Fixed.
Comment 11 Geoffrey Garen 2020-07-19 17:29:28 PDT
Created attachment 404690 [details]
Patch for landing
Comment 12 EWS 2020-07-19 18:22:02 PDT
Committed r264586: <https://trac.webkit.org/changeset/264586>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 404690 [details].
Comment 13 Radar WebKit Bug Importer 2020-07-19 18:23:15 PDT
<rdar://problem/65801842>