[CrOS Tether] Remove spammy log from Initializer.

[CrOS Tether] Remove spammy log from Initializer.

Previously, we printed a warning log each time Init() was called when the feature had already been initialized. This resulted in spammy logs since this is expected to happen under some circumstances.

This CL also adds some extra tests in TetherService which ensure that Init() is called correctly.

BUG=672263, 736443
Review-Url: https://codereview.chromium.org/2961733002
Cr-Commit-Position: refs/heads/master@{#483195}
Committed: https://chromium.googlesource.com/chromium/src/+/9aa44ab0d44d18475363d33873f8160d9022cab1

[Kyle Horimoto, 2017-06-26 23:41:00]

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

[Ryan Hansberry, 2017-06-26 23:55:08]

https://codereview.chromium.org/2961733002/diff/1/chrome/browser/chromeos/tet...
File chrome/browser/chromeos/tether/tether_service.cc (right):

https://codereview.chromium.org/2961733002/diff/1/chrome/browser/chromeos/tet...
chrome/browser/chromeos/tether/tether_service.cc:209: if (!was_tether_enabled &&
Please add a test.

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

https://codereview.chromium.org/2961733002/diff/1/chromeos/components/tether/...
chromeos/components/tether/initializer.cc:76: PA_LOG(INFO) << "Initializer
Tether feature.";
Is this log needed? If so, the phrasing is awkward.

[Kyle Horimoto, 2017-06-27 00:53:38]

Description was changed from

==========
[CrOS Tether] Remove spammy log from Initializer.

Previously, we printed a warning log each time Init() was called when the
feature had already been initialized. This resulted in spammy logs since this is
expected to happen under some circumstances.

That being said, Init() was still being called more often than was needed. In
TetherService, anytime a property changes which may affect whether tethering
should be initialized or shut down, we would make a call to Init(). Instead, we
only need to make that call when it is now enabled but was not previously
enabled. I also introduced that change in this CL.

BUG=672263,736443
==========

to

==========
[CrOS Tether] Remove spammy log from Initializer.

Previously, we printed a warning log each time Init() was called when the
feature had already been initialized. This resulted in spammy logs since this is
expected to happen under some circumstances.

This CL also adds some extra tests in TetherService which ensure that Init() is
called correctly.

BUG=672263,736443
==========

[Kyle Horimoto, 2017-06-27 00:56:01]

https://codereview.chromium.org/2961733002/diff/1/chrome/browser/chromeos/tet...
File chrome/browser/chromeos/tether/tether_service.cc (right):

https://codereview.chromium.org/2961733002/diff/1/chrome/browser/chromeos/tet...
chrome/browser/chromeos/tether/tether_service.cc:209: if (!was_tether_enabled &&
On 2017/06/26 23:55:08, Ryan Hansberry wrote:
> Please add a test.

Added tests. When testing, I realized that this isn't actually correct all the
time. When changes occur as part of the tether toggle changing in settings, this
would not start/stop at the right time. Kept tests though.

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

https://codereview.chromium.org/2961733002/diff/1/chromeos/components/tether/...
chromeos/components/tether/initializer.cc:76: PA_LOG(INFO) << "Initializer
Tether feature.";
On 2017/06/26 23:55:08, Ryan Hansberry wrote:
> Is this log needed? If so, the phrasing is awkward.

Fixed typo. Yes, I think this log is useful. There previously was no log for the
success case.

[Ryan Hansberry, 2017-06-28 17:02:40]

lgtm with nit

https://codereview.chromium.org/2961733002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/tether/tether_service.cc (right):

https://codereview.chromium.org/2961733002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/tether/tether_service.cc:49: void
TetherService::TetherService::InitializerDelegate::InitializeTether(
Sorry for the noob question here: why is the double TetherService scoping
necessary? ("TetherService::TetherService::")

https://codereview.chromium.org/2961733002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/tether/tether_service.h (right):

https://codereview.chromium.org/2961733002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/tether/tether_service.h:71: class InitializerDelegate {
Please add a comment that this is a necessary instanced wrapper around
Initializer explicitly meant for testing.

[Kyle Horimoto, 2017-06-28 21:57:48]

https://codereview.chromium.org/2961733002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/tether/tether_service.cc (right):

https://codereview.chromium.org/2961733002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/tether/tether_service.cc:49: void
TetherService::TetherService::InitializerDelegate::InitializeTether(
On 2017/06/28 17:02:40, Ryan Hansberry wrote:
> Sorry for the noob question here: why is the double TetherService scoping
> necessary? ("TetherService::TetherService::")

Whoa, good catch. I didn't intend for this, and I didn't actually know this
would compile this way. Fixed.

https://codereview.chromium.org/2961733002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/tether/tether_service.h (right):

https://codereview.chromium.org/2961733002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/tether/tether_service.h:71: class InitializerDelegate {
On 2017/06/28 17:02:40, Ryan Hansberry wrote:
> Please add a comment that this is a necessary instanced wrapper around
> Initializer explicitly meant for testing. 

Done.

[Kyle Horimoto, 2017-06-28 21:57:52]

The CQ bit was checked by khorimoto@chromium.org

[Kyle Horimoto, 2017-06-28 21:57:52]

The patchset sent to the CQ was uploaded after l-g-t-m from
hansberry@chromium.org
Link to the patchset: https://codereview.chromium.org/2961733002/#ps60001
(title: "hansberry@ comments.")

[commit-bot: I haz the power, 2017-06-28 21:58:24]

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-06-28 23:33:11]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1498687072101450,
"parent_rev": "1f4be4da171cead27321f786d3b1306fd6bffeb3", "commit_rev":
"d18b5781407a6484d47965f926b9015a8004d618"}

[commit-bot: I haz the power, 2017-06-28 23:33:41]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1498687072101450,
"parent_rev": "2858a4923b7489ed301b13bbea89f71999553edd", "commit_rev":
"9aa44ab0d44d18475363d33873f8160d9022cab1"}

[commit-bot: I haz the power, 2017-06-28 23:33:55]

Description was changed from

==========
[CrOS Tether] Remove spammy log from Initializer.

Previously, we printed a warning log each time Init() was called when the
feature had already been initialized. This resulted in spammy logs since this is
expected to happen under some circumstances.

This CL also adds some extra tests in TetherService which ensure that Init() is
called correctly.

BUG=672263,736443
==========

to

==========
[CrOS Tether] Remove spammy log from Initializer.

Previously, we printed a warning log each time Init() was called when the
feature had already been initialized. This resulted in spammy logs since this is
expected to happen under some circumstances.

This CL also adds some extra tests in TetherService which ensure that Init() is
called correctly.

BUG=672263,736443

Review-Url: https://codereview.chromium.org/2961733002
Cr-Commit-Position: refs/heads/master@{#483195}
Committed:
https://chromium.googlesource.com/chromium/src/+/9aa44ab0d44d18475363d33873f8...
==========

[commit-bot: I haz the power, 2017-06-28 23:33:56]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/9aa44ab0d44d18475363d33873f8...