[CrOS Tether] Transform NetworkConnectionHandler to an interface.

[CrOS Tether] Transform NetworkConnectionHandler to an interface.

The implementation is now part of a class private to network_connection_handler.cc.

This is in preparation for using a test double for NetworkConnectionHandler.

BUG=672263
Review-Url: https://codereview.chromium.org/2861883002
Cr-Commit-Position: refs/heads/master@{#469229}
Committed: https://chromium.googlesource.com/chromium/src/+/a2ff3163407b76f7b8bfbc66b96d4bd0e21596b2

[Kyle Horimoto, 2017-05-03 21:46:58]

khorimoto@chromium.org changed reviewers:
+ stevenjb@chromium.org

[stevenjb, 2017-05-03 22:05:38]

https://codereview.chromium.org/2861883002/diff/1/chromeos/components/tether/...
File chromeos/components/tether/initializer_unittest.cc (right):

https://codereview.chromium.org/2861883002/diff/1/chromeos/components/tether/...
chromeos/components/tether/initializer_unittest.cc:97: bool check_error_state)
override {}
blank line between methods with an implementation. (inline accessors / setters
can be exempted, e.g. set_foo(int foo) { foo_ = foo}).

https://codereview.chromium.org/2861883002/diff/1/chromeos/components/tether/...
File chromeos/components/tether/tether_connector_unittest.cc (right):

https://codereview.chromium.org/2861883002/diff/1/chromeos/components/tether/...
chromeos/components/tether/tether_connector_unittest.cc:68: bool
check_error_state) override {}
blank lines

https://codereview.chromium.org/2861883002/diff/1/chromeos/network/network_co...
File chromeos/network/network_connection_handler.cc (right):

https://codereview.chromium.org/2861883002/diff/1/chromeos/network/network_co...
chromeos/network/network_connection_handler.cc:129: // Implementation.
Move this to a separate file.

https://codereview.chromium.org/2861883002/diff/1/chromeos/network/network_co...
File chromeos/network/network_connection_handler.h (right):

https://codereview.chromium.org/2861883002/diff/1/chromeos/network/network_co...
chromeos/network/network_connection_handler.h:47: public
NetworkStateHandlerObserver {
Put these in the Impl

https://codereview.chromium.org/2861883002/diff/1/chromeos/network/network_co...
chromeos/network/network_connection_handler.h:187: const
network_handler::ErrorCallback& error_callback);
These should also be in the Impl

https://codereview.chromium.org/2861883002/diff/1/chromeos/network/network_co...
chromeos/network/network_connection_handler.h:195: friend class NetworkHandler;
Don't do this, see below.

https://codereview.chromium.org/2861883002/diff/1/chromeos/network/network_co...
chromeos/network/network_connection_handler.h:196: friend class
NetworkConnectionHandlerTest;
Do we need this?

https://codereview.chromium.org/2861883002/diff/1/chromeos/network/network_co...
chromeos/network/network_connection_handler.h:199: static
std::unique_ptr<NetworkConnectionHandler> Create();
Just make this public, or better yet, just have NetworkHandler include the _impl
header and construct a NCHImpl (and remove ths).

https://codereview.chromium.org/2861883002/diff/1/chromeos/network/network_co...
File chromeos/network/network_connection_handler_unittest.cc (right):

https://codereview.chromium.org/2861883002/diff/1/chromeos/network/network_co...
chromeos/network/network_connection_handler_unittest.cc:155:
network_connection_handler_ = NetworkConnectionHandler::Create();
new NetworkConnectionHandlerImpl(), just like we do for
ManagedNetworkConfigurationHandlerImpl above.

https://codereview.chromium.org/2861883002/diff/1/chromeos/network/network_ha...
File chromeos/network/network_handler.cc (right):

https://codereview.chromium.org/2861883002/diff/1/chromeos/network/network_ha...
chromeos/network/network_handler.cc:48: network_connection_handler_ =
NetworkConnectionHandler::Create();
same here

[Kyle Horimoto, 2017-05-03 22:38:30]

https://codereview.chromium.org/2861883002/diff/1/chromeos/components/tether/...
File chromeos/components/tether/initializer_unittest.cc (right):

https://codereview.chromium.org/2861883002/diff/1/chromeos/components/tether/...
chromeos/components/tether/initializer_unittest.cc:97: bool check_error_state)
override {}
On 2017/05/03 22:05:38, stevenjb wrote:
> blank line between methods with an implementation. (inline accessors / setters
> can be exempted, e.g. set_foo(int foo) { foo_ = foo}).

Done.

https://codereview.chromium.org/2861883002/diff/1/chromeos/components/tether/...
File chromeos/components/tether/tether_connector_unittest.cc (right):

