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

Issue 143673004: Fix the crash issue caused by vibrate permission (Closed)

Created:
6 years, 11 months ago by guangzhen.li
Modified:
6 years, 10 months ago
CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

Fix the crash issue caused by vibrate permission The app will be crashed when using vibrate API, without VIBRATE permission specified. Catch the security exception instead of crash should be more friendly. BUG=338151 TEST= Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=251289

Patch Set 1 #

Patch Set 2 : Check permissin instead of try-catch #

Total comments: 3

Patch Set 3 : Change error message, and cleaner condition #

Patch Set 4 : Add myself in the AUTHORS file #

Unified diffs Side-by-side diffs Delta from patch set Stats (+14 lines, -2 lines) Patch
M AUTHORS View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/VibrationProvider.java View 1 2 3 chunks +13 lines, -2 lines 0 comments Download

Messages

Total messages: 31 (0 generated)
guangzhen.li
6 years, 11 months ago (2014-01-26 06:39:59 UTC) #1
jdduke (slow)
This looks pretty reasonable, but I'll defer to mvanouwerkerk@ who originally wrote this. On a ...
6 years, 11 months ago (2014-01-26 15:49:10 UTC) #2
jdduke (slow)
On 2014/01/26 15:49:10, jdduke wrote: > This looks pretty reasonable, but I'll defer to mvanouwerkerk@ ...
6 years, 11 months ago (2014-01-27 01:13:39 UTC) #3
guangzhen.li
On 2014/01/27 01:13:39, jdduke wrote: > On 2014/01/26 15:49:10, jdduke wrote: > > This looks ...
6 years, 11 months ago (2014-01-27 08:56:37 UTC) #4
Michael van Ouwerkerk
Thanks for tackling this issue, I wasn't aware this was a concern! I like Jared ...
6 years, 11 months ago (2014-01-27 16:36:51 UTC) #5
guangzhen.li
On 2014/01/27 16:36:51, Michael van Ouwerkerk wrote: > Thanks for tackling this issue, I wasn't ...
6 years, 10 months ago (2014-01-28 08:57:34 UTC) #6
Michael van Ouwerkerk
I discussed the logging options with Pavel, and I summarized his advice in this bug: ...
6 years, 10 months ago (2014-01-28 15:09:04 UTC) #7
jdduke (slow)
https://codereview.chromium.org/143673004/diff/80001/content/public/android/java/src/org/chromium/content/browser/VibrationProvider.java File content/public/android/java/src/org/chromium/content/browser/VibrationProvider.java (right): https://codereview.chromium.org/143673004/diff/80001/content/public/android/java/src/org/chromium/content/browser/VibrationProvider.java#newcode50 content/public/android/java/src/org/chromium/content/browser/VibrationProvider.java:50: Log.e(TAG, "Caught security exception, requires VIBRATE permission."); On 2014/01/28 ...
6 years, 10 months ago (2014-01-28 15:45:32 UTC) #8
guangzhen.li
Thanks for your comments. I'm currently in holiday till Feb. 6. Will fix these when ...
6 years, 10 months ago (2014-01-29 16:26:05 UTC) #9
benm (inactive)
Thanks; this looks like the right approach for WebView.
6 years, 10 months ago (2014-01-31 12:06:44 UTC) #10
guangzhen.li
On 2014/01/28 15:45:32, jdduke wrote: > https://codereview.chromium.org/143673004/diff/80001/content/public/android/java/src/org/chromium/content/browser/VibrationProvider.java > File > content/public/android/java/src/org/chromium/content/browser/VibrationProvider.java > (right): > > ...
6 years, 10 months ago (2014-02-07 07:43:12 UTC) #11
Michael van Ouwerkerk
Thanks! lgtm
6 years, 10 months ago (2014-02-07 13:05:02 UTC) #12
benm (inactive)
lgtm
6 years, 10 months ago (2014-02-10 21:40:26 UTC) #13
guangzhen.li
The CQ bit was checked by guangzhen.li@intel.com
6 years, 10 months ago (2014-02-12 01:25:50 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/guangzhen.li@intel.com/143673004/100001
6 years, 10 months ago (2014-02-12 01:27:22 UTC) #15
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 10 months ago (2014-02-12 01:53:05 UTC) #16
commit-bot: I haz the power
Retried try job too often on chromium_presubmit for step(s) presubmit http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_presubmit&number=49645
6 years, 10 months ago (2014-02-12 01:53:06 UTC) #17
jdduke (slow)
Sorry, lgtm!
6 years, 10 months ago (2014-02-12 17:48:44 UTC) #18
jdduke (slow)
The CQ bit was checked by jdduke@chromium.org
6 years, 10 months ago (2014-02-12 17:49:08 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/guangzhen.li@intel.com/143673004/100001
6 years, 10 months ago (2014-02-12 17:49:50 UTC) #20
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 10 months ago (2014-02-12 18:12:48 UTC) #21
commit-bot: I haz the power
Retried try job too often on chromium_presubmit for step(s) presubmit http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_presubmit&number=49846
6 years, 10 months ago (2014-02-12 18:12:49 UTC) #22
guangzhen.li
The CQ bit was checked by guangzhen.li@intel.com
6 years, 10 months ago (2014-02-13 08:25:50 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/guangzhen.li@intel.com/143673004/100001
6 years, 10 months ago (2014-02-13 08:29:34 UTC) #24
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 10 months ago (2014-02-13 08:45:32 UTC) #25
commit-bot: I haz the power
Retried try job too often on chromium_presubmit for step(s) presubmit http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_presubmit&number=50031
6 years, 10 months ago (2014-02-13 08:45:33 UTC) #26
Michael van Ouwerkerk
I think isn't going so submit until you are a registered author. ** Presubmit Warnings ...
6 years, 10 months ago (2014-02-13 10:36:28 UTC) #27
guangzhen.li
The CQ bit was checked by guangzhen.li@intel.com
6 years, 10 months ago (2014-02-14 02:27:10 UTC) #28
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/guangzhen.li@intel.com/143673004/540001
6 years, 10 months ago (2014-02-14 02:28:49 UTC) #29
guangzhen.li
On 2014/02/13 10:36:28, Michael van Ouwerkerk wrote: > I think isn't going so submit until ...
6 years, 10 months ago (2014-02-14 02:29:35 UTC) #30
commit-bot: I haz the power
6 years, 10 months ago (2014-02-14 13:42:59 UTC) #31
Message was sent while issue was closed.
Change committed as 251289

Powered by Google App Engine
This is Rietveld 408576698