(Android) Removing ErrorProne warnings

(Android) Removing ErrorProne warnings

BUG=685605
Review-Url: https://codereview.chromium.org/2650403004
Cr-Commit-Position: refs/heads/master@{#447352}
Committed: https://chromium.googlesource.com/chromium/src/+/f4fb13aa6886c4a1f68d638599c40ec113b47eee

[marcin, 2017-01-26 12:34:21]

The CQ bit was checked by marcin@mwiacek.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-26 12:34:33]

Dry run: 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-26 12:34:33]

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

[commit-bot: I haz the power, 2017-01-26 12:34:34]

Dry run: No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[marcin, 2017-01-26 12:41:52]

Description was changed from

==========
Warnings

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

to

==========
Removing ErrorProne warnings

BUG=685605
==========

[marcin, 2017-01-26 22:34:43]

marcin@mwiacek.com changed reviewers:
+ qinmin@chromium.org, yfriedman@chromium.org, yusufo@chromium.org

[marcin, 2017-01-26 22:34:44]

Hi,

Please review, after first LGTM I will be able to make dry run and maybe add
code for more.

[marcin, 2017-01-27 22:40:28]

Description was changed from

==========
Removing ErrorProne warnings

BUG=685605
==========

to

==========
(Android) Removing ErrorProne warnings

BUG=685605
==========

[marcin, 2017-01-31 00:11:01]

Patchset #2 (id:20001) has been deleted

[marcin, 2017-01-31 00:11:41]

marcin@mwiacek.com changed reviewers:
+ mariakhomenko@chromium.org
- qinmin@chromium.org, yfriedman@chromium.org, yusufo@chromium.org

[Maria, 2017-01-31 00:29:18]

https://codereview.chromium.org/2650403004/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/document/ChromeLauncherActivity.java
(right):

