Observe IP address change in connectivity_checker.

Observe IP address change in connectivity_checker.

Observing ConnectionType alone doesn't detect network
changes on some OEM platforms. Observe IP address
to address this.

BUG=

Committed: https://crrev.com/0c95f2d29d7f869035d09d45944490f7e10acba2
Cr-Commit-Position: refs/heads/master@{#320148}

[wzhong, 2015-03-10 19:59:53]

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

[wzhong, 2015-03-11 17:09:54]

Ping.

[byungchul, 2015-03-11 17:18:03]

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

https://codereview.chromium.org/995933002/diff/1/chromecast/net/connectivity_...
chromecast/net/connectivity_checker.cc:71:
net::NetworkChangeNotifier::RemoveIPAddressObserver(this);
reverse the order.

https://codereview.chromium.org/995933002/diff/1/chromecast/net/connectivity_...
chromecast/net/connectivity_checker.cc:133: SetConnectivity(false);
Please comment why it sets connectivity false. Shouldn't it set false in
connection type changed to non-NONE?

[derekjchow1, 2015-03-11 17:25:01]

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

https://codereview.chromium.org/995933002/diff/1/chromecast/net/connectivity_...
chromecast/net/connectivity_checker.cc:133: SetConnectivity(false);
Don't SetConnectivity here since NetworkChangeNotifierLinux will notify if the
IP on any netif changes. This can result in a disconnect loop. Consider the
following scenario:

1) Ethernet cable is plugged, IP address is changed and SetConnectivity(false)
is called.
2) ConnectivityObserver brings up hotspot netif. IP address is changed again.
3) Connectivity checks occur, and SetConnectivity(true) is called.
4) Hotspot is brought down and IP address changes again. Repeat step 3.

Instead, we should rely on Check() to properly call SetConnectivity.

https://codereview.chromium.org/995933002/diff/1/chromecast/net/connectivity_...
File chromecast/net/connectivity_checker.h (right):

https://codereview.chromium.org/995933002/diff/1/chromecast/net/connectivity_...
chromecast/net/connectivity_checker.h:32: public
net::NetworkChangeNotifier::IPAddressObserver{
Add space:
"public net::NetworkChangeNotifier::IPAddressObserver {"

[wzhong, 2015-03-11 17:38:21]

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

https://codereview.chromium.org/995933002/diff/1/chromecast/net/connectivity_...
chromecast/net/connectivity_checker.cc:71:
net::NetworkChangeNotifier::RemoveIPAddressObserver(this);
On 2015/03/11 17:18:03, byungchul wrote:
> reverse the order.

Done.

https://codereview.chromium.org/995933002/diff/1/chromecast/net/connectivity_...
chromecast/net/connectivity_checker.cc:133: SetConnectivity(false);
On 2015/03/11 17:25:00, derekjchow1 wrote:
> Don't SetConnectivity here since NetworkChangeNotifierLinux will notify if the
> IP on any netif changes. This can result in a disconnect loop. Consider the
> following scenario:
> 
> 1) Ethernet cable is plugged, IP address is changed and SetConnectivity(false)
> is called.
> 2) ConnectivityObserver brings up hotspot netif. IP address is changed again.
> 3) Connectivity checks occur, and SetConnectivity(true) is called.
> 4) Hotspot is brought down and IP address changes again. Repeat step 3.
> 
> Instead, we should rely on Check() to properly call SetConnectivity.

Removed. Triggering a connectivity checking is sufficient.

https://codereview.chromium.org/995933002/diff/1/chromecast/net/connectivity_...
File chromecast/net/connectivity_checker.h (right):

https://codereview.chromium.org/995933002/diff/1/chromecast/net/connectivity_...
chromecast/net/connectivity_checker.h:32: public
net::NetworkChangeNotifier::IPAddressObserver{
On 2015/03/11 17:25:00, derekjchow1 wrote:
> Add space:
> "public net::NetworkChangeNotifier::IPAddressObserver {"

Done.

[byungchul, 2015-03-11 17:47:31]

lgtm

[wzhong, 2015-03-11 17:53:35]

The CQ bit was checked by wzhong@chromium.org

[commit-bot: I haz the power, 2015-03-11 17:53:48]

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

[commit-bot: I haz the power, 2015-03-11 17:53:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-11 17:53:50]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[derekjchow1, 2015-03-11 20:46:22]

lgtm

[gunsch, 2015-03-11 20:50:45]

lgtm

[wzhong, 2015-03-11 20:54:02]

The CQ bit was checked by wzhong@chromium.org

[commit-bot: I haz the power, 2015-03-11 20:54:37]

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

[commit-bot: I haz the power, 2015-03-11 21:35:43]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-03-11 21:36:29]

Patchset 2 (id:??) landed as
https://crrev.com/0c95f2d29d7f869035d09d45944490f7e10acba2
Cr-Commit-Position: refs/heads/master@{#320148}