🐝 Convert a few CHECKs to LOG(FATAL)

Convert a few CHECKs to LOG(FATAL)

After https://codereview.chromium.org/1937613002/, these error messages will be
lost.  We'd prefer to keep them to help with reports from the field.

Committed: https://crrev.com/e524fe76b40554f2ab571dc3f2c26ac0144ea1a1
Cr-Commit-Position: refs/heads/master@{#391878}

[Yaron, 2016-05-05 14:37:16]

Description was changed from

==========
Convert a few CHECKs to LOG(FATAL)

After https://codereview.chromium.org/1937613002/, these error messages will be
lost.  We'd prefer to keep them to help with reports from the field.
==========

to

==========
Convert a few CHECKs to LOG(FATAL)

After https://codereview.chromium.org/1937613002/, these error messages will be
lost.  We'd prefer to keep them to help with reports from the field.
==========

[Yaron, 2016-05-05 17:08:29]

yfriedman@chromium.org changed reviewers:
+ torne@chromium.org

[Yaron, 2016-05-05 17:08:30]

https://codereview.chromium.org/1955573002/diff/20001/base/android/jni_androi...
File base/android/jni_android.cc (left):

https://codereview.chromium.org/1955573002/diff/20001/base/android/jni_androi...
base/android/jni_android.cc:159: CHECK(base::android::ClearException(env) || id)
<<
Am I crazy? Wasn't this inverted?

[Torne, 2016-05-05 17:33:22]

LGTM

https://codereview.chromium.org/1955573002/diff/20001/base/android/jni_androi...
File base/android/jni_android.cc (left):

https://codereview.chromium.org/1955573002/diff/20001/base/android/jni_androi...
base/android/jni_android.cc:159: CHECK(base::android::ClearException(env) || id)
<<
On 2016/05/05 17:08:30, Yaron wrote:
> Am I crazy? Wasn't this inverted?

Yes, this was inverted, you aren't crazy. It just didn't actually matter,
because if id is non-null then there won't have been an exception set, and so
the only way there can now be an exception is if there was already a pending
uncleared exception before this call.

I'm actually not sure whether it's even idiomatic/sensible to check for
exceptions when you get valid return values from the JNI functions. In Python-C
binding it's idiomatic to check if you get NULL back and only then care about
the exception state..

[Yaron, 2016-05-05 19:46:20]

The CQ bit was checked by yfriedman@chromium.org

[commit-bot: I haz the power, 2016-05-05 19:47:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1955573002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1955573002/20001

[Yaron, 2016-05-05 19:48:12]

On 2016/05/05 17:33:22, Torne wrote:
> LGTM
> 
>
https://codereview.chromium.org/1955573002/diff/20001/base/android/jni_androi...
> File base/android/jni_android.cc (left):
> 
>
https://codereview.chromium.org/1955573002/diff/20001/base/android/jni_androi...
> base/android/jni_android.cc:159: CHECK(base::android::ClearException(env) ||
id)
> <<
> On 2016/05/05 17:08:30, Yaron wrote:
> > Am I crazy? Wasn't this inverted?
> 
> Yes, this was inverted, you aren't crazy. It just didn't actually matter,
> because if id is non-null then there won't have been an exception set, and so
> the only way there can now be an exception is if there was already a pending
> uncleared exception before this call.
> 
> I'm actually not sure whether it's even idiomatic/sensible to check for
> exceptions when you get valid return values from the JNI functions. In
Python-C
> binding it's idiomatic to check if you get NULL back and only then care about
> the exception state..

Ya.. we kinda just standardized on always calling CheckException when returning
from a java/jni call but I see what you're saying. Probably not much harm in it,
at least the logic is correct now.

[commit-bot: I haz the power, 2016-05-05 19:51:47]

Description was changed from

==========
Convert a few CHECKs to LOG(FATAL)

After https://codereview.chromium.org/1937613002/, these error messages will be
lost.  We'd prefer to keep them to help with reports from the field.
==========

to

==========
Convert a few CHECKs to LOG(FATAL)

After https://codereview.chromium.org/1937613002/, these error messages will be
lost.  We'd prefer to keep them to help with reports from the field.
==========

[commit-bot: I haz the power, 2016-05-05 19:51:48]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-05-05 19:53:24]

Description was changed from

==========
Convert a few CHECKs to LOG(FATAL)

After https://codereview.chromium.org/1937613002/, these error messages will be
lost.  We'd prefer to keep them to help with reports from the field.
==========

to

==========
Convert a few CHECKs to LOG(FATAL)

After https://codereview.chromium.org/1937613002/, these error messages will be
lost.  We'd prefer to keep them to help with reports from the field.

Committed: https://crrev.com/e524fe76b40554f2ab571dc3f2c26ac0144ea1a1
Cr-Commit-Position: refs/heads/master@{#391878}
==========

[commit-bot: I haz the power, 2016-05-05 19:53:25]

Patchset 2 (id:??) landed as
https://crrev.com/e524fe76b40554f2ab571dc3f2c26ac0144ea1a1
Cr-Commit-Position: refs/heads/master@{#391878}