https://codereview.chromium.org/2650403004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/document/ChromeLauncherActivity.java:215:
if (getIntent() != null && getIntent().getAction().equals(Intent.ACTION_VIEW)
Please make this Intent.ACTION_VIEW.equals(getIntent().getAction()) since action
is not guaranteed to be non-null

https://codereview.chromium.org/2650403004/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java
(left):

https://codereview.chromium.org/2650403004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java:675:
switch (intent.getAction()) {
It was my understanding that switching on strings is supported in later versions
of Java -- I also don't see any complaints about this switch in your bug

https://codereview.chromium.org/2650403004/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java
(right):

https://codereview.chromium.org/2650403004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java:187:
if (intent.getAction().equals(ACTION_DOWNLOAD_RESUME_ALL)) {
As in the other file, please reverse comparisons so that it's in the form
CONSTANT.equals(value). Here and below.

[Maria, 2017-01-31 00:29:32]

The CQ bit was checked by mariakhomenko@chromium.org to run a CQ dry run

[Maria, 2017-01-31 00:29:47]

On 2017/01/31 00:29:18, Maria wrote:
>
https://codereview.chromium.org/2650403004/diff/1/chrome/android/java/src/org...
> File
>
chrome/android/java/src/org/chromium/chrome/browser/document/ChromeLauncherActivity.java
> (right):
> 
>
https://codereview.chromium.org/2650403004/diff/1/chrome/android/java/src/org...
>
chrome/android/java/src/org/chromium/chrome/browser/document/ChromeLauncherActivity.java:215:
> if (getIntent() != null && getIntent().getAction().equals(Intent.ACTION_VIEW)
> Please make this Intent.ACTION_VIEW.equals(getIntent().getAction()) since
action
> is not guaranteed to be non-null
> 
>
https://codereview.chromium.org/2650403004/diff/1/chrome/android/java/src/org...
> File
>
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java
> (left):
> 
>
https://codereview.chromium.org/2650403004/diff/1/chrome/android/java/src/org...
>
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java:675:
> switch (intent.getAction()) {
> It was my understanding that switching on strings is supported in later
versions
> of Java -- I also don't see any complaints about this switch in your bug
> 
>
https://codereview.chromium.org/2650403004/diff/1/chrome/android/java/src/org...
> File
>
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java
> (right):
> 
>
https://codereview.chromium.org/2650403004/diff/1/chrome/android/java/src/org...
>
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java:187:
> if (intent.getAction().equals(ACTION_DOWNLOAD_RESUME_ALL)) {
> As in the other file, please reverse comparisons so that it's in the form
> CONSTANT.equals(value). Here and below.

FYI, I sent your CL through CQ dry run

[commit-bot: I haz the power, 2017-01-31 00:30:16]

Dry run: 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-31 01:03:15]

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

[commit-bot: I haz the power, 2017-01-31 01:03:15]

Dry run: This issue passed the CQ dry run.

[marcin, 2017-01-31 01:06:17]

The CQ bit was checked by marcin@mwiacek.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-31 01:06:56]

Dry run: 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-31 01:06:57]

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

[commit-bot: I haz the power, 2017-01-31 01:06:58]

Dry run: No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[marcin, 2017-01-31 01:09:15]

The CQ bit was checked by marcin@mwiacek.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-31 01:10:00]

Dry run: 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-31 01:10:01]

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

[commit-bot: I haz the power, 2017-01-31 01:10:01]

Dry run: No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[marcin, 2017-01-31 01:17:03]

https://codereview.chromium.org/2650403004/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/document/ChromeLauncherActivity.java
(right):

https://codereview.chromium.org/2650403004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/document/ChromeLauncherActivity.java:215:
if (getIntent() != null && getIntent().getAction().equals(Intent.ACTION_VIEW)
On 2017/01/31 00:29:17, Maria wrote:
> Please make this Intent.ACTION_VIEW.equals(getIntent().getAction()) since
action
> is not guaranteed to be non-null

Done.

https://codereview.chromium.org/2650403004/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java
(left):

https://codereview.chromium.org/2650403004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java:675:
switch (intent.getAction()) {
On 2017/01/31 00:29:17, Maria wrote:
> It was my understanding that switching on strings is supported in later
versions
> of Java -- I also don't see any complaints about this switch in your bug

ifs are shorter than switch .... break; .... break; .... break;

https://codereview.chromium.org/2650403004/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java
(right):

https://codereview.chromium.org/2650403004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java:187:
if (intent.getAction().equals(ACTION_DOWNLOAD_RESUME_ALL)) {
On 2017/01/31 00:29:17, Maria wrote:
> As in the other file, please reverse comparisons so that it's in the form
> CONSTANT.equals(value). Here and below.

Done.

[marcin, 2017-01-31 02:19:27]

Patchset #2 (id:40001) has been deleted

[marcin, 2017-01-31 02:45:18]

Patchset #2 (id:60001) has been deleted

[marcin, 2017-01-31 02:46:17]

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

[Torne, 2017-01-31 11:54:20]

android_webview LGTM, thanks

[qinmin, 2017-01-31 18:53:36]

qinmin@chromium.org changed reviewers:
+ qinmin@chromium.org

[qinmin, 2017-01-31 18:53:37]

DownloadNotificationService lgtm

[marcin, 2017-01-31 21:21:06]

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

[Maria, 2017-01-31 21:27:23]

lgtm

[marcin, 2017-01-31 21:29:12]

The CQ bit was checked by marcin@mwiacek.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-31 21:29:55]

Dry run: 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-31 22:25:01]

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

[commit-bot: I haz the power, 2017-01-31 22:25:02]

Dry run: This issue passed the CQ dry run.

[marcin, 2017-01-31 22:49:28]

The CQ bit was checked by marcin@mwiacek.com

[commit-bot: I haz the power, 2017-01-31 22:50:10]

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-31 23:04:31]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1485902968762960,
"parent_rev": "a09bd8e908a66bedfd344063695c7fd8ed10b6ec", "commit_rev":
"f4fb13aa6886c4a1f68d638599c40ec113b47eee"}

[commit-bot: I haz the power, 2017-01-31 23:09:15]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1485902968762960,
"parent_rev": "a09bd8e908a66bedfd344063695c7fd8ed10b6ec", "commit_rev":
"f4fb13aa6886c4a1f68d638599c40ec113b47eee"}

[commit-bot: I haz the power, 2017-01-31 23:09:22]

Description was changed from

==========
(Android) Removing ErrorProne warnings

BUG=685605
==========

to

==========
(Android) Removing ErrorProne warnings

BUG=685605

Review-Url: https://codereview.chromium.org/2650403004
Cr-Commit-Position: refs/heads/master@{#447352}
Committed:
https://chromium.googlesource.com/chromium/src/+/f4fb13aa6886c4a1f68d638599c4...
==========

[commit-bot: I haz the power, 2017-01-31 23:09:24]

Committed patchset #2 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/f4fb13aa6886c4a1f68d638599c4...