Log device/usb messages to chrome://device-log.

Log device/usb messages to chrome://device-log.

Log events and debugging information related to USB devices through
the device event log so that these errors are visible on Chrome OS
at chrome://device-log.

BUG=455336
Committed: https://crrev.com/0f2dc63836f1d614069cc17b8452866e891b558c
Cr-Commit-Position: refs/heads/master@{#317635}

[Reilly Grant (use Gerrit), 2015-02-19 23:32:21]

reillyg@chromium.org changed reviewers:
+ stevenjb@chromium.org

[Reilly Grant (use Gerrit), 2015-02-19 23:34:53]

Steven, I've split out the use case for
https://codereview.chromium.org/919183002/ into this independent patch so that
I'm not blocked on getting chrome://device-log onto all platforms. The bug I'm
trying to investigate is on Chrome OS.

[Reilly Grant (use Gerrit), 2015-02-19 23:41:44]

Patchset #1 (id:1) has been deleted

[Reilly Grant (use Gerrit), 2015-02-19 23:46:49]

Patchset #1 (id:20001) has been deleted

[stevenjb, 2015-02-20 00:01:27]

https://codereview.chromium.org/941063003/diff/40001/device/usb/usb_device_ha...
File device/usb/usb_device_handle_impl.cc (right):

https://codereview.chromium.org/941063003/diff/40001/device/usb/usb_device_ha...
device/usb/usb_device_handle_impl.cc:150: <<
ConvertPlatformUsbErrorToString(rv);
You might want to use type EVENT here. DEBUG doesn't show up in Feedback
reports. 

General Note: Anything that might be spammy (e.g. an event that occurs every
second) is not a good candidate for DEVICE_LOG.

https://codereview.chromium.org/941063003/diff/40001/device/usb/usb_device_ha...
device/usb/usb_device_handle_impl.cc:237: USB_LOG(DEBUG) << "Failed to allocate
control transfer.";
If LOG(ERROR) was correct before, this should also use ERROR. Anything that
shouldn't normally occur should be fine as type ERROR (which will also output to
the chrome log).

https://codereview.chromium.org/941063003/diff/40001/device/usb/usb_device_ha...
device/usb/usb_device_handle_impl.cc:265: USB_LOG(DEBUG) << "Failed to allocate
bulk transfer.";
Double check all of these for ERROR / USER / EVENT / DEBUG

https://codereview.chromium.org/941063003/diff/40001/device/usb/usb_device_im...
File device/usb/usb_device_impl.cc (right):

https://codereview.chromium.org/941063003/diff/40001/device/usb/usb_device_im...
device/usb/usb_device_impl.cc:226: << ConvertPlatformUsbErrorToString(rv);
These seem like they should be actual ERROR events.

https://codereview.chromium.org/941063003/diff/40001/device/usb/usb_log.h
File device/usb/usb_log.h (right):

https://codereview.chromium.org/941063003/diff/40001/device/usb/usb_log.h#new...
device/usb/usb_log.h:11: #define USB_LOG(x) VLOG(1)
You might want to try something like:

#define USB_LOG_DEBUG VLOG(2)
#define USB_LOG_USER VLOG(1)
#define USB_LOG_ERROR LOG(ERROR)
#define USB_LOG(level) USB_LOG_##level

https://codereview.chromium.org/941063003/diff/40001/device/usb/usb_service.cc
File device/usb/usb_service.cc (right):

https://codereview.chromium.org/941063003/diff/40001/device/usb/usb_service.c...
device/usb/usb_service.cc:90: << ", uniqueId = " << device->unique_id();
These might be a good candidate for 'USER' since it might represent a user
inserting or removing a device. Even if some devices are built in and not
physically added I think that would be OK. It's a convenient way to identify
"significant" events that are likely to drive other events.

[Reilly Grant (use Gerrit), 2015-02-20 00:10:22]

https://codereview.chromium.org/941063003/diff/40001/device/usb/usb_device_ha...
File device/usb/usb_device_handle_impl.cc (right):

https://codereview.chromium.org/941063003/diff/40001/device/usb/usb_device_ha...
device/usb/usb_device_handle_impl.cc:150: <<
ConvertPlatformUsbErrorToString(rv);
On 2015/02/20 00:01:27, stevenjb wrote:
> You might want to use type EVENT here. DEBUG doesn't show up in Feedback
> reports. 
> 
> General Note: Anything that might be spammy (e.g. an event that occurs every
> second) is not a good candidate for DEVICE_LOG.

Spamminess is somewhat controlled by the Chrome app driving this code. If it
does something that generates an error every second, then there will be a log
every second. That said, such failures should be rare and so I'm willing to
accept the possibility of a message becoming spammy if things are really broken.

https://codereview.chromium.org/941063003/diff/40001/device/usb/usb_service.cc
File device/usb/usb_service.cc (right):

