Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(542)

Issue 2269993004: Add unit test for UsbChooserController (Closed)

Created:
4 years, 4 months ago by juncai
Modified:
4 years, 4 months ago
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@modify_UsbDeviceFilter_MatchesAny
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add unit test for UsbChooserController This CL adds some unit tests for code that deals with displaying USB devices that have the same name on the chooser. BUG=637402 Committed: https://crrev.com/2ef45ff9caf531d43a366186a7c80d7307d91252 Cr-Commit-Position: refs/heads/master@{#413929}

Patch Set 1 : added unit test for UsbChooserController #

Patch Set 2 : removed and added some include files #

Total comments: 2

Patch Set 3 : address comments #

Patch Set 4 : added more test #

Total comments: 2

Patch Set 5 : address more comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+144 lines, -0 lines) Patch
A chrome/browser/usb/usb_chooser_controller_unittest.cc View 1 2 3 4 1 chunk +136 lines, -0 lines 0 comments Download
M chrome/chrome_tests_unit.gypi View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M device/usb/mock_usb_device.h View 1 2 2 chunks +7 lines, -0 lines 0 comments Download

Messages

Total messages: 28 (18 generated)
juncai
sky@chromium.org: Please review changes in //chrome/chrome_tests_unit.gypi reillyg@chromium.org: Please review changes in //chrome/browser/usb/usb_chooser_controller_unittest.cc
4 years, 4 months ago (2016-08-23 18:11:07 UTC) #6
Reilly Grant (use Gerrit)
https://codereview.chromium.org/2269993004/diff/20001/chrome/browser/usb/usb_chooser_controller_unittest.cc File chrome/browser/usb/usb_chooser_controller_unittest.cc (right): https://codereview.chromium.org/2269993004/diff/20001/chrome/browser/usb/usb_chooser_controller_unittest.cc#newcode25 chrome/browser/usb/usb_chooser_controller_unittest.cc:25: switches::kDisableWebUsbSecurity); I'm planning to remove this flag so I'd ...
4 years, 4 months ago (2016-08-23 18:30:19 UTC) #7
sky
chrome/*.gypi has owners of *, so you can pick anyone you want. That said, LGTM
4 years, 4 months ago (2016-08-23 20:43:20 UTC) #10
juncai
On 2016/08/23 20:43:20, sky wrote: > chrome/*.gypi has owners of *, so you can pick ...
4 years, 4 months ago (2016-08-23 23:21:58 UTC) #13
juncai
https://codereview.chromium.org/2269993004/diff/20001/chrome/browser/usb/usb_chooser_controller_unittest.cc File chrome/browser/usb/usb_chooser_controller_unittest.cc (right): https://codereview.chromium.org/2269993004/diff/20001/chrome/browser/usb/usb_chooser_controller_unittest.cc#newcode25 chrome/browser/usb/usb_chooser_controller_unittest.cc:25: switches::kDisableWebUsbSecurity); On 2016/08/23 18:30:18, Reilly Grant wrote: > I'm ...
4 years, 4 months ago (2016-08-23 23:22:06 UTC) #14
Reilly Grant (use Gerrit)
lgtm with a nit https://codereview.chromium.org/2269993004/diff/60001/chrome/browser/usb/usb_chooser_controller_unittest.cc File chrome/browser/usb/usb_chooser_controller_unittest.cc (right): https://codereview.chromium.org/2269993004/diff/60001/chrome/browser/usb/usb_chooser_controller_unittest.cc#newcode70 chrome/browser/usb/usb_chooser_controller_unittest.cc:70: CreateMockUsbDevice(0, 1, "Google", "a", "001"); ...
4 years, 4 months ago (2016-08-24 00:19:06 UTC) #17
juncai
https://codereview.chromium.org/2269993004/diff/60001/chrome/browser/usb/usb_chooser_controller_unittest.cc File chrome/browser/usb/usb_chooser_controller_unittest.cc (right): https://codereview.chromium.org/2269993004/diff/60001/chrome/browser/usb/usb_chooser_controller_unittest.cc#newcode70 chrome/browser/usb/usb_chooser_controller_unittest.cc:70: CreateMockUsbDevice(0, 1, "Google", "a", "001"); On 2016/08/24 00:19:06, Reilly ...
4 years, 4 months ago (2016-08-24 01:26:21 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2269993004/80001
4 years, 4 months ago (2016-08-24 01:27:12 UTC) #25
commit-bot: I haz the power
Committed patchset #5 (id:80001)
4 years, 4 months ago (2016-08-24 01:34:16 UTC) #26
commit-bot: I haz the power
4 years, 4 months ago (2016-08-24 01:37:37 UTC) #28
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/2ef45ff9caf531d43a366186a7c80d7307d91252
Cr-Commit-Position: refs/heads/master@{#413929}

Powered by Google App Engine
This is Rietveld 408576698