<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body><span class="vcard"><a class="email" href="mailto:darin@apple.com" title="Darin Adler <darin@apple.com>"> <span class="fn">Darin Adler</span></a>
</span> changed
<a class="bz_bug_link
bz_status_NEW "
title="NEW - Make every port implement MainThreadSharedTimer instead of using global functions"
href="https://bugs.webkit.org/show_bug.cgi?id=150498">bug 150498</a>
<br>
<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>What</th>
<th>Removed</th>
<th>Added</th>
</tr>
<tr>
<td style="text-align:right;">Attachment #264207 Flags</td>
<td>review?
</td>
<td>review+
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - Make every port implement MainThreadSharedTimer instead of using global functions"
href="https://bugs.webkit.org/show_bug.cgi?id=150498#c32">Comment # 32</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - Make every port implement MainThreadSharedTimer instead of using global functions"
href="https://bugs.webkit.org/show_bug.cgi?id=150498">bug 150498</a>
from <span class="vcard"><a class="email" href="mailto:darin@apple.com" title="Darin Adler <darin@apple.com>"> <span class="fn">Darin Adler</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=264207&action=diff" name="attach_264207" title="Try to fix win build">attachment 264207</a> <a href="attachment.cgi?id=264207&action=edit" title="Try to fix win build">[details]</a></span>
Try to fix win build
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=264207&action=review">https://bugs.webkit.org/attachment.cgi?id=264207&action=review</a>
<span class="quote">> 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.</span >
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.
<span class="quote">> Source/WebCore/platform/MainThreadSharedTimer.h:52
> + std::function<void()> m_firedFunction { nullptr };</span >
A std::function will be initialized to null without explicitly specifying nullptr. Because of that we can simply omit the { nullptr } here.
<span class="quote">> Source/WebCore/platform/SharedTimer.h:41
> + SharedTimer() { }</span >
This line of code can simply be deleted. It has no effect.
<span class="quote">> Source/WebCore/platform/cf/MainThreadSharedTimerCF.cpp:60
> + static PowerObserver* powerObserver;
> + if (!powerObserver)
> + powerObserver = std::make_unique<PowerObserver>(restartSharedTimer).release();</span >
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);
<span class="quote">> Source/WebCore/workers/WorkerRunLoop.cpp:56
> + std::function<void()> m_sharedTimerFunction { nullptr };</span >
No need for the { nullptr } here since std::function will initialize itself to null.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>