Makes NetworkChangeType enum public.

Makes NetworkChangeType enum visible in the net namespace for use in tests.
(Breaking out connection migration CL https://codereview.chromium.org/1327923002/.)

R=pauljensen@chromium.org, rch@chromium.org
BUG=

[Jana, 2015-12-22 23:03:32]

Description was changed from

==========
Adding Notify method to NetworkChangeNotifier for tests.
(Breaking out connection migration CL
https://codereview.chromium.org/1327923002/.)

R=pauljensen@chromium.org, rch@chromium.org
BUG=
==========

to

==========
Adding Notify method to NetworkChangeNotifier for tests. Also makes
NetworkChangeType enum visible in the net namespace for use in tests.
(Breaking out connection migration CL
https://codereview.chromium.org/1327923002/.)

R=pauljensen@chromium.org, rch@chromium.org
BUG=
==========

[Ryan Hamilton, 2015-12-22 23:12:57]

https://codereview.chromium.org/1549613002/diff/1/net/base/network_change_not...
File net/base/network_change_notifier.cc (right):

https://codereview.chromium.org/1549613002/diff/1/net/base/network_change_not...
net/base/network_change_notifier.cc:1083:
!NetworkChangeNotifier::test_notifications_only_) {
If you can arrange for this variable to be false (it looks like there is a
setter for it) then I think you can simply use this method?

[Jana, 2015-12-28 20:15:25]

On 2015/12/22 23:12:57, Ryan Hamilton wrote:
>
https://codereview.chromium.org/1549613002/diff/1/net/base/network_change_not...
> File net/base/network_change_notifier.cc (right):
> 
>
https://codereview.chromium.org/1549613002/diff/1/net/base/network_change_not...
> net/base/network_change_notifier.cc:1083:
> !NetworkChangeNotifier::test_notifications_only_) {
> If you can arrange for this variable to be false (it looks like there is a
> setter for it) then I think you can simply use this method?

This setter is a protected method and so cannot be called directly from the
test. I'd like Paul's input on whether we should make this method public.

[pauljensen, 2015-12-29 13:35:41]

https://codereview.chromium.org/1549613002/diff/1/net/base/network_change_not...
File net/base/network_change_notifier.cc (right):

https://codereview.chromium.org/1549613002/diff/1/net/base/network_change_not...
net/base/network_change_notifier.cc:1083:
!NetworkChangeNotifier::test_notifications_only_) {
On 2015/12/22 23:12:57, Ryan Hamilton wrote:
> If you can arrange for this variable to be false (it looks like there is a
> setter for it) then I think you can simply use this method?

+1 to Ryan's comment.  It should be false in your tests as your don't explicitly
set it to true (it defaults to false).  This method is protected, but you have
your own NCN impl in your test so it should be able to freely call this
function.

[Jana, 2015-12-29 20:13:14]

Description was changed from

==========
Adding Notify method to NetworkChangeNotifier for tests. Also makes
NetworkChangeType enum visible in the net namespace for use in tests.
(Breaking out connection migration CL
https://codereview.chromium.org/1327923002/.)

R=pauljensen@chromium.org, rch@chromium.org
BUG=
==========

to

==========
Makes NetworkChangeType enum visible in the net namespace for use in tests.
(Breaking out connection migration CL
https://codereview.chromium.org/1327923002/.)

R=pauljensen@chromium.org, rch@chromium.org
BUG=
==========

[Jana, 2015-12-29 20:13:25]

On 2015/12/29 13:35:41, pauljensen wrote:
>
https://codereview.chromium.org/1549613002/diff/1/net/base/network_change_not...
> File net/base/network_change_notifier.cc (right):
> 
>
https://codereview.chromium.org/1549613002/diff/1/net/base/network_change_not...
> net/base/network_change_notifier.cc:1083:
> !NetworkChangeNotifier::test_notifications_only_) {
> On 2015/12/22 23:12:57, Ryan Hamilton wrote:
> > If you can arrange for this variable to be false (it looks like there is a
> > setter for it) then I think you can simply use this method?
> 
> +1 to Ryan's comment.  It should be false in your tests as your don't
explicitly
> set it to true (it defaults to false).  This method is protected, but you have
> your own NCN impl in your test so it should be able to freely call this
> function.

Done.

[Ryan Hamilton, 2015-12-29 20:24:56]

On 2015/12/29 20:13:25, Jana wrote:
> On 2015/12/29 13:35:41, pauljensen wrote:
> >
>
https://codereview.chromium.org/1549613002/diff/1/net/base/network_change_not...
> > File net/base/network_change_notifier.cc (right):
> > 
> >
>
https://codereview.chromium.org/1549613002/diff/1/net/base/network_change_not...
> > net/base/network_change_notifier.cc:1083:
> > !NetworkChangeNotifier::test_notifications_only_) {
> > On 2015/12/22 23:12:57, Ryan Hamilton wrote:
> > > If you can arrange for this variable to be false (it looks like there is a
> > > setter for it) then I think you can simply use this method?
> > 
> > +1 to Ryan's comment.  It should be false in your tests as your don't
> explicitly
> > set it to true (it defaults to false).  This method is protected, but you
have
> > your own NCN impl in your test so it should be able to freely call this
> > function.
> 
> Done.

LGTM, though Paul should have the final say.

[pauljensen, 2015-12-30 13:21:57]

I'd really rather you didn't expose this enum publicly.  It has no pertinence to
the public NCN API, which is already a massive 400+ lines, so I think it's just
confusing things.  It looks like you're using just SOON_TO_BE_DISCONNECTED and
DISCONNECTED, so why not just add two functions to your
MockNetworkChangeNotifier: NotifyNetworkSoonToDisconnect(Network network) and
NotifyNetworkDisconnected(Network network) that use these enum values?

[Jana, 2016-01-07 17:23:00]

On 2015/12/30 13:21:57, pauljensen wrote:
> I'd really rather you didn't expose this enum publicly.  It has no pertinence
to
> the public NCN API, which is already a massive 400+ lines, so I think it's
just
> confusing things.  It looks like you're using just SOON_TO_BE_DISCONNECTED and
> DISCONNECTED, so why not just add two functions to your
> MockNetworkChangeNotifier: NotifyNetworkSoonToDisconnect(Network network) and
> NotifyNetworkDisconnected(Network network) that use these enum values?

Done.

[Jana, 2016-01-07 17:29:15]

Not submitting CL and closing this issue.