(Android) Replacing deprecated API with current

(Android) Replacing deprecated API with current 

BUG=681421
Review-Url: https://codereview.chromium.org/2632103002
Cr-Commit-Position: refs/heads/master@{#444426}
Committed: https://chromium.googlesource.com/chromium/src/+/834246eadace04e43bc7f49d2f9deca590364a54

[marcin, 2017-01-15 22:52:37]

Description was changed from

==========
Formatting for deprecated changes


Remove depreceated elements

BUG=
==========

to

==========
Replacing deprecated API with current 

BUG=681421
==========

[marcin, 2017-01-15 22:55:10]

Description was changed from

==========
Replacing deprecated API with current 

BUG=681421
==========

to

==========
(Android) Replacing deprecated API with current 

BUG=681421
==========

[marcin, 2017-01-15 22:58:31]

marcin@mwiacek.com changed reviewers:
+ avayvod@chromium.org, bauerb@chromium.org, jaekyun@chromium.org,
mdjones@chromium.org, mvanouwerkerk@chromium.org, yusufo@chromium.org

[Michael van Ouwerkerk, 2017-01-16 11:03:32]

https://codereview.chromium.org/2632103002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java
(right):

https://codereview.chromium.org/2632103002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java:1032:
private static int getResourcesColor(Resources resources, int id) {
Please use ApiCompatibilityUtils#getColor instead.

https://codereview.chromium.org/2632103002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/notifications/CustomNotificationBuilder.java
(right):

https://codereview.chromium.org/2632103002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/notifications/CustomNotificationBuilder.java:87:
private void setContentViewForNotificationBuilder(Notification.Builder builder,
nit: this can be static

https://codereview.chromium.org/2632103002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/notifications/CustomNotificationBuilder.java:97:
private Notification notificationWithBigContentView(Notification.Builder
builder,
nit: this can be static

[Bernhard Bauer, 2017-01-16 11:54:21]

https://codereview.chromium.org/2632103002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java
(right):

https://codereview.chromium.org/2632103002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java:400:
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.LOLLIPOP) {
Should we move this also into a method in ApiCompatUtils?

https://codereview.chromium.org/2632103002/diff/1/ui/android/java/src/org/chr...
File ui/android/java/src/org/chromium/ui/UiUtils.java (right):

https://codereview.chromium.org/2632103002/diff/1/ui/android/java/src/org/chr...
ui/android/java/src/org/chromium/ui/UiUtils.java:153: private static String
getLocale(InputMethodSubtype inputMethodSubType) {
And this one already is a method, so we could just move it to ApiCompatUtils.

[marcin, 2017-01-16 20:07:02]

marcin@mwiacek.com changed reviewers:
+ rmcilroy@chromium.org

[marcin, 2017-01-16 20:08:01]

[...]

Thank you for review for patch set 1

I have done necessary changes and we have patch set 2, please review

[whywhat, 2017-01-17 03:58:25]

media/router/ lgtm with nits

https://codereview.chromium.org/2632103002/diff/2/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java
(right):

https://codereview.chromium.org/2632103002/diff/2/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:239:
// TODO(avayvod): hide this behind the flag or remove
nit: this and the lines below should be indented by 4 more spaces
You can also just initialize the builder with the ctor and call the method below
on apiOptionsBuilder in a separate statement.

https://codereview.chromium.org/2632103002/diff/2/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:254:
(new LaunchOptions.Builder())
nit: I'd prefer this to be extracted into a local variable above the return
statement

[Yusuf, 2017-01-17 07:48:22]

CustomTabActivity lgtm

[Michael van Ouwerkerk, 2017-01-17 11:44:24]

/notifications lgtm

[mdjones, 2017-01-17 16:40:10]

CompositorViewHolder lgtm

[Bernhard Bauer, 2017-01-17 17:01:29]

ChromeTabbedActivity and ShareHelper LGTM

[marcin, 2017-01-17 18:37:39]

marcin@mwiacek.com changed reviewers:
+ agrieve@chromium.org, nyquist@chromium.org, tedchoc@chromium.org,
torne@chromium.org, twellington@chromium.org, yfriedman@chromium.org

[marcin, 2017-01-17 20:25:40]

On 2017/01/17 03:58:25, whywhat wrote:
> media/router/ lgtm with nits
> 
>
https://codereview.chromium.org/2632103002/diff/2/chrome/android/java/src/org...
> File
>
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java
> (right):
> 
>
https://codereview.chromium.org/2632103002/diff/2/chrome/android/java/src/org...
>
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:239:
> // TODO(avayvod): hide this behind the flag or remove
> nit: this and the lines below should be indented by 4 more spaces
> You can also just initialize the builder with the ctor and call the method
below
> on apiOptionsBuilder in a separate statement.
> 
>
https://codereview.chromium.org/2632103002/diff/2/chrome/android/java/src/org...
>
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:254:
> (new LaunchOptions.Builder())
> nit: I'd prefer this to be extracted into a local variable above the return
> statement

I have made changed in patch set 3, please review

[nyquist, 2017-01-17 22:44:58]

marcin, which part do you want me to review?

[Theresa, 2017-01-17 22:46:12]

ui/android lgtm

[marcin, 2017-01-17 22:49:38]

On 2017/01/17 22:44:58, nyquist wrote:
> marcin, which part do you want me to review?

If I see correctly I need review for 

base/android/java/src/org/chromium/base/ApiCompatibilityUtils.java 

and you're in base/android (I don't see anything deeper)

I'm sorry if I made mistake

[Ted C, 2017-01-17 22:51:29]

indenting needs to be tweaked a bit before submitting

https://codereview.chromium.org/2632103002/diff/30001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java
(right):

https://codereview.chromium.org/2632103002/diff/30001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java:1041:
context.getResources(),
indenting of this looks off, should be 8 (appears to be 6) from the previous
line.

https://codereview.chromium.org/2632103002/diff/30001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java
(right):

https://codereview.chromium.org/2632103002/diff/30001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:255:
builder.setRelaunchIfRunning(relaunchIfRunning)
+4 indent

https://codereview.chromium.org/2632103002/diff/30001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/media/router/cast/CreateRouteRequest.java:256:
.build());
+4 indent here too

https://codereview.chromium.org/2632103002/diff/30001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/share/ShareHelper.java
(right):

https://codereview.chromium.org/2632103002/diff/30001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/share/ShareHelper.java:363:
saveFile);
should be 8 from the start of the previous line

https://codereview.chromium.org/2632103002/diff/30001/ui/android/java/src/org...
File ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java (right):

https://codereview.chromium.org/2632103002/diff/30001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java:224:
getFileForImageCapture(context));
indenting nit here too

[Ted C, 2017-01-17 22:54:13]

On 2017/01/17 22:49:38, marcin wrote:
> On 2017/01/17 22:44:58, nyquist wrote:
> > marcin, which part do you want me to review?
> 
> If I see correctly I need review for 
> 
> base/android/java/src/org/chromium/base/ApiCompatibilityUtils.java 
> 
> and you're in base/android (I don't see anything deeper)
> 
> I'm sorry if I made mistake

