73937 2011-12-06 11:18:29 -0800 [chromium] Guard access to WebKitPlatformSupport::currentThread with a null check 2011-12-06 13:06:44 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 73851 1 adamk adamk fishd levin ojan rafaelw webkit.review.bot oldest_to_newest 516056 0 adamk 2011-12-06 11:18:29 -0800 [chromium] Guard access to WebKitPlatformSupport::currentThread with a null check 516058 1 118076 adamk 2011-12-06 11:20:49 -0800 Created attachment 118076 Patch 516084 2 118076 abarth 2011-12-06 11:33:21 -0800 Comment on attachment 118076 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=118076&action=review > Source/WebKit/chromium/ChangeLog:10 > + Though |currentThread| is never null in production code, it is null in > + unit tests (such as Chromium's unit_tests) that call WebKit::initialize() > + without first constructing a MessageLoop. Can you add this comment to the code as well? We don't want folks cargo-cult copying this null check. 516098 3 levin 2011-12-06 11:40:42 -0800 Why is it a bad thing to construct a MessageLoop in the test code? 516101 4 118076 adamk 2011-12-06 11:44:00 -0800 Comment on attachment 118076 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=118076&action=review >> Source/WebKit/chromium/ChangeLog:10 >> + without first constructing a MessageLoop. > > Can you add this comment to the code as well? We don't want folks cargo-cult copying this null check. Done. 516102 5 118081 adamk 2011-12-06 11:44:17 -0800 Created attachment 118081 Patch for landing 516103 6 118081 levin 2011-12-06 11:45:15 -0800 Comment on attachment 118081 Patch for landing Just blocking for the moment since there is an unanswered question. 516111 7 adamk 2011-12-06 11:50:14 -0800 (In reply to comment #3) > Why is it a bad thing to construct a MessageLoop in the test code? It's not so much that tests don't construct MessageLoops, but that each test case inside, e.g., unit_tests wants to have control over its own MessageLoop. For example, some tests need an IO loop, others might need a UI loop. And only one can exist per thread. So at initialization time (presumably during test startup), there's no loop, even if each test case creates a loop later (in its GTest test fixture). 516117 8 118081 levin 2011-12-06 11:52:56 -0800 Comment on attachment 118081 Patch for landing Thanks! Sorry for the hold up. 516182 9 118081 webkit.review.bot 2011-12-06 13:06:39 -0800 Comment on attachment 118081 Patch for landing Clearing flags on attachment: 118081 Committed r102171: <http://trac.webkit.org/changeset/102171> 516183 10 webkit.review.bot 2011-12-06 13:06:44 -0800 All reviewed patches have been landed. Closing bug. 118076 2011-12-06 11:20:49 -0800 2011-12-06 11:44:13 -0800 Patch bug-73937-20111206112048.patch text/plain 2137 adamk U3VidmVyc2lvbiBSZXZpc2lvbjogMTAyMTUyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L2No cm9taXVtL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvY2hyb21pdW0vQ2hhbmdlTG9nCmluZGV4 IGNjNGIwYWFjNDZkMjliMjE3MjIwMzJjNDE5MzI2ZGVlMmNiZjZkNzQuLjhiZmNiYWI3MTdlNmVi ZDY4ZWQ0NzEzOGE1Y2MwOWYxZDVkMmM5ZTUgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvY2hy b21pdW0vQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJLaXQvY2hyb21pdW0vQ2hhbmdlTG9nCkBA IC0xLDMgKzEsMTggQEAKKzIwMTEtMTItMDYgIEFkYW0gS2xlaW4gIDxhZGFta0BjaHJvbWl1bS5v cmc+CisKKyAgICAgICAgW2Nocm9taXVtXSBHdWFyZCBhY2Nlc3MgdG8gV2ViS2l0UGxhdGZvcm1T dXBwb3J0OjpjdXJyZW50VGhyZWFkIHdpdGggYSBudWxsIGNoZWNrCisgICAgICAgIGh0dHBzOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD03MzkzNworCisgICAgICAgIFJldmlld2Vk IGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRob3VnaCB8Y3VycmVudFRocmVhZHwgaXMg bmV2ZXIgbnVsbCBpbiBwcm9kdWN0aW9uIGNvZGUsIGl0IGlzIG51bGwgaW4KKyAgICAgICAgdW5p dCB0ZXN0cyAoc3VjaCBhcyBDaHJvbWl1bSdzIHVuaXRfdGVzdHMpIHRoYXQgY2FsbCBXZWJLaXQ6 OmluaXRpYWxpemUoKQorICAgICAgICB3aXRob3V0IGZpcnN0IGNvbnN0cnVjdGluZyBhIE1lc3Nh Z2VMb29wLgorCisgICAgICAgICogc3JjL1dlYktpdC5jcHA6CisgICAgICAgIChXZWJLaXQ6Omlu aXRpYWxpemUpOgorICAgICAgICAoV2ViS2l0OjpzaHV0ZG93bik6CisKIDIwMTEtMTItMDYgIFNo ZXJpZmYgQm90ICA8d2Via2l0LnJldmlldy5ib3RAZ21haWwuY29tPgogCiAgICAgICAgIFVucmV2 aWV3ZWQsIHJvbGxpbmcgb3V0IHIxMDIxMDIuCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L2No cm9taXVtL3NyYy9XZWJLaXQuY3BwIGIvU291cmNlL1dlYktpdC9jaHJvbWl1bS9zcmMvV2ViS2l0 LmNwcAppbmRleCA3NmU3MjlkYzNmN2Y3ZTRhNTkwYzE2NGM3MDE5YmUyOWQyNmI0ZjRhLi5jYTgz MDJhYWI1ZTBkMWE5MDU2N2IzYmQyMDE4Mjk0YjM1MDdiZTZjIDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViS2l0L2Nocm9taXVtL3NyYy9XZWJLaXQuY3BwCisrKyBiL1NvdXJjZS9XZWJLaXQvY2hyb21p dW0vc3JjL1dlYktpdC5jcHAKQEAgLTkzLDkgKzkzLDExIEBAIHZvaWQgaW5pdGlhbGl6ZShXZWJL aXRQbGF0Zm9ybVN1cHBvcnQqIHdlYktpdFBsYXRmb3JtU3VwcG9ydCkKICAgICBXZWJDb3JlOjpW OEJpbmRpbmdQZXJJc29sYXRlRGF0YTo6ZW5zdXJlSW5pdGlhbGl6ZWQodjg6Oklzb2xhdGU6Okdl dEN1cnJlbnQoKSk7CiAKICNpZiBFTkFCTEUoTVVUQVRJT05fT0JTRVJWRVJTKQotICAgIEFTU0VS VCghc19lbmRPZlRhc2tSdW5uZXIpOwotICAgIHNfZW5kT2ZUYXNrUnVubmVyID0gbmV3IEVuZE9m VGFza1J1bm5lcjsKLSAgICB3ZWJLaXRQbGF0Zm9ybVN1cHBvcnQtPmN1cnJlbnRUaHJlYWQoKS0+ YWRkVGFza09ic2VydmVyKHNfZW5kT2ZUYXNrUnVubmVyKTsKKyAgICBpZiAoV2ViVGhyZWFkKiBj dXJyZW50VGhyZWFkID0gd2ViS2l0UGxhdGZvcm1TdXBwb3J0LT5jdXJyZW50VGhyZWFkKCkpIHsK KyAgICAgICAgQVNTRVJUKCFzX2VuZE9mVGFza1J1bm5lcik7CisgICAgICAgIHNfZW5kT2ZUYXNr UnVubmVyID0gbmV3IEVuZE9mVGFza1J1bm5lcjsKKyAgICAgICAgY3VycmVudFRocmVhZC0+YWRk VGFza09ic2VydmVyKHNfZW5kT2ZUYXNrUnVubmVyKTsKKyAgICB9CiAjZW5kaWYKIH0KIApAQCAt MTI3LDYgKzEyOSw3IEBAIHZvaWQgc2h1dGRvd24oKQogewogI2lmIEVOQUJMRShNVVRBVElPTl9P QlNFUlZFUlMpCiAgICAgaWYgKHNfZW5kT2ZUYXNrUnVubmVyKSB7CisgICAgICAgIEFTU0VSVChz X3dlYktpdFBsYXRmb3JtU3VwcG9ydC0+Y3VycmVudFRocmVhZCgpKTsKICAgICAgICAgc193ZWJL aXRQbGF0Zm9ybVN1cHBvcnQtPmN1cnJlbnRUaHJlYWQoKS0+cmVtb3ZlVGFza09ic2VydmVyKHNf ZW5kT2ZUYXNrUnVubmVyKTsKICAgICAgICAgZGVsZXRlIHNfZW5kT2ZUYXNrUnVubmVyOwogICAg ICAgICBzX2VuZE9mVGFza1J1bm5lciA9IDA7Cg== 118081 2011-12-06 11:44:17 -0800 2011-12-06 13:06:39 -0800 Patch for landing bug-73937-20111206114416.patch text/plain 2234 adamk U3VidmVyc2lvbiBSZXZpc2lvbjogMTAyMTUyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L2No cm9taXVtL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvY2hyb21pdW0vQ2hhbmdlTG9nCmluZGV4 IGNjNGIwYWFjNDZkMjliMjE3MjIwMzJjNDE5MzI2ZGVlMmNiZjZkNzQuLjI4MmQ2YTY1ZDRmZWE3 YWQ5N2UxYWIzMWYwYzA4NTBjMmM5MWFjYjkgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvY2hy b21pdW0vQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJLaXQvY2hyb21pdW0vQ2hhbmdlTG9nCkBA IC0xLDMgKzEsMTggQEAKKzIwMTEtMTItMDYgIEFkYW0gS2xlaW4gIDxhZGFta0BjaHJvbWl1bS5v cmc+CisKKyAgICAgICAgW2Nocm9taXVtXSBHdWFyZCBhY2Nlc3MgdG8gV2ViS2l0UGxhdGZvcm1T dXBwb3J0OjpjdXJyZW50VGhyZWFkIHdpdGggYSBudWxsIGNoZWNrCisgICAgICAgIGh0dHBzOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD03MzkzNworCisgICAgICAgIFJldmlld2Vk IGJ5IEFkYW0gQmFydGguCisKKyAgICAgICAgVGhvdWdoIHxjdXJyZW50VGhyZWFkfCBpcyBuZXZl ciBudWxsIGluIHByb2R1Y3Rpb24gY29kZSwgaXQgaXMgbnVsbCBpbgorICAgICAgICB1bml0IHRl c3RzIChzdWNoIGFzIENocm9taXVtJ3MgdW5pdF90ZXN0cykgdGhhdCBjYWxsIFdlYktpdDo6aW5p dGlhbGl6ZSgpCisgICAgICAgIHdpdGhvdXQgZmlyc3QgY29uc3RydWN0aW5nIGEgTWVzc2FnZUxv b3AuCisKKyAgICAgICAgKiBzcmMvV2ViS2l0LmNwcDoKKyAgICAgICAgKFdlYktpdDo6aW5pdGlh bGl6ZSk6CisgICAgICAgIChXZWJLaXQ6OnNodXRkb3duKToKKwogMjAxMS0xMi0wNiAgU2hlcmlm ZiBCb3QgIDx3ZWJraXQucmV2aWV3LmJvdEBnbWFpbC5jb20+CiAKICAgICAgICAgVW5yZXZpZXdl ZCwgcm9sbGluZyBvdXQgcjEwMjEwMi4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvY2hyb21p dW0vc3JjL1dlYktpdC5jcHAgYi9Tb3VyY2UvV2ViS2l0L2Nocm9taXVtL3NyYy9XZWJLaXQuY3Bw CmluZGV4IDc2ZTcyOWRjM2Y3ZjdlNGE1OTBjMTY0YzcwMTliZTI5ZDI2YjRmNGEuLjJiMmIyNTk3 Y2ZjYmUzNTY5ZDY1YWE2OTI1MDdhMDEzM2VhNzgyZTYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJL aXQvY2hyb21pdW0vc3JjL1dlYktpdC5jcHAKKysrIGIvU291cmNlL1dlYktpdC9jaHJvbWl1bS9z cmMvV2ViS2l0LmNwcApAQCAtOTMsOSArOTMsMTIgQEAgdm9pZCBpbml0aWFsaXplKFdlYktpdFBs YXRmb3JtU3VwcG9ydCogd2ViS2l0UGxhdGZvcm1TdXBwb3J0KQogICAgIFdlYkNvcmU6OlY4Qmlu ZGluZ1Blcklzb2xhdGVEYXRhOjplbnN1cmVJbml0aWFsaXplZCh2ODo6SXNvbGF0ZTo6R2V0Q3Vy cmVudCgpKTsKIAogI2lmIEVOQUJMRShNVVRBVElPTl9PQlNFUlZFUlMpCi0gICAgQVNTRVJUKCFz X2VuZE9mVGFza1J1bm5lcik7Ci0gICAgc19lbmRPZlRhc2tSdW5uZXIgPSBuZXcgRW5kT2ZUYXNr UnVubmVyOwotICAgIHdlYktpdFBsYXRmb3JtU3VwcG9ydC0+Y3VycmVudFRocmVhZCgpLT5hZGRU YXNrT2JzZXJ2ZXIoc19lbmRPZlRhc2tSdW5uZXIpOworICAgIC8vIGN1cnJlbnRUaHJlYWQgd2ls bCBhbHdheXMgYmUgbm9uLW51bGwgaW4gcHJvZHVjdGlvbiwgYnV0IGNhbiBiZSBudWxsIGluIENo cm9taXVtIHVuaXQgdGVzdHMuCisgICAgaWYgKFdlYlRocmVhZCogY3VycmVudFRocmVhZCA9IHdl YktpdFBsYXRmb3JtU3VwcG9ydC0+Y3VycmVudFRocmVhZCgpKSB7CisgICAgICAgIEFTU0VSVCgh c19lbmRPZlRhc2tSdW5uZXIpOworICAgICAgICBzX2VuZE9mVGFza1J1bm5lciA9IG5ldyBFbmRP ZlRhc2tSdW5uZXI7CisgICAgICAgIGN1cnJlbnRUaHJlYWQtPmFkZFRhc2tPYnNlcnZlcihzX2Vu ZE9mVGFza1J1bm5lcik7CisgICAgfQogI2VuZGlmCiB9CiAKQEAgLTEyNyw2ICsxMzAsNyBAQCB2 b2lkIHNodXRkb3duKCkKIHsKICNpZiBFTkFCTEUoTVVUQVRJT05fT0JTRVJWRVJTKQogICAgIGlm IChzX2VuZE9mVGFza1J1bm5lcikgeworICAgICAgICBBU1NFUlQoc193ZWJLaXRQbGF0Zm9ybVN1 cHBvcnQtPmN1cnJlbnRUaHJlYWQoKSk7CiAgICAgICAgIHNfd2ViS2l0UGxhdGZvcm1TdXBwb3J0 LT5jdXJyZW50VGhyZWFkKCktPnJlbW92ZVRhc2tPYnNlcnZlcihzX2VuZE9mVGFza1J1bm5lcik7 CiAgICAgICAgIGRlbGV0ZSBzX2VuZE9mVGFza1J1bm5lcjsKICAgICAgICAgc19lbmRPZlRhc2tS dW5uZXIgPSAwOwo=