Bug 223844 - Allow DisplayRefreshMonitors to be more long-lived objects
Summary: Allow DisplayRefreshMonitors to be more long-lived objects
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-03-27 14:48 PDT by Simon Fraser (smfr)
Modified: 2021-03-27 19:53 PDT (History)
13 users (show)

See Also:

Attachments
Patch (43.12 KB, patch)
2021-03-27 14:58 PDT, Simon Fraser (smfr) 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (43.13 KB, patch)
2021-03-27 15:07 PDT, Simon Fraser (smfr) 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (43.13 KB, patch)
2021-03-27 15:12 PDT, Simon Fraser (smfr) 		cdumez: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (43.73 KB, patch)
2021-03-27 15:48 PDT, Simon Fraser (smfr) 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (45.20 KB, patch)
2021-03-27 18:09 PDT, Simon Fraser (smfr) 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2021-03-27 14:48:36 PDT 
Allow DisplayRefreshMonitor to be more long-lived objects
Comment 1 Simon Fraser (smfr) 2021-03-27 14:58:59 PDT 
Created attachment 424470 [details]
Patch
Comment 2 Simon Fraser (smfr) 2021-03-27 15:07:58 PDT 
Created attachment 424471 [details]
Patch
Comment 3 Simon Fraser (smfr) 2021-03-27 15:12:49 PDT 
Created attachment 424472 [details]
Patch
Comment 4 Chris Dumez 2021-03-27 15:31:03 PDT 
Comment on attachment 424472 [details]
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?
Comment 5 Simon Fraser (smfr) 2021-03-27 15:48:03 PDT 
Created attachment 424473 [details]
Patch
Comment 6 Simon Fraser (smfr) 2021-03-27 18:09:19 PDT 
Created attachment 424477 [details]
Patch
Comment 7 EWS 2021-03-27 19:52:43 PDT 
Committed r275144: <https://commits.webkit.org/r275144>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 424477 [details].
Comment 8 Radar WebKit Bug Importer 2021-03-27 19:53:19 PDT 
<rdar://problem/75926530>