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

Issue 1289423002: Add webusb notification UI code (Closed)

Created:
5 years, 4 months ago by juncai
Modified:
5 years, 3 months ago
CC:
chromium-reviews, blundell+watchlist_chromium.org, sdefresne+watchlist_chromium.org, asvitkine+watch_chromium.org, droger+watchlist_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add webusb notification UI code This patch added code to display notification on chrome when a USB device is plugged in. BUG=492204 Committed: https://crrev.com/1b115256c55c867df10795991178c766e5604aa6 Cr-Commit-Position: refs/heads/master@{#347310}

Patch Set 1 : added code to display notification on chrome when a USB device is plugged in #

Patch Set 2 : moved chrome browser related code to chrome/browser #

Patch Set 3 : added some #include files #

Patch Set 4 : updated some .gn and .gyp(i) files #

Patch Set 5 : added some #ifdef flags #

Total comments: 4

Patch Set 6 : moved components webusb related code back to components/webusb #

Patch Set 7 : updated comments, gn, gypi, DEPS files #

Patch Set 8 : updated gn file #

Total comments: 2

Patch Set 9 : updated code so that WebUsbDetector is not a subclass of KeyedService #

Patch Set 10 : call webusb::WebUsbBrowserClient::Set at browser_process_impl.cc #

Patch Set 11 : moved webusb::WebUsbDetector::GetInstance() call to chrome_browser_main.cc #

Total comments: 8

Patch Set 12 : updated chrome switches name, WebUsbBrowserClient::Set function, etc. #

Patch Set 13 : merged changes from master and updated code since message_center/notification interface changed. #

Patch Set 14 : modified WebUsbBrowserClient::Set function #

Total comments: 4

Patch Set 15 : modified WebUsbBrowserClient::Set function again #

Total comments: 2

Patch Set 16 : modified WebUsbDetector destructor using override #

Patch Set 17 : moved chrome related code back to chrome directory #

Total comments: 2

Patch Set 18 : make WebUsbDetector owned by the Chrome browser process #

Patch Set 19 : changed variable name #

Total comments: 2

Patch Set 20 : removed OpenURL member function from WebUsbBrowserClient #

Total comments: 8

Patch Set 21 : added code to check if url is secure #

Total comments: 2

Patch Set 22 : updated code so that Chrome browser process owns WebUsbBrowserClient and WebUsbDetector objects #

Total comments: 4

Patch Set 23 : modified WebUsbDetector constructor #

Patch Set 24 : merged changes from master, resolved conflicts at about_flags.cc #

