168233 2017-02-13 10:08:02 -0800 [WebRTC] libwebrtc socket factory is not assigning the right socket type 2017-02-13 10:39:08 -0800 1 1 1 Unclassified WebKit WebKit2 WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 youennf youennf bfulgham commit-queue eric.carlson oldest_to_newest 1276223 0 youennf 2017-02-13 10:08:02 -0800 As spotted by Brent, libwebrtc socket factory is not assigning the right socket type. 1276225 1 301358 youennf 2017-02-13 10:09:06 -0800 Created attachment 301358 Patch 1276227 2 301358 bfulgham 2017-02-13 10:10:45 -0800 Comment on attachment 301358 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=301358&action=review It seems like we should have some kind of test for these code paths; I only stumbled upon this by chance. > Source/WebKit2/WebProcess/Network/webrtc/LibWebRTCSocketFactory.cpp:64 > + auto socket = std::make_unique<LibWebRTCSocket>(*this, ++s_uniqueSocketIdentifier, LibWebRTCSocket::Type::UDP, socketAddress, rtc::SocketAddress()); I approve this change! :-) > Source/WebKit2/WebProcess/Network/webrtc/LibWebRTCSocketFactory.cpp:81 > + auto socket = std::make_unique<LibWebRTCSocket>(*this, ++s_uniqueSocketIdentifier, LibWebRTCSocket::Type::ClientTCP, localSocketAddress, remoteSocketAddress); Hey! You caught one I missed! :-) 1276232 3 youennf 2017-02-13 10:29:22 -0800 (In reply to comment #2) > Comment on attachment 301358 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=301358&action=review > > It seems like we should have some kind of test for these code paths; I only > stumbled upon this by chance. I totally agree! I would like to test the whole code path: WebKit2 WebProcess creating webrtc sockets and calling NetworkProcess to do that. But then, the bots might not like that we do networking, although maybe 127.0.0.1 networking might work and be good enough for that purpose. 1276238 4 301358 commit-queue 2017-02-13 10:39:04 -0800 Comment on attachment 301358 Patch Clearing flags on attachment: 301358 Committed r212232: <http://trac.webkit.org/changeset/212232> 1276239 5 commit-queue 2017-02-13 10:39:08 -0800 All reviewed patches have been landed. Closing bug. 301358 2017-02-13 10:09:06 -0800 2017-02-13 10:39:04 -0800 Patch bug-168233-20170213100634.patch text/plain 2689 youennf U3VidmVyc2lvbiBSZXZpc2lvbjogMjEyMTk2CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggMDA5OTA3NjNiMjE1YTIz MDQzMTk1OTY1NzFjZjllMDhiN2MyYmJiMi4uYTc1YTkwMDIwMjY4MTJmNjcwY2JlNmUzYTE2MWNl MTUxYzliOThhNSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE0IEBACisyMDE3LTAyLTEzICBZb3Vl bm4gRmFibGV0ICA8eW91ZW5uZkBnbWFpbC5jb20+CisKKyAgICAgICAgW1dlYlJUQ10gbGlid2Vi cnRjIHNvY2tldCBmYWN0b3J5IGlzIG5vdCBhc3NpZ25pbmcgdGhlIHJpZ2h0IHNvY2tldCB0eXBl CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNjgyMzMK KworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFdlYlBy b2Nlc3MvTmV0d29yay93ZWJydGMvTGliV2ViUlRDU29ja2V0RmFjdG9yeS5jcHA6CisgICAgICAg IChXZWJLaXQ6OkxpYldlYlJUQ1NvY2tldEZhY3Rvcnk6OkNyZWF0ZVVkcFNvY2tldCk6CisgICAg ICAgIChXZWJLaXQ6OkxpYldlYlJUQ1NvY2tldEZhY3Rvcnk6OkNyZWF0ZUNsaWVudFRjcFNvY2tl dCk6CisKIDIwMTctMDItMTAgIEpvaG4gV2lsYW5kZXIgIDx3aWxhbmRlckBhcHBsZS5jb20+CiAK ICAgICAgICAgVXBkYXRlcyB0byBSZXNvdXJjZSBMb2FkIFN0YXRpc3RpY3M6IEdldCB0aGUgcmln aHQgd2Vic2l0ZSBkYXRhIHN0b3JlIGFuZCBpbnRyb2R1Y2UgdGltZW91dCBmb3IgdXNlciBpbnRl cmFjdGlvbgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9OZXR3b3JrL3dl YnJ0Yy9MaWJXZWJSVENTb2NrZXRGYWN0b3J5LmNwcCBiL1NvdXJjZS9XZWJLaXQyL1dlYlByb2Nl c3MvTmV0d29yay93ZWJydGMvTGliV2ViUlRDU29ja2V0RmFjdG9yeS5jcHAKaW5kZXggMzI5Njc2 NmNhMjJlNjY3ZDc3YjEyZTQ5NWEzMGI3OTZkZGRjOTcyZC4uMDFiNTRiNjk1NTE2M2ZhYjVkNTEw NmJiYzUzMTNmZTZiMDczZmJkNCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvV2ViUHJvY2Vz cy9OZXR3b3JrL3dlYnJ0Yy9MaWJXZWJSVENTb2NrZXRGYWN0b3J5LmNwcAorKysgYi9Tb3VyY2Uv V2ViS2l0Mi9XZWJQcm9jZXNzL05ldHdvcmsvd2VicnRjL0xpYldlYlJUQ1NvY2tldEZhY3Rvcnku Y3BwCkBAIC02MSw3ICs2MSw3IEBAIHJ0Yzo6QXN5bmNQYWNrZXRTb2NrZXQqIExpYldlYlJUQ1Nv Y2tldEZhY3Rvcnk6OkNyZWF0ZVNlcnZlclRjcFNvY2tldChjb25zdCBydGM6CiAKIHJ0Yzo6QXN5 bmNQYWNrZXRTb2NrZXQqIExpYldlYlJUQ1NvY2tldEZhY3Rvcnk6OkNyZWF0ZVVkcFNvY2tldChj b25zdCBydGM6OlNvY2tldEFkZHJlc3MmIHNvY2tldEFkZHJlc3MsIHVpbnQxNl90IG1pblBvcnQs IHVpbnQxNl90IG1heFBvcnQpCiB7Ci0gICAgYXV0byBzb2NrZXQgPSBzdGQ6Om1ha2VfdW5pcXVl PExpYldlYlJUQ1NvY2tldD4oKnRoaXMsICsrc191bmlxdWVTb2NrZXRJZGVudGlmaWVyLCBMaWJX ZWJSVENTb2NrZXQ6OlR5cGU6OlNlcnZlclRDUCwgc29ja2V0QWRkcmVzcywgcnRjOjpTb2NrZXRB ZGRyZXNzKCkpOworICAgIGF1dG8gc29ja2V0ID0gc3RkOjptYWtlX3VuaXF1ZTxMaWJXZWJSVENT b2NrZXQ+KCp0aGlzLCArK3NfdW5pcXVlU29ja2V0SWRlbnRpZmllciwgTGliV2ViUlRDU29ja2V0 OjpUeXBlOjpVRFAsIHNvY2tldEFkZHJlc3MsIHJ0Yzo6U29ja2V0QWRkcmVzcygpKTsKICAgICBt X3NvY2tldHMuc2V0KHNvY2tldC0+aWRlbnRpZmllcigpLCBzb2NrZXQuZ2V0KCkpOwogCiAgICAg YXV0byBpZGVudGlmaWVyID0gc29ja2V0LT5pZGVudGlmaWVyKCk7CkBAIC03OCw3ICs3OCw3IEBA IHJ0Yzo6QXN5bmNQYWNrZXRTb2NrZXQqIExpYldlYlJUQ1NvY2tldEZhY3Rvcnk6OkNyZWF0ZVVk cFNvY2tldChjb25zdCBydGM6OlNvY2tlCiAKIHJ0Yzo6QXN5bmNQYWNrZXRTb2NrZXQqIExpYldl YlJUQ1NvY2tldEZhY3Rvcnk6OkNyZWF0ZUNsaWVudFRjcFNvY2tldChjb25zdCBydGM6OlNvY2tl dEFkZHJlc3MmIGxvY2FsU29ja2V0QWRkcmVzcywgY29uc3QgcnRjOjpTb2NrZXRBZGRyZXNzJiBy ZW1vdGVTb2NrZXRBZGRyZXNzLCBjb25zdCBydGM6OlByb3h5SW5mbyYsIGNvbnN0IHN0ZDo6c3Ry aW5nJiwgaW50IG9wdGlvbnMpCiB7Ci0gICAgYXV0byBzb2NrZXQgPSBzdGQ6Om1ha2VfdW5pcXVl PExpYldlYlJUQ1NvY2tldD4oKnRoaXMsICsrc191bmlxdWVTb2NrZXRJZGVudGlmaWVyLCBMaWJX ZWJSVENTb2NrZXQ6OlR5cGU6OlNlcnZlclRDUCwgbG9jYWxTb2NrZXRBZGRyZXNzLCByZW1vdGVT b2NrZXRBZGRyZXNzKTsKKyAgICBhdXRvIHNvY2tldCA9IHN0ZDo6bWFrZV91bmlxdWU8TGliV2Vi UlRDU29ja2V0PigqdGhpcywgKytzX3VuaXF1ZVNvY2tldElkZW50aWZpZXIsIExpYldlYlJUQ1Nv Y2tldDo6VHlwZTo6Q2xpZW50VENQLCBsb2NhbFNvY2tldEFkZHJlc3MsIHJlbW90ZVNvY2tldEFk ZHJlc3MpOwogICAgIHNvY2tldC0+c2V0U3RhdGUoTGliV2ViUlRDU29ja2V0OjpTVEFURV9DT05O RUNUSU5HKTsKICAgICBtX3NvY2tldHMuc2V0KHNvY2tldC0+aWRlbnRpZmllcigpLCBzb2NrZXQu Z2V0KCkpOwogCg==