Bug 150498 – Make every port implement MainThreadSharedTimer instead of using global functions

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Carlos Garcia Campos]

Keep common code in MainThreadSharedTimer and implement the rest of the members in the platform specific files. It simplifies both the header and the port implementations.

[Carlos Garcia Campos]

Created attachment 263917 [details]
Patch

I'm not setting r? because the windows part is missing. I need help with that because I don't know how to pass the instance pointer to TimerWindowWndProc.

[Carlos Garcia Campos]

I plan to use RunLoop::Timer for the GTK+ implementation, since our RunLoop::Timer implementation is basically the same to what we do in SharedTimerGtk. If this is the case of the other ports we could have a cross-platform implementation of MainThreadSharedTimer.

[Carlos Garcia Campos]

hmm, the idea of moving the shared function to the class as a member to make setFiredFunction doesn't work for CF because of the restartSharedTimer, so I'll leave it as global var in every port file for now.

[Carlos Garcia Campos]

Created attachment 263919 [details]
Updated patch

Switched to use a static member, that should work for windows too.

[Carlos Garcia Campos]

Created attachment 263920 [details]
Try to fix Mac build

[Carlos Garcia Campos]

Created attachment 263922 [details]
Try to fix Mac build

[Carlos Garcia Campos]

Created attachment 263923 [details]
Try to fix Mac build

[Carlos Garcia Campos]

Created attachment 263924 [details]
Try to fix Mac build

Hopefully the last one, sorry for the noise.

[Darin Adler]

Comment on attachment 263924 [details]
Try to fix Mac build

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

This is a little better, although not a huge improvement. I think there is some other strangeness here that can be fixed.

