Add more methods to APICompatibilityUtils

Add more methods to APICompatibilityUtils

BUG=415352
Committed: https://crrev.com/ebbafa30ff50bcf7be653caeea60c371f4deddec
Cr-Commit-Position: refs/heads/master@{#296490}

[Yusuf, 2014-09-17 23:44:09]

yusufo@chromium.org changed reviewers:
+ nyquist@chromium.org

[nyquist, 2014-09-18 00:06:21]

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

https://codereview.chromium.org/575253003/diff/1/base/android/java/src/org/ch...
base/android/java/src/org/chromium/base/ApiCompatibilityUtils.java:271: return
null;
I am assuming passing in null as the bundle to startActivity works?
Also; how about returning an empty bundle instead here?
I guess that would make it hard to know what to do if you care about the result
of toBundle() (but at which point you might want to do more fancy stuff with the
API yourself)

https://codereview.chromium.org/575253003/diff/1/base/android/java/src/org/ch...
base/android/java/src/org/chromium/base/ApiCompatibilityUtils.java:278: public
static Notification.Builder setLocalOnly(
Do we need support for flags as well?

Something like:

public static int setLocalOnly(int flags) {
  if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT_WATCH) {
    return flags | Notification.FLAG_LOCAL_ONLY;
  } else {
    return flags;
  }
}

Or do you plan on changing usages of the flag to always use the builder?

https://codereview.chromium.org/575253003/diff/1/base/android/java/src/org/ch...
base/android/java/src/org/chromium/base/ApiCompatibilityUtils.java:339: * @see
android.app.Notification.Builder#setLocalOnly(boolean)
I don't think this comment is correct.

[Yusuf, 2014-09-23 21:39:01]

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

https://codereview.chromium.org/575253003/diff/1/base/android/java/src/org/ch...
base/android/java/src/org/chromium/base/ApiCompatibilityUtils.java:271: return
null;
Yeah I was thinking this way we can at least check the return value to guess
what happened.


On 2014/09/18 00:06:21, nyquist wrote:
> I am assuming passing in null as the bundle to startActivity works?
> Also; how about returning an empty bundle instead here?
> I guess that would make it hard to know what to do if you care about the
result
> of toBundle() (but at which point you might want to do more fancy stuff with
the
> API yourself)

https://codereview.chromium.org/575253003/diff/1/base/android/java/src/org/ch...
base/android/java/src/org/chromium/base/ApiCompatibilityUtils.java:278: public
static Notification.Builder setLocalOnly(
Will add the flags version too.

On 2014/09/18 00:06:21, nyquist wrote:
> Do we need support for flags as well?
> 
> Something like:
> 
> public static int setLocalOnly(int flags) {
>   if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT_WATCH) {
>     return flags | Notification.FLAG_LOCAL_ONLY;
>   } else {
>     return flags;
>   }
> }
> 
> Or do you plan on changing usages of the flag to always use the builder?

https://codereview.chromium.org/575253003/diff/1/base/android/java/src/org/ch...
base/android/java/src/org/chromium/base/ApiCompatibilityUtils.java:339: * @see
android.app.Notification.Builder#setLocalOnly(boolean)
Umm. Seems OK to me. What was the error here?

On 2014/09/18 00:06:20, nyquist wrote:
> I don't think this comment is correct.

[nyquist, 2014-09-24 00:24:40]

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

https://codereview.chromium.org/575253003/diff/20001/base/android/java/src/or...
base/android/java/src/org/chromium/base/ApiCompatibilityUtils.java:381: public
static boolean datePickerRequiresAccept() {
intentional?

[nyquist, 2014-09-24 00:27:19]

I think this might not be OK for android_aosp yet. kicked off a tryjob to see.

https://codereview.chromium.org/575253003/diff/40001/base/android/java/src/or...
File base/android/java/src/org/chromium/base/ApiCompatibilityUtils.java (right):

https://codereview.chromium.org/575253003/diff/40001/base/android/java/src/or...
base/android/java/src/org/chromium/base/ApiCompatibilityUtils.java:312: if
(Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT_WATCH) {
Is this version code available on android_aosp yet?

[Yusuf, 2014-09-24 17:12:59]

https://codereview.chromium.org/575253003/diff/40001/base/android/java/src/or...
File base/android/java/src/org/chromium/base/ApiCompatibilityUtils.java (right):

https://codereview.chromium.org/575253003/diff/40001/base/android/java/src/or...
base/android/java/src/org/chromium/base/ApiCompatibilityUtils.java:312: if
(Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT_WATCH) {
On 2014/09/24 00:27:18, nyquist wrote:
> Is this version code available on android_aosp yet?

Done. Removed for now

[nyquist, 2014-09-24 18:11:10]

lgtm

[Yusuf, 2014-09-24 18:29:27]

The CQ bit was checked by yusufo@chromium.org

[commit-bot: I haz the power, 2014-09-24 18:29:44]

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

[commit-bot: I haz the power, 2014-09-24 19:32:17]

Committed patchset #4 (id:60001) as b540dd7c6c837ac02cc3760cefd2c4d0284a2577

[commit-bot: I haz the power, 2014-09-24 19:33:03]

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