bluetooth: Add service list to DeviceDetailsPage in internals page.

bluetooth: Add service list to DeviceDetailsPage in internals page.

Adds service list to DeviceDetailsPage and creates the structure for displaying
hierachical data using collapsible panels.
Adds id property to ServiceInfo for display in service list.
Adds expandable list control for collapsible tree view of service list items.

GIF: https://goo.gl/photos/rqnDxZYmx1TDnBJ26

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

Review-Url: https://codereview.chromium.org/2617923002
Cr-Commit-Position: refs/heads/master@{#443782}
Committed: https://chromium.googlesource.com/chromium/src/+/ae05be6e566364f83f864fcf8a941d04b4cd768c

[mbrunson, 2017-01-06 02:14:59]

Description was changed from

==========
bluetooth: Add service list to DeviceDetailsPage in internals page.

Adds service list to DeviceDetailsPage and creates the structure for displaying
hierachical data using collapsible panels.
Adds id property to ServiceInfo for display in service list.
Adds expandable list control for collapsible tree view of service list items.

BUG=651282
==========

to

==========
bluetooth: Add service list to DeviceDetailsPage in internals page.

Adds service list to DeviceDetailsPage and creates the structure for displaying
hierachical data using collapsible panels.
Adds id property to ServiceInfo for display in service list.
Adds expandable list control for collapsible tree view of service list items.

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

[mbrunson, 2017-01-11 21:55:49]

Description was changed from

==========
bluetooth: Add service list to DeviceDetailsPage in internals page.

Adds service list to DeviceDetailsPage and creates the structure for displaying
hierachical data using collapsible panels.
Adds id property to ServiceInfo for display in service list.
Adds expandable list control for collapsible tree view of service list items.

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

to

==========
bluetooth: Add service list to DeviceDetailsPage in internals page.

Adds service list to DeviceDetailsPage and creates the structure for displaying
hierachical data using collapsible panels.
Adds id property to ServiceInfo for display in service list.
Adds expandable list control for collapsible tree view of service list items.

GIF: https://goo.gl/photos/rqnDxZYmx1TDnBJ26

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

[mbrunson, 2017-01-11 21:56:44]

mbrunson@chromium.org changed reviewers:
+ scheib@chromium.org

[scheib, 2017-01-12 21:53:16]

LGTM, small change request:

https://codereview.chromium.org/2617923002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/bluetooth_internals/service_list.js (right):

https://codereview.chromium.org/2617923002/diff/60001/chrome/browser/resource...
chrome/browser/resources/bluetooth_internals/service_list.js:49: var uuidString
= this.info.uuid.uuid.split('-')[0];
For a basic display of UUID (which I think we should stick with for now), you'd
best display the entire UUID because vendor chosen UUIDs may have meaningful
data at any location in the UUID, including the last character. E.g. there could
easily be a vendor who chose to use these:
12341234-1234-efde-1523-785fea000001
12341234-1234-efde-1523-785fea000002
12341234-1234-efde-1523-785fea000003


Later, we could have a smarter display that detects if the UUID is using the
bluetooth base UUID
https://www.bluetooth.com/specifications/assigned-numbers/service-discovery and
if so then simplify it. More so, we can look up all standard UUIDs and replace
them with their name and number, e.g. 'generic_access (0x1800)'.


Keep the uuid field in the info-container.

[mbrunson, 2017-01-12 22:59:59]

https://codereview.chromium.org/2617923002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/bluetooth_internals/service_list.js (right):

https://codereview.chromium.org/2617923002/diff/60001/chrome/browser/resource...
chrome/browser/resources/bluetooth_internals/service_list.js:49: var uuidString
= this.info.uuid.uuid.split('-')[0];
On 2017/01/12 21:53:16, scheib wrote:
> For a basic display of UUID (which I think we should stick with for now),
you'd
> best display the entire UUID because vendor chosen UUIDs may have meaningful
> data at any location in the UUID, including the last character. E.g. there
could
> easily be a vendor who chose to use these:
> 12341234-1234-efde-1523-785fea000001
> 12341234-1234-efde-1523-785fea000002
> 12341234-1234-efde-1523-785fea000003
> 
> 
> Later, we could have a smarter display that detects if the UUID is using the
> bluetooth base UUID
> https://www.bluetooth.com/specifications/assigned-numbers/service-discovery
and
> if so then simplify it. More so, we can look up all standard UUIDs and replace
> them with their name and number, e.g. 'generic_access (0x1800)'.
> 
> 
> Keep the uuid field in the info-container.

Done.

[mbrunson, 2017-01-12 23:10:17]

mbrunson@chromium.org changed reviewers:
+ dpapad@chromium.org

[mbrunson, 2017-01-12 23:10:18]

dpapad: WebUI OWNERS review, please.

[mbrunson, 2017-01-12 23:42:28]

mbrunson@chromium.org changed reviewers:
+ tsepez@chromium.org

[mbrunson, 2017-01-12 23:42:28]

Mojom review, please.

tsepez:
  device/bluetooth/public/interfaces/device.mojom

[Tom Sepez, 2017-01-12 23:45:07]

mojom LGTM

[dpapad, 2017-01-13 22:17:20]

https://codereview.chromium.org/2617923002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/bluetooth_internals/bluetooth_internals.css
(right):

https://codereview.chromium.org/2617923002/diff/80001/chrome/browser/resource...
chrome/browser/resources/bluetooth_internals/bluetooth_internals.css:52:
border-left: 1px solid gray;
Let's use a CSS variable where possible.

https://codereview.chromium.org/2617923002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/bluetooth_internals/expandable_list.js (right):

https://codereview.chromium.org/2617923002/diff/80001/chrome/browser/resource...
chrome/browser/resources/bluetooth_internals/expandable_list.js:33:
this.briefContent_.addEventListener(
Is decorate only called once (if not the listener will be registered multiple
times)? Is there a better lifycycle method for registering listeners?

[mbrunson, 2017-01-13 23:26:47]

https://codereview.chromium.org/2617923002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/bluetooth_internals/bluetooth_internals.css
(right):

https://codereview.chromium.org/2617923002/diff/80001/chrome/browser/resource...
chrome/browser/resources/bluetooth_internals/bluetooth_internals.css:52:
border-left: 1px solid gray;
On 2017/01/13 22:17:20, dpapad wrote:
> Let's use a CSS variable where possible.

Done.

https://codereview.chromium.org/2617923002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/bluetooth_internals/expandable_list.js (right):

https://codereview.chromium.org/2617923002/diff/80001/chrome/browser/resource...
chrome/browser/resources/bluetooth_internals/expandable_list.js:33:
this.briefContent_.addEventListener(
On 2017/01/13 22:17:20, dpapad wrote:
> Is decorate only called once (if not the listener will be registered multiple
> times)? Is there a better lifycycle method for registering listeners?

Decorate is only called once. The cr.ui.define function defines a constructor
that calls decorate and is the only place where decorate is called[1]. I don't
think there's a better lifecycle method for this. Decorate is required to be
defined when using cr.ui.define so I thought I might as well use it.

[1]
https://cs.chromium.org/chromium/src/ui/webui/resources/js/cr/ui.js?rcl=0&l=96

[dpapad, 2017-01-14 00:32:57]

LGTM with nits.

https://codereview.chromium.org/2617923002/diff/100001/chrome/browser/resourc...
File chrome/browser/resources/bluetooth_internals/expandable_list.js (right):

https://codereview.chromium.org/2617923002/diff/100001/chrome/browser/resourc...
chrome/browser/resources/bluetooth_internals/expandable_list.js:111:
this.spinner_.hidden = !loading;
FWIW, a hidden spinner still takes up precious CPU, doesn't it?

https://codereview.chromium.org/2617923002/diff/100001/chrome/browser/resourc...
File chrome/browser/resources/bluetooth_internals/service_list.js (right):

https://codereview.chromium.org/2617923002/diff/100001/chrome/browser/resourc...
chrome/browser/resources/bluetooth_internals/service_list.js:22: }
Semicolon missing.

https://codereview.chromium.org/2617923002/diff/100001/chrome/browser/resourc...
chrome/browser/resources/bluetooth_internals/service_list.js:60: var
serviceViewObj = {
Nit: How about inlining as follows?

this.serviceFieldSet_.setObject({
  ...
  ...
});

[mbrunson, 2017-01-14 01:36:12]

https://codereview.chromium.org/2617923002/diff/100001/chrome/browser/resourc...
File chrome/browser/resources/bluetooth_internals/expandable_list.js (right):

https://codereview.chromium.org/2617923002/diff/100001/chrome/browser/resourc...
chrome/browser/resources/bluetooth_internals/expandable_list.js:111:
this.spinner_.hidden = !loading;
On 2017/01/14 00:32:57, dpapad wrote:
> FWIW, a hidden spinner still takes up precious CPU, doesn't it?

Does it? That seems to defeat the purpose of hidden/display:none if images still
take CPU cycles when they're not displayed...

https://codereview.chromium.org/2617923002/diff/100001/chrome/browser/resourc...
File chrome/browser/resources/bluetooth_internals/service_list.js (right):

https://codereview.chromium.org/2617923002/diff/100001/chrome/browser/resourc...
chrome/browser/resources/bluetooth_internals/service_list.js:22: }
On 2017/01/14 00:32:57, dpapad wrote:
> Semicolon missing.

Done.

https://codereview.chromium.org/2617923002/diff/100001/chrome/browser/resourc...
chrome/browser/resources/bluetooth_internals/service_list.js:60: var
serviceViewObj = {
On 2017/01/14 00:32:57, dpapad wrote:
> Nit: How about inlining as follows?
> 
> this.serviceFieldSet_.setObject({
>   ...
>   ...
> });

Done.

[mbrunson, 2017-01-14 01:53:11]

The CQ bit was checked by mbrunson@chromium.org

[mbrunson, 2017-01-14 01:53:11]

The patchset sent to the CQ was uploaded after l-g-t-m from scheib@chromium.org,
tsepez@chromium.org, dpapad@chromium.org
Link to the patchset: https://codereview.chromium.org/2617923002/#ps120001
(title: "Add semicolon, inline serviceViewObj")

[commit-bot: I haz the power, 2017-01-14 01:53:31]

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-01-14 03:58:58]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1484358791040870,
"parent_rev": "5671bf116d315cb3a5d4b0d25c1d0e168d90779d", "commit_rev":
"ae05be6e566364f83f864fcf8a941d04b4cd768c"}

[commit-bot: I haz the power, 2017-01-14 03:59:30]

Description was changed from

==========
bluetooth: Add service list to DeviceDetailsPage in internals page.

Adds service list to DeviceDetailsPage and creates the structure for displaying
hierachical data using collapsible panels.
Adds id property to ServiceInfo for display in service list.
Adds expandable list control for collapsible tree view of service list items.

GIF: https://goo.gl/photos/rqnDxZYmx1TDnBJ26

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

to

==========
bluetooth: Add service list to DeviceDetailsPage in internals page.

Adds service list to DeviceDetailsPage and creates the structure for displaying
hierachical data using collapsible panels.
Adds id property to ServiceInfo for display in service list.
Adds expandable list control for collapsible tree view of service list items.

GIF: https://goo.gl/photos/rqnDxZYmx1TDnBJ26

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

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

[commit-bot: I haz the power, 2017-01-14 03:59:31]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/ae05be6e566364f83f864fcf8a94...