[Android] Use exceptions rather that Log.wtf on Android

[Android] Use exceptions rather that Log.wtf on Android

AudioManagerAndroid uses Log.wtf to signal error conditions. However,
the undefined behavior of this function depending on device
configuration makes debugging difficult. Throw exceptions instead.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=278807

[benm (inactive), 2014-06-18 17:52:46]

Hey Henrik, I noticed the use of Log.wtf in AudioManagerAndroid while we were
looking at the WebRTC permission crash, it doesn't seem quite right to me and
made debugging the permission crash quite hard. I don't think there should be
any functional difference with this change to excetions, other than we'll get a
stack trace in the logcat.

WDYT?

[henrika (OOO until Aug 14), 2014-06-18 17:56:41]

Looks great. I will check details tomorrow.

What exactly happens now when you do the "throw new ...." dance? Gives a clear
output in logcat?

[benm (inactive), 2014-06-18 17:59:39]

Right, it will crash the app (as nothing will catch the exception) and
output the stack trace to logcat. Log.wtf would crash the app, but hide the
stack trace in the event dropbox (which is why it was hard to debug, I
forgot to look there).


On 18 June 2014 18:56, <henrika@chromium.org> wrote:

> Looks great. I will check details tomorrow.
>
> What exactly happens now when you do the "throw new ...." dance? Gives a
> clear
> output in logcat?
>
>
> https://codereview.chromium.org/340083002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[henrika (OOO until Aug 14), 2014-06-18 18:02:25]

Thx. Do you need OK now or can I check tomorrow?

[benm (inactive), 2014-06-18 18:48:59]

Tomorrow is fine!

Thanks!

Sent from my Android
On 18 Jun 2014 19:02, <henrika@chromium.org> wrote:

> Thx. Do you need OK now or can I check tomorrow?
>
> https://codereview.chromium.org/340083002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[henrika (OOO until Aug 14), 2014-06-19 08:56:28]

LGTM with question.

[henrika (OOO until Aug 14), 2014-06-19 08:56:34]

https://codereview.chromium.org/340083002/diff/1/media/base/android/java/src/...
File media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java
(right):

https://codereview.chromium.org/340083002/diff/1/media/base/android/java/src/...
media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java:657:
BluetoothManager btManager =
Why do you remove try-catch here?

[benm (inactive), 2014-06-19 09:23:10]

https://codereview.chromium.org/340083002/diff/1/media/base/android/java/src/...
File media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java
(right):

https://codereview.chromium.org/340083002/diff/1/media/base/android/java/src/...
media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java:657:
BluetoothManager btManager =
On 2014/06/19 08:56:34, henrika (OOO June 23-July 7) wrote:
> Why do you remove try-catch here?

Because before all we would do was catch the exception and then invoke Log.wtf,
which will crash the app. Now we just crash due to an uncaught exception.

If the intent was to just log the exception but continue execution of the app,
we could use a less severe log and put the try/catch back in.

[henrika (OOO until Aug 14), 2014-06-19 09:44:30]

Fine as is. LGTM++


On Thu, Jun 19, 2014 at 11:23 AM, <benm@chromium.org> wrote:

>
> https://codereview.chromium.org/340083002/diff/1/media/
> base/android/java/src/org/chromium/media/AudioManagerAndroid.java
> File
> media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java
> (right):
>
> https://codereview.chromium.org/340083002/diff/1/media/
> base/android/java/src/org/chromium/media/AudioManagerAndroid.java#
> newcode657
> media/base/android/java/src/org/chromium/media/
> AudioManagerAndroid.java:657:
> BluetoothManager btManager =
> On 2014/06/19 08:56:34, henrika (OOO June 23-July 7) wrote:
>
>> Why do you remove try-catch here?
>>
>
> Because before all we would do was catch the exception and then invoke
> Log.wtf, which will crash the app. Now we just crash due to an uncaught
> exception.
>
> If the intent was to just log the exception but continue execution of
> the app, we could use a less severe log and put the try/catch back in.
>
> https://codereview.chromium.org/340083002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[benm (inactive), 2014-06-19 09:45:50]

thanks!

[benm (inactive), 2014-06-19 09:45:52]

The CQ bit was checked by benm@chromium.org

[commit-bot: I haz the power, 2014-06-19 09:47:14]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/benm@chromium.org/340083002/1

[commit-bot: I haz the power, 2014-06-19 16:41:09]

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

[commit-bot: I haz the power, 2014-06-19 16:41:10]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)

[benm (inactive), 2014-06-20 12:42:46]

The CQ bit was checked by benm@chromium.org

[benm (inactive), 2014-06-20 12:42:48]

The CQ bit was unchecked by benm@chromium.org

[benm (inactive), 2014-06-20 12:44:23]

+marcus for owners

[bulach, 2014-06-20 14:12:09]

lgtm, tiny nit:

https://codereview.chromium.org/340083002/diff/1/media/base/android/java/src/...
File media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java
(right):

https://codereview.chromium.org/340083002/diff/1/media/base/android/java/src/...
media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java:668:
profileConnectionState = btAdapter.getProfileConnectionState(
nit: unindent

[benm (inactive), 2014-06-20 16:21:40]

https://codereview.chromium.org/340083002/diff/1/media/base/android/java/src/...
File media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java
(right):

https://codereview.chromium.org/340083002/diff/1/media/base/android/java/src/...
media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java:668:
profileConnectionState = btAdapter.getProfileConnectionState(
On 2014/06/20 14:12:08, bulach wrote:
> nit: unindent

Done.

[benm (inactive), 2014-06-20 16:21:53]

The CQ bit was checked by benm@chromium.org

[commit-bot: I haz the power, 2014-06-20 16:22:32]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/benm@chromium.org/340083002/20001

[commit-bot: I haz the power, 2014-06-20 20:03:40]

Change committed as 278807