[Webkit-unassigned] [Bug 150498] Make every port implement MainThreadSharedTimer instead of using global functions

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=150498

Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #264207|review?                     |review+
              Flags|                            |

--- Comment #32 from Darin Adler <darin at apple.com> ---
Comment on attachment 264207
  --> https://bugs.webkit.org/attachment.cgi?id=264207
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.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20151028/2f295dba/attachment.html>