Bug 203181 – imported/w3c/web-platform-tests/clipboard-apis/async-navigator-clipboard-basics.https.html is flaky

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Aakash Jain]

The following layout test is flaky on iOS (debug/release) and linux.

imported/w3c/web-platform-tests/clipboard-apis/async-navigator-clipboard-basics.https.html

Flakiness Dashboard:

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=imported%2Fw3c%2Fweb-platform-tests%2Fclipboard-apis%2Fasync-navigator-clipboard-basics.https.html

[Aakash Jain]

This test failed on clean tree (run-layout-tests-without-patch step) in https://ews-build.webkit.org/#/builders/24/builds/2042

[Wenson Hsieh]

Created attachment 381396 [details]
Fixes the test

[Wenson Hsieh]

Created attachment 381447 [details]
Fix GTK/WPE builds

[Ryosuke Niwa]

Comment on attachment 381447 [details]
Fix GTK/WPE builds

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

Since change count is a Cocoa specific concept, we need to hide that from the platform agnostic code. Perhaps something like PasteboardToken.

> Source/WebCore/Modules/async-clipboard/Clipboard.h:70
> -        int changeCount;
> +        long changeCount;

Can we use uint32_t or uint64_t to be more explicit about the width of this count?
Also, changeCount is cocoa specific concept.
It probably doesn't belong in this platform agnostic class not that we should fix it in this patch.

> Source/WebCore/platform/Pasteboard.h:325
> +    long m_changeCount { 0 };
> +

I don't think this change makes sense. The concept of change count doesn't exist in platforms like Windows.

[Wenson Hsieh]

Comment on attachment 381447 [details]
Fix GTK/WPE builds

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

Thanks for the review!

>> Source/WebCore/Modules/async-clipboard/Clipboard.h:70
>> +        long changeCount;
> 
> Can we use uint32_t or uint64_t to be more explicit about the width of this count?
> Also, changeCount is cocoa specific concept.
> It probably doesn't belong in this platform agnostic class not that we should fix it in this patch.

Good point. This should be a int64_t, to match macOS/iOS — I’ll change it to that.

I’ll also refactor changeCount out of platform-agnostic code in a followup.

>> Source/WebCore/platform/Pasteboard.h:325
>> +
> 
> I don't think this change makes sense. The concept of change count doesn't exist in platforms like Windows.

Okay — I’ll undo this change.

[Wenson Hsieh]

Created attachment 381499 [details]
Patch for EWS

[Wenson Hsieh]

Created attachment 381500 [details]
Fix GTK/WPE builds

[WebKit Commit Bot]

Comment on attachment 381500 [details]
Fix GTK/WPE builds

Clearing flags on attachment: 381500

Committed r251421: <https://trac.webkit.org/changeset/251421>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/56491382>