https://codereview.chromium.org/941063003/diff/40001/device/usb/usb_service.c...
device/usb/usb_service.cc:90: << ", uniqueId = " << device->unique_id();
On 2015/02/20 00:01:27, stevenjb wrote:
> These might be a good candidate for 'USER' since it might represent a user
> inserting or removing a device. Even if some devices are built in and not
> physically added I think that would be OK. It's a convenient way to identify
> "significant" events that are likely to drive other events.

Fair enough. There will be a small burst of events when the service is
initialized but otherwise this should be mostly user-driven.

[Reilly Grant (use Gerrit), 2015-02-20 23:30:40]

I've rebased this patch and re-evaluated the log levels.

[stevenjb, 2015-02-20 23:40:32]

lgtm w/ nit

https://codereview.chromium.org/941063003/diff/60001/device/usb/usb_device_ha...
File device/usb/usb_device_handle_impl.cc (right):

https://codereview.chromium.org/941063003/diff/60001/device/usb/usb_device_ha...
device/usb/usb_device_handle_impl.cc:672: USB_LOG(USER) << "Transfer too long.";
These feel like errors; it seems like we would want our attention drawn to these
if they happen.

[Reilly Grant (use Gerrit), 2015-02-20 23:43:55]

https://codereview.chromium.org/941063003/diff/60001/device/usb/usb_device_ha...
File device/usb/usb_device_handle_impl.cc (right):

https://codereview.chromium.org/941063003/diff/60001/device/usb/usb_device_ha...
device/usb/usb_device_handle_impl.cc:672: USB_LOG(USER) << "Transfer too long.";
On 2015/02/20 23:40:32, stevenjb wrote:
> These feel like errors; it seems like we would want our attention drawn to
these
> if they happen.

They're not "Chrome is having a problem" errors, they're "the user app is doing
something wrong" errors, so I thought that USER would be appropriate.

[Reilly Grant (use Gerrit), 2015-02-21 00:28:45]

New patchsets have been uploaded after l-g-t-m from stevenjb@chromium.org

[stevenjb, 2015-02-21 00:55:28]

https://codereview.chromium.org/941063003/diff/60001/device/usb/usb_device_ha...
File device/usb/usb_device_handle_impl.cc (right):

https://codereview.chromium.org/941063003/diff/60001/device/usb/usb_device_ha...
device/usb/usb_device_handle_impl.cc:672: USB_LOG(USER) << "Transfer too long.";
On 2015/02/20 23:43:54, reillyg wrote:
> On 2015/02/20 23:40:32, stevenjb wrote:
> > These feel like errors; it seems like we would want our attention drawn to
> these
> > if they happen.
> 
> They're not "Chrome is having a problem" errors, they're "the user app is
doing
> something wrong" errors, so I thought that USER would be appropriate.

It's still an error, imho. I've been associating USER with "actions". The source
of an ERROR is less interesting than the fact that something unexpected
occurred. It's fine either way.

[Reilly Grant (use Gerrit), 2015-02-21 01:09:04]

The CQ bit was checked by reillyg@chromium.org

[Reilly Grant (use Gerrit), 2015-02-21 01:09:17]

The CQ bit was unchecked by reillyg@chromium.org

[Reilly Grant (use Gerrit), 2015-02-21 01:21:38]

reillyg@chromium.org changed reviewers:
+ bauerb@chromium.org

[Reilly Grant (use Gerrit), 2015-02-21 01:21:59]

bauerb@, please take a look at the chrome/browser/ui/webui/device_log_ui.cc
change.

[stevenjb, 2015-02-21 01:27:55]

On 2015/02/21 01:21:59, reillyg wrote:
> bauerb@, please take a look at the chrome/browser/ui/webui/device_log_ui.cc
> change.

I'm an owner of that directory, and (effectively the only) author of that file.

[Reilly Grant (use Gerrit), 2015-02-21 01:43:50]

reillyg@chromium.org changed reviewers:
+ xiyuan@chromium.org
- bauerb@chromium.org

[Reilly Grant (use Gerrit), 2015-02-21 01:44:35]

Adding stevenjb@ as an OWNER of this file (since it was moved out of
.../webui/chromeos) and suggesting xiyuan@ to review.

[xiyuan, 2015-02-22 17:51:50]

lgtm

[Reilly Grant (use Gerrit), 2015-02-22 18:31:29]

The CQ bit was checked by reillyg@chromium.org

[commit-bot: I haz the power, 2015-02-22 18:31:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/941063003/100001

[commit-bot: I haz the power, 2015-02-22 18:34:00]

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

[commit-bot: I haz the power, 2015-02-22 18:34:02]

Try jobs failed on following builders:
  ios_dbg_simulator on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)

[Reilly Grant (use Gerrit), 2015-02-23 19:09:42]

New patchsets have been uploaded after l-g-t-m from xiyuan@chromium.org

[Reilly Grant (use Gerrit), 2015-02-23 19:09:58]

The CQ bit was checked by reillyg@chromium.org

[commit-bot: I haz the power, 2015-02-23 19:10:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/941063003/120001

[commit-bot: I haz the power, 2015-02-23 20:02:16]

Committed patchset #5 (id:120001)

[commit-bot: I haz the power, 2015-02-23 20:03:37]

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