cricket::P2PTransportChannel::OnUnknownAddress duplicates connection creation logic
Reported by aab11223...@hotmail.com, Dec 28 2014
What steps will reproduce the problem? 1. look @ cricket::P2PTransportChannel::OnUnknownAddress What is the expected result? uses cricket::P2PTransportChannel::CreateConnection to create prflx connections if needed. What do you see instead? subset (e.g. some checks and logging missing) logic in cricket::P2PTransportChannel::CreateConnection is duplicated. What version of the product are you using? On what operating system? r7977 Please provide any additional information below. attached patch just reuses cricket::P2PTransportChannel::CreateConnection in cricket::P2PTransportChannel::OnUnknownAddress. was useful for me since e.g. does stuff log connection creates, etc.
Dec 29 2014,
@jiay, please help to evaluate this!
Dec 29 2014,
Guoweis, can you work to land this patch? Please make sure the author fills in one of the agreements at: http://www.webrtc.org/contributing#TOC-Contributor-Agreement and get added to the AUTHORS file.
Jan 10 2015,
In CreateConnection, a look up is done to make sure there is no existing connection with that remote candidate address. A similar but not exactly the same check was done priority to invoke port->CreateConnection inside the OnUnknownAddress. I'm not totally sure about this change. However, if you could tell me which logging you think is missing, we could consider adding it instead. Does this address your concern?
Feb 8 2016,
Honghai, can you take a quick look and see if this is worth taking still? This code has changed a lot in the last year.
Nov 8 2016,
Dec 5 2016,
[bulk-edit] This issue appears to have been triaged (as evidenced by the presence of the EngTriaged label) and also has an Owner -- hence, changing its status to Assigned. If the currently set Owner is wrong, then please re-assign to a correct Owner, or remove Owner and set status to Available.
Clearing owner and setting status to Available, since there haven't been any updates for > 1 year. Will be assigned again once priority is high enough.
Sign in to add a comment