Address NewApi lint warnings in base.

Address NewApi lint warnings in base.

BUG=411461
Committed: https://crrev.com/990941506160c4256f556c7938c34122def0a537
Cr-Commit-Position: refs/heads/master@{#320110}

[newt (away), 2015-03-03 23:59:39]

newt@chromium.org changed reviewers:
+ jamesr@chromium.org, kkimlabs@chromium.org, yfriedman@chromium.org

[newt (away), 2015-03-03 23:59:40]

PTAL

kkimlabs: AnimationFrameTimeHistogram.java
jamesr: JavaHandlerThread.java
yfriedman: SysUtils.java and everything else for OWNERS

[jamesr, 2015-03-04 00:01:38]

https://codereview.chromium.org/974103002/diff/1/base/android/java/src/org/ch...
File base/android/java/src/org/chromium/base/JavaHandlerThread.java (right):

https://codereview.chromium.org/974103002/diff/1/base/android/java/src/org/ch...
base/android/java/src/org/chromium/base/JavaHandlerThread.java:46: // Question
for jamesr: What should we do here on versions of Android before JB MR2
I don't know.  The reason I added this call was for microbenchmarking purposes. 
We probably don't care about running these microbenchmarks on older versions of
Android.

[newt (away), 2015-03-04 00:24:07]

https://codereview.chromium.org/974103002/diff/1/base/android/java/src/org/ch...
File base/android/java/src/org/chromium/base/JavaHandlerThread.java (right):

https://codereview.chromium.org/974103002/diff/1/base/android/java/src/org/ch...
base/android/java/src/org/chromium/base/JavaHandlerThread.java:46: // Question
for jamesr: What should we do here on versions of Android before JB MR2
On 2015/03/04 00:01:38, jamesr wrote:
> I don't know.  The reason I added this call was for microbenchmarking
purposes. 
> We probably don't care about running these microbenchmarks on older versions
of
> Android.

Will the code work if I call mThread.quit() on earlier versions of Android, or
should I just throw an exception and call it a day?

[jamesr, 2015-03-04 00:25:53]

On 2015/03/04 00:24:07, newt wrote:
>
https://codereview.chromium.org/974103002/diff/1/base/android/java/src/org/ch...
> File base/android/java/src/org/chromium/base/JavaHandlerThread.java (right):
> 
>
https://codereview.chromium.org/974103002/diff/1/base/android/java/src/org/ch...
> base/android/java/src/org/chromium/base/JavaHandlerThread.java:46: // Question
> for jamesr: What should we do here on versions of Android before JB MR2
> On 2015/03/04 00:01:38, jamesr wrote:
> > I don't know.  The reason I added this call was for microbenchmarking
> purposes. 
> > We probably don't care about running these microbenchmarks on older versions
> of
> > Android.
> 
> Will the code work if I call mThread.quit() on earlier versions of Android

I'm not sure what would happen.  If you have the ability to test on an earlier
version of Android try running base_perftests on it with your proposed change
and see what happens.

> or should I just throw an exception and call it a day?

Seems fine to me, so long as we aren't trying to run base_perftests on such
devices (which we probably aren't)

[Kibeom Kim (inactive), 2015-03-04 00:34:15]

https://codereview.chromium.org/974103002/diff/1/base/android/java/src/org/ch...
File base/android/java/src/org/chromium/base/AnimationFrameTimeHistogram.java
(right):

https://codereview.chromium.org/974103002/diff/1/base/android/java/src/org/ch...
base/android/java/src/org/chromium/base/AnimationFrameTimeHistogram.java:22:
@TargetApi(Build.VERSION_CODES.JELLY_BEAN)
Can we narrow the scope by moving this to "class Recorder.." down there? (I
guess it cannot be applied to mAnimator directly)

btw, I wish that there is more straightforward way to express it. Setting
TargetApi seems kind of indirect. Or how about @SuppressLint("NewApi") ?

[newt (away), 2015-03-04 00:54:28]

https://codereview.chromium.org/974103002/diff/1/base/android/java/src/org/ch...
File base/android/java/src/org/chromium/base/AnimationFrameTimeHistogram.java
(right):

https://codereview.chromium.org/974103002/diff/1/base/android/java/src/org/ch...
base/android/java/src/org/chromium/base/AnimationFrameTimeHistogram.java:22:
@TargetApi(Build.VERSION_CODES.JELLY_BEAN)
On 2015/03/04 00:34:15, Kibeom Kim wrote:
> Can we narrow the scope by moving this to "class Recorder.." down there? (I
> guess it cannot be applied to mAnimator directly)

Yes. Good idea. (It could be applied to mAnimator, but it would also need to be
applied to Record, which implements TimeListener, and to all the places where
mAnimator is used)

> btw, I wish that there is more straightforward way to express it. Setting
> TargetApi seems kind of indirect. Or how about @SuppressLint("NewApi") ?

TargetApi is actually preferable. It expresses that this code uses features
added in API levels up to and including, say, JELLY_BEAN, and that the code is
safe in its use of these features. If someone later tries to use an API added in
LOLLIPOP, lint would still complain. @SuppressLint("NewApi") is a broader and
vaguer suppression of all NewApi warnings.

[newt (away), 2015-03-04 00:54:37]

PTAL

[Kibeom Kim (inactive), 2015-03-04 00:56:13]

AnimationFrameTimeHistogram lgtm

[newt (away), 2015-03-04 00:56:21]

On Tue, Mar 3, 2015 at 4:25 PM, <jamesr@chromium.org> wrote:

> On 2015/03/04 00:24:07, newt wrote:
>
> https://codereview.chromium.org/974103002/diff/1/base/
> android/java/src/org/chromium/base/JavaHandlerThread.java
>
>> File base/android/java/src/org/chromium/base/JavaHandlerThread.java
>> (right):
>>
>
>
> https://codereview.chromium.org/974103002/diff/1/base/
> android/java/src/org/chromium/base/JavaHandlerThread.java#newcode46
>
>> base/android/java/src/org/chromium/base/JavaHandlerThread.java:46: //
>> Question
>> for jamesr: What should we do here on versions of Android before JB MR2
>> On 2015/03/04 00:01:38, jamesr wrote:
>> > I don't know.  The reason I added this call was for microbenchmarking
>> purposes.
>> > We probably don't care about running these microbenchmarks on older
>> versions
>> of
>> > Android.
>>
>
>  Will the code work if I call mThread.quit() on earlier versions of Android
>>
>
> I'm not sure what would happen.  If you have the ability to test on an
> earlier
> version of Android try running base_perftests on it with your proposed
> change
> and see what happens.
>
>  or should I just throw an exception and call it a day?
>>
>
> Seems fine to me, so long as we aren't trying to run base_perftests on such
> devices (which we probably aren't)
>

base_perftests crashes on these devices, so I expect we're not running
these tests
on these devices.


>
> https://codereview.chromium.org/974103002/
>

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

[jamesr, 2015-03-04 01:16:41]

On 2015/03/04 00:56:21, newt wrote:
> base_perftests crashes on these devices, so I expect we're not running
> these tests
> on these devices.

That doesn't really surprise me.  Testing performance on <= JB1 probably isn't
all that interesting.  I think whatever you feel like doing here is fine in that
case.

[Yaron, 2015-03-05 17:09:50]

https://codereview.chromium.org/974103002/diff/20001/base/android/java/src/or...
File base/android/java/src/org/chromium/base/JavaHandlerThread.java (right):

https://codereview.chromium.org/974103002/diff/20001/base/android/java/src/or...
base/android/java/src/org/chromium/base/JavaHandlerThread.java:44: throw new
UnsupportedOperationException("stop() not supported before JB MR2");
This is because of the call to quitSafely, right? What's to prevent this from
being called pre-mr2? Is there a check somewhere in native? I didn't see it at
first blush.

[newt (away), 2015-03-05 22:29:40]

https://codereview.chromium.org/974103002/diff/20001/base/android/java/src/or...
File base/android/java/src/org/chromium/base/JavaHandlerThread.java (right):

https://codereview.chromium.org/974103002/diff/20001/base/android/java/src/or...
base/android/java/src/org/chromium/base/JavaHandlerThread.java:44: throw new
UnsupportedOperationException("stop() not supported before JB MR2");
On 2015/03/05 17:09:49, Yaron wrote:
> This is because of the call to quitSafely, right? What's to prevent this from
> being called pre-mr2? Is there a check somewhere in native? I didn't see it at
> first blush.

This method is used only in base_perftests, which AFAICT isn't run on older
devices (or if it is, it's already crashing with a NoSuchMethodError).

Anyway, I updated the code so it works on all devices.

[jamesr, 2015-03-05 22:36:46]

This'll somewhat break base_perftests since it is depending on all posted tasks
running whereas calling HandlerThread.stop() will simply stop at some arbitrary
point.  The resulting performance data will then be bogus.  I think it'd be
better to crash or something rather than produce valid-looking but bogus data,
but I don't really care all that much about what happens given that we don't run
perf tests on such OSes

[newt (away), 2015-03-05 23:01:23]

On 2015/03/05 22:36:46, jamesr wrote:
> This'll somewhat break base_perftests since it is depending on all posted
tasks
> running whereas calling HandlerThread.stop() will simply stop at some
arbitrary
> point.  The resulting performance data will then be bogus.  I think it'd be
> better to crash or something rather than produce valid-looking but bogus data,
> but I don't really care all that much about what happens given that we don't
run
> perf tests on such OSes

I think this does work; see if you agree. mThread.quit() is called in a posted
task, so the thread quits only after all the prior posted tasks have run. This
is identical to calling mThread.quitSafely() immediately (not via post()) with
one difference: calls to Handler.post() after mThread.quitSafely() has been
called will return false, whereas calls to Handler.post() after the
"mThread.quit() task" has been posted will return true.

[jamesr, 2015-03-05 23:02:17]

Ah good point.

[newt (away), 2015-03-06 17:24:52]

yfriedman: PTAL

[newt (away), 2015-03-09 21:20:57]

yfriedman: friendly ping

[Yaron, 2015-03-11 14:32:00]

lgtm (sorry I missed it somehow)

[newt (away), 2015-03-11 17:06:20]

The CQ bit was checked by newt@chromium.org

[newt (away), 2015-03-11 17:06:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
kkimlabs@chromium.org
Link to the patchset: https://codereview.chromium.org/974103002/#ps40001 (title:
"JavaHandlerThread.stop() now works on all devices")

[commit-bot: I haz the power, 2015-03-11 17:06:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/974103002/40001

[commit-bot: I haz the power, 2015-03-11 18:08:20]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-03-11 18:09:18]

Patchset 3 (id:??) landed as
https://crrev.com/990941506160c4256f556c7938c34122def0a537
Cr-Commit-Position: refs/heads/master@{#320110}