Add multiple selection support to chooser on desktops

Add multiple selection support to chooser on desktops

This CL is part 1 of changing Chrome Apps device permissions dialog
using the same chooser dialog as WebUSB and WebBluetooth. Since the
device permissions dialog allows multiple selection, this CL added code
to support multiple selection for chooser.

BUG=653222
Committed: https://crrev.com/0f6d2799e18907d3419645099fd9b920318f5310
Cr-Commit-Position: refs/heads/master@{#434209}

[juncai, 2016-11-22 01:35:50]

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

[commit-bot: I haz the power, 2016-11-22 01:36:35]

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

[juncai, 2016-11-22 01:40:26]

Description was changed from

==========
Add multiple selection support to chooser on desktops

This CL is part 1 of changing Chrome Apps device permissions dialog using
the same chooser dialog as WebUSB and WebBluetooth. Since the device
permissions dialog allows multiple selection, this CL added code to
support multiple selection for chooser.

BUG=652445
==========

to

==========
Add multiple selection support to chooser on desktops

This CL is part 1 of changing Chrome Apps device permissions dialog
using the same chooser dialog as WebUSB and WebBluetooth. Since the 
device permissions dialog allows multiple selection, this CL added code
to support multiple selection for chooser.

BUG=652445
==========

[juncai, 2016-11-22 01:40:58]

Description was changed from

==========
Add multiple selection support to chooser on desktops

This CL is part 1 of changing Chrome Apps device permissions dialog
using the same chooser dialog as WebUSB and WebBluetooth. Since the 
device permissions dialog allows multiple selection, this CL added code
to support multiple selection for chooser.

BUG=652445
==========

to

==========
Add multiple selection support to chooser on desktops

This CL is part 1 of changing Chrome Apps device permissions dialog
using the same chooser dialog as WebUSB and WebBluetooth. Since the 
device permissions dialog allows multiple selection, this CL added code
to support multiple selection for chooser.

BUG=653222
==========

[juncai, 2016-11-22 01:44:02]

juncai@chromium.org changed reviewers:
+ reillyg@chromium.org, rsesek@chromium.org, sky@chromium.org

[juncai, 2016-11-22 01:44:02]

rsesek@chromium.org: Please review changes in 
//chrome/browser/ui/cocoa

sky@chromium.org: Please review changes in 
//chrome/browser/ui/views

reillyg@chromium.org: Please review changes in 
//chrome/browser/chooser_controller

[commit-bot: I haz the power, 2016-11-22 02:30:58]

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

[commit-bot: I haz the power, 2016-11-22 02:30:58]

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_...)

[sky, 2016-11-22 04:37:35]

https://codereview.chromium.org/2518933004/diff/1/chrome/browser/ui/views/cho...
File chrome/browser/ui/views/chooser_content_view.cc (right):

https://codereview.chromium.org/2518933004/diff/1/chrome/browser/ui/views/cho...
chrome/browser/ui/views/chooser_content_view.cc:289:
chooser_controller_->Select(index);
Isn't Select() a one time type call? I would expect you to pass it all the
selected indices, not just one.

[juncai, 2016-11-22 18:57:30]

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

[commit-bot: I haz the power, 2016-11-22 18:58:07]

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

[juncai, 2016-11-22 19:07:00]

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

[juncai, 2016-11-22 19:07:00]

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

https://codereview.chromium.org/2518933004/diff/1/chrome/browser/ui/views/cho...
File chrome/browser/ui/views/chooser_content_view.cc (right):

https://codereview.chromium.org/2518933004/diff/1/chrome/browser/ui/views/cho...
chrome/browser/ui/views/chooser_content_view.cc:289:
chooser_controller_->Select(index);
On 2016/11/22 04:37:35, sky wrote:
> Isn't Select() a one time type call? I would expect you to pass it all the
> selected indices, not just one.

Done.

[Robert Sesek, 2016-11-22 19:07:29]

https://codereview.chromium.org/2518933004/diff/1/chrome/browser/ui/cocoa/cho...
File chrome/browser/ui/cocoa/chooser_content_view_cocoa.mm (right):

https://codereview.chromium.org/2518933004/diff/1/chrome/browser/ui/cocoa/cho...
chrome/browser/ui/cocoa/chooser_content_view_cocoa.mm:318: NSUInteger idx =
static_cast<NSUInteger>(index);
Is the cast necessary? size_t should be the same as NSUInteger.

[Reilly Grant (use Gerrit), 2016-11-22 20:55:28]

//chrome/browser/chooser_controller lgtm with some minor changes in other
places.

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

https://codereview.chromium.org/2518933004/diff/20001/chrome/browser/ui/bluet...
chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc:111:
DCHECK(indices.size() == 1);
DCHECK_EQ(indices.size(), 1)

https://codereview.chromium.org/2518933004/diff/20001/chrome/browser/ui/bluet...
File chrome/browser/ui/bluetooth/bluetooth_chooser_controller_unittest.cc
(right):

https://codereview.chromium.org/2518933004/diff/20001/chrome/browser/ui/bluet...
chrome/browser/ui/bluetooth/bluetooth_chooser_controller_unittest.cc:355:
indices.push_back(0);
std::vector<size_t> indices{1};

https://codereview.chromium.org/2518933004/diff/20001/chrome/browser/usb/usb_...
File chrome/browser/usb/usb_chooser_controller.cc (right):

