Bootstrapping: Display Bluetooth name of CrOS device on OOBE screen.

Bootstrapping: Display Bluetooth name of CrOS device on OOBE screen.

BUG=720638
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2876753003
Cr-Commit-Position: refs/heads/master@{#472854}
Committed: https://chromium.googlesource.com/chromium/src/+/9102badab31062609acf65f4ddf9c3a332e85705

[xdai1, 2017-05-11 23:52:07]

Description was changed from

==========
Bootstrapping: Display Bluetooth name of CrOS device on OOBE screen.

BUG=720638
==========

to

==========
Bootstrapping: Display Bluetooth name of CrOS device on OOBE screen.

BUG=720638
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[xdai1, 2017-05-16 22:19:31]

Patchset #1 (id:1) has been deleted

[xdai1, 2017-05-16 22:20:03]

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

[commit-bot: I haz the power, 2017-05-16 22:22:03]

Dry run: CQ is trying da patch.

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

[xdai1, 2017-05-16 23:51:54]

xdai@chromium.org changed reviewers:
+ achuith@chromium.org

[xdai1, 2017-05-16 23:51:54]

achuith@, could you help review this CL please? Thanks! (Please refer to the bug
to see the screenshot.)

[achuithb, 2017-05-17 00:02:50]

https://codereview.chromium.org/2876753003/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/version_info_updater.cc (right):

https://codereview.chromium.org/2876753003/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/login/version_info_updater.cc:45: const char
kBluetoothDeviceNamePrefix[] = "Bluetooth device name: ";
nit newline after this.

This doesn't need to be localized, right?

https://codereview.chromium.org/2876753003/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/login/version_info_updater.cc:155: void
VersionInfoUpdater::UpdateDeviceInfo() {
Why not get rid of this function since it's just one line?

https://codereview.chromium.org/2876753003/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/login/version_info_updater.cc:165:
delegate_->OnDeviceInfoUpdated(bluetooth_name_text);
Why not
delegate_->OnDeviceInfoUpdated(kBluetoothDeviceNamePrefix + adapter->GetName());

https://codereview.chromium.org/2876753003/diff/40001/ui/login/oobe.css
File ui/login/oobe.css (right):

https://codereview.chromium.org/2876753003/diff/40001/ui/login/oobe.css#newco...
ui/login/oobe.css:159: #bluetooth-name {
Can you check what this looks like in the VM as well?

[commit-bot: I haz the power, 2017-05-17 02:13:10]

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

[commit-bot: I haz the power, 2017-05-17 02:13:11]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[xdai1, 2017-05-17 23:55:44]

achuith@, I've addressed your comments. Please take another look, thanks!

https://codereview.chromium.org/2876753003/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/version_info_updater.cc (right):

https://codereview.chromium.org/2876753003/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/login/version_info_updater.cc:45: const char
kBluetoothDeviceNamePrefix[] = "Bluetooth device name: ";
On 2017/05/17 00:02:50, achuithb wrote:
> nit newline after this.
> 
> This doesn't need to be localized, right?

I don't think it needs to be localized, especially the string is only visible
for a very small amount of users.

https://codereview.chromium.org/2876753003/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/login/version_info_updater.cc:155: void
VersionInfoUpdater::UpdateDeviceInfo() {
On 2017/05/17 00:02:50, achuithb wrote:
> Why not get rid of this function since it's just one line?

Done.

https://codereview.chromium.org/2876753003/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/login/version_info_updater.cc:165:
delegate_->OnDeviceInfoUpdated(bluetooth_name_text);
On 2017/05/17 00:02:50, achuithb wrote:
> Why not
> delegate_->OnDeviceInfoUpdated(kBluetoothDeviceNamePrefix +
adapter->GetName());

Done.

https://codereview.chromium.org/2876753003/diff/40001/ui/login/oobe.css
File ui/login/oobe.css (right):

https://codereview.chromium.org/2876753003/diff/40001/ui/login/oobe.css#newco...
ui/login/oobe.css:159: #bluetooth-name {
On 2017/05/17 00:02:50, achuithb wrote:
> Can you check what this looks like in the VM as well?

The string doesn't show up in the VM. The string will only show up if 1) the
device is in discoverable mode and 2) the device name is not empty. The two
conditions are both false in the VM.

[achuithb, 2017-05-18 00:00:16]

lgtm

[xdai1, 2017-05-18 00:00:37]

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

[commit-bot: I haz the power, 2017-05-18 00:03:40]

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, 2017-05-18 03:20:10]

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

[commit-bot: I haz the power, 2017-05-18 03:20:11]

Dry run: This issue passed the CQ dry run.

[xdai1, 2017-05-18 17:08:37]

The CQ bit was checked by xdai@chromium.org

[commit-bot: I haz the power, 2017-05-18 17: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-18 17:16:54]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1495127317935150,
"parent_rev": "64e280e83e00b3d5217595c55312a3251f682fdf", "commit_rev":
"9102badab31062609acf65f4ddf9c3a332e85705"}

[commit-bot: I haz the power, 2017-05-18 17:17:07]

Description was changed from

==========
Bootstrapping: Display Bluetooth name of CrOS device on OOBE screen.

BUG=720638
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Bootstrapping: Display Bluetooth name of CrOS device on OOBE screen.

BUG=720638
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2876753003
Cr-Commit-Position: refs/heads/master@{#472854}
Committed:
https://chromium.googlesource.com/chromium/src/+/9102badab31062609acf65f4ddf9...
==========

[commit-bot: I haz the power, 2017-05-18 17:17:08]

Committed patchset #3 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/9102badab31062609acf65f4ddf9...