Don't check connectivity if network is offline.

Don't check connectivity if network is offline.

Some netifs could be ignored by command line flag, --netifs-to-ignore, and
internet could be accessible via those netifs.
Many testing scripts and developers set this for ethernet link and
use ethernet to get testing results.

BUG=internal b/19593970

Committed: https://crrev.com/39d09da9ea1c6dbf3529b4febe4277db104a135f
Cr-Commit-Position: refs/heads/master@{#322288}

[byungchul, 2015-03-25 22:17:20]

byungchul@chromium.org changed reviewers:
+ derekjchow@chromium.org, gunsch@chromium.org, lcwu@chromium.org

[gunsch, 2015-03-25 22:36:03]

lgtm

[derekjchow1, 2015-03-25 22:37:06]

The CQ bit was checked by derekjchow@chromium.org

[derekjchow1, 2015-03-25 22:37:07]

The CQ bit was unchecked by derekjchow@chromium.org

[derekjchow1, 2015-03-25 22:37:27]

not lgtm

https://codereview.chromium.org/1037693005/diff/1/chromecast/net/connectivity...
File chromecast/net/connectivity_checker.cc (right):

https://codereview.chromium.org/1037693005/diff/1/chromecast/net/connectivity...
chromecast/net/connectivity_checker.cc:110: if
(net::NetworkChangeNotifier::IsOffline())
I don't think we should check here. What happens for non-test situations?
I think the following can occur.:

1) Ethernet plug gets pulled
2) Connection type is changed to NONE
3) OnConnectionTypeChanged calls Check
4) Check returns prematurely

SetConnectivity(false) is never called in this process.

https://codereview.chromium.org/1037693005/diff/1/chromecast/net/connectivity...
chromecast/net/connectivity_checker.cc:152: if (http_response_code < 400) {
Perhaps we should check (http_response_code < 400 &&
!net::NetworkChangeNotifier::IsOffline()) here?

[byungchul, 2015-03-25 22:44:40]

https://codereview.chromium.org/1037693005/diff/1/chromecast/net/connectivity...
File chromecast/net/connectivity_checker.cc (right):

https://codereview.chromium.org/1037693005/diff/1/chromecast/net/connectivity...
chromecast/net/connectivity_checker.cc:110: if
(net::NetworkChangeNotifier::IsOffline())
On 2015/03/25 22:37:26, derekjchow1 wrote:
> I don't think we should check here. What happens for non-test situations?
> I think the following can occur.:
> 
> 1) Ethernet plug gets pulled
> 2) Connection type is changed to NONE
> 3) OnConnectionTypeChanged calls Check
> 4) Check returns prematurely
> 
> SetConnectivity(false) is never called in this process.

SetConnectivity(false) is called by OnConnectionTypeChanged().

[derekjchow1, 2015-03-25 22:44:41]

Nevermind, we explicitly SetConnectivity to false in OnConnectionTypeChanged.

lgtm

[byungchul, 2015-03-26 00:22:52]

The CQ bit was checked by byungchul@chromium.org

[commit-bot: I haz the power, 2015-03-26 00:23:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1037693005/1

[commit-bot: I haz the power, 2015-03-26 01:28:36]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-03-26 01:29:31]

Patchset 1 (id:??) landed as
https://crrev.com/39d09da9ea1c6dbf3529b4febe4277db104a135f
Cr-Commit-Position: refs/heads/master@{#322288}