Bug 181151 - [WTF] Align assumption in RunLoopWin to the other platform's RunLoop
Summary: [WTF] Align assumption in RunLoopWin to the other platform's RunLoop
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-12-25 06:53 PST by Yusuke Suzuki
Modified: 2019-03-10 19:37 PDT (History)
10 users (show)

See Also:


Attachments
Patch (9.89 KB, patch)
2017-12-25 06:57 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (16.10 KB, patch)
2017-12-25 07:24 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (15.58 KB, patch)
2017-12-25 07:30 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (17.41 KB, patch)
2017-12-25 07:41 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews104 for mac-elcapitan-wk2 (2.66 MB, application/zip)
2017-12-25 09:38 PST, EWS Watchlist
no flags Details
Patch (19.09 KB, patch)
2018-01-21 00:51 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (19.11 KB, patch)
2018-01-21 00:55 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (19.66 KB, patch)
2018-01-21 06:00 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (19.66 KB, patch)
2018-01-21 06:09 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (17.34 KB, patch)
2018-01-21 07:34 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (17.50 KB, patch)
2018-01-21 08:16 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (17.49 KB, patch)
2018-01-21 08:30 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (17.92 KB, patch)
2018-01-21 17:23 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (20.50 KB, patch)
2018-01-21 21:43 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (20.54 KB, patch)
2018-04-25 04:33 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (20.48 KB, patch)
2018-04-25 06:42 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (20.55 KB, patch)
2018-04-25 08:40 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (20.56 KB, patch)
2018-04-25 09:12 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (17.29 KB, patch)
2018-04-25 10:15 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (17.73 KB, patch)
2018-04-26 12:37 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (17.99 KB, patch)
2018-04-26 12:40 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (18.00 KB, patch)
2018-05-13 08:56 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Redone patch (15.05 KB, patch)
2019-03-05 17:12 PST, Don Olmstead
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews204 for win-future (12.84 MB, application/zip)
2019-03-05 19:26 PST, EWS Watchlist
no flags Details
Patch (16.64 KB, patch)
2019-03-07 22:27 PST, Fujii Hironori
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews105 for mac-highsierra-wk2 (2.61 MB, application/zip)
2019-03-08 00:34 PST, EWS Watchlist
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2017-12-25 06:53:44 PST
[WTF] Support feature-rich RunLoop in Windows
Comment 1 Yusuke Suzuki 2017-12-25 06:57:20 PST
Created attachment 330179 [details]
Patch
Comment 2 Yusuke Suzuki 2017-12-25 07:24:59 PST
Created attachment 330180 [details]
Patch
Comment 3 Yusuke Suzuki 2017-12-25 07:30:29 PST
Created attachment 330181 [details]
Patch
Comment 4 Yusuke Suzuki 2017-12-25 07:41:11 PST
Created attachment 330182 [details]
Patch
Comment 5 EWS Watchlist 2017-12-25 09:38:16 PST
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
Comment 6 EWS Watchlist 2017-12-25 09:38:17 PST
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
Comment 7 Yusuke Suzuki 2018-01-21 00:51:58 PST
Created attachment 331860 [details]
Patch
Comment 8 EWS Watchlist 2018-01-21 00:54:05 PST
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.
Comment 9 Yusuke Suzuki 2018-01-21 00:55:53 PST
Created attachment 331861 [details]
Patch
Comment 10 EWS Watchlist 2018-01-21 00:56:52 PST
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.
Comment 11 Yusuke Suzuki 2018-01-21 06:00:59 PST
Created attachment 331866 [details]
Patch
Comment 12 EWS Watchlist 2018-01-21 06:03:37 PST
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.
Comment 13 Yusuke Suzuki 2018-01-21 06:09:41 PST
Created attachment 331867 [details]
Patch
Comment 14 EWS Watchlist 2018-01-21 06:10:37 PST
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.
Comment 15 Yusuke Suzuki 2018-01-21 07:34:20 PST
Created attachment 331869 [details]
Patch
Comment 16 EWS Watchlist 2018-01-21 07:35:39 PST
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.
Comment 17 Yusuke Suzuki 2018-01-21 08:16:07 PST
Created attachment 331870 [details]
Patch
Comment 18 EWS Watchlist 2018-01-21 08:18:23 PST
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.
Comment 19 Yusuke Suzuki 2018-01-21 08:30:57 PST
Created attachment 331871 [details]
Patch
Comment 20 EWS Watchlist 2018-01-21 08:33:39 PST
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.
Comment 21 Yusuke Suzuki 2018-01-21 17:23:54 PST
Created attachment 331889 [details]
Patch
Comment 22 EWS Watchlist 2018-01-21 17:25:46 PST
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.
Comment 23 Yusuke Suzuki 2018-01-21 21:43:50 PST
Created attachment 331894 [details]
Patch
Comment 24 EWS Watchlist 2018-01-21 21:45:01 PST
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 25 Alex Christensen 2018-01-23 22:06:36 PST
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?
Comment 26 Yusuke Suzuki 2018-04-25 04:33:30 PDT
Created attachment 338719 [details]
Patch
Comment 27 Yusuke Suzuki 2018-04-25 06:42:36 PDT
Created attachment 338725 [details]
Patch
Comment 28 Yusuke Suzuki 2018-04-25 08:40:48 PDT
Created attachment 338740 [details]
Patch
Comment 29 Yusuke Suzuki 2018-04-25 09:12:02 PDT
Created attachment 338742 [details]
Patch
Comment 30 Yusuke Suzuki 2018-04-25 10:15:23 PDT
Created attachment 338748 [details]
Patch
Comment 31 Yusuke Suzuki 2018-04-26 12:37:11 PDT
Created attachment 338904 [details]
Patch
Comment 32 Yusuke Suzuki 2018-04-26 12:40:12 PDT
Created attachment 338905 [details]
Patch
Comment 33 Yusuke Suzuki 2018-05-13 08:56:32 PDT
Created attachment 340264 [details]
Patch
Comment 34 Don Olmstead 2019-03-05 17:12:51 PST
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 35 EWS Watchlist 2019-03-05 19:26:36 PST
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
Comment 36 EWS Watchlist 2019-03-05 19:26:48 PST
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 37 Fujii Hironori 2019-03-05 20:40:42 PST
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 38 Fujii Hironori 2019-03-07 22:15:17 PST
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 39 Fujii Hironori 2019-03-07 22:18:33 PST
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.
Comment 40 Fujii Hironori 2019-03-07 22:27:48 PST
Created attachment 363987 [details]
Patch

* Addressed my review feedbacks
Comment 41 EWS Watchlist 2019-03-08 00:34:05 PST
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
Comment 42 EWS Watchlist 2019-03-08 00:34:07 PST
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 43 Don Olmstead 2019-03-08 14:20:02 PST
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?
Comment 44 Fujii Hironori 2019-03-10 19:33:24 PDT
Thanks. They all seem false positive.
Comment 45 Fujii Hironori 2019-03-10 19:36:51 PDT
Comment on attachment 363987 [details]
Patch

Clearing flags on attachment: 363987

Committed r242694: <https://trac.webkit.org/changeset/242694>
Comment 46 Fujii Hironori 2019-03-10 19:36:54 PDT
All reviewed patches have been landed.  Closing bug.
Comment 47 Radar WebKit Bug Importer 2019-03-10 19:37:24 PDT
<rdar://problem/48756033>