https://codereview.chromium.org/2518933004/diff/20001/chrome/browser/usb/usb_...
chrome/browser/usb/usb_chooser_controller.cc:113: DCHECK(indices.size() == 1);
DCHECK_EQ(indices.size(), 1)

[sky, 2016-11-22 21:20:55]

LGTM

[commit-bot: I haz the power, 2016-11-22 21:45:26]

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

[commit-bot: I haz the power, 2016-11-22 21:45:27]

Dry run: This issue passed the CQ dry run.

[juncai, 2016-11-22 23:11:42]

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

[juncai, 2016-11-22 23:12:07]

https://codereview.chromium.org/2518933004/diff/1/chrome/browser/ui/cocoa/cho...
File chrome/browser/ui/cocoa/chooser_content_view_cocoa.mm (right):

https://codereview.chromium.org/2518933004/diff/1/chrome/browser/ui/cocoa/cho...
chrome/browser/ui/cocoa/chooser_content_view_cocoa.mm:318: NSUInteger idx =
static_cast<NSUInteger>(index);
On 2016/11/22 19:07:29, Robert Sesek wrote:
> Is the cast necessary? size_t should be the same as NSUInteger.

Done.

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

https://codereview.chromium.org/2518933004/diff/20001/chrome/browser/ui/bluet...
chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc:111:
DCHECK(indices.size() == 1);
On 2016/11/22 20:55:27, Reilly Grant wrote:
> DCHECK_EQ(indices.size(), 1)

Done.

https://codereview.chromium.org/2518933004/diff/20001/chrome/browser/ui/bluet...
File chrome/browser/ui/bluetooth/bluetooth_chooser_controller_unittest.cc
(right):

https://codereview.chromium.org/2518933004/diff/20001/chrome/browser/ui/bluet...
chrome/browser/ui/bluetooth/bluetooth_chooser_controller_unittest.cc:355:
indices.push_back(0);
On 2016/11/22 20:55:27, Reilly Grant wrote:
> std::vector<size_t> indices{1};

Done.

https://codereview.chromium.org/2518933004/diff/20001/chrome/browser/usb/usb_...
File chrome/browser/usb/usb_chooser_controller.cc (right):

https://codereview.chromium.org/2518933004/diff/20001/chrome/browser/usb/usb_...
chrome/browser/usb/usb_chooser_controller.cc:113: DCHECK(indices.size() == 1);
On 2016/11/22 20:55:27, Reilly Grant wrote:
> DCHECK_EQ(indices.size(), 1)

Done.

[commit-bot: I haz the power, 2016-11-22 23:12:09]

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

[scheib, 2016-11-22 23:30:16]

//chrome/browser/ui/bluetooth LGTM

[commit-bot: I haz the power, 2016-11-23 01:00:07]

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

[commit-bot: I haz the power, 2016-11-23 01:00:08]

Dry run: This issue passed the CQ dry run.

[Robert Sesek, 2016-11-23 18:15:23]

LGTM

[juncai, 2016-11-23 18:26:58]

The CQ bit was checked by juncai@chromium.org

[juncai, 2016-11-23 18:26:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
reillyg@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2518933004/#ps40001
(title: "address comments")

[commit-bot: I haz the power, 2016-11-23 18:27:39]

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-11-23 18:38:13]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1479925618766770,
"parent_rev": "774e5b312d62d8b4e45da1927ae769faa03f2b14", "commit_rev":
"2624e59169353ba129915f486a24e24fd4dba705"}

[commit-bot: I haz the power, 2016-11-23 18:39:13]

Description was changed from

==========
Add multiple selection support to chooser on desktops

This CL is part 1 of changing Chrome Apps device permissions dialog
using the same chooser dialog as WebUSB and WebBluetooth. Since the 
device permissions dialog allows multiple selection, this CL added code
to support multiple selection for chooser.

BUG=653222
==========

to

==========
Add multiple selection support to chooser on desktops

This CL is part 1 of changing Chrome Apps device permissions dialog
using the same chooser dialog as WebUSB and WebBluetooth. Since the 
device permissions dialog allows multiple selection, this CL added code
to support multiple selection for chooser.

BUG=653222
==========

[commit-bot: I haz the power, 2016-11-23 18:39:14]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-23 18:42:10]

Description was changed from

==========
Add multiple selection support to chooser on desktops

This CL is part 1 of changing Chrome Apps device permissions dialog
using the same chooser dialog as WebUSB and WebBluetooth. Since the 
device permissions dialog allows multiple selection, this CL added code
to support multiple selection for chooser.

BUG=653222
==========

to

==========
Add multiple selection support to chooser on desktops

This CL is part 1 of changing Chrome Apps device permissions dialog
using the same chooser dialog as WebUSB and WebBluetooth. Since the
device permissions dialog allows multiple selection, this CL added code
to support multiple selection for chooser.

BUG=653222

Committed: https://crrev.com/0f6d2799e18907d3419645099fd9b920318f5310
Cr-Commit-Position: refs/heads/master@{#434209}
==========

[commit-bot: I haz the power, 2016-11-23 18:42:12]

Patchset 3 (id:??) landed as
https://crrev.com/0f6d2799e18907d3419645099fd9b920318f5310
Cr-Commit-Position: refs/heads/master@{#434209}