Tether: Display a 'setup required' notification when appropriate.

Tether: Display a 'setup required' notification when appropriate.

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

[Ryan Hansberry, 2017-05-31 22:52:25]

Description was changed from

==========
Tether: Display a 'setup required' notification when appropriate.

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

to

==========
Tether: Display a 'setup required' notification when appropriate.

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

[Ryan Hansberry, 2017-05-31 22:52:26]

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

[Kyle Horimoto, 2017-05-31 23:29:06]

lgtm

https://codereview.chromium.org/2917803002/diff/20001/chromeos/components/tet...
File chromeos/components/tether/initializer.cc (right):

https://codereview.chromium.org/2917803002/diff/20001/chromeos/components/tet...
chromeos/components/tether/initializer.cc:200: clock_ =
base::MakeUnique<base::DefaultClock>();
I commented on https://codereview.chromium.org/2870393003/, but I think you
missed it. Is it even necessary to create an instance field?

https://codereview.chromium.org/2917803002/diff/20001/chromeos/components/tet...
File chromeos/components/tether/tether_connector.h (right):

https://codereview.chromium.org/2917803002/diff/20001/chromeos/components/tet...
chromeos/components/tether/tether_connector.h:29: // UI, TetherConnector
receives a callback from NetworkConnectionHandler and
nit: This is my fault for forgetting to change this comment, but while you're
here, can you update this description? It's actually called by
NetworkConnectionHandlerTetherDelegate.

[Ryan Hansberry, 2017-06-01 21:14:13]

I've confirmed on a real device (i.e. the full flow with a phone returning
SETUP_REQUIRED) that this all works as expected. *However*, crbug.com/726866 is
preventing the notification from visually showing up.

https://codereview.chromium.org/2917803002/diff/20001/chromeos/components/tet...
File chromeos/components/tether/initializer.cc (right):

https://codereview.chromium.org/2917803002/diff/20001/chromeos/components/tet...
chromeos/components/tether/initializer.cc:200: clock_ =
base::MakeUnique<base::DefaultClock>();
On 2017/05/31 23:29:06, Kyle Horimoto wrote:
> I commented on https://codereview.chromium.org/2870393003/, but I think you
> missed it. Is it even necessary to create an instance field?

You're right this is unnecessary. Changing this will mean tweaking tests for
initializer and host_scanner, which I'd prefer not to do in this CL since it's
unrelated. I'll fix this in a subsequent CL.

https://codereview.chromium.org/2917803002/diff/20001/chromeos/components/tet...
File chromeos/components/tether/tether_connector.h (right):

https://codereview.chromium.org/2917803002/diff/20001/chromeos/components/tet...
chromeos/components/tether/tether_connector.h:29: // UI, TetherConnector
receives a callback from NetworkConnectionHandler and
On 2017/05/31 23:29:06, Kyle Horimoto wrote:
> nit: This is my fault for forgetting to change this comment, but while you're
> here, can you update this description? It's actually called by
> NetworkConnectionHandlerTetherDelegate.

Done.

[Ryan Hansberry, 2017-06-01 23:48:14]

The CQ bit was checked by hansberry@chromium.org

[Ryan Hansberry, 2017-06-01 23:48:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
khorimoto@chromium.org
Link to the patchset: https://codereview.chromium.org/2917803002/#ps40001
(title: "Rebase.")

[commit-bot: I haz the power, 2017-06-01 23:49:32]

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-02 00:26:33]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1496360894360110,
"parent_rev": "c32f05f1e460ccc7be3cd69b31fc05cccc191008", "commit_rev":
"968db9c86d7b8d6a949edecb4548ce33091b0a97"}

[commit-bot: I haz the power, 2017-06-02 00:26:44]

Description was changed from

==========
Tether: Display a 'setup required' notification when appropriate.

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

to

==========
Tether: Display a 'setup required' notification when appropriate.

BUG=672263

Review-Url: https://codereview.chromium.org/2917803002
Cr-Commit-Position: refs/heads/master@{#476495}
Committed:
https://chromium.googlesource.com/chromium/src/+/968db9c86d7b8d6a949edecb4548...
==========

[commit-bot: I haz the power, 2017-06-02 00:26:45]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/968db9c86d7b8d6a949edecb4548...