Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2823323002:
cros: close dialog when shill detects captive portal network switched to online (Closed)
|
Created:
3 years, 8 months ago by Qiang(Joe) Xu Modified:
3 years, 8 months ago CC:
chromium-reviews, cbentzel+watch_chromium.org, stevenjb+watch_chromium.org, oshima+watch_chromium.org, davemoore+watch_chromium.org, net-reviews_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
Descriptioncros: close dialog when shill detects captive portal network switched to online
Changes:
(1) On crrev.com/2756643002, we stop the chrome portal detector attempts when chromebook is successfully behind the captive portal. Thus, the following passive attempts are only triggered by network state change or proxy change, like auth supplied. Currently, the portal dialog auto-close happens if chrome portal detection detects an online result. This used to be robust since we keep doing active chrome detection when we are behind the portal. But now if shill detects a online state, it triggers chrome portal detection but this detection times out (indicating that wifi is bad so that fetching gives no response). To make it robust, we can close the dialog when shill detects captive portal network switched to online state.
(2) SetIgnoreNoNetworkForTesting() is not needed any more since in tests, SetConnected happens before dialog is shown. The dialog wont be auto-closed for the tests.
BUG=705813
TEST=test that portal dialog can be closed when wifi signal is not that good.
Review-Url: https://codereview.chromium.org/2823323002
Cr-Commit-Position: refs/heads/master@{#465773}
Committed: https://chromium.googlesource.com/chromium/src/+/4179f08f0a422e3f78c822565cc067fee99987e9
Patch Set 1 #
Messages
Total messages: 23 (18 generated)
Description was changed from ========== cros: close dialog when shill detects captive portal network switched to online BUG=705813 ========== to ========== cros: close dialog when shill detects captive portal network switched to online BUG=705813 ==========
Description was changed from ========== cros: close dialog when shill detects captive portal network switched to online BUG=705813 ========== to ========== cros: close dialog when shill detects captive portal network switched to online BUG=705813 ==========
Description was changed from ========== cros: close dialog when shill detects captive portal network switched to online BUG=705813 ========== to ========== cros: close dialog when shill detects captive portal network switched to online Changes: (1) On crrev.com/2756643002, we stop the chrome portal detector attempts when chromebook is successfully behind the captive portal. Thus, the following passive attempts are only triggered by network state change or proxy change. Currently, the portal dialog auto-close happens if chrome portal detection detects an online result. This used to be robust since we are keep doing active chrome detection when we are behind the portal. But now if shill detects a online state, it triggers chrome portal detection but this detection times out (indicating that wifi is bad). To make it robust, we can close the dialog when shill detects captive portal network switched to online state. (2) SetIgnoreNoNetworkForTesting() is not needed in tests as portal dialog is not shown in network_portal_detector_impl_browsertest.cc at all. BUG=705813 TEST=test that portal dialog can be closed when wifi signal is not that good. ==========
warx@chromium.org changed reviewers: + stevenjb@chromium.org
Description was changed from ========== cros: close dialog when shill detects captive portal network switched to online Changes: (1) On crrev.com/2756643002, we stop the chrome portal detector attempts when chromebook is successfully behind the captive portal. Thus, the following passive attempts are only triggered by network state change or proxy change. Currently, the portal dialog auto-close happens if chrome portal detection detects an online result. This used to be robust since we are keep doing active chrome detection when we are behind the portal. But now if shill detects a online state, it triggers chrome portal detection but this detection times out (indicating that wifi is bad). To make it robust, we can close the dialog when shill detects captive portal network switched to online state. (2) SetIgnoreNoNetworkForTesting() is not needed in tests as portal dialog is not shown in network_portal_detector_impl_browsertest.cc at all. BUG=705813 TEST=test that portal dialog can be closed when wifi signal is not that good. ========== to ========== cros: close dialog when shill detects captive portal network switched to online Changes: (1) On crrev.com/2756643002, we stop the chrome portal detector attempts when chromebook is successfully behind the captive portal. Thus, the following passive attempts are only triggered by network state change or proxy change. Currently, the portal dialog auto-close happens if chrome portal detection detects an online result. This used to be robust since we keep doing active chrome detection when we are behind the portal. But now if shill detects a online state, it triggers chrome portal detection but this detection times out (indicating that wifi is bad). To make it robust, we can close the dialog when shill detects captive portal network switched to online state. (2) SetIgnoreNoNetworkForTesting() is not needed in tests as portal dialog is not shown in network_portal_detector_impl_browsertest.cc at all. BUG=705813 TEST=test that portal dialog can be closed when wifi signal is not that good. ==========
The CQ bit was checked by warx@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
warx@chromium.org changed reviewers: - stevenjb@chromium.org
Description was changed from ========== cros: close dialog when shill detects captive portal network switched to online Changes: (1) On crrev.com/2756643002, we stop the chrome portal detector attempts when chromebook is successfully behind the captive portal. Thus, the following passive attempts are only triggered by network state change or proxy change. Currently, the portal dialog auto-close happens if chrome portal detection detects an online result. This used to be robust since we keep doing active chrome detection when we are behind the portal. But now if shill detects a online state, it triggers chrome portal detection but this detection times out (indicating that wifi is bad). To make it robust, we can close the dialog when shill detects captive portal network switched to online state. (2) SetIgnoreNoNetworkForTesting() is not needed in tests as portal dialog is not shown in network_portal_detector_impl_browsertest.cc at all. BUG=705813 TEST=test that portal dialog can be closed when wifi signal is not that good. ========== to ========== cros: close dialog when shill detects captive portal network switched to online Changes: (1) On crrev.com/2756643002, we stop the chrome portal detector attempts when chromebook is successfully behind the captive portal. Thus, the following passive attempts are only triggered by network state change or proxy change. Currently, the portal dialog auto-close happens if chrome portal detection detects an online result. This used to be robust since we keep doing active chrome detection when we are behind the portal. But now if shill detects a online state, it triggers chrome portal detection but this detection times out (indicating that wifi is bad). To make it robust, we can close the dialog when shill detects captive portal network switched to online state. (2) SetIgnoreNoNetworkForTesting() is not needed any more since in tests, SetConnected happens before dialog is shown. The dialog wont be auto-closed for the tests. BUG=705813 TEST=test that portal dialog can be closed when wifi signal is not that good. ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== cros: close dialog when shill detects captive portal network switched to online Changes: (1) On crrev.com/2756643002, we stop the chrome portal detector attempts when chromebook is successfully behind the captive portal. Thus, the following passive attempts are only triggered by network state change or proxy change. Currently, the portal dialog auto-close happens if chrome portal detection detects an online result. This used to be robust since we keep doing active chrome detection when we are behind the portal. But now if shill detects a online state, it triggers chrome portal detection but this detection times out (indicating that wifi is bad). To make it robust, we can close the dialog when shill detects captive portal network switched to online state. (2) SetIgnoreNoNetworkForTesting() is not needed any more since in tests, SetConnected happens before dialog is shown. The dialog wont be auto-closed for the tests. BUG=705813 TEST=test that portal dialog can be closed when wifi signal is not that good. ========== to ========== cros: close dialog when shill detects captive portal network switched to online Changes: (1) On crrev.com/2756643002, we stop the chrome portal detector attempts when chromebook is successfully behind the captive portal. Thus, the following passive attempts are only triggered by network state change or proxy change. Currently, the portal dialog auto-close happens if chrome portal detection detects an online result. This used to be robust since we keep doing active chrome detection when we are behind the portal. But now if shill detects a online state, it triggers chrome portal detection but this detection times out (indicating that wifi is bad so that fetching gives on response). To make it robust, we can close the dialog when shill detects captive portal network switched to online state. (2) SetIgnoreNoNetworkForTesting() is not needed any more since in tests, SetConnected happens before dialog is shown. The dialog wont be auto-closed for the tests. BUG=705813 TEST=test that portal dialog can be closed when wifi signal is not that good. ==========
warx@chromium.org changed reviewers: + stevenjb@chromium.org
warx@chromium.org changed reviewers: + alemate@chromium.org
Description was changed from ========== cros: close dialog when shill detects captive portal network switched to online Changes: (1) On crrev.com/2756643002, we stop the chrome portal detector attempts when chromebook is successfully behind the captive portal. Thus, the following passive attempts are only triggered by network state change or proxy change. Currently, the portal dialog auto-close happens if chrome portal detection detects an online result. This used to be robust since we keep doing active chrome detection when we are behind the portal. But now if shill detects a online state, it triggers chrome portal detection but this detection times out (indicating that wifi is bad so that fetching gives on response). To make it robust, we can close the dialog when shill detects captive portal network switched to online state. (2) SetIgnoreNoNetworkForTesting() is not needed any more since in tests, SetConnected happens before dialog is shown. The dialog wont be auto-closed for the tests. BUG=705813 TEST=test that portal dialog can be closed when wifi signal is not that good. ========== to ========== cros: close dialog when shill detects captive portal network switched to online Changes: (1) On crrev.com/2756643002, we stop the chrome portal detector attempts when chromebook is successfully behind the captive portal. Thus, the following passive attempts are only triggered by network state change or proxy change, like auth supplied. Currently, the portal dialog auto-close happens if chrome portal detection detects an online result. This used to be robust since we keep doing active chrome detection when we are behind the portal. But now if shill detects a online state, it triggers chrome portal detection but this detection times out (indicating that wifi is bad so that fetching gives no response). To make it robust, we can close the dialog when shill detects captive portal network switched to online state. (2) SetIgnoreNoNetworkForTesting() is not needed any more since in tests, SetConnected happens before dialog is shown. The dialog wont be auto-closed for the tests. BUG=705813 TEST=test that portal dialog can be closed when wifi signal is not that good. ==========
PTAL, thanks When auth info is supplied, shill shall detect the state change from "portal" to "online". I think we can close the dialog right at this point: https://www.chromium.org/chromium-os/chromiumos-design-docs/network-portal-de...
lgtm
lgtm
The CQ bit was checked by warx@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1492637603669470, "parent_rev":
"f7fcc45c7cfb05f8fed045d38153792baba57e89", "commit_rev":
"4179f08f0a422e3f78c822565cc067fee99987e9"}
Message was sent while issue was closed.
Description was changed from ========== cros: close dialog when shill detects captive portal network switched to online Changes: (1) On crrev.com/2756643002, we stop the chrome portal detector attempts when chromebook is successfully behind the captive portal. Thus, the following passive attempts are only triggered by network state change or proxy change, like auth supplied. Currently, the portal dialog auto-close happens if chrome portal detection detects an online result. This used to be robust since we keep doing active chrome detection when we are behind the portal. But now if shill detects a online state, it triggers chrome portal detection but this detection times out (indicating that wifi is bad so that fetching gives no response). To make it robust, we can close the dialog when shill detects captive portal network switched to online state. (2) SetIgnoreNoNetworkForTesting() is not needed any more since in tests, SetConnected happens before dialog is shown. The dialog wont be auto-closed for the tests. BUG=705813 TEST=test that portal dialog can be closed when wifi signal is not that good. ========== to ========== cros: close dialog when shill detects captive portal network switched to online Changes: (1) On crrev.com/2756643002, we stop the chrome portal detector attempts when chromebook is successfully behind the captive portal. Thus, the following passive attempts are only triggered by network state change or proxy change, like auth supplied. Currently, the portal dialog auto-close happens if chrome portal detection detects an online result. This used to be robust since we keep doing active chrome detection when we are behind the portal. But now if shill detects a online state, it triggers chrome portal detection but this detection times out (indicating that wifi is bad so that fetching gives no response). To make it robust, we can close the dialog when shill detects captive portal network switched to online state. (2) SetIgnoreNoNetworkForTesting() is not needed any more since in tests, SetConnected happens before dialog is shown. The dialog wont be auto-closed for the tests. BUG=705813 TEST=test that portal dialog can be closed when wifi signal is not that good. Review-Url: https://codereview.chromium.org/2823323002 Cr-Commit-Position: refs/heads/master@{#465773} Committed: https://chromium.googlesource.com/chromium/src/+/4179f08f0a422e3f78c822565cc0... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/4179f08f0a422e3f78c822565cc0... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