Patch Set 25 : use USB_DEVICE_NAME instead of USB_DEVICE_PRODUCT_NAME in generated_resources.grd #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats (+355 lines, -12 lines) Patch
M chrome/app/generated_resources.grd View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 1 chunk +18 lines, -0 lines 0 comments Download
M chrome/browser/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/DEPS View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/OWNERS View 2 3 4 5 6 7 8 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/about_flags.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +7 lines, -0 lines 0 comments Download
M chrome/browser/chrome_browser_main.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 2 chunks +8 lines, -0 lines 0 comments Download
M chrome/browser/chrome_browser_main.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 3 chunks +19 lines, -0 lines 2 comments Download
A chrome/browser/chrome_webusb_browser_client.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +31 lines, -0 lines 0 comments Download
A chrome/browser/chrome_webusb_browser_client.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 1 chunk +101 lines, -0 lines 0 comments Download
M chrome/chrome_browser.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 2 chunks +3 lines, -0 lines 0 comments Download
M chrome/common/chrome_switches.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/chrome_switches.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +2 lines, -0 lines 2 comments Download
M components/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +1 line, -0 lines 0 comments Download
M components/OWNERS View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +4 lines, -0 lines 0 comments Download
M components/components.gyp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +1 line, -0 lines 0 comments Download
A + components/webusb.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +9 lines, -8 lines 0 comments Download
A + components/webusb/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +6 lines, -3 lines 0 comments Download
A + components/webusb/DEPS View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +2 lines, -1 line 0 comments Download
A + components/webusb/OWNERS View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +1 line, -0 lines 0 comments Download
A components/webusb/webusb_browser_client.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +32 lines, -0 lines 0 comments Download
A components/webusb/webusb_detector.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +45 lines, -0 lines 2 comments Download
A components/webusb/webusb_detector.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +57 lines, -0 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 68 (17 generated)
juncai
Please review this patch.
5 years, 4 months ago (2015-08-18 00:19:26 UTC) #2
Reilly Grant (use Gerrit)
https://codereview.chromium.org/1289423002/diff/80001/chrome/browser/about_flags.cc File chrome/browser/about_flags.cc (right): https://codereview.chromium.org/1289423002/diff/80001/chrome/browser/about_flags.cc#newcode2151 chrome/browser/about_flags.cc:2151: kOsAll, kOsAllDesktop https://codereview.chromium.org/1289423002/diff/80001/chrome/browser/chrome_webusb_browser_client.cc File chrome/browser/chrome_webusb_browser_client.cc (right): https://codereview.chromium.org/1289423002/diff/80001/chrome/browser/chrome_webusb_browser_client.cc#newcode62 chrome/browser/chrome_webusb_browser_client.cc:62: void ...
5 years, 4 months ago (2015-08-18 00:23:43 UTC) #3
juncai
Please review the change. https://codereview.chromium.org/1289423002/diff/80001/chrome/browser/about_flags.cc File chrome/browser/about_flags.cc (right): https://codereview.chromium.org/1289423002/diff/80001/chrome/browser/about_flags.cc#newcode2151 chrome/browser/about_flags.cc:2151: kOsAll, On 2015/08/18 00:23:43, reillyg ...
5 years, 4 months ago (2015-08-18 17:39:59 UTC) #4
Reilly Grant (use Gerrit)
https://codereview.chromium.org/1289423002/diff/140001/chrome/browser/chrome_webusb_browser_client.cc File chrome/browser/chrome_webusb_browser_client.cc (right): https://codereview.chromium.org/1289423002/diff/140001/chrome/browser/chrome_webusb_browser_client.cc#newcode24 chrome/browser/chrome_webusb_browser_client.cc:24: chrome::FindBrowserWithProfile(profile, chrome::HOST_DESKTOP_TYPE_NATIVE); I think we can use ProfileManager::GetActiveProfile() to ...
5 years, 4 months ago (2015-08-18 18:30:02 UTC) #5
juncai
Please review the change. https://codereview.chromium.org/1289423002/diff/140001/chrome/browser/chrome_webusb_browser_client.cc File chrome/browser/chrome_webusb_browser_client.cc (right): https://codereview.chromium.org/1289423002/diff/140001/chrome/browser/chrome_webusb_browser_client.cc#newcode24 chrome/browser/chrome_webusb_browser_client.cc:24: chrome::FindBrowserWithProfile(profile, chrome::HOST_DESKTOP_TYPE_NATIVE); On 2015/08/18 18:30:02, ...
5 years, 4 months ago (2015-08-20 15:56:33 UTC) #6
Reilly Grant (use Gerrit)
https://codereview.chromium.org/1289423002/diff/200001/chrome/common/chrome_switches.cc File chrome/common/chrome_switches.cc (right): https://codereview.chromium.org/1289423002/diff/200001/chrome/common/chrome_switches.cc#newcode561 chrome/common/chrome_switches.cc:561: extern const char kEnableWebUsbNotification[] = "enable-webusb-notification"; Please rename the ...
5 years, 4 months ago (2015-08-20 17:02:45 UTC) #7
juncai
Please review the change. https://codereview.chromium.org/1289423002/diff/200001/chrome/common/chrome_switches.cc File chrome/common/chrome_switches.cc (right): https://codereview.chromium.org/1289423002/diff/200001/chrome/common/chrome_switches.cc#newcode561 chrome/common/chrome_switches.cc:561: extern const char kEnableWebUsbNotification[] = ...
5 years, 4 months ago (2015-08-21 05:16:04 UTC) #8
Reilly Grant (use Gerrit)
https://codereview.chromium.org/1289423002/diff/260001/chrome/browser/about_flags.cc File chrome/browser/about_flags.cc (right): https://codereview.chromium.org/1289423002/diff/260001/chrome/browser/about_flags.cc#newcode2140 chrome/browser/about_flags.cc:2140: {"enable-webusb-notification", Here and other places "notification" is still singular. ...
5 years, 4 months ago (2015-08-21 17:23:02 UTC) #9
juncai
Please review the change. https://codereview.chromium.org/1289423002/diff/260001/chrome/browser/about_flags.cc File chrome/browser/about_flags.cc (right): https://codereview.chromium.org/1289423002/diff/260001/chrome/browser/about_flags.cc#newcode2140 chrome/browser/about_flags.cc:2140: {"enable-webusb-notification", On 2015/08/21 17:23:02, Reilly ...
5 years, 4 months ago (2015-08-21 20:18:27 UTC) #10
Reilly Grant (use Gerrit)
lgtm
5 years, 4 months ago (2015-08-21 20:43:51 UTC) #11
juncai
jwd@chromium.org: Please review changes in tools/metrics/histograms/ caitkp@chromium.org: Please review changes in components/ thestig@chromium.org: Please review ...
5 years, 4 months ago (2015-08-21 21:05:58 UTC) #13
Lei Zhang
lgtm
5 years, 4 months ago (2015-08-21 22:11:39 UTC) #14
Cait (Slow)
Hi Juncai -- Is there a design doc or an "intent to implement" thread you ...
5 years, 4 months ago (2015-08-24 14:35:04 UTC) #15
juncai
On 2015/08/24 14:35:04, Cait wrote: > Hi Juncai -- Is there a design doc or ...
5 years, 4 months ago (2015-08-24 16:14:59 UTC) #16
jwd
lgtm
5 years, 4 months ago (2015-08-24 17:10:59 UTC) #17
juncai
On 2015/08/24 14:35:04, Cait wrote: > Hi Juncai -- Is there a design doc or ...
5 years, 4 months ago (2015-08-24 19:31:11 UTC) #18
Cait (Slow)
https://codereview.chromium.org/1289423002/diff/280001/components/webusb/webusb_detector.h File components/webusb/webusb_detector.h (right): https://codereview.chromium.org/1289423002/diff/280001/components/webusb/webusb_detector.h#newcode26 components/webusb/webusb_detector.h:26: virtual ~WebUsbDetector(); I think this should be ~WebUsbDetector() override; ...
5 years, 4 months ago (2015-08-24 19:50:31 UTC) #19
juncai
Please review the change. https://codereview.chromium.org/1289423002/diff/280001/components/webusb/webusb_detector.h File components/webusb/webusb_detector.h (right): https://codereview.chromium.org/1289423002/diff/280001/components/webusb/webusb_detector.h#newcode26 components/webusb/webusb_detector.h:26: virtual ~WebUsbDetector(); On 2015/08/24 19:50:31, ...
5 years, 4 months ago (2015-08-25 02:10:07 UTC) #20
Cait (Slow)
components/ lgtm -- thanks!
5 years, 4 months ago (2015-08-25 04:12:38 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1289423002/300001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1289423002/300001
5 years, 4 months ago (2015-08-25 15:37:01 UTC) #24
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/92457)
5 years, 4 months ago (2015-08-25 15:47:24 UTC) #26
juncai
jshin@chromium.org: Please review changes in the dependency '+ui/base/l10n', that I added to components/webusb/DEPS rsesek@chromium.org: Please ...
5 years, 4 months ago (2015-08-25 16:26:08 UTC) #28
Robert Sesek
ui/gfx/image DEPS LGTM
5 years, 4 months ago (2015-08-25 16:34:43 UTC) #29
jungshik at Google
LGTM
5 years, 4 months ago (2015-08-25 17:23:18 UTC) #30
stevenjb
src/components does not seem like the right place to me for UI that triggers notifications. ...
5 years, 3 months ago (2015-08-25 22:28:32 UTC) #31
Ken Rockot(use gerrit already)
On 2015/08/25 22:28:32, stevenjb wrote: > src/components does not seem like the right place to ...
5 years, 3 months ago (2015-08-25 23:36:17 UTC) #32
Cait (Slow)
On 2015/08/25 23:36:17, Ken Rockot wrote: > On 2015/08/25 22:28:32, stevenjb wrote: > > src/components ...
5 years, 3 months ago (2015-08-26 14:42:24 UTC) #33
Reilly Grant (use Gerrit)
https://codereview.chromium.org/1289423002/diff/320001/components/webusb/webusb_detector.cc File components/webusb/webusb_detector.cc (right): https://codereview.chromium.org/1289423002/diff/320001/components/webusb/webusb_detector.cc#newcode17 components/webusb/webusb_detector.cc:17: return Singleton<WebUsbDetector>::get(); Issues I've been trying to resolve recently ...
5 years, 3 months ago (2015-08-26 22:32:13 UTC) #34
juncai
Please review the change. https://codereview.chromium.org/1289423002/diff/320001/components/webusb/webusb_detector.cc File components/webusb/webusb_detector.cc (right): https://codereview.chromium.org/1289423002/diff/320001/components/webusb/webusb_detector.cc#newcode17 components/webusb/webusb_detector.cc:17: return Singleton<WebUsbDetector>::get(); On 2015/08/26 22:32:12, ...
5 years, 3 months ago (2015-08-27 00:54:42 UTC) #35
Reilly Grant (use Gerrit)
https://codereview.chromium.org/1289423002/diff/360001/chrome/browser/chrome_webusb_browser_client.h File chrome/browser/chrome_webusb_browser_client.h (right): https://codereview.chromium.org/1289423002/diff/360001/chrome/browser/chrome_webusb_browser_client.h#newcode28 chrome/browser/chrome_webusb_browser_client.h:28: void OpenURL(const GURL& url) override; This function is no ...
5 years, 3 months ago (2015-08-27 01:03:34 UTC) #36
juncai
Please review the change. https://codereview.chromium.org/1289423002/diff/360001/chrome/browser/chrome_webusb_browser_client.h File chrome/browser/chrome_webusb_browser_client.h (right): https://codereview.chromium.org/1289423002/diff/360001/chrome/browser/chrome_webusb_browser_client.h#newcode28 chrome/browser/chrome_webusb_browser_client.h:28: void OpenURL(const GURL& url) override; ...
5 years, 3 months ago (2015-08-27 03:42:42 UTC) #38
stevenjb
https://codereview.chromium.org/1289423002/diff/380001/chrome/browser/chrome_browser_main.cc File chrome/browser/chrome_browser_main.cc (right): https://codereview.chromium.org/1289423002/diff/380001/chrome/browser/chrome_browser_main.cc#newcode1176 chrome/browser/chrome_browser_main.cc:1176: webusb::WebUsbDetector::Set(new webusb::WebUsbDetector()); Where do these get destroyed? Do they ...
5 years, 3 months ago (2015-08-27 16:56:03 UTC) #40
Reilly Grant (use Gerrit)
https://codereview.chromium.org/1289423002/diff/380001/chrome/browser/chrome_browser_main.cc File chrome/browser/chrome_browser_main.cc (right): https://codereview.chromium.org/1289423002/diff/380001/chrome/browser/chrome_browser_main.cc#newcode1176 chrome/browser/chrome_browser_main.cc:1176: webusb::WebUsbDetector::Set(new webusb::WebUsbDetector()); On 2015/08/27 16:56:02, stevenjb wrote: > Where ...
5 years, 3 months ago (2015-08-27 17:03:00 UTC) #41
juncai
Please review the change. https://codereview.chromium.org/1289423002/diff/380001/chrome/browser/chrome_webusb_browser_client.cc File chrome/browser/chrome_webusb_browser_client.cc (right): https://codereview.chromium.org/1289423002/diff/380001/chrome/browser/chrome_webusb_browser_client.cc#newcode43 chrome/browser/chrome_webusb_browser_client.cc:43: OpenURL(landing_page_); On 2015/08/27 16:56:02, stevenjb ...
5 years, 3 months ago (2015-08-27 19:58:39 UTC) #42
Reilly Grant (use Gerrit)
https://codereview.chromium.org/1289423002/diff/400001/chrome/browser/chrome_browser_main.cc File chrome/browser/chrome_browser_main.cc (right): https://codereview.chromium.org/1289423002/diff/400001/chrome/browser/chrome_browser_main.cc#newcode1176 chrome/browser/chrome_browser_main.cc:1176: webusb::WebUsbDetector::Set(new webusb::WebUsbDetector()); These are still being leaked. I'm sorry ...
5 years, 3 months ago (2015-08-27 22:17:17 UTC) #43
juncai
stevenjb, Reilly, please review the change. https://codereview.chromium.org/1289423002/diff/380001/chrome/browser/chrome_browser_main.cc File chrome/browser/chrome_browser_main.cc (right): https://codereview.chromium.org/1289423002/diff/380001/chrome/browser/chrome_browser_main.cc#newcode1176 chrome/browser/chrome_browser_main.cc:1176: webusb::WebUsbDetector::Set(new webusb::WebUsbDetector()); On ...
5 years, 3 months ago (2015-08-28 01:15:52 UTC) #44
Reilly Grant (use Gerrit)
lgtm, thanks for the updates.
5 years, 3 months ago (2015-08-28 01:24:53 UTC) #45
stevenjb
https://codereview.chromium.org/1289423002/diff/420001/chrome/browser/chrome_browser_main.cc File chrome/browser/chrome_browser_main.cc (right): https://codereview.chromium.org/1289423002/diff/420001/chrome/browser/chrome_browser_main.cc#newcode1177 chrome/browser/chrome_browser_main.cc:1177: webusb_detector_.reset(new webusb::WebUsbDetector()); Couldn't this just be: webusb_detector_.reset(new webusb::WebUsbDetector(webusb_browser_client_.get())? Also: ...
5 years, 3 months ago (2015-08-28 16:55:06 UTC) #46
juncai
stevenjb: Please review the change. https://codereview.chromium.org/1289423002/diff/420001/chrome/browser/chrome_browser_main.cc File chrome/browser/chrome_browser_main.cc (right): https://codereview.chromium.org/1289423002/diff/420001/chrome/browser/chrome_browser_main.cc#newcode1177 chrome/browser/chrome_browser_main.cc:1177: webusb_detector_.reset(new webusb::WebUsbDetector()); On 2015/08/28 ...
5 years, 3 months ago (2015-08-28 20:23:57 UTC) #47
juncai
On 2015/08/28 20:23:57, juncai wrote: > stevenjb: > > Please review the change. > > ...
5 years, 3 months ago (2015-09-01 19:19:27 UTC) #48
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1289423002/440001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1289423002/440001
5 years, 3 months ago (2015-09-03 21:27:48 UTC) #51
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_compile_dbg_32_ng/builds/93631) ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED, ...
5 years, 3 months ago (2015-09-03 21:30:40 UTC) #53
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1289423002/460001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1289423002/460001
5 years, 3 months ago (2015-09-04 00:32:53 UTC) #56
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/96411)
5 years, 3 months ago (2015-09-04 00:43:45 UTC) #58
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1289423002/480001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1289423002/480001
5 years, 3 months ago (2015-09-04 02:03:42 UTC) #61
commit-bot: I haz the power
Committed patchset #25 (id:480001)
5 years, 3 months ago (2015-09-04 02:36:15 UTC) #62
commit-bot: I haz the power
Patchset 25 (id:??) landed as https://crrev.com/1b115256c55c867df10795991178c766e5604aa6 Cr-Commit-Position: refs/heads/master@{#347310}
5 years, 3 months ago (2015-09-04 02:37:36 UTC) #63
Alexei Svitkine (slow)
https://codereview.chromium.org/1289423002/diff/480001/chrome/common/chrome_switches.cc File chrome/common/chrome_switches.cc (right): https://codereview.chromium.org/1289423002/diff/480001/chrome/common/chrome_switches.cc#newcode562 chrome/common/chrome_switches.cc:562: extern const char kEnableWebUsbNotifications[] = "enable-webusb-notifications"; Nit: Why is ...
5 years, 3 months ago (2015-09-04 15:34:35 UTC) #65
juncai
Thanks asvitkine@! https://codereview.chromium.org/1289423002/diff/480001/chrome/common/chrome_switches.cc File chrome/common/chrome_switches.cc (right): https://codereview.chromium.org/1289423002/diff/480001/chrome/common/chrome_switches.cc#newcode562 chrome/common/chrome_switches.cc:562: extern const char kEnableWebUsbNotifications[] = "enable-webusb-notifications"; On ...
5 years, 3 months ago (2015-09-04 16:39:14 UTC) #66
stevenjb
Sorry, I was OOO last week. A couple of comments that can be addressed in ...
5 years, 3 months ago (2015-09-09 19:35:26 UTC) #67
juncai
5 years, 3 months ago (2015-09-09 20:30:29 UTC) #68
Message was sent while issue was closed.
https://codereview.chromium.org/1289423002/diff/480001/chrome/browser/chrome_...
File chrome/browser/chrome_browser_main.cc (right):

https://codereview.chromium.org/1289423002/diff/480001/chrome/browser/chrome_...
chrome/browser/chrome_browser_main.cc:1794: webusb_detector_.reset();
On 2015/09/09 19:35:26, stevenjb wrote:
> Note: We should really reset the detector before the browser client since the
> detector depends on the browser client. As-is if the detector code changes to
> access the browser client in the destructor, a crash would likely be
introduced.

Done in a new patch.

https://codereview.chromium.org/1289423002/diff/480001/components/webusb/webu...
File components/webusb/webusb_detector.h (right):

https://codereview.chromium.org/1289423002/diff/480001/components/webusb/webu...
components/webusb/webusb_detector.h:22: WebUsbDetector(WebUsbBrowserClient*
webusb_browser_client);
On 2015/09/09 19:35:26, stevenjb wrote:
> explicit

Done in a new patch.

Powered by Google App Engine
This is Rietveld 408576698