Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2054583002:
Do not reconnect the network change signal each time the network manager is started (Closed)
DescriptionDo not reconnect the network change signal each time the network manager is started
Due to a bug, the NetworkManager was reconnecting to the NetworkMonitor's NetworkChanged signal every time the network manager is stopped and restarted. After each calls, one more listener was added to the signal and never removed - which caused OnNetworksChanged to be called multiple times on each actual network change.
Not sure if this had any negative effect other than the extraneous "Network changed" lines in WebRTC logs, but it wasn't working correctly either way.
The fix is to only subscribe to the signal once, when the NetworkMonitor is created.
TBR=pthatcher
BUG=
NOTRY=True
Committed: https://crrev.com/979c268830794316dc1d05ea7242eb7310f6bc23
Cr-Commit-Position: refs/heads/master@{#13105}
Patch Set 1 #
Total comments: 1
Patch Set 2 : Fixed spacing #Messages
Total messages: 21 (11 generated)
skvlad@webrtc.org changed reviewers: + honghaiz@webrtc.org, pthatcher@webrtc.org
lgtm Thanks for fixing this! Since this is a confirmed fix and it is a short one, do you want to put a TBR and land it? https://codereview.webrtc.org/2054583002/diff/1/webrtc/base/network.cc File webrtc/base/network.cc (left): https://codereview.webrtc.org/2054583002/diff/1/webrtc/base/network.cc#oldcod... webrtc/base/network.cc:767: this, &BasicNetworkManager::OnNetworksChanged); indent this line by 4 spaces from the previous one.
Description was changed from ========== Do not reconnect the network change signal each time the network manager is started Due to a bug, the NetworkManager was reconnecting to the NetworkMonitor's NetworkChanged signal every time the network manager is stopped and restarted. After each calls, one more listener was added to the signal and never removed - which caused OnNetworksChanged to be called multiple times on each actual network change. Not sure if this had any negative effect other than the extraneous "Network changed" lines in WebRTC logs, but it wasn't working correctly either way. The fix is to only subscribe to the signal once, when the NetworkMonitor is created. BUG= ========== to ========== Do not reconnect the network change signal each time the network manager is started Due to a bug, the NetworkManager was reconnecting to the NetworkMonitor's NetworkChanged signal every time the network manager is stopped and restarted. After each calls, one more listener was added to the signal and never removed - which caused OnNetworksChanged to be called multiple times on each actual network change. Not sure if this had any negative effect other than the extraneous "Network changed" lines in WebRTC logs, but it wasn't working correctly either way. The fix is to only subscribe to the signal once, when the NetworkMonitor is created. TBR=pthatcher BUG= ==========
The CQ bit was checked by skvlad@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from honghaiz@webrtc.org Link to the patchset: https://codereview.webrtc.org/2054583002/#ps20001 (title: "Fixed spacing")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2054583002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_dbg/builds/14038)
The CQ bit was checked by skvlad@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2054583002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_dbg/builds/14041)
Description was changed from ========== Do not reconnect the network change signal each time the network manager is started Due to a bug, the NetworkManager was reconnecting to the NetworkMonitor's NetworkChanged signal every time the network manager is stopped and restarted. After each calls, one more listener was added to the signal and never removed - which caused OnNetworksChanged to be called multiple times on each actual network change. Not sure if this had any negative effect other than the extraneous "Network changed" lines in WebRTC logs, but it wasn't working correctly either way. The fix is to only subscribe to the signal once, when the NetworkMonitor is created. TBR=pthatcher BUG= ========== to ========== Do not reconnect the network change signal each time the network manager is started Due to a bug, the NetworkManager was reconnecting to the NetworkMonitor's NetworkChanged signal every time the network manager is stopped and restarted. After each calls, one more listener was added to the signal and never removed - which caused OnNetworksChanged to be called multiple times on each actual network change. Not sure if this had any negative effect other than the extraneous "Network changed" lines in WebRTC logs, but it wasn't working correctly either way. The fix is to only subscribe to the signal once, when the NetworkMonitor is created. TBR=pthatcher BUG= NOTRY=True ==========
The CQ bit was checked by skvlad@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2054583002/20001
Message was sent while issue was closed.
Description was changed from ========== Do not reconnect the network change signal each time the network manager is started Due to a bug, the NetworkManager was reconnecting to the NetworkMonitor's NetworkChanged signal every time the network manager is stopped and restarted. After each calls, one more listener was added to the signal and never removed - which caused OnNetworksChanged to be called multiple times on each actual network change. Not sure if this had any negative effect other than the extraneous "Network changed" lines in WebRTC logs, but it wasn't working correctly either way. The fix is to only subscribe to the signal once, when the NetworkMonitor is created. TBR=pthatcher BUG= NOTRY=True ========== to ========== Do not reconnect the network change signal each time the network manager is started Due to a bug, the NetworkManager was reconnecting to the NetworkMonitor's NetworkChanged signal every time the network manager is stopped and restarted. After each calls, one more listener was added to the signal and never removed - which caused OnNetworksChanged to be called multiple times on each actual network change. Not sure if this had any negative effect other than the extraneous "Network changed" lines in WebRTC logs, but it wasn't working correctly either way. The fix is to only subscribe to the signal once, when the NetworkMonitor is created. TBR=pthatcher BUG= NOTRY=True ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
CQ bit was unchecked
Message was sent while issue was closed.
Description was changed from ========== Do not reconnect the network change signal each time the network manager is started Due to a bug, the NetworkManager was reconnecting to the NetworkMonitor's NetworkChanged signal every time the network manager is stopped and restarted. After each calls, one more listener was added to the signal and never removed - which caused OnNetworksChanged to be called multiple times on each actual network change. Not sure if this had any negative effect other than the extraneous "Network changed" lines in WebRTC logs, but it wasn't working correctly either way. The fix is to only subscribe to the signal once, when the NetworkMonitor is created. TBR=pthatcher BUG= NOTRY=True ========== to ========== Do not reconnect the network change signal each time the network manager is started Due to a bug, the NetworkManager was reconnecting to the NetworkMonitor's NetworkChanged signal every time the network manager is stopped and restarted. After each calls, one more listener was added to the signal and never removed - which caused OnNetworksChanged to be called multiple times on each actual network change. Not sure if this had any negative effect other than the extraneous "Network changed" lines in WebRTC logs, but it wasn't working correctly either way. The fix is to only subscribe to the signal once, when the NetworkMonitor is created. TBR=pthatcher BUG= NOTRY=True Committed: https://crrev.com/979c268830794316dc1d05ea7242eb7310f6bc23 Cr-Commit-Position: refs/heads/master@{#13105} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/979c268830794316dc1d05ea7242eb7310f6bc23 Cr-Commit-Position: refs/heads/master@{#13105} |