https://codereview.chromium.org/2861883002/diff/1/chromeos/components/tether/...
chromeos/components/tether/tether_connector_unittest.cc:68: bool
check_error_state) override {}
On 2017/05/03 22:05:38, stevenjb wrote:
> blank lines

Done.

https://codereview.chromium.org/2861883002/diff/1/chromeos/network/network_co...
File chromeos/network/network_connection_handler.cc (right):

https://codereview.chromium.org/2861883002/diff/1/chromeos/network/network_co...
chromeos/network/network_connection_handler.cc:129: // Implementation.
On 2017/05/03 22:05:38, stevenjb wrote:
> Move this to a separate file.

Done.

https://codereview.chromium.org/2861883002/diff/1/chromeos/network/network_co...
File chromeos/network/network_connection_handler.h (right):

https://codereview.chromium.org/2861883002/diff/1/chromeos/network/network_co...
chromeos/network/network_connection_handler.h:47: public
NetworkStateHandlerObserver {
On 2017/05/03 22:05:38, stevenjb wrote:
> Put these in the Impl

Done.

https://codereview.chromium.org/2861883002/diff/1/chromeos/network/network_co...
chromeos/network/network_connection_handler.h:187: const
network_handler::ErrorCallback& error_callback);
On 2017/05/03 22:05:38, stevenjb wrote:
> These should also be in the Impl

I'd like to keep these three functions in the base class as non-virtual
functions. I use these functions in my TetherConnector test and will also be
using them in my upcoming TetherDisconnector test.

https://codereview.chromium.org/2861883002/diff/1/chromeos/network/network_co...
chromeos/network/network_connection_handler.h:195: friend class NetworkHandler;
On 2017/05/03 22:05:38, stevenjb wrote:
> Don't do this, see below.

Done.

https://codereview.chromium.org/2861883002/diff/1/chromeos/network/network_co...
chromeos/network/network_connection_handler.h:196: friend class
NetworkConnectionHandlerTest;
On 2017/05/03 22:05:38, stevenjb wrote:
> Do we need this?

Done.

https://codereview.chromium.org/2861883002/diff/1/chromeos/network/network_co...
chromeos/network/network_connection_handler.h:199: static
std::unique_ptr<NetworkConnectionHandler> Create();
On 2017/05/03 22:05:38, stevenjb wrote:
> Just make this public, or better yet, just have NetworkHandler include the
_impl
> header and construct a NCHImpl (and remove ths).

Done.

https://codereview.chromium.org/2861883002/diff/1/chromeos/network/network_co...
File chromeos/network/network_connection_handler_unittest.cc (right):

https://codereview.chromium.org/2861883002/diff/1/chromeos/network/network_co...
chromeos/network/network_connection_handler_unittest.cc:155:
network_connection_handler_ = NetworkConnectionHandler::Create();
On 2017/05/03 22:05:38, stevenjb wrote:
> new NetworkConnectionHandlerImpl(), just like we do for
> ManagedNetworkConfigurationHandlerImpl above.

Done.

https://codereview.chromium.org/2861883002/diff/1/chromeos/network/network_ha...
File chromeos/network/network_handler.cc (right):

https://codereview.chromium.org/2861883002/diff/1/chromeos/network/network_ha...
chromeos/network/network_handler.cc:48: network_connection_handler_ =
NetworkConnectionHandler::Create();
On 2017/05/03 22:05:38, stevenjb wrote:
> same here

Done.

[stevenjb, 2017-05-04 00:05:06]

lgtm

[Kyle Horimoto, 2017-05-04 00:08:47]

The CQ bit was checked by khorimoto@chromium.org

[commit-bot: I haz the power, 2017-05-04 00:09:30]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-04 01:20:28]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493856527103150,
"parent_rev": "275c19ecdc32a7d1fc994f77b9972b041a86f7a7", "commit_rev":
"a2ff3163407b76f7b8bfbc66b96d4bd0e21596b2"}

[commit-bot: I haz the power, 2017-05-04 01:20:41]

Description was changed from

==========
[CrOS Tether] Transform NetworkConnectionHandler to an interface.

The implementation is now part of a class private to
network_connection_handler.cc.

This is in preparation for using a test double for NetworkConnectionHandler.

BUG=672263
==========

to

==========
[CrOS Tether] Transform NetworkConnectionHandler to an interface.

The implementation is now part of a class private to
network_connection_handler.cc.

This is in preparation for using a test double for NetworkConnectionHandler.

BUG=672263

Review-Url: https://codereview.chromium.org/2861883002
Cr-Commit-Position: refs/heads/master@{#469229}
Committed:
https://chromium.googlesource.com/chromium/src/+/a2ff3163407b76f7b8bfbc66b96d...
==========

[commit-bot: I haz the power, 2017-05-04 01:20:42]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/a2ff3163407b76f7b8bfbc66b96d...