[webkit-reviews] review granted: [Bug 223844] Allow DisplayRefreshMonitors to be more long-lived objects : [Attachment 424472] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 223844: Allow DisplayRefreshMonitors to be more long-lived objects
https://bugs.webkit.org/show_bug.cgi?id=223844

Attachment 424472: Patch

https://bugs.webkit.org/attachment.cgi?id=424472&action=review




--- Comment #4 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 424472
  --> https://bugs.webkit.org/attachment.cgi?id=424472
Patch

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

R=me assuming bots are happy

> Source/WebCore/platform/graphics/DisplayRefreshMonitor.cpp:106
> +    LockHolder lock(mutex());

I think we usually use locker for such variables, e.g:
Auto locker = holdLock(lock());

> Source/WebCore/platform/graphics/DisplayRefreshMonitor.cpp:111
> +    auto started = startNotificationMechanism();

I don’t think we need this local. Could write:
If(!startNotifocationMechanism())
    return false;

> Source/WebCore/platform/graphics/DisplayRefreshMonitor.cpp:128
> +    ++m_unscheduledFireCount;

Could be merged into the next line.

> Source/WebCore/platform/graphics/DisplayRefreshMonitor.cpp:135
>	   LockHolder lock(m_mutex);

Same comment about locker.

> Source/WebCore/platform/graphics/DisplayRefreshMonitor.h:75
> +    Lock& mutex() { return m_mutex; }

Why don’t we call this lock instead of mutex since this is what we call it
nowadays?