Bug 225556 - Add web process drawing to the focus test in MotionMark
Summary: Add web process drawing to the focus test in MotionMark
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-05-07 22:58 PDT by Myles C. Maxfield
Modified: 2021-07-22 11:01 PDT (History)
10 users (show)

See Also:


Attachments
Patch (4.45 KB, patch)
2021-05-07 23:24 PDT, Myles C. Maxfield
simon.fraser: review+
Details | Formatted Diff | Diff
Patch for committing (3.07 KB, patch)
2021-05-11 22:52 PDT, Myles C. Maxfield
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2021-05-07 22:58:21 PDT
Add web process drawing to the focus test in MotionMark
Comment 1 Myles C. Maxfield 2021-05-07 23:24:46 PDT
Created attachment 428079 [details]
Patch
Comment 2 Simon Fraser (smfr) 2021-05-08 10:29:40 PDT
Comment on attachment 428079 [details]
Patch

When we do version bumps, are they normally part of the patch?
Comment 3 Myles C. Maxfield 2021-05-08 17:41:50 PDT
(In reply to Simon Fraser (smfr) from comment #2)
> Comment on attachment 428079 [details]
> Patch
> 
> When we do version bumps, are they normally part of the patch?

I think we batch up multiple patches into a single version bump.

This patch only updates what's in PerformanceTests. When we want to publish, we copy that into Websites/browserbench.org.
Comment 4 Darin Adler 2021-05-08 21:29:56 PDT
Comment on attachment 428079 [details]
Patch

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

> PerformanceTests/ChangeLog:11
> +        Previously, the MotionMark focus test just animated the blur, opacity, and position of each element.
> +        All of these get mapped to properties on the Core Animation layers, which means that each rAF() tick,
> +        the only thing we were doing was pushing layer state changes to the window server.

This comment talks a lot about window server and Core Animation. But this is a cross-platform, cross-browser benchmark, and window server is specific to Mac, and Core Animation specific to Apple’s macOS/iOS/iPadOS/tvOS/watchOS.

I’m sure that these these improvements help us have a more repeatable test on macOS, but are also helpful on other platforms, including iOS, and to accurately test other web browsers as well. But that’s not stated here and probably should be. We would not want to make the changes if they were harmful in those other configurations.

I’m not sure we need quite so long a change log comment. Maybe there is somewhere else we should leave this information if we are going to need it in the future. In the change log it might or might not be found.
Comment 5 Simon Fraser (smfr) 2021-05-11 10:35:33 PDT
Comment on attachment 428079 [details]
Patch

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

>> PerformanceTests/ChangeLog:11
>> +        the only thing we were doing was pushing layer state changes to the window server.
> 
> This comment talks a lot about window server and Core Animation. But this is a cross-platform, cross-browser benchmark, and window server is specific to Mac, and Core Animation specific to Apple’s macOS/iOS/iPadOS/tvOS/watchOS.
> 
> I’m sure that these these improvements help us have a more repeatable test on macOS, but are also helpful on other platforms, including iOS, and to accurately test other web browsers as well. But that’s not stated here and probably should be. We would not want to make the changes if they were harmful in those other configurations.
> 
> I’m not sure we need quite so long a change log comment. Maybe there is somewhere else we should leave this information if we are going to need it in the future. In the change log it might or might not be found.

Agreed that we can trim down this changelog.
Comment 6 Myles C. Maxfield 2021-05-11 22:52:43 PDT
Created attachment 428347 [details]
Patch for committing
Comment 7 Radar WebKit Bug Importer 2021-05-14 22:59:19 PDT
<rdar://problem/78047813>
Comment 8 Myles C. Maxfield 2021-07-22 11:01:41 PDT
Focus test has been removed.