Check UsbDevice string fields for null.

Check UsbDevice string fields for null.

Android API allows these methods to return null, but converters from
jni_string.h expect jstrings to be non-null with a DCHECK. See
https://developer.android.com/reference/android/hardware/usb/UsbDevice.html
> Returns ... or null if the property could not be read

R=reillyg@chromium.org
TEST=Don't crash in debug on null jstring DCHECK on KeyedService creation, or
any other scenarios that call UsbDeviceAndroid::Create. Was reproducible only
on certain devices, like Motorola Nexus 6.

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

[Igor Eremeev, 2017-05-29 13:07:24]

Hello! I've found the issue when browser was crashing on DCHECK in this method,
because the device returns null from
getManufacturerName/getProductName/getSerialNumber. According to API page, this
case is valid for Android devices, so I think the code should check these fields
for null. I didn't put this null check in ChromeUsbDevice.java, because having
any logic in simple wrapper seemed wrong to me.

[Reilly Grant (use Gerrit), 2017-05-30 18:50:29]

https://codereview.chromium.org/2914453002/diff/1/device/usb/usb_device_andro...
File device/usb/usb_device_android.cc (right):

https://codereview.chromium.org/2914453002/diff/1/device/usb/usb_device_andro...
device/usb/usb_device_android.cc:38: const JavaRef<jstring>&
manufacturer_jstring =
These locals must be ScopedJavaLocalRef<jstring>. Using a const
JavaRef<jstring>& here is unsafe because you are holding a reference to a return
value.

[Igor Eremeev, 2017-05-31 06:50:44]

https://codereview.chromium.org/2914453002/diff/1/device/usb/usb_device_andro...
File device/usb/usb_device_android.cc (right):

https://codereview.chromium.org/2914453002/diff/1/device/usb/usb_device_andro...
device/usb/usb_device_android.cc:38: const JavaRef<jstring>&
manufacturer_jstring =
On 2017/05/30 18:50:29, Reilly Grant wrote:
> These locals must be ScopedJavaLocalRef<jstring>. Using a const
> JavaRef<jstring>& here is unsafe because you are holding a reference to a
return
> value.

Const ref binding would've saved the day :) However, I agree that this feature
is rather exotic to use in production code, fixed.

https://codereview.chromium.org/2914453002/diff/1/device/usb/usb_device_andro...
device/usb/usb_device_android.cc:38: const JavaRef<jstring>&
manufacturer_jstring =
On 2017/05/30 18:50:29, Reilly Grant wrote:
> These locals must be ScopedJavaLocalRef<jstring>. Using a const
> JavaRef<jstring>& here is unsafe because you are holding a reference to a
return
> value.

Done.

[Reilly Grant (use Gerrit), 2017-05-31 06:59:05]

lgtm

[Igor Eremeev, 2017-05-31 07:00:56]

The CQ bit was checked by myrth@yandex-team.ru

[commit-bot: I haz the power, 2017-05-31 07:01:12]

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-05-31 07:47:18]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1496214056930890,
"parent_rev": "68adbbbf1a52544dbd52c2248e530f722d638b34", "commit_rev":
"eed4be16b9d9e00cfb5708277c78969d9c652810"}

[commit-bot: I haz the power, 2017-05-31 07:47:27]

Description was changed from

==========
Check UsbDevice string fields for null.

Android API allows these methods to return null, but converters from
jni_string.h expect jstrings to be non-null with a DCHECK. See
https://developer.android.com/reference/android/hardware/usb/UsbDevice.html
> Returns ... or null if the property could not be read

R=reillyg@chromium.org
TEST=Don't crash in debug on null jstring DCHECK on KeyedService creation, or
any other scenarios that call UsbDeviceAndroid::Create. Was reproducible only
on certain devices, like Motorola Nexus 6.
==========

to

==========
Check UsbDevice string fields for null.

Android API allows these methods to return null, but converters from
jni_string.h expect jstrings to be non-null with a DCHECK. See
https://developer.android.com/reference/android/hardware/usb/UsbDevice.html
> Returns ... or null if the property could not be read

R=reillyg@chromium.org
TEST=Don't crash in debug on null jstring DCHECK on KeyedService creation, or
any other scenarios that call UsbDeviceAndroid::Create. Was reproducible only
on certain devices, like Motorola Nexus 6.

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

[commit-bot: I haz the power, 2017-05-31 07:47:28]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/eed4be16b9d9e00cfb5708277c78...