Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1910793002:
Android VideoCaptureCamera2: Catch all exceptions in getNumberOfCameras() (Closed)
|
Created:
4 years, 8 months ago by magjed_chromium Modified:
4 years, 8 months ago Reviewers:
mcasas CC:
chromium-reviews, posciak+watch_chromium.org, feature-media-reviews_chromium.org, mcasas+watch+vc_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAndroid VideoCaptureCamera2: Catch all exceptions in getNumberOfCameras()
BUG=605424
Committed: https://crrev.com/a74b8ccb63e527837449638b13e2c1534c40bddf
Cr-Commit-Position: refs/heads/master@{#389062}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Only catch CameraAccessException and SecurityException #Patch Set 3 : Add link to bug. Also clean up other catch clauses. #Messages
Total messages: 18 (7 generated)
The CQ bit was checked by magjed@chromium.org to run a CQ dry run
Description was changed from ========== Android VideoCaptureCamera2: Catch all exceptions in getNumberOfCameras() BUG=605424 ========== to ========== Android VideoCaptureCamera2: Catch all exceptions in getNumberOfCameras() BUG=605424 ==========
magjed@chromium.org changed reviewers: + mcasas@chromium.org
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1910793002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1910793002/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_chromium_gn_compile_dbg on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...) linux_chromium_compile_dbg_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Miguel - Please take a look.
https://codereview.chromium.org/1910793002/diff/1/media/capture/video/android... File media/capture/video/android/java/src/org/chromium/media/VideoCaptureCamera2.java (right): https://codereview.chromium.org/1910793002/diff/1/media/capture/video/android... media/capture/video/android/java/src/org/chromium/media/VideoCaptureCamera2.java:298: } catch (Exception ex) { Seems like Android is spitting a non documented Exception [1] right? In any case, don't catch generic Exceptions [2], add a case for the SecurityException being thrown. [1] http://developer.android.com/reference/android/hardware/camera2/CameraManager... [2] https://source.android.com/source/code-style.html#dont-catch-generic-exception
https://codereview.chromium.org/1910793002/diff/1/media/capture/video/android... File media/capture/video/android/java/src/org/chromium/media/VideoCaptureCamera2.java (right): https://codereview.chromium.org/1910793002/diff/1/media/capture/video/android... media/capture/video/android/java/src/org/chromium/media/VideoCaptureCamera2.java:298: } catch (Exception ex) { On 2016/04/21 14:51:59, mcasas wrote: > Seems like Android is spitting a non documented > Exception [1] right? > > In any case, don't catch generic Exceptions [2], > add a case for the SecurityException being thrown. > > [1] > http://developer.android.com/reference/android/hardware/camera2/CameraManager... > [2] > https://source.android.com/source/code-style.html#dont-catch-generic-exception #1. Yes exactly, it spits 'SecurityException: Lacking privileges to access camera service'. #2. Done.
LGTM % please add a comment with the bug so we will remember why we added an undocumented exception catch().
PTAL, I cleaned up the other catch clauses as well.
On 2016/04/21 15:29:42, magjed_chromium wrote: > PTAL, I cleaned up the other catch clauses as well. still LGTM thanks
The CQ bit was checked by magjed@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1910793002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1910793002/40001
Message was sent while issue was closed.
Description was changed from ========== Android VideoCaptureCamera2: Catch all exceptions in getNumberOfCameras() BUG=605424 ========== to ========== Android VideoCaptureCamera2: Catch all exceptions in getNumberOfCameras() BUG=605424 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Android VideoCaptureCamera2: Catch all exceptions in getNumberOfCameras() BUG=605424 ========== to ========== Android VideoCaptureCamera2: Catch all exceptions in getNumberOfCameras() BUG=605424 Committed: https://crrev.com/a74b8ccb63e527837449638b13e2c1534c40bddf Cr-Commit-Position: refs/heads/master@{#389062} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/a74b8ccb63e527837449638b13e2c1534c40bddf Cr-Commit-Position: refs/heads/master@{#389062} |