To nyquist's point, if you include multiple reviewers, you want do
something like this:

nyquist - base/*
yusufo - CustomTabActivity.java
twellington - ui/*
etc...

It makes it clearer the scope each reviewer needs to look at
and will get you answers faster (or at least hopefully should).

[marcin, 2017-01-17 23:06:44]

On 2017/01/17 22:51:29, Ted C wrote:
> indenting needs to be tweaked a bit before submitting
[...]

done, please review

+ I need review for base/* from somebody (nyquist ?)

[Ted C, 2017-01-18 01:00:21]

lgtm w/ final nit

https://codereview.chromium.org/2632103002/diff/50001/ui/android/java/src/org...
File ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java (right):

https://codereview.chromium.org/2632103002/diff/50001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java:224:
getFileForImageCapture(context));
+4 indent

[Torne, 2017-01-18 14:03:04]

base LGTM, but please in future don't add every single owner for a directory to
get a review - just pick one person, and if they don't respond after a
reasonable time try someone else ;)

[marcin, 2017-01-18 17:57:40]

The CQ bit was checked by marcin@mwiacek.com

[marcin, 2017-01-18 17:57:40]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org,
yusufo@chromium.org, mvanouwerkerk@chromium.org, mdjones@chromium.org,
avayvod@chromium.org, twellington@chromium.org, torne@chromium.org,
tedchoc@chromium.org
Link to the patchset: https://codereview.chromium.org/2632103002/#ps70001
(title: "Indentation fix after previous patch set")

[commit-bot: I haz the power, 2017-01-18 17:58:03]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-18 18:43:07]

CQ is committing da patch.

Bot data: {"patchset_id": 70001, "attempt_start_ts": 1484762260013630,
"parent_rev": "7b6995d78f7b2ee86376158ed18a4c1c64483e9c", "commit_rev":
"834246eadace04e43bc7f49d2f9deca590364a54"}

[commit-bot: I haz the power, 2017-01-18 18:43:36]

Description was changed from

==========
(Android) Replacing deprecated API with current 

BUG=681421
==========

to

==========
(Android) Replacing deprecated API with current 

BUG=681421

Review-Url: https://codereview.chromium.org/2632103002
Cr-Commit-Position: refs/heads/master@{#444426}
Committed:
https://chromium.googlesource.com/chromium/src/+/834246eadace04e43bc7f49d2f9d...
==========

[commit-bot: I haz the power, 2017-01-18 18:43:38]

Committed patchset #5 (id:70001) as
https://chromium.googlesource.com/chromium/src/+/834246eadace04e43bc7f49d2f9d...