Summary: | imported/w3c/web-platform-tests/clipboard-apis/async-navigator-clipboard-basics.https.html is flaky | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Aakash Jain <aakash_jain> | ||||||||||
Component: | Tools / Tests | Assignee: | Wenson Hsieh <wenson_hsieh> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | aakash_jain, bdakin, commit-queue, megan_gardner, rniwa, thorton, webkit-bot-watchers-bugzilla, webkit-bug-importer, wenson_hsieh | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | Other | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Aakash Jain
2019-10-19 08:33:54 PDT
This test failed on clean tree (run-layout-tests-without-patch step) in https://ews-build.webkit.org/#/builders/24/builds/2042 Created attachment 381396 [details]
Fixes the test
Created attachment 381447 [details]
Fix GTK/WPE builds
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. 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. Created attachment 381499 [details]
Patch for EWS
Created attachment 381500 [details]
Fix GTK/WPE builds
Comment on attachment 381500 [details] Fix GTK/WPE builds Clearing flags on attachment: 381500 Committed r251421: <https://trac.webkit.org/changeset/251421> All reviewed patches have been landed. Closing bug. |