76781 2012-01-21 04:40:33 -0800 Port RunLoop to WinCE 2012-02-13 10:09:17 -0800 1 1 1 Unclassified WebKit Platform 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 78340 78341 1 paroga paroga aroben webkit.review.bot oldest_to_newest 540122 0 paroga 2012-01-21 04:40:33 -0800 Port RunLoop to WinCE 540123 1 123445 paroga 2012-01-21 04:42:26 -0800 Created attachment 123445 Patch 540556 2 123445 aroben 2012-01-23 08:23:44 -0800 Comment on attachment 123445 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=123445&action=review > Source/WebCore/platform/win/RunLoopWin.cpp:44 > +#if OS(WINCE) > + LONG longPtr = ::GetWindowLong(hWnd, 0); > +#else > LONG_PTR longPtr = ::GetWindowLongPtr(hWnd, 0); > - > +#endif Maybe we should take a different approach here. wtf/MathExtras.h and wtf/StringExtras.h have shim functions that provide compatibility between various compilers/runtimes. Maybe we should have a WindowsExtras.h (maybe in WTF, maybe in WebCore/platform) that does the same thing to bridge the gap between WinCE and other versions of Windows? For example, WindowsExtras.h could have an inline GetWIndowLongPtr function that is defined only on WinCE that just calls through to GetWindowLong. We could do the same thing for SetWindowLong[Ptr]. And we could even have a WinCE-only HWND_MESSAGE that is just 0. What do you think? Seems like that would make it easier to fix these kinds of issues in the future (by just including WindowsExtras.h). 540693 3 paroga 2012-01-23 11:33:18 -0800 (In reply to comment #2) > (From update of attachment 123445 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=123445&action=review > > > Source/WebCore/platform/win/RunLoopWin.cpp:44 > > +#if OS(WINCE) > > + LONG longPtr = ::GetWindowLong(hWnd, 0); > > +#else > > LONG_PTR longPtr = ::GetWindowLongPtr(hWnd, 0); > > - > > +#endif > > Maybe we should take a different approach here. wtf/MathExtras.h and wtf/StringExtras.h have shim functions that provide compatibility between various compilers/runtimes. Maybe we should have a WindowsExtras.h (maybe in WTF, maybe in WebCore/platform) that does the same thing to bridge the gap between WinCE and other versions of Windows? For example, WindowsExtras.h could have an inline GetWIndowLongPtr function that is defined only on WinCE that just calls through to GetWindowLong. We could do the same thing for SetWindowLong[Ptr]. And we could even have a WinCE-only HWND_MESSAGE that is just 0. What do you think? Seems like that would make it easier to fix these kinds of issues in the future (by just including WindowsExtras.h). I'd prefere a solution with WindowsExtras.h too, but I'm not a big fan of "implementing" a function with the same name on WinCE. IMHO a wrapper function witch does the correct stuff for both platform would be better. This function can do the cast to LONG_PTR on Win32 then too. Defining HWND_MESSAGE to 0 should be ok and can be done via an #ifndef HWND_MESSAGE. 540735 4 123445 aroben 2012-01-23 12:12:48 -0800 Comment on attachment 123445 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=123445&action=review >>> Source/WebCore/platform/win/RunLoopWin.cpp:44 >>> +#endif >> >> Maybe we should take a different approach here. wtf/MathExtras.h and wtf/StringExtras.h have shim functions that provide compatibility between various compilers/runtimes. Maybe we should have a WindowsExtras.h (maybe in WTF, maybe in WebCore/platform) that does the same thing to bridge the gap between WinCE and other versions of Windows? For example, WindowsExtras.h could have an inline GetWIndowLongPtr function that is defined only on WinCE that just calls through to GetWindowLong. We could do the same thing for SetWindowLong[Ptr]. And we could even have a WinCE-only HWND_MESSAGE that is just 0. What do you think? Seems like that would make it easier to fix these kinds of issues in the future (by just including WindowsExtras.h). > > I'd prefere a solution with WindowsExtras.h too, but I'm not a big fan of "implementing" a function with the same name on WinCE. IMHO a wrapper function witch does the correct stuff for both platform would be better. This function can do the cast to LONG_PTR on Win32 then too. Defining HWND_MESSAGE to 0 should be ok and can be done via an #ifndef HWND_MESSAGE. What specifically concerns you about implementing GetWindowLongPtr on WinCE? A differently-named-and-typed wrapper function could be good if it made the code clearer; what do you suggest as a name and signature? We tend not to use macros for constants, so I'd recommend a "const HWND HWND_MESSAGE = 0;", similar to constants like piFloat in MathExtras.h. 553977 5 126485 paroga 2012-02-10 02:55:36 -0800 Created attachment 126485 Patch 555351 6 126485 webkit.review.bot 2012-02-13 10:09:10 -0800 Comment on attachment 126485 Patch Clearing flags on attachment: 126485 Committed r107585: <http://trac.webkit.org/changeset/107585> 555352 7 webkit.review.bot 2012-02-13 10:09:17 -0800 All reviewed patches have been landed. Closing bug. 123445 2012-01-21 04:42:26 -0800 2012-02-10 02:55:36 -0800 Patch bug-76781.patch text/plain 2906 paroga ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCAzNzljYWY1Li5iYjAyMWM4IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUg QEAKKzIwMTItMDEtMjEgIFBhdHJpY2sgR2Fuc3RlcmVyICA8cGFyb2dhQHdlYmtpdC5vcmc+CisK KyAgICAgICAgUG9ydCBSdW5Mb29wIHRvIFdpbkNFCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD03Njc4MQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgICogcGxhdGZvcm0vd2luL1J1bkxvb3BXaW4uY3BwOgorICAg ICAgICAoV2ViQ29yZTo6UnVuTG9vcDo6UnVuTG9vcFduZFByb2MpOgorICAgICAgICAoV2ViQ29y ZTo6UnVuTG9vcDo6cmVnaXN0ZXJSdW5Mb29wTWVzc2FnZVdpbmRvd0NsYXNzKToKKyAgICAgICAg KFdlYkNvcmU6OlJ1bkxvb3A6OlJ1bkxvb3ApOgorCiAyMDEyLTAxLTIxICBKb2NoZW4gRWlzaW5n ZXIgIDxqb2NoZW5AY2hyb21pdW0ub3JnPgogCiAgICAgICAgIGhpc3RvcnkucmVwbGFjZVN0YXRl IHNob3VsZCB1cGRhdGUgdGhlIEhpc3RvcnlJdGVtIHRvIHVzZSBhIEdFVCBtZXRob2QgaWYgcHJl dmlvdXNseSBub24tR0VUIChQT1NUKQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcGxhdGZv cm0vd2luL1J1bkxvb3BXaW4uY3BwIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vd2luL1J1bkxv b3BXaW4uY3BwCmluZGV4IDEyMGMyNTIuLjg0OGM3NDAgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJD b3JlL3BsYXRmb3JtL3dpbi9SdW5Mb29wV2luLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0 Zm9ybS93aW4vUnVuTG9vcFdpbi5jcHAKQEAgLTM3LDggKzM3LDEyIEBAIHN0YXRpYyBjb25zdCBM UFdTVFIga1J1bkxvb3BNZXNzYWdlV2luZG93Q2xhc3NOYW1lID0gTCJSdW5Mb29wTWVzc2FnZVdp bmRvdyI7CiAKIExSRVNVTFQgQ0FMTEJBQ0sgUnVuTG9vcDo6UnVuTG9vcFduZFByb2MoSFdORCBo V25kLCBVSU5UIG1lc3NhZ2UsIFdQQVJBTSB3UGFyYW0sIExQQVJBTSBsUGFyYW0pCiB7CisjaWYg T1MoV0lOQ0UpCisgICAgTE9ORyBsb25nUHRyID0gOjpHZXRXaW5kb3dMb25nKGhXbmQsIDApOwor I2Vsc2UKICAgICBMT05HX1BUUiBsb25nUHRyID0gOjpHZXRXaW5kb3dMb25nUHRyKGhXbmQsIDAp OwotICAgIAorI2VuZGlmCisKICAgICBpZiAoUnVuTG9vcCogcnVuTG9vcCA9IHJlaW50ZXJwcmV0 X2Nhc3Q8UnVuTG9vcCo+KGxvbmdQdHIpKQogICAgICAgICByZXR1cm4gcnVuTG9vcC0+d25kUHJv YyhoV25kLCBtZXNzYWdlLCB3UGFyYW0sIGxQYXJhbSk7CiAKQEAgLTQ2LDcgKzUwLDExIEBAIExS RVNVTFQgQ0FMTEJBQ0sgUnVuTG9vcDo6UnVuTG9vcFduZFByb2MoSFdORCBoV25kLCBVSU5UIG1l c3NhZ2UsIFdQQVJBTSB3UGFyYW0sCiAgICAgICAgIExQQ1JFQVRFU1RSVUNUIGNyZWF0ZVN0cnVj dCA9IHJlaW50ZXJwcmV0X2Nhc3Q8TFBDUkVBVEVTVFJVQ1Q+KGxQYXJhbSk7CiAKICAgICAgICAg Ly8gQXNzb2NpYXRlIHRoZSBSdW5Mb29wIHdpdGggdGhlIHdpbmRvdy4KKyNpZiBPUyhXSU5DRSkK KyAgICAgICAgOjpTZXRXaW5kb3dMb25nKGhXbmQsIDAsIChMT05HKWNyZWF0ZVN0cnVjdC0+bHBD cmVhdGVQYXJhbXMpOworI2Vsc2UKICAgICAgICAgOjpTZXRXaW5kb3dMb25nUHRyKGhXbmQsIDAs IChMT05HX1BUUiljcmVhdGVTdHJ1Y3QtPmxwQ3JlYXRlUGFyYW1zKTsKKyNlbmRpZgogICAgICAg ICByZXR1cm4gMDsKICAgICB9CiAKQEAgLTg3LDIyICs5NSwyNyBAQCBib29sIFJ1bkxvb3A6OnJl Z2lzdGVyUnVuTG9vcE1lc3NhZ2VXaW5kb3dDbGFzcygpCiB7CiAgICAgLy8gRklYTUU6IFRoaXMg cmVhbGx5IG9ubHkgbmVlZHMgdG8gYmUgY2FsbGVkIG9uY2UuCiAKLSAgICBXTkRDTEFTU0VYIHdp bmRvd0NsYXNzID0geyAwIH07Ci0gICAgd2luZG93Q2xhc3MuY2JTaXplICAgICAgICAgID0gc2l6 ZW9mKHdpbmRvd0NsYXNzKTsKKyAgICBXTkRDTEFTUyB3aW5kb3dDbGFzcyA9IHsgMCB9OwogICAg IHdpbmRvd0NsYXNzLmxwZm5XbmRQcm9jICAgICA9IFJ1bkxvb3A6OlJ1bkxvb3BXbmRQcm9jOwog ICAgIHdpbmRvd0NsYXNzLmNiV25kRXh0cmEgICAgICA9IHNpemVvZihSdW5Mb29wKik7CiAgICAg d2luZG93Q2xhc3MubHBzekNsYXNzTmFtZSAgID0ga1J1bkxvb3BNZXNzYWdlV2luZG93Q2xhc3NO YW1lOwogCi0gICAgcmV0dXJuICEhOjpSZWdpc3RlckNsYXNzRXgoJndpbmRvd0NsYXNzKTsKKyAg ICByZXR1cm4gISE6OlJlZ2lzdGVyQ2xhc3MoJndpbmRvd0NsYXNzKTsKIH0KIAogUnVuTG9vcDo6 UnVuTG9vcCgpCiB7CiAgICAgcmVnaXN0ZXJSdW5Mb29wTWVzc2FnZVdpbmRvd0NsYXNzKCk7CiAK KyNpZiBPUyhXSU5DRSkKKyAgICBIV05EIGhXbmRQYXJlbnQgPSAwOworI2Vsc2UKKyAgICBIV05E IGhXbmRQYXJlbnQgPSBIV05EX01FU1NBR0U7CisjZW5kaWYKKwogICAgIG1fcnVuTG9vcE1lc3Nh Z2VXaW5kb3cgPSA6OkNyZWF0ZVdpbmRvdyhrUnVuTG9vcE1lc3NhZ2VXaW5kb3dDbGFzc05hbWUs IDAsIDAsCiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIENXX1VT RURFRkFVTFQsIDAsIENXX1VTRURFRkFVTFQsIDAsCi0gICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgIEhXTkRfTUVTU0FHRSwgMCwgMCwgdGhpcyk7CisgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGhXbmRQYXJlbnQsIDAsIDAsIHRoaXMp OwogICAgIEFTU0VSVCg6OklzV2luZG93KG1fcnVuTG9vcE1lc3NhZ2VXaW5kb3cpKTsKIH0KIAo= 126485 2012-02-10 02:55:36 -0800 2012-02-13 10:09:10 -0800 Patch bug-76781b.patch text/plain 1351 paroga ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCA5MjdkNjNkLi5hY2MwZWU4IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTMg QEAKKzIwMTItMDItMTAgIFBhdHJpY2sgR2Fuc3RlcmVyICA8cGFyb2dhQHdlYmtpdC5vcmc+CisK KyAgICAgICAgUG9ydCBSdW5Mb29wIHRvIFdpbkNFCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD03Njc4MQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgICogcGxhdGZvcm0vd2luL1J1bkxvb3BXaW4uY3BwOgorICAg ICAgICAoV2ViQ29yZTo6UnVuTG9vcDo6cmVnaXN0ZXJSdW5Mb29wTWVzc2FnZVdpbmRvd0NsYXNz KToKKwogMjAxMi0wMi0xMCAgUGF0cmljayBHYW5zdGVyZXIgIDxwYXJvZ2FAd2Via2l0Lm9yZz4K IAogICAgICAgICBbV0lOXSBBZGQgYWJzdHJhY3Rpb24gZm9yIEhXTkRfTUVTU0FHRQpkaWZmIC0t Z2l0IGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vd2luL1J1bkxvb3BXaW4uY3BwIGIvU291cmNl L1dlYkNvcmUvcGxhdGZvcm0vd2luL1J1bkxvb3BXaW4uY3BwCmluZGV4IGRmOGM3ZTYuLjE1ZmVl OGMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL3dpbi9SdW5Mb29wV2luLmNw cAorKysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS93aW4vUnVuTG9vcFdpbi5jcHAKQEAgLTg2 LDEzICs4NiwxMiBAQCBib29sIFJ1bkxvb3A6OnJlZ2lzdGVyUnVuTG9vcE1lc3NhZ2VXaW5kb3dD bGFzcygpCiB7CiAgICAgLy8gRklYTUU6IFRoaXMgcmVhbGx5IG9ubHkgbmVlZHMgdG8gYmUgY2Fs bGVkIG9uY2UuCiAKLSAgICBXTkRDTEFTU0VYIHdpbmRvd0NsYXNzID0geyAwIH07Ci0gICAgd2lu ZG93Q2xhc3MuY2JTaXplICAgICAgICAgID0gc2l6ZW9mKHdpbmRvd0NsYXNzKTsKKyAgICBXTkRD TEFTUyB3aW5kb3dDbGFzcyA9IHsgMCB9OwogICAgIHdpbmRvd0NsYXNzLmxwZm5XbmRQcm9jICAg ICA9IFJ1bkxvb3A6OlJ1bkxvb3BXbmRQcm9jOwogICAgIHdpbmRvd0NsYXNzLmNiV25kRXh0cmEg ICAgICA9IHNpemVvZihSdW5Mb29wKik7CiAgICAgd2luZG93Q2xhc3MubHBzekNsYXNzTmFtZSAg ID0ga1J1bkxvb3BNZXNzYWdlV2luZG93Q2xhc3NOYW1lOwogCi0gICAgcmV0dXJuICEhOjpSZWdp c3RlckNsYXNzRXgoJndpbmRvd0NsYXNzKTsKKyAgICByZXR1cm4gISE6OlJlZ2lzdGVyQ2xhc3Mo JndpbmRvd0NsYXNzKTsKIH0KIAogUnVuTG9vcDo6UnVuTG9vcCgpCg==