[Chromecast] Add connectivity checking for Chromecast.

[Chromecast] Add connectivity checking for Chromecast.

Adds ConnectivityChecker and CastResourceDispatcherHostDelegate
classes. Performs connectivity checks on startup, network changes
and failed resource loads.

R=gunsch@chromium.org,byungchul@chromium.org
BUG=336640
Committed: https://crrev.com/30d4c2223ff7482572ecbb1269083b58e4d90fb9
Cr-Commit-Position: refs/heads/master@{#317388}

[gunsch, 2015-02-20 01:13:16]

Please pick a better commit message / change the subject on this. Start with
"[Chromecast]" and describe something that will be useful in `git log
--oneline`.

[gunsch, 2015-02-20 01:14:07]

On 2015/02/20 01:13:16, gunsch wrote:
> Please pick a better commit message / change the subject on this. Start with
> "[Chromecast]" and describe something that will be useful in `git log
> --oneline`.

Also, BUG=336640

[derekjchow1, 2015-02-20 01:16:35]

On 2015/02/20 01:14:07, gunsch wrote:
> On 2015/02/20 01:13:16, gunsch wrote:
> > Please pick a better commit message / change the subject on this. Start with
> > "[Chromecast]" and describe something that will be useful in `git log
> > --oneline`.
> 
> Also, BUG=336640

Gah, thought Rietveld was asking me for the first patchset name.

Done.

[byungchul, 2015-02-20 01:32:39]

https://codereview.chromium.org/925183003/diff/1/chromecast/chromecast.gyp
File chromecast/chromecast.gyp (left):

https://codereview.chromium.org/925183003/diff/1/chromecast/chromecast.gyp#ol...
chromecast/chromecast.gyp:499: 'cast_net',
Not necessary? Internal source has this.

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

https://codereview.chromium.org/925183003/diff/1/chromecast/net/connectivity_...
chromecast/net/connectivity_checker.h:40: virtual ~ConnectivityObserver() {};
';' not necessary.

And, as far as I know, 'virtual' is not necessary though many pure classes has
that. Can you check if chromium complaints if it is not a virtual dtor?

[gunsch, 2015-02-20 01:56:03]

https://codereview.chromium.org/925183003/diff/1/chromecast/browser/cast_reso...
File chromecast/browser/cast_resource_dispatcher_host_delegate.h (right):

https://codereview.chromium.org/925183003/diff/1/chromecast/browser/cast_reso...
chromecast/browser/cast_resource_dispatcher_host_delegate.h:19: //
content::ResourceDispatcherHostDelegate implementation.
nit: prefer "implementation:"

https://codereview.chromium.org/925183003/diff/1/chromecast/chromecast.gyp
File chromecast/chromecast.gyp (right):

https://codereview.chromium.org/925183003/diff/1/chromecast/chromecast.gyp#ne...
chromecast/chromecast.gyp:80: 'net/network_change_notifier_cast.cc',
do you want to delete the NetworkChangeNotifierCast files, since you're making
cast_net a common target?

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

https://codereview.chromium.org/925183003/diff/1/chromecast/net/connectivity_...
chromecast/net/connectivity_checker.cc:24: // How often we perform connectivity
checks in seconds
nit: avoid first-person references in comments, here and line 27

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

https://codereview.chromium.org/925183003/diff/1/chromecast/net/connectivity_...
chromecast/net/connectivity_checker.h:59: // Empty constructor for test
alternately, make it private and add a protected

static ConnectivityChecker* CreateConnectivityCheckerForTesting()

Chromium has checks that enforce methods ending in "ForTesting" are only called
in test contexts

[gunsch, 2015-02-20 01:57:28]

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

https://codereview.chromium.org/925183003/diff/1/chromecast/net/connectivity_...
chromecast/net/connectivity_checker.cc:90: bad_responses_ = 0;
how about moving "bad_responses_ = 0" to "OnResponseStarted"? Then all the logic
involving the bad-response counting is in one single method.

[derekjchow1, 2015-02-20 02:24:17]

https://codereview.chromium.org/925183003/diff/1/chromecast/browser/cast_reso...
File chromecast/browser/cast_resource_dispatcher_host_delegate.h (right):

https://codereview.chromium.org/925183003/diff/1/chromecast/browser/cast_reso...
chromecast/browser/cast_resource_dispatcher_host_delegate.h:19: //
content::ResourceDispatcherHostDelegate implementation.
On 2015/02/20 01:56:03, gunsch wrote:
> nit: prefer "implementation:"

Done.

https://codereview.chromium.org/925183003/diff/1/chromecast/chromecast.gyp
File chromecast/chromecast.gyp (left):

https://codereview.chromium.org/925183003/diff/1/chromecast/chromecast.gyp#ol...
chromecast/chromecast.gyp:499: 'cast_net',
On 2015/02/20 01:32:39, byungchul wrote:
> Not necessary? Internal source has this.

In the context of this CL it is needed because Android also uses
ConnectivityChecker as well.

Internally, "cast_net" here is split into two different targets. I'm working to
remove one the internal targets to bring it in line with this CL.

https://codereview.chromium.org/925183003/diff/1/chromecast/chromecast.gyp
File chromecast/chromecast.gyp (right):

https://codereview.chromium.org/925183003/diff/1/chromecast/chromecast.gyp#ne...
chromecast/chromecast.gyp:80: 'net/network_change_notifier_cast.cc',
On 2015/02/20 01:56:03, gunsch wrote:
> do you want to delete the NetworkChangeNotifierCast files, since you're making
> cast_net a common target?

Given we're not finished moving to NetworkChangeNotifierLinux internally no.
I'll do a second CL to remove these files once we have done so.

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

https://codereview.chromium.org/925183003/diff/1/chromecast/net/connectivity_...
chromecast/net/connectivity_checker.cc:24: // How often we perform connectivity
checks in seconds
On 2015/02/20 01:56:03, gunsch wrote:
> nit: avoid first-person references in comments, here and line 27

Done.

https://codereview.chromium.org/925183003/diff/1/chromecast/net/connectivity_...
chromecast/net/connectivity_checker.cc:90: bad_responses_ = 0;
On 2015/02/20 01:57:28, gunsch wrote:
> how about moving "bad_responses_ = 0" to "OnResponseStarted"? Then all the
logic
> involving the bad-response counting is in one single method.

Done.

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

https://codereview.chromium.org/925183003/diff/1/chromecast/net/connectivity_...
chromecast/net/connectivity_checker.h:40: virtual ~ConnectivityObserver() {};
On 2015/02/20 01:32:39, byungchul wrote:
> ';' not necessary.
> 
> And, as far as I know, 'virtual' is not necessary though many pure classes has
> that. Can you check if chromium complaints if it is not a virtual dtor?

Done.

I removed the virtual keyword for the dtor in this instance and cast_shell built
fine. That being said, the style guide says that pure virtual classes should
have a virtual destructor.

https://codereview.chromium.org/925183003/diff/1/chromecast/net/connectivity_...
chromecast/net/connectivity_checker.h:59: // Empty constructor for test
On 2015/02/20 01:56:03, gunsch wrote:
> alternately, make it private and add a protected
> 
> static ConnectivityChecker* CreateConnectivityCheckerForTesting()
> 
> Chromium has checks that enforce methods ending in "ForTesting" are only
called
> in test contexts

This actually isn't referenced now. Removed.

[gunsch, 2015-02-20 19:51:20]

lgtm

[derekjchow1, 2015-02-20 20:09:31]

The CQ bit was checked by derekjchow@chromium.org

[commit-bot: I haz the power, 2015-02-20 20:10:56]

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

[commit-bot: I haz the power, 2015-02-20 20:30:36]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-02-20 20:31:22]

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