| Summary: | [WTF] Align assumption in RunLoopWin to the other platform's RunLoop | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Yusuke Suzuki <ysuzuki> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Component: | New Bugs | Assignee: | Yusuke Suzuki <ysuzuki> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Severity: | Normal | CC: | achristensen, bfulgham, darin, don.olmstead, ews-watchlist, Hironori.Fujii, pvollan, rniwa, ross.kirsling, webkit-bug-importer | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| OS: | Unspecified | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Description
Yusuke Suzuki
2017-12-25 06:53:44 PST
Created attachment 330179 [details]
Patch
Created attachment 330180 [details]
Patch
Created attachment 330181 [details]
Patch
Created attachment 330182 [details]
Patch
Comment on attachment 330182 [details] Patch Attachment 330182 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/5827210 New failing tests: imported/w3c/web-platform-tests/service-workers/service-worker/register-same-scope-different-script-url.https.html fast/mediastream/MediaStream-MediaElement-setObject-null.html imported/w3c/web-platform-tests/service-workers/service-worker/ServiceWorkerGlobalScope/update.https.html Created attachment 330185 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Created attachment 331860 [details]
Patch
Attachment 331860 [details] did not pass style-queue: ERROR: Tools/TestWebKitAPI/Tests/WTF/RunLoop.cpp:29: Alphabetical sorting problem. [build/include_order] [4] WARNING: This machine could support 4 simulators, but is only configured for 3. WARNING: Please see <https://trac.webkit.org/wiki/IncreasingKernelLimits>. Total errors found: 1 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style. Created attachment 331861 [details]
Patch
Attachment 331861 [details] did not pass style-queue: ERROR: Tools/TestWebKitAPI/Tests/WTF/RunLoop.cpp:29: Alphabetical sorting problem. [build/include_order] [4] WARNING: This machine could support 4 simulators, but is only configured for 3. WARNING: Please see <https://trac.webkit.org/wiki/IncreasingKernelLimits>. Total errors found: 1 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style. Created attachment 331866 [details]
Patch
Attachment 331866 [details] did not pass style-queue: ERROR: Tools/TestWebKitAPI/Tests/WTF/RunLoop.cpp:29: Alphabetical sorting problem. [build/include_order] [4] WARNING: This machine could support 4 simulators, but is only configured for 3. WARNING: Please see <https://trac.webkit.org/wiki/IncreasingKernelLimits>. Total errors found: 1 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style. Created attachment 331867 [details]
Patch
Attachment 331867 [details] did not pass style-queue: ERROR: Tools/TestWebKitAPI/Tests/WTF/RunLoop.cpp:29: Alphabetical sorting problem. [build/include_order] [4] WARNING: This machine could support 4 simulators, but is only configured for 3. WARNING: Please see <https://trac.webkit.org/wiki/IncreasingKernelLimits>. Total errors found: 1 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style. Created attachment 331869 [details]
Patch
Attachment 331869 [details] did not pass style-queue: ERROR: Tools/TestWebKitAPI/Tests/WTF/RunLoop.cpp:29: Alphabetical sorting problem. [build/include_order] [4] WARNING: This machine could support 4 simulators, but is only configured for 3. WARNING: Please see <https://trac.webkit.org/wiki/IncreasingKernelLimits>. Total errors found: 1 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style. Created attachment 331870 [details]
Patch
Attachment 331870 [details] did not pass style-queue: ERROR: Tools/TestWebKitAPI/Tests/WTF/RunLoop.cpp:29: Alphabetical sorting problem. [build/include_order] [4] WARNING: This machine could support 4 simulators, but is only configured for 3. WARNING: Please see <https://trac.webkit.org/wiki/IncreasingKernelLimits>. Total errors found: 1 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style. Created attachment 331871 [details]
Patch
Attachment 331871 [details] did not pass style-queue: ERROR: Tools/TestWebKitAPI/Tests/WTF/RunLoop.cpp:29: Alphabetical sorting problem. [build/include_order] [4] WARNING: This machine could support 4 simulators, but is only configured for 3. WARNING: Please see <https://trac.webkit.org/wiki/IncreasingKernelLimits>. Total errors found: 1 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style. Created attachment 331889 [details]
Patch
Attachment 331889 [details] did not pass style-queue: ERROR: Tools/TestWebKitAPI/Tests/WTF/RunLoop.cpp:29: Alphabetical sorting problem. [build/include_order] [4] WARNING: This machine could support 4 simulators, but is only configured for 3. WARNING: Please see <https://trac.webkit.org/wiki/IncreasingKernelLimits>. Total errors found: 1 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style. Created attachment 331894 [details]
Patch
Attachment 331894 [details] did not pass style-queue: ERROR: Tools/TestWebKitAPI/Tests/WTF/RunLoop.cpp:29: Alphabetical sorting problem. [build/include_order] [4] WARNING: This machine could support 4 simulators, but is only configured for 3. WARNING: Please see <https://trac.webkit.org/wiki/IncreasingKernelLimits>. Total errors found: 1 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style. Comment on attachment 331894 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=331894&action=review This seems like a good thing and it would allow better cross-platform code, but the Windows bots are red. > Source/WTF/ChangeLog:3 > + [WTF] Support feature-rich RunLoop in Windows This could be a better description. Exactly what functions are you trying to enable? Created attachment 338719 [details]
Patch
Created attachment 338725 [details]
Patch
Created attachment 338740 [details]
Patch
Created attachment 338742 [details]
Patch
Created attachment 338748 [details]
Patch
Created attachment 338904 [details]
Patch
Created attachment 338905 [details]
Patch
Created attachment 340264 [details]
Patch
Created attachment 363711 [details]
Redone patch
Here's the previous patch redone on ToT.
Yusuke can you double check that I got everything from your original patch?
Comment on attachment 363711 [details] Redone patch Attachment 363711 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/11388338 New failing tests: fast/scrolling/scroll-container-horizontally.html Created attachment 363721 [details]
Archive of layout-test-results from ews204 for win-future
The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews204 Port: win-future Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit
Comment on attachment 363711 [details] Redone patch View in context: https://bugs.webkit.org/attachment.cgi?id=363711&action=review > Source/WTF/wtf/win/RunLoopWin.cpp:80 > + while (BOOL result = ::PeekMessage(&message, 0, 0, 0, PM_REMOVE)) { Unused variable 'result'. > Source/WTF/wtf/win/RunLoopWin.cpp:98 > + std::call_once(onceKey, [] { Theoretically, other thread can create a window before RegisterClass is finish. Should registerRunLoopMessageWindowClass be called from initializeMainThread? > Source/WTF/wtf/win/RunLoopWin.cpp:101 > + windowClass.cbWndExtra = sizeof(RunLoop*); cbWndExtra is for dialog box. You don't need to set. Comment on attachment 363711 [details] Redone patch View in context: https://bugs.webkit.org/attachment.cgi?id=363711&action=review >> Source/WTF/wtf/win/RunLoopWin.cpp:101 >> + windowClass.cbWndExtra = sizeof(RunLoop*); > > cbWndExtra is for dialog box. You don't need to set. Err. This is needed for GetWindowLongPtr. Comment on attachment 363711 [details] Redone patch View in context: https://bugs.webkit.org/attachment.cgi?id=363711&action=review > Source/WTF/wtf/win/RunLoopWin.cpp:139 > + ::KillTimer(m_runLoop->m_runLoopMessageWindow, bitwise_cast<uintptr_t>(this)); Set m_isActive = false. Created attachment 363987 [details]
Patch
* Addressed my review feedbacks
Comment on attachment 363987 [details] Patch Attachment 363987 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/11424255 New failing tests: accessibility/mac/selection-notification-focus-change.html Created attachment 363994 [details]
Archive of layout-test-results from ews105 for mac-highsierra-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 363987 [details]
Patch
r=me
Code looks fine. I'm worried about this getting a rollout with the AppleWin test failures. You analyze any of those?
Thanks. They all seem false positive. Comment on attachment 363987 [details] Patch Clearing flags on attachment: 363987 Committed r242694: <https://trac.webkit.org/changeset/242694> All reviewed patches have been landed. Closing bug. |