Add throbber and status text to WebBluetooth chooser UI on non-Mac desktops

Add throbber and status text to WebBluetooth chooser UI on non-Mac desktops

This is part 3 of matching WebBluetooth chooser views to mocks.

This CL adds:
1. a throbber that starts when the the chooser scans for Bluetooth devices
and stops when the scan completes.
2. status text:
"Bluetooth turned off" when the Bluetooth adapter is turned off.
"Scanning for devices..." when chooser is scanning for Bluetooth devices.
"Re-scan" link and clicks it to re-scan Bluetooth devices.

The screenshots were uploaded on the issue page.

BUG=583452
Committed: https://crrev.com/9c9d751f358ea3e5e9b2076aecde6842016cbf0f
Cr-Commit-Position: refs/heads/master@{#406433}

[juncai, 2016-07-15 18:43:16]

The CQ bit was checked by juncai@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-15 18:43:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[juncai, 2016-07-15 19:34:53]

The CQ bit was checked by juncai@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-15 19:35:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[juncai, 2016-07-15 19:40:22]

juncai@chromium.org changed reviewers:
+ msw@chromium.org, reillyg@chromium.org, scheib@chromium.org

[juncai, 2016-07-15 19:40:23]

msw@chromium.org, please review changes in:
//chrome/browser/ui/cocoa/
//chrome/browser/ui/views/

scheib@chromium.org, please review changes in:
//chrome/browser/ui/bluetooth/

reillyg@chromium.org, please review changes in:
//chrome/browser/chooser_controller/
//chrome/browser/usb/

[juncai, 2016-07-15 19:42:18]

Description was changed from

==========
Add throbber and status text to WebBluetooth chooser UI on non-Mac desktops

This is part 3 of matching WebBluetooth chooser views to mocks.

This CL adds:
1. a throbber that starts when the the chooser scans for Bluetooth devices
and stops when the scan completes.
2. status text:
"Bluetooth turned off" when the Bluetooth adapter is turned off.
"Scanning for devices..." when chooser is scanning for Bluetooth devices.
"Re-scan" link and clicks it to re-scan Bluetooth devices.

BUG=583452
==========

to

==========
Add throbber and status text to WebBluetooth chooser UI on non-Mac desktops

This is part 3 of matching WebBluetooth chooser views to mocks.

This CL adds:
1. a throbber that starts when the the chooser scans for Bluetooth devices
and stops when the scan completes.
2. status text:
"Bluetooth turned off" when the Bluetooth adapter is turned off.
"Scanning for devices..." when chooser is scanning for Bluetooth devices.
"Re-scan" link and clicks it to re-scan Bluetooth devices.

The screenshots were uploaded on the issue page.

BUG=583452
==========

[commit-bot: I haz the power, 2016-07-15 20:47:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-15 20:47:22]

Dry run: This issue passed the CQ dry run.

[Reilly Grant (use Gerrit), 2016-07-15 21:55:20]

https://codereview.chromium.org/2155743002/diff/20001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2155743002/diff/20001/chrome/app/generated_re...
chrome/app/generated_resources.grd:14871: +      <message
name="IDS_BLUETOOTH_DEVICE_CHOOSER_RE_SCAN" desc="Text of the 'Re-scan' link.">
This description is tautological. I suggest, "Text of a link the user can click
to start a new scan for Bluetooth devices."

https://codereview.chromium.org/2155743002/diff/20001/chrome/browser/chooser_...
File chrome/browser/chooser_controller/chooser_controller.h (right):

https://codereview.chromium.org/2155743002/diff/20001/chrome/browser/chooser_...
chrome/browser/chooser_controller/chooser_controller.h:49: // Called when the
device adapter is truned on.
s/truned/turned/

https://codereview.chromium.org/2155743002/diff/20001/chrome/browser/chooser_...
chrome/browser/chooser_controller/chooser_controller.h:52: // Called when the
device adapter is truned off.
s/truned/turned/

https://codereview.chromium.org/2155743002/diff/20001/chrome/browser/chooser_...
chrome/browser/chooser_controller/chooser_controller.h:56: virtual void
Refresh() = 0;
I suggest calling these "RefreshInProgress" and "RefreshComplete" or having a
single "SetRefreshing" method that takes a boolean. Same above, you can have a
single "AdapterEnabled" method.

https://codereview.chromium.org/2155743002/diff/20001/chrome/browser/chooser_...
chrome/browser/chooser_controller/chooser_controller.h:84: virtual bool
NeedsThrobber() const = 0;
This seems redundant. If the set of options is refreshing we should show a
throbber. The USB chooser will never be in the refreshing state. Actually, it
could be during the initial enumeration. It would be nice to have a throbber
then.

https://codereview.chromium.org/2155743002/diff/20001/chrome/browser/chooser_...
chrome/browser/chooser_controller/chooser_controller.h:87: virtual bool
NeedsStatus() const = 0;
Maybe just call this "GetStatus" and return an empty string if no status text is
necessary.

[juncai, 2016-07-18 20:10:47]

The CQ bit was checked by juncai@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-18 20:11:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[juncai, 2016-07-18 20:14:16]

https://codereview.chromium.org/2155743002/diff/20001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2155743002/diff/20001/chrome/app/generated_re...
chrome/app/generated_resources.grd:14871: +      <message
name="IDS_BLUETOOTH_DEVICE_CHOOSER_RE_SCAN" desc="Text of the 'Re-scan' link.">
On 2016/07/15 21:55:19, Reilly Grant wrote:
> This description is tautological. I suggest, "Text of a link the user can
click
> to start a new scan for Bluetooth devices."

Done.

https://codereview.chromium.org/2155743002/diff/20001/chrome/browser/chooser_...
File chrome/browser/chooser_controller/chooser_controller.h (right):

https://codereview.chromium.org/2155743002/diff/20001/chrome/browser/chooser_...
chrome/browser/chooser_controller/chooser_controller.h:49: // Called when the
device adapter is truned on.
On 2016/07/15 21:55:20, Reilly Grant wrote:
> s/truned/turned/

Done.

https://codereview.chromium.org/2155743002/diff/20001/chrome/browser/chooser_...
chrome/browser/chooser_controller/chooser_controller.h:52: // Called when the
device adapter is truned off.
On 2016/07/15 21:55:20, Reilly Grant wrote:
> s/truned/turned/

Done.

https://codereview.chromium.org/2155743002/diff/20001/chrome/browser/chooser_...
chrome/browser/chooser_controller/chooser_controller.h:56: virtual void
Refresh() = 0;
On 2016/07/15 21:55:20, Reilly Grant wrote:
> I suggest calling these "RefreshInProgress" and "RefreshComplete" or having a
> single "SetRefreshing" method that takes a boolean. Same above, you can have a
> single "AdapterEnabled" method.

Done.

https://codereview.chromium.org/2155743002/diff/20001/chrome/browser/chooser_...
chrome/browser/chooser_controller/chooser_controller.h:84: virtual bool
NeedsThrobber() const = 0;
On 2016/07/15 21:55:20, Reilly Grant wrote:
> This seems redundant. If the set of options is refreshing we should show a
> throbber. The USB chooser will never be in the refreshing state. Actually, it
> could be during the initial enumeration. It would be nice to have a throbber
> then.

Done.

https://codereview.chromium.org/2155743002/diff/20001/chrome/browser/chooser_...
chrome/browser/chooser_controller/chooser_controller.h:87: virtual bool
NeedsStatus() const = 0;
On 2016/07/15 21:55:20, Reilly Grant wrote:
> Maybe just call this "GetStatus" and return an empty string if no status text
is
> necessary.

Done.

[Reilly Grant (use Gerrit), 2016-07-18 21:15:03]

lgtm with nits

https://codereview.chromium.org/2155743002/diff/40001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2155743002/diff/40001/chrome/app/generated_re...
chrome/app/generated_resources.grd:14866: +        No devices found. Bluetooth
turned off.
Just "Bluetooth turned off." It's simpler.

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/chooser_...
File chrome/browser/chooser_controller/chooser_controller.h (right):

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/chooser_...
chrome/browser/chooser_controller/chooser_controller.h:62: // Returns the text
to be displayed in the chooser when there is no option.
s/is no option/are no options/

[commit-bot: I haz the power, 2016-07-18 21:47:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-18 21:47:38]

Dry run: This issue passed the CQ dry run.

[msw, 2016-07-18 23:01:31]

https://codereview.chromium.org/2155743002/diff/40001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2155743002/diff/40001/chrome/app/generated_re...
chrome/app/generated_resources.grd:14866: +        No devices found. Bluetooth
turned off.
On 2016/07/18 21:15:03, Reilly Grant wrote:
> Just "Bluetooth turned off." It's simpler.

Ditto for the id (IDS_BLUETOOTH_DEVICE_CHOOSER_ADAPTER_OFF?) & comment.

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/chooser_...
File chrome/browser/chooser_controller/chooser_controller.h (right):

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/chooser_...
chrome/browser/chooser_controller/chooser_controller.h:50: virtual void
AdapterEnabled(bool enabled) = 0;
nit: OnAdapterEnabledChanged? SetAdapterEnabled?

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/chooser_...
chrome/browser/chooser_controller/chooser_controller.h:53: virtual void
SetRefreshing(bool refreshing) = 0;
nit: OnRefreshStateChanged?

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/chooser_...
File chrome/browser/chooser_controller/mock_chooser_controller.cc (right):

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/chooser_...
chrome/browser/chooser_controller/mock_chooser_controller.cc:18: return
l10n_util::GetStringUTF16(IDS_DEVICE_CHOOSER_NO_DEVICES_FOUND_PROMPT);
Could the mock return an empty string like GetStatus?

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/bluet...
File chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc (right):

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/bluet...
chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc:102: case
content::BluetoothChooser::AdapterPresence::ABSENT:
Should this case send observer()->AdapterEnabled(false)?

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/bluet...
chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc:109:
observer()->AdapterEnabled(false /* Bluetooth adapter is truned off */);
nit: 'turned'

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/bluet...
File chrome/browser/ui/bluetooth/bluetooth_chooser_controller.h (right):

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/bluet...
chrome/browser/ui/bluetooth/bluetooth_chooser_controller.h:43: void
UpdateAdapterPresence(
nit: OnAdapterPresenceChanged? SetAdapterPresence?

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/bluet...
chrome/browser/ui/bluetooth/bluetooth_chooser_controller.h:48: void
UpdateDiscoveryState(content::BluetoothChooser::DiscoveryState state);
nit: OnDiscoveryStateChanged? SetDiscoveryState?

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/chooser_content_view.cc (right):

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view.cc:39: SetLayoutManager(layout);
nit: inline |new views::FillLayout()| here; |layout| isn't used otherwise

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view.cc:44: // Set the throbber in the
center of the chooser rectangular.
nit: 'rectangle' or just "in the center of the chooser"

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view.cc:100: // No row is selected since
the adapter status is changed.
nit: "has changed"

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view.cc:101: // This will also disable
the OK button if it is enabled because
nit: "if it was"

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view.cc:102: // of previously selected
row.
nit: "a previously"

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view.cc:116: // This will also disable
the OK button if it is enabled because
nit: "if it was"

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view.cc:117: // of previously selected
row.
nit: "a previously"

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view.cc:128: // When refreshing, show
|throbber_|. When refreshing is complete, hide
nit: "When complete"

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/chooser_content_view.h (right):

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view.h:76: views::TableView*
table_view_;
nit: = nullptr;

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view.h:77: views::Throbber* throbber_;
nit: = nullptr;

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/chooser_content_view_unittest.cc (left):

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view_unittest.cc:309:
TEST_F(ChooserContentViewTest, ClickStyledLabelLink) {
nit: can you test the same with the ChooserContentView?

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/extensions/chooser_dialog_view.cc (right):

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/extensions/chooser_dialog_view.cc:15: #include
"ui/views/controls/styled_label.h"
nit: remove

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc
(left):

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc:83:
IN_PROC_BROWSER_TEST_F(ChooserDialogViewTest, ClickStyledLabelLink) {
nit: can you test the same with ChooserContentView?

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/website_settings/chooser_bubble_ui_view.cc (right):

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/website_settings/chooser_bubble_ui_view.cc:25: #include
"ui/views/controls/styled_label.h"
nit: remove

[juncai, 2016-07-19 20:33:05]

The CQ bit was checked by juncai@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-19 20:34:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[juncai, 2016-07-19 20:42:46]

https://codereview.chromium.org/2155743002/diff/40001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2155743002/diff/40001/chrome/app/generated_re...
chrome/app/generated_resources.grd:14866: +        No devices found. Bluetooth
turned off.
On 2016/07/18 21:15:03, Reilly Grant wrote:
> Just "Bluetooth turned off." It's simpler.

Done.

https://codereview.chromium.org/2155743002/diff/40001/chrome/app/generated_re...
chrome/app/generated_resources.grd:14866: +        No devices found. Bluetooth
turned off.
On 2016/07/18 23:01:30, msw wrote:
> On 2016/07/18 21:15:03, Reilly Grant wrote:
> > Just "Bluetooth turned off." It's simpler.
> 
> Ditto for the id (IDS_BLUETOOTH_DEVICE_CHOOSER_ADAPTER_OFF?) & comment.

Done.

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/chooser_...
File chrome/browser/chooser_controller/chooser_controller.h (right):

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/chooser_...
chrome/browser/chooser_controller/chooser_controller.h:50: virtual void
AdapterEnabled(bool enabled) = 0;
On 2016/07/18 23:01:30, msw wrote:
> nit: OnAdapterEnabledChanged? SetAdapterEnabled?

Done.

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/chooser_...
chrome/browser/chooser_controller/chooser_controller.h:53: virtual void
SetRefreshing(bool refreshing) = 0;
On 2016/07/18 23:01:30, msw wrote:
> nit: OnRefreshStateChanged?

Done.

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/chooser_...
chrome/browser/chooser_controller/chooser_controller.h:62: // Returns the text
to be displayed in the chooser when there is no option.
On 2016/07/18 21:15:03, Reilly Grant wrote:
> s/is no option/are no options/

Done.

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/chooser_...
File chrome/browser/chooser_controller/mock_chooser_controller.cc (right):

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/chooser_...
chrome/browser/chooser_controller/mock_chooser_controller.cc:18: return
l10n_util::GetStringUTF16(IDS_DEVICE_CHOOSER_NO_DEVICES_FOUND_PROMPT);
On 2016/07/18 23:01:30, msw wrote:
> Could the mock return an empty string like GetStatus?

It could, but the unit test needs to check what is shown on the table view when
there are no devices:
https://cs.chromium.org/chromium/src/chrome/browser/ui/views/chooser_content_...

Also maybe it is better to use a string literal in the unit test instead of
using IDS_DEVICE_CHOOSER_NO_DEVICES_FOUND_PROMPT? Just in case the
IDS_DEVICE_CHOOSER_NO_DEVICES_FOUND_PROMPT is accidentally modified. I made the
changes in chooser_content_view_unittest.cc.

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/bluet...
File chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc (right):

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/bluet...
chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc:102: case
content::BluetoothChooser::AdapterPresence::ABSENT:
On 2016/07/18 23:01:30, msw wrote:
> Should this case send observer()->AdapterEnabled(false)?

When there is no Bluetooth adapter, the Bluetooth chooser won't show up. So this
line of code won't be reached. Maybe add a NOTREACHED();?

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/bluet...
chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc:109:
observer()->AdapterEnabled(false /* Bluetooth adapter is truned off */);
On 2016/07/18 23:01:30, msw wrote:
> nit: 'turned'

Done.

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/bluet...
File chrome/browser/ui/bluetooth/bluetooth_chooser_controller.h (right):

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/bluet...
chrome/browser/ui/bluetooth/bluetooth_chooser_controller.h:43: void
UpdateAdapterPresence(
On 2016/07/18 23:01:30, msw wrote:
> nit: OnAdapterPresenceChanged? SetAdapterPresence?

Done.

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/bluet...
chrome/browser/ui/bluetooth/bluetooth_chooser_controller.h:48: void
UpdateDiscoveryState(content::BluetoothChooser::DiscoveryState state);
On 2016/07/18 23:01:30, msw wrote:
> nit: OnDiscoveryStateChanged? SetDiscoveryState?

Done.

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/chooser_content_view.cc (right):

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view.cc:39: SetLayoutManager(layout);
On 2016/07/18 23:01:31, msw wrote:
> nit: inline |new views::FillLayout()| here; |layout| isn't used otherwise

Done.

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view.cc:44: // Set the throbber in the
center of the chooser rectangular.
On 2016/07/18 23:01:30, msw wrote:
> nit: 'rectangle' or just "in the center of the chooser"

Done.

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view.cc:100: // No row is selected since
the adapter status is changed.
On 2016/07/18 23:01:30, msw wrote:
> nit: "has changed"

Done.

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view.cc:101: // This will also disable
the OK button if it is enabled because
On 2016/07/18 23:01:30, msw wrote:
> nit: "if it was"

Done.

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view.cc:102: // of previously selected
row.
On 2016/07/18 23:01:30, msw wrote:
> nit: "a previously"

Done.

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view.cc:116: // This will also disable
the OK button if it is enabled because
On 2016/07/18 23:01:30, msw wrote:
> nit: "if it was"

Done.

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view.cc:117: // of previously selected
row.
On 2016/07/18 23:01:30, msw wrote:
> nit: "a previously"

Done.

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view.cc:128: // When refreshing, show
|throbber_|. When refreshing is complete, hide
On 2016/07/18 23:01:31, msw wrote:
> nit: "When complete"

Done.

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/chooser_content_view.h (right):

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view.h:76: views::TableView*
table_view_;
On 2016/07/18 23:01:31, msw wrote:
> nit: = nullptr;

Since |table_view_| is always initialized in the constructor, setting it to
nullptr here seems redundant?

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view.h:77: views::Throbber* throbber_;
On 2016/07/18 23:01:31, msw wrote:
> nit: = nullptr;

ditto.

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/chooser_content_view_unittest.cc (left):

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view_unittest.cc:309:
TEST_F(ChooserContentViewTest, ClickStyledLabelLink) {
On 2016/07/18 23:01:31, msw wrote:
> nit: can you test the same with the ChooserContentView?

Done.

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/extensions/chooser_dialog_view.cc (right):

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/extensions/chooser_dialog_view.cc:15: #include
"ui/views/controls/styled_label.h"
On 2016/07/18 23:01:31, msw wrote:
> nit: remove

If removed,
return chooser_content_view_->CreateFootnoteView();
will have compile error:
cannot initialize return object of type 'views::View *' with an rvalue of type
'views::StyledLabel *'

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc
(left):

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc:83:
IN_PROC_BROWSER_TEST_F(ChooserDialogViewTest, ClickStyledLabelLink) {
On 2016/07/18 23:01:31, msw wrote:
> nit: can you test the same with ChooserContentView?

Done.

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/website_settings/chooser_bubble_ui_view.cc (right):

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/website_settings/chooser_bubble_ui_view.cc:25: #include
"ui/views/controls/styled_label.h"
On 2016/07/18 23:01:31, msw wrote:
> nit: remove

If removed,
return chooser_content_view_->CreateFootnoteView();
will have compile error:
cannot initialize return object of type 'views::View *' with an rvalue of type
'views::StyledLabel *'

[juncai, 2016-07-19 20:58:19]

The CQ bit was checked by juncai@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-19 20:58:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[scheib, 2016-07-19 21:18:13]

//chrome/browser/ui/bluetooth/ LGTM

https://codereview.chromium.org/2155743002/diff/80001/chrome/browser/ui/bluet...
File chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc (right):

https://codereview.chromium.org/2155743002/diff/80001/chrome/browser/ui/bluet...
chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc:111:
observer()->OnAdapterEnabledChanged(
In a seperate patch, consider naming ChooserController::Observer and observer()
to be more specific. For example, here I would expect
view()->OnAdapterEnabledChanged().

[commit-bot: I haz the power, 2016-07-19 22:05:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-19 22:05:21]

Dry run: This issue passed the CQ dry run.

[msw, 2016-07-19 22:08:11]

lgtm with minor nits.

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/chooser_...
File chrome/browser/chooser_controller/mock_chooser_controller.cc (right):

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/chooser_...
chrome/browser/chooser_controller/mock_chooser_controller.cc:18: return
l10n_util::GetStringUTF16(IDS_DEVICE_CHOOSER_NO_DEVICES_FOUND_PROMPT);
On 2016/07/19 20:42:45, juncai wrote:
> On 2016/07/18 23:01:30, msw wrote:
> > Could the mock return an empty string like GetStatus?
> 
> It could, but the unit test needs to check what is shown on the table view
when
> there are no devices:
>
https://cs.chromium.org/chromium/src/chrome/browser/ui/views/chooser_content_...
> 
> Also maybe it is better to use a string literal in the unit test instead of
> using IDS_DEVICE_CHOOSER_NO_DEVICES_FOUND_PROMPT? Just in case the
> IDS_DEVICE_CHOOSER_NO_DEVICES_FOUND_PROMPT is accidentally modified. I made
the
> changes in chooser_content_view_unittest.cc.

I don't think it's a good idea to use a string literal in the test. That's
basically a change detector test. Using
IDS_DEVICE_CHOOSER_NO_DEVICES_FOUND_PROMPT here is fine.

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/bluet...
File chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc (right):

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/bluet...
chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc:102: case
content::BluetoothChooser::AdapterPresence::ABSENT:
On 2016/07/19 20:42:46, juncai wrote:
> On 2016/07/18 23:01:30, msw wrote:
> > Should this case send observer()->AdapterEnabled(false)?
> 
> When there is no Bluetooth adapter, the Bluetooth chooser won't show up. So
this
> line of code won't be reached. Maybe add a NOTREACHED();?

Acknowledged.

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/chooser_content_view.h (right):

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view.h:76: views::TableView*
table_view_;
On 2016/07/19 20:42:46, juncai wrote:
> On 2016/07/18 23:01:31, msw wrote:
> > nit: = nullptr;
> 
> Since |table_view_| is always initialized in the constructor, setting it to
> nullptr here seems redundant?

Acknowledged.

https://codereview.chromium.org/2155743002/diff/80001/chrome/browser/ui/views...
File chrome/browser/ui/views/chooser_content_view.h (right):

https://codereview.chromium.org/2155743002/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view.h:81: views::StyledLabel*
styled_label_ = nullptr;  // Weak. For testing only.
nit: I'm not sure I'd mark |styled_label_| "For testing only.", even if the only
reason to cache the pointer is for test, the object is still used in production.

https://codereview.chromium.org/2155743002/diff/80001/chrome/browser/ui/views...
File chrome/browser/ui/views/chooser_content_view_unittest.cc (right):

https://codereview.chromium.org/2155743002/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view_unittest.cc:20: const char
kNoDevicesFound[] = "No devices found.";
nit: don't do this; use IDS_DEVICE_CHOOSER_NO_DEVICES_FOUND_PROMPT.
(otherwise, changes to the string require parallel changes to the test)

https://codereview.chromium.org/2155743002/diff/80001/chrome/browser/ui/views...
File chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc
(right):

https://codereview.chromium.org/2155743002/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc:23: const
char kNoDevicesFound[] = "No devices found.";
ditto nit: don't do this.

[juncai, 2016-07-19 22:51:31]

The CQ bit was checked by juncai@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-19 22:52:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[juncai, 2016-07-19 22:57:22]

The CQ bit was checked by juncai@chromium.org to run a CQ dry run

[juncai, 2016-07-19 22:58:24]

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/chooser_...
File chrome/browser/chooser_controller/mock_chooser_controller.cc (right):

https://codereview.chromium.org/2155743002/diff/40001/chrome/browser/chooser_...
chrome/browser/chooser_controller/mock_chooser_controller.cc:18: return
l10n_util::GetStringUTF16(IDS_DEVICE_CHOOSER_NO_DEVICES_FOUND_PROMPT);
On 2016/07/19 22:08:10, msw wrote:
> On 2016/07/19 20:42:45, juncai wrote:
> > On 2016/07/18 23:01:30, msw wrote:
> > > Could the mock return an empty string like GetStatus?
> > 
> > It could, but the unit test needs to check what is shown on the table view
> when
> > there are no devices:
> >
>
https://cs.chromium.org/chromium/src/chrome/browser/ui/views/chooser_content_...
> > 
> > Also maybe it is better to use a string literal in the unit test instead of
> > using IDS_DEVICE_CHOOSER_NO_DEVICES_FOUND_PROMPT? Just in case the
> > IDS_DEVICE_CHOOSER_NO_DEVICES_FOUND_PROMPT is accidentally modified. I made
> the
> > changes in chooser_content_view_unittest.cc.
> 
> I don't think it's a good idea to use a string literal in the test. That's
> basically a change detector test. Using
> IDS_DEVICE_CHOOSER_NO_DEVICES_FOUND_PROMPT here is fine.

Done.

https://codereview.chromium.org/2155743002/diff/80001/chrome/browser/ui/bluet...
File chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc (right):

https://codereview.chromium.org/2155743002/diff/80001/chrome/browser/ui/bluet...
chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc:111:
observer()->OnAdapterEnabledChanged(
On 2016/07/19 21:18:13, scheib wrote:
> In a seperate patch, consider naming ChooserController::Observer and
observer()
> to be more specific. For example, here I would expect
> view()->OnAdapterEnabledChanged().

I will do this in a separate patch.

https://codereview.chromium.org/2155743002/diff/80001/chrome/browser/ui/views...
File chrome/browser/ui/views/chooser_content_view.h (right):

https://codereview.chromium.org/2155743002/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view.h:81: views::StyledLabel*
styled_label_ = nullptr;  // Weak. For testing only.
On 2016/07/19 22:08:11, msw wrote:
> nit: I'm not sure I'd mark |styled_label_| "For testing only.", even if the
only
> reason to cache the pointer is for test, the object is still used in
production.

Done.

https://codereview.chromium.org/2155743002/diff/80001/chrome/browser/ui/views...
File chrome/browser/ui/views/chooser_content_view_unittest.cc (right):

https://codereview.chromium.org/2155743002/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/chooser_content_view_unittest.cc:20: const char
kNoDevicesFound[] = "No devices found.";
On 2016/07/19 22:08:11, msw wrote:
> nit: don't do this; use IDS_DEVICE_CHOOSER_NO_DEVICES_FOUND_PROMPT.
> (otherwise, changes to the string require parallel changes to the test)

Done.

https://codereview.chromium.org/2155743002/diff/80001/chrome/browser/ui/views...
File chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc
(right):

https://codereview.chromium.org/2155743002/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc:23: const
char kNoDevicesFound[] = "No devices found.";
On 2016/07/19 22:08:11, msw wrote:
> ditto nit: don't do this.

Done.

[commit-bot: I haz the power, 2016-07-19 22:58:26]

Dry run: 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, 2016-07-20 00:48:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-20 00:48:46]

Dry run: This issue passed the CQ dry run.

[juncai, 2016-07-20 00:49:38]

The CQ bit was checked by juncai@chromium.org

[juncai, 2016-07-20 00:49:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
reillyg@chromium.org, msw@chromium.org, scheib@chromium.org
Link to the patchset: https://codereview.chromium.org/2155743002/#ps120001
(title: "removed unnecessary include file")

[commit-bot: I haz the power, 2016-07-20 00:50:38]

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, 2016-07-20 00:56:38]

Description was changed from

==========
Add throbber and status text to WebBluetooth chooser UI on non-Mac desktops

This is part 3 of matching WebBluetooth chooser views to mocks.

This CL adds:
1. a throbber that starts when the the chooser scans for Bluetooth devices
and stops when the scan completes.
2. status text:
"Bluetooth turned off" when the Bluetooth adapter is turned off.
"Scanning for devices..." when chooser is scanning for Bluetooth devices.
"Re-scan" link and clicks it to re-scan Bluetooth devices.

The screenshots were uploaded on the issue page.

BUG=583452
==========

to

==========
Add throbber and status text to WebBluetooth chooser UI on non-Mac desktops

This is part 3 of matching WebBluetooth chooser views to mocks.

This CL adds:
1. a throbber that starts when the the chooser scans for Bluetooth devices
and stops when the scan completes.
2. status text:
"Bluetooth turned off" when the Bluetooth adapter is turned off.
"Scanning for devices..." when chooser is scanning for Bluetooth devices.
"Re-scan" link and clicks it to re-scan Bluetooth devices.

The screenshots were uploaded on the issue page.

BUG=583452
==========

[commit-bot: I haz the power, 2016-07-20 00:56:40]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-07-20 00:59:26]

Description was changed from

==========
Add throbber and status text to WebBluetooth chooser UI on non-Mac desktops

This is part 3 of matching WebBluetooth chooser views to mocks.

This CL adds:
1. a throbber that starts when the the chooser scans for Bluetooth devices
and stops when the scan completes.
2. status text:
"Bluetooth turned off" when the Bluetooth adapter is turned off.
"Scanning for devices..." when chooser is scanning for Bluetooth devices.
"Re-scan" link and clicks it to re-scan Bluetooth devices.

The screenshots were uploaded on the issue page.

BUG=583452
==========

to

==========
Add throbber and status text to WebBluetooth chooser UI on non-Mac desktops

This is part 3 of matching WebBluetooth chooser views to mocks.

This CL adds:
1. a throbber that starts when the the chooser scans for Bluetooth devices
and stops when the scan completes.
2. status text:
"Bluetooth turned off" when the Bluetooth adapter is turned off.
"Scanning for devices..." when chooser is scanning for Bluetooth devices.
"Re-scan" link and clicks it to re-scan Bluetooth devices.

The screenshots were uploaded on the issue page.

BUG=583452

Committed: https://crrev.com/9c9d751f358ea3e5e9b2076aecde6842016cbf0f
Cr-Commit-Position: refs/heads/master@{#406433}
==========

[commit-bot: I haz the power, 2016-07-20 00:59:28]

Patchset 7 (id:??) landed as
https://crrev.com/9c9d751f358ea3e5e9b2076aecde6842016cbf0f
Cr-Commit-Position: refs/heads/master@{#406433}