> Source/WebCore/platform/SharedTimer.h:52
>      class MainThreadSharedTimer final : public SharedTimer {

This class uses global state, but instead it should be a singleton. We would replace the mainThreadSharedTimer() function from ThreadTimers with a MainThreadSharedTimer::singleton() function that returns a reference to a single global shared timer.

The code for this should go into MainThreadSharedTimer.cpp and this class should get its own header MainThreadSharedTimer.h instead of being tacked on to the bottom of SharedTimer.h. If you don’t want to add those two new files in this patch, you could instead put the code into ThreadTimers.cpp for now, but that would just be a stopgap until we can do it right.

> Source/WebCore/platform/SharedTimer.h:54
>          void setFiredFunction(void (*function)()) override

This function’s body should not be in the header. Instead it should be in MainThreadSharedTimer.cpp (or ThreadTimers.cpp for now, I guess).

> Source/WebCore/platform/SharedTimer.h:62
> +        void setFireInterval(double) override;
> +        void stop() override;
> +        void invalidate() override;

Can any of these be private?

> Source/WebCore/platform/SharedTimer.h:64
> +        static void timerFired()

This function’s body should not be in the header. Instead it should be in MainThreadSharedTimer.cpp (or ThreadTimers.cpp for now, I guess).

The class is named timer, so I think this should just be named fired(), not timerFired().

This should be a member function, not a static member function; callers can call MainThreadSharedTimer::singleton().fired(). We should also consider making it private if we can. Unfortunately doing that means all the thread-specific callback functions would need to be either static member functions or friend functions so they could access this. But it’s peculiar to have this be public.

> Source/WebCore/platform/SharedTimer.h:70
> +        static void (*s_firedFunction)();

This should be a non-static member, and also should probably be initialized to nullptr here in the header once it is like that.

> Source/WebCore/platform/cf/SharedTimerCF.cpp:41
> +void (*MainThreadSharedTimer::s_firedFunction)() = nullptr;

We don’t want code like this to be repeated for each platform. This is only here because we don’t have a MainThreadSharedTimer.cpp to put it in. For now, I suggest putting code like this in ThreadTimers.cpp or creating MainThreadSharedTimer.cpp. But we won’t need this if we change the function to be a member instead of a static member.

> Source/WebCore/platform/cf/SharedTimerCF.cpp:75
> +    MainThreadSharedTimer::timerFired();

This should be:

    MainThreadSharedTimer::singleton().timerFired();

> Source/WebCore/platform/gtk/SharedTimerGtk.cpp:40
> +    ASSERT(MainThreadSharedTimer::s_firedFunction);

No need to repeat the class name when we are inside a member function of that class.

> Source/WebCore/platform/gtk/SharedTimerGtk.cpp:44
> +    gSharedTimer.scheduleAfterDelay("[WebKit] sharedTimerTimeoutCallback", [this] { MainThreadSharedTimer::timerFired(); },

No need to repeat the class name when we are inside a member function of that class.

> Source/WebCore/platform/win/SharedTimerWin.cpp:128
> +    ASSERT(MainThreadSharedTimer::s_firedFunction);

No need to repeat the class name when we are inside a member function of that class.

[Carlos Garcia Campos]

(In reply to comment #9)
> Comment on attachment 263924 [details]
> Try to fix Mac build
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=263924&action=review
> 
> This is a little better, although not a huge improvement. I think there is
> some other strangeness here that can be fixed.

Thanks for the review.

> > Source/WebCore/platform/SharedTimer.h:52
> >      class MainThreadSharedTimer final : public SharedTimer {
> 
> This class uses global state, but instead it should be a singleton. We would
> replace the mainThreadSharedTimer() function from ThreadTimers with a
> MainThreadSharedTimer::singleton() function that returns a reference to a
> single global shared timer.
> 
> The code for this should go into MainThreadSharedTimer.cpp and this class
> should get its own header MainThreadSharedTimer.h instead of being tacked on
> to the bottom of SharedTimer.h. If you don’t want to add those two new files
> in this patch, you could instead put the code into ThreadTimers.cpp for now,
> but that would just be a stopgap until we can do it right.

I don't mind to add new files as long as someone can help me with the XCode files.

> > Source/WebCore/platform/SharedTimer.h:54
> >          void setFiredFunction(void (*function)()) override
> 
> This function’s body should not be in the header. Instead it should be in
> MainThreadSharedTimer.cpp (or ThreadTimers.cpp for now, I guess).

Ok.

> > Source/WebCore/platform/SharedTimer.h:62
> > +        void setFireInterval(double) override;
> > +        void stop() override;
> > +        void invalidate() override;
> 
> Can any of these be private?

No, all of them are used by ThreadTimers.

> > Source/WebCore/platform/SharedTimer.h:64
> > +        static void timerFired()
> 
> This function’s body should not be in the header. Instead it should be in
> MainThreadSharedTimer.cpp (or ThreadTimers.cpp for now, I guess).
> 
> The class is named timer, so I think this should just be named fired(), not
> timerFired().

Sure.

> This should be a member function, not a static member function; callers can
> call MainThreadSharedTimer::singleton().fired(). We should also consider
> making it private if we can. Unfortunately doing that means all the
> thread-specific callback functions would need to be either static member
> functions or friend functions so they could access this. But it’s peculiar
> to have this be public.

Yes, making it private would require changes in CF and win implementations that I'm afraid I can't do my self easily. So, I'll leave a FIXME.

> > Source/WebCore/platform/SharedTimer.h:70
> > +        static void (*s_firedFunction)();
> 
> This should be a non-static member, and also should probably be initialized
> to nullptr here in the header once it is like that.

Yes, and not a pointer function but a std::function instead as well, I guess.

> > Source/WebCore/platform/cf/SharedTimerCF.cpp:41
> > +void (*MainThreadSharedTimer::s_firedFunction)() = nullptr;
> 
> We don’t want code like this to be repeated for each platform. This is only
> here because we don’t have a MainThreadSharedTimer.cpp to put it in. For
> now, I suggest putting code like this in ThreadTimers.cpp or creating
> MainThreadSharedTimer.cpp. But we won’t need this if we change the function
> to be a member instead of a static member.

Right.

> > Source/WebCore/platform/cf/SharedTimerCF.cpp:75
> > +    MainThreadSharedTimer::timerFired();
> 
> This should be:
> 
>     MainThreadSharedTimer::singleton().timerFired();
> 
> > Source/WebCore/platform/gtk/SharedTimerGtk.cpp:40
> > +    ASSERT(MainThreadSharedTimer::s_firedFunction);
> 
> No need to repeat the class name when we are inside a member function of
> that class.
> 
> > Source/WebCore/platform/gtk/SharedTimerGtk.cpp:44
> > +    gSharedTimer.scheduleAfterDelay("[WebKit] sharedTimerTimeoutCallback", [this] { MainThreadSharedTimer::timerFired(); },
> 
> No need to repeat the class name when we are inside a member function of
> that class.
> 
> > Source/WebCore/platform/win/SharedTimerWin.cpp:128
> > +    ASSERT(MainThreadSharedTimer::s_firedFunction);
> 
> No need to repeat the class name when we are inside a member function of
> that class.

Ok, I'll submit a new patch adding all new files.

[Carlos Garcia Campos]

Created attachment 264004 [details]
New patch

It still needs the changes in the XCode files. Could someone with a mac add them, please?

[WebKit Commit Bot]

Attachment 264004 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:72:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:73:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:113:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/ThreadTimers.cpp:73:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 5 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Carlos Garcia Campos]

Created attachment 264005 [details]
Fix EFL build

[WebKit Commit Bot]

Attachment 264005 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:72:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:73:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:113:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/ThreadTimers.cpp:73:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 5 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Carlos Garcia Campos]

Created attachment 264007 [details]
Try to fix win build

It seems windows needs functional to be included

[WebKit Commit Bot]

Attachment 264007 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:72:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:73:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:113:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/ThreadTimers.cpp:73:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 5 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Carlos Garcia Campos]

I don't understand the windows build error:

C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\DerivedSources\ForwardingHeaders\wtf/NeverDestroyed.h(53): error C2248: 'WebCore::MainThreadSharedTimer::MainThreadSharedTimer': cannot access private member declared in class 'WebCore::MainThreadSharedTimer' (compiling source file C:\cygwin\home\buildbot\WebKit\Source\WebCore\platform\MainThreadSharedTimer.cpp) [C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Source\WebCore\WebCore.vcxproj]
  C:\cygwin\home\buildbot\WebKit\Source\WebCore\platform\MainThreadSharedTimer.cpp(37): note: see declaration of 'WebCore::MainThreadSharedTimer::MainThreadSharedTimer' (compiling source file C:\cygwin\home\buildbot\WebKit\Source\WebCore\platform\MainThreadSharedTimer.cpp)
  c:\cygwin\home\buildbot\webkit\source\webcore\platform\MainThreadSharedTimer.h(35): note: see declaration of 'WebCore::MainThreadSharedTimer' (compiling source file C:\cygwin\home\buildbot\WebKit\Source\WebCore\platform\MainThreadSharedTimer.cpp)
  C:\cygwin\home\buildbot\WebKit\Source\WebCore\platform\MainThreadSharedTimer.cpp(33): note: see reference to function template instantiation 'WTF::NeverDestroyed<WebCore::MainThreadSharedTimer>::NeverDestroyed<>(void)' being compiled
  C:\cygwin\home\buildbot\WebKit\Source\WebCore\platform\MainThreadSharedTimer.cpp(33): note: see reference to function template instantiation 'WTF::NeverDestroyed<WebCore::MainThreadSharedTimer>::NeverDestroyed<>(void)' being compiled

Doesn't friend class work in windows?

[Carlos Garcia Campos]

Created attachment 264053 [details]
Try to fix win build

Let's see if this fixes the win build.

[WebKit Commit Bot]

Attachment 264053 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:72:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:73:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:113:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/ThreadTimers.cpp:73:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 5 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Carlos Garcia Campos]

hmm, the current win failure doesn't seem related to this patch.

[Carlos Garcia Campos]

Created attachment 264126 [details]
Try to build on Mac

Now with the xcode changes, thanks Philippe!

[WebKit Commit Bot]

Attachment 264126 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:72:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:73:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:113:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/ThreadTimers.cpp:73:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 5 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Carlos Garcia Campos]

Created attachment 264127 [details]
Try to fix mac build

It's failing when linking due to undefined symbols, trying with WEBCORE_EXPORT

[WebKit Commit Bot]

Attachment 264127 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:72:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:73:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:113:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/ThreadTimers.cpp:73:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 5 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Carlos Garcia Campos]

I'm lost with the build failures. Darin, could you help me with the mac builds?

[Darin Adler]

I believe the Mac build failure is a mistake in the project file patch; MainThreadSharedTimer.cpp was not added to the list of files to compile. You’ll note that the old SharedTimerCF.cpp had a PBXBuildFile line that was added but the patch doesn’t add new PBXBuildFile for the new files. Whoever generated the Xcode project files has to redo it, or someone else needs to apply the patch and do that.

[Carlos Garcia Campos]

(In reply to comment #26)
> I believe the Mac build failure is a mistake in the project file patch;
> MainThreadSharedTimer.cpp was not added to the list of files to compile.
> You’ll note that the old SharedTimerCF.cpp had a PBXBuildFile line that was
> added but the patch doesn’t add new PBXBuildFile for the new files. Whoever
> generated the Xcode project files has to redo it, or someone else needs to
> apply the patch and do that.

Thanks Darin, it's amazing how difficult is to add new files to xcode project files even using xcode. I'll try to get a new patch.

[Carlos Garcia Campos]

Created attachment 264203 [details]
Try to fix mac builds

Updated the xcode changes

[WebKit Commit Bot]

Attachment 264203 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:72:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:73:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:113:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/ThreadTimers.cpp:73:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 5 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Carlos Garcia Campos]

Created attachment 264207 [details]
Try to fix win build

I think std::function<void()> = 0 was the problem in windows.

[WebKit Commit Bot]

Attachment 264207 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:72:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:73:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/win/MainThreadSharedTimerWin.cpp:113:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/ThreadTimers.cpp:73:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 5 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Darin Adler]

Comment on attachment 264207 [details]
Try to fix win build

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

> Source/WebCore/platform/MainThreadSharedTimer.h:46
> +    // FIXME: this should be private, but CF and Windows implementations
> +    // need to call this from static methods at the moment.

I wouldn’t bother with this FIXME even though I agree with what it says.

If we were keeping it: we normally use sentence capitalization so "this" would be capitalized, and I don't think "static methods" is the term we'd want to use here, since that is a way to refer to static member functions, but if these were in fact static member functions then there would be no problem. I think we mean "non-member functions". The fact that those functions are also declared with "static" so they get internal linkage is not relevant.

> Source/WebCore/platform/MainThreadSharedTimer.h:52
> +    std::function<void()> m_firedFunction { nullptr };

A std::function will be initialized to null without explicitly specifying nullptr. Because of that we can simply omit the { nullptr } here.

> Source/WebCore/platform/SharedTimer.h:41
> +    SharedTimer() { }

This line of code can simply be deleted. It has no effect.

> Source/WebCore/platform/cf/MainThreadSharedTimerCF.cpp:60
> +    static PowerObserver* powerObserver;
> +    if (!powerObserver)
> +        powerObserver = std::make_unique<PowerObserver>(restartSharedTimer).release();

Not new code, but someone should return to this, since this is a peculiar way to write it. I’d write this:

    static NeverDestroyed<PowerObserver> powerObserver(restartSharedTimer);

> Source/WebCore/workers/WorkerRunLoop.cpp:56
> +    std::function<void()> m_sharedTimerFunction { nullptr };

No need for the { nullptr } here since std::function will initialize itself to null.

[Carlos Garcia Campos]

(In reply to comment #32)
> Comment on attachment 264207 [details]
> Try to fix win build
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=264207&action=review
> 
> > Source/WebCore/platform/MainThreadSharedTimer.h:46
> > +    // FIXME: this should be private, but CF and Windows implementations
> > +    // need to call this from static methods at the moment.
> 
> I wouldn’t bother with this FIXME even though I agree with what it says.
> 
> If we were keeping it: we normally use sentence capitalization so "this"
> would be capitalized, and I don't think "static methods" is the term we'd
> want to use here, since that is a way to refer to static member functions,
> but if these were in fact static member functions then there would be no
> problem. I think we mean "non-member functions". The fact that those
> functions are also declared with "static" so they get internal linkage is
> not relevant.

Ok, I'll fix the comment then.

> > Source/WebCore/platform/MainThreadSharedTimer.h:52
> > +    std::function<void()> m_firedFunction { nullptr };
> 
> A std::function will be initialized to null without explicitly specifying
> nullptr. Because of that we can simply omit the { nullptr } here.

Ok.

> > Source/WebCore/platform/SharedTimer.h:41
> > +    SharedTimer() { }
> 
> This line of code can simply be deleted. It has no effect.

It seems to be needed by derived classes constructor, if I remove it I get a build failure:

../../Source/WebCore/platform/MainThreadSharedTimer.cpp: In constructor 'WebCore::MainThreadSharedTimer::MainThreadSharedTimer()':
../../Source/WebCore/platform/MainThreadSharedTimer.cpp:37:46: error: no matching function for call to 'WebCore::SharedTimer::SharedTimer()'
In file included from ../../Source/WebCore/platform/MainThreadSharedTimer.h:30:0,
                 from ../../Source/WebCore/platform/MainThreadSharedTimer.cpp:27:
../../Source/WebCore/platform/SharedTimer.h:39:14: note: candidate: WebCore::SharedTimer::SharedTimer(const WebCore::SharedTimer&) <deleted>
../../Source/WebCore/platform/SharedTimer.h:39:14: note:   candidate expects 1 argument, 0 provided

I guess I can do SharedTimer() = default; instead, though.

> > Source/WebCore/platform/cf/MainThreadSharedTimerCF.cpp:60
> > +    static PowerObserver* powerObserver;
> > +    if (!powerObserver)
> > +        powerObserver = std::make_unique<PowerObserver>(restartSharedTimer).release();
> 
> Not new code, but someone should return to this, since this is a peculiar
> way to write it. I’d write this:
> 
>     static NeverDestroyed<PowerObserver> powerObserver(restartSharedTimer);

Yes, but I'll leave this for someone who can build mac port :-)

> > Source/WebCore/workers/WorkerRunLoop.cpp:56
> > +    std::function<void()> m_sharedTimerFunction { nullptr };
> 
> No need for the { nullptr } here since std::function will initialize itself
> to null.

Ok.

[Carlos Garcia Campos]

Committed r191789: <http://trac.webkit.org/changeset/191789>