Update to newer Android Lint and suppress new Lint errors

Update to newer Android Lint and suppress new Lint errors

This CL updates DEPS to enable updated Android Lint that would work for
Java 8. Also, it adds inline suppression for newly discovered Lint
errors. Additionally, it suppresses Lint deprecation warning for
"android:singleLine". Moreover, it suppresses Lint warnings "AuthLeak",
"VisibleForTests", etc. for tests, and Lint error "ByteOrderMark" in
generated android_chrome_strings.xml (crbug.com/715196).

BUG=635567, 669999
Review-Url: https://codereview.chromium.org/2830843004
Cr-Commit-Position: refs/heads/master@{#468545}
Committed: https://chromium.googlesource.com/chromium/src/+/5512e732f9513b2e5b3f8c47f146dfa5c3582b6d

[F, 2017-04-25 17:49:32]

Patchset #1 (id:1) has been deleted

[F, 2017-04-25 17:50:03]

Patchset #1 (id:1) has been deleted

[F, 2017-04-25 17:50:15]

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

[commit-bot: I haz the power, 2017-04-25 17:50:42]

Dry run: CQ is trying da patch.

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

[F, 2017-04-25 17:55:00]

Description was changed from

==========
Update Lint and suppress errors


lint WIP

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

to

==========
Update DEPS to newer Android Lint and suppress new Lint errors

This CL updates DEPS to enable updated Android Lint that would work for
Java 8. Also, it adds inline suppression for newly discovered Lint
errors. Additionally, it fixes the Lint deprecation warnings on
android:singleLine by replacing it with android:maxLines.

BUG=635567,669999
==========

[commit-bot: I haz the power, 2017-04-25 17:55:17]

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

[commit-bot: I haz the power, 2017-04-25 17:55:18]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[F, 2017-04-25 18:08:50]

Description was changed from

==========
Update DEPS to newer Android Lint and suppress new Lint errors

This CL updates DEPS to enable updated Android Lint that would work for
Java 8. Also, it adds inline suppression for newly discovered Lint
errors. Additionally, it fixes the Lint deprecation warnings on
android:singleLine by replacing it with android:maxLines.

BUG=635567,669999
==========

to

==========
Update DEPS to newer Android Lint and suppress new Lint errors

This CL updates DEPS to enable updated Android Lint that would work for
Java 8. Also, it adds inline suppression for newly discovered Lint
errors. Additionally, it fixes the Lint deprecation warnings on
android:singleLine by replacing it with android:maxLines. Moreover, it
suppresses Lint warnings "AuthLeak" and "VisibleForTests" for tests,
and "ByteOrderMark" in generated android_chrome_strings.xml.

BUG=635567,669999
==========

[F, 2017-04-25 18:48:36]

Description was changed from

==========
Update DEPS to newer Android Lint and suppress new Lint errors

This CL updates DEPS to enable updated Android Lint that would work for
Java 8. Also, it adds inline suppression for newly discovered Lint
errors. Additionally, it fixes the Lint deprecation warnings on
android:singleLine by replacing it with android:maxLines. Moreover, it
suppresses Lint warnings "AuthLeak" and "VisibleForTests" for tests,
and "ByteOrderMark" in generated android_chrome_strings.xml.

BUG=635567,669999
==========

to

==========
Update DEPS to newer Android Lint and suppress new Lint errors

This CL updates DEPS to enable updated Android Lint that would work for
Java 8. Also, it adds inline suppression for newly discovered Lint
errors. Additionally, it fixes the Lint deprecation warnings on
android:singleLine by replacing it with android:maxLines. Moreover, it
suppresses Lint warnings "AuthLeak" and "VisibleForTests" for tests,
and "ByteOrderMark" in generated android_chrome_strings.xml
(crbug.com/715196). 

BUG=635567,669999
==========

[F, 2017-04-25 18:48:55]

Description was changed from

==========
Update DEPS to newer Android Lint and suppress new Lint errors

This CL updates DEPS to enable updated Android Lint that would work for
Java 8. Also, it adds inline suppression for newly discovered Lint
errors. Additionally, it fixes the Lint deprecation warnings on
android:singleLine by replacing it with android:maxLines. Moreover, it
suppresses Lint warnings "AuthLeak" and "VisibleForTests" for tests,
and "ByteOrderMark" in generated android_chrome_strings.xml
(crbug.com/715196). 

BUG=635567,669999
==========

to

==========
Update DEPS to newer Android Lint and suppress new Lint errors

This CL updates DEPS to enable updated Android Lint that would work for
Java 8. Also, it adds inline suppression for newly discovered Lint
errors. Additionally, it fixes the Lint deprecation warnings on
android:singleLine by replacing it with android:maxLines. Moreover, it
suppresses Lint warnings "AuthLeak" and "VisibleForTests" for tests,
and Lint error "ByteOrderMark" in generated android_chrome_strings.xml
(crbug.com/715196). 

BUG=635567,669999
==========

[F, 2017-04-25 18:52:24]

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

[commit-bot: I haz the power, 2017-04-25 18:53:18]

Dry run: CQ is trying da patch.

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

[F, 2017-04-25 18:55:50]

zpeng@chromium.org changed reviewers:
+ agrieve@chromium.org

[F, 2017-04-25 18:55:52]

Hi Andrew, PTAL. Thanks!

[commit-bot: I haz the power, 2017-04-25 19:12:35]

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

[commit-bot: I haz the power, 2017-04-25 19:12:36]

Dry run: Try jobs failed on following builders:
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)

[agrieve, 2017-04-25 19:42:40]

agrieve@chromium.org changed reviewers:
+ tedchoc@chromium.org

[agrieve, 2017-04-25 19:42:41]

lgtm, only questionable change is the maxlines one.

SO says it is subtly different:
http://stackoverflow.com/questions/17124985/why-does-androidsingleline-true-m...

Maybe Ted would know better?

[F, 2017-04-25 19:52:28]

On 2017/04/25 19:42:41, agrieve wrote:
> lgtm, only questionable change is the maxlines one.
> 
> SO says it is subtly different:
>
http://stackoverflow.com/questions/17124985/why-does-androidsingleline-true-m...
> 
> Maybe Ted would know better?

I see. Thanks for the SO link :)
I replaced android:singleLine with android:maxLines because in the lint warning
message, there is a recommendation to replace with android:maxLines.

[F, 2017-04-25 19:52:41]

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

[commit-bot: I haz the power, 2017-04-25 19:53:22]

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-04-25 20:31:16]

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

[commit-bot: I haz the power, 2017-04-25 20:31:17]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[Ted C, 2017-04-25 20:59:15]

tedchoc@chromium.org changed reviewers:
+ dfalcantara@chromium.org

[Ted C, 2017-04-25 20:59:16]

I don't think singleLine and maxLines are equivalent.  If I recall, one makes
ellipsizing work and the other doesn't.

Without testing that on all android versions, I would not feel comfortable
making that change.

+dfalcantara to add his thoughts

[gone, 2017-04-25 21:03:24]

On 2017/04/25 20:59:16, Ted C wrote:
> I don't think singleLine and maxLines are equivalent.  If I recall, one makes
> ellipsizing work and the other doesn't.
> 
> Without testing that on all android versions, I would not feel comfortable
> making that change.
> 
> +dfalcantara to add his thoughts

They're not equivalent.  We've hit bugs with this before, with the most recent
thread being here:

https://groups.google.com/a/google.com/forum/#!searchin/clank-frontend/maxlin...

[F, 2017-04-25 21:39:17]

On 2017/04/25 21:03:24, slow (dfalcantara) wrote:
> On 2017/04/25 20:59:16, Ted C wrote:
> > I don't think singleLine and maxLines are equivalent.  If I recall, one
makes
> > ellipsizing work and the other doesn't.
> > 
> > Without testing that on all android versions, I would not feel comfortable
> > making that change.
> > 
> > +dfalcantara to add his thoughts
> 
> They're not equivalent.  We've hit bugs with this before, with the most recent
> thread being here:
> 
>
https://groups.google.com/a/google.com/forum/#!searchin/clank-frontend/maxlin...

Thanks for the info! I'll revert the changes and add new suppression instead

[Ted C, 2017-04-26 18:09:18]

On 2017/04/25 21:39:17, F wrote:
> On 2017/04/25 21:03:24, slow (dfalcantara) wrote:
> > On 2017/04/25 20:59:16, Ted C wrote:
> > > I don't think singleLine and maxLines are equivalent.  If I recall, one
> makes
> > > ellipsizing work and the other doesn't.
> > > 
> > > Without testing that on all android versions, I would not feel comfortable
> > > making that change.
> > > 
> > > +dfalcantara to add his thoughts
> > 
> > They're not equivalent.  We've hit bugs with this before, with the most
recent
> > thread being here:
> > 
> >
>
https://groups.google.com/a/google.com/forum/#!searchin/clank-frontend/maxlin...
> 
> Thanks for the info! I'll revert the changes and add new suppression instead

Cool...just ping this when it is ready to be reviewed again.

[F, 2017-04-26 20:19:44]

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

[commit-bot: I haz the power, 2017-04-26 20:20:28]

Dry run: CQ is trying da patch.

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

[F, 2017-04-26 20:21:07]

The CQ bit was unchecked by zpeng@chromium.org

[F, 2017-04-26 20:25:51]

Description was changed from

==========
Update DEPS to newer Android Lint and suppress new Lint errors

This CL updates DEPS to enable updated Android Lint that would work for
Java 8. Also, it adds inline suppression for newly discovered Lint
errors. Additionally, it fixes the Lint deprecation warnings on
android:singleLine by replacing it with android:maxLines. Moreover, it
suppresses Lint warnings "AuthLeak" and "VisibleForTests" for tests,
and Lint error "ByteOrderMark" in generated android_chrome_strings.xml
(crbug.com/715196). 

BUG=635567,669999
==========

to

==========
Update DEPS to newer Android Lint and suppress new Lint errors

This CL updates DEPS to enable updated Android Lint that would work for
Java 8. Also, it adds inline suppression for newly discovered Lint
errors. Additionally, it suppresses Lint deprecation warning for
"android:singleLine". Moreover, it
suppresses Lint warnings "AuthLeak" and "VisibleForTests" for tests,
and Lint error "ByteOrderMark" in generated android_chrome_strings.xml
(crbug.com/715196). 

BUG=635567,669999
==========

[F, 2017-04-26 20:26:10]

Description was changed from

==========
Update DEPS to newer Android Lint and suppress new Lint errors

This CL updates DEPS to enable updated Android Lint that would work for
Java 8. Also, it adds inline suppression for newly discovered Lint
errors. Additionally, it suppresses Lint deprecation warning for
"android:singleLine". Moreover, it
suppresses Lint warnings "AuthLeak" and "VisibleForTests" for tests,
and Lint error "ByteOrderMark" in generated android_chrome_strings.xml
(crbug.com/715196). 

BUG=635567,669999
==========

to

==========
Update DEPS to newer Android Lint and suppress new Lint errors

This CL updates DEPS to enable updated Android Lint that would work for
Java 8. Also, it adds inline suppression for newly discovered Lint
errors. Additionally, it suppresses Lint deprecation warning for
"android:singleLine". Moreover, it suppresses Lint warnings "AuthLeak"
and "VisibleForTests" for tests, and Lint error "ByteOrderMark" in
generated android_chrome_strings.xml (crbug.com/715196). 

BUG=635567,669999
==========

[F, 2017-04-26 21:33:28]

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

[commit-bot: I haz the power, 2017-04-26 21:34:10]

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-04-26 21:54:29]

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

[commit-bot: I haz the power, 2017-04-26 21:54:30]

Dry run: Try jobs failed on following builders:
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)

[F, 2017-04-26 22:20:33]

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

[commit-bot: I haz the power, 2017-04-26 22:21:23]

Dry run: CQ is trying da patch.

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

[F, 2017-04-26 22:34:14]

Description was changed from

==========
Update DEPS to newer Android Lint and suppress new Lint errors

This CL updates DEPS to enable updated Android Lint that would work for
Java 8. Also, it adds inline suppression for newly discovered Lint
errors. Additionally, it suppresses Lint deprecation warning for
"android:singleLine". Moreover, it suppresses Lint warnings "AuthLeak"
and "VisibleForTests" for tests, and Lint error "ByteOrderMark" in
generated android_chrome_strings.xml (crbug.com/715196). 

BUG=635567,669999
==========

to

==========
Update to newer Android Lint and suppress new Lint errors

This CL updates DEPS to enable updated Android Lint that would work for
Java 8. Also, it adds inline suppression for newly discovered Lint
errors. Additionally, it suppresses Lint deprecation warning for
"android:singleLine". Moreover, it suppresses Lint warnings "AuthLeak"
and "VisibleForTests" for tests, and Lint error "ByteOrderMark" in
generated android_chrome_strings.xml (crbug.com/715196).

BUG=635567,669999
==========

[F, 2017-04-26 22:35:13]

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

[commit-bot: I haz the power, 2017-04-26 22:36: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-04-26 23:10:33]

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

[commit-bot: I haz the power, 2017-04-26 23:10:34]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[F, 2017-04-26 23:24:48]

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

[commit-bot: I haz the power, 2017-04-26 23:26:14]

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-04-26 23:53:36]

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

[commit-bot: I haz the power, 2017-04-26 23:53:37]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[F, 2017-04-27 14:08:22]

Thanks Andrew, Dan, and Ted! PTAL

I've reverted the android:singleLine changes. Since DEPS is already ahead, no
longer updating DEPS

[F, 2017-04-27 14:08:59]

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

[commit-bot: I haz the power, 2017-04-27 14:09:18]

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-04-27 14:32:44]

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

[commit-bot: I haz the power, 2017-04-27 14:32:45]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[F, 2017-04-27 15:41:27]

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

[commit-bot: I haz the power, 2017-04-27 15:42:05]

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-04-27 17:25:00]

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

[commit-bot: I haz the power, 2017-04-27 17:25:01]

Dry run: This issue passed the CQ dry run.

[Ted C, 2017-04-27 19:02:48]

I'm pretty sure all the static lint todos should be removed.

https://codereview.chromium.org/2830843004/diff/220001/base/android/java/src/...
File base/android/java/src/org/chromium/base/ApplicationStatus.java (right):

https://codereview.chromium.org/2830843004/diff/220001/base/android/java/src/...
base/android/java/src/org/chromium/base/ApplicationStatus.java:64: //
TODO(crbug.com/635567): Fix this properly.
no fix needed, this is cleared when applicable already.

https://codereview.chromium.org/2830843004/diff/220001/base/android/java/src/...
base/android/java/src/org/chromium/base/ApplicationStatus.java:362: //
TODO(crbug.com/635567): Fix this properly.
fix what?  activity.isDestroyed() is available in O and guarded by that check,
so I don't think anything needs to be fixed here.

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/ChromeActivitySessionTracker.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/ChromeActivitySessionTracker.java:47:
// TODO(crbug.com/635567): Fix this properly.
Again, this is a leaky singleton intentionally

So no fix should be needed here

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/DeferredStartupHandler.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/DeferredStartupHandler.java:73:
// TODO(crbug.com/635567): Fix this properly.
same comment, this is correct and no fix is needed...I have a feeling that I'm
going to say this on all of these.

For all the statics, let's remove these todos because I think there is no action
that would address it IMO.

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
File chrome/android/java/src/org/chromium/chrome/browser/appmenu/AppMenu.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/appmenu/AppMenu.java:154: //
TODO(crbug.com/635567): Fix this properly.
What needs to be fixed?  Can we put the todo above the line(s) that lint is
complaining about?

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java:639:
// TODO(crbug.com/635567): Fix this properly.
move to the line that is wrong.

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/crash/MinidumpUploadRetry.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/crash/MinidumpUploadRetry.java:24:
// TODO(crbug.com/635567): Fix this properly.
remove

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/identity/SettingsSecureBasedIdentificationGenerator.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/identity/SettingsSecureBasedIdentificationGenerator.java:43:
// TODO(crbug.com/635567): Fix this properly.
This class was designed around ANDROID_ID.  Again, I don't think anything
will/needs to be done here.  We could certainly have a discussion around this,
but I don't think we should have any TODOs unless we know something is wrong
(otherwise these TODOs will live on forever).

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
File chrome/android/java/src/org/chromium/chrome/browser/nfc/BeamCallback.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/nfc/BeamCallback.java:58: //
TODO(crbug.com/635567): Fix this properly.
Here is a case where we should delete the code that uses this since we only
support JB+...this is ok to leave in

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PackageManagerDelegate.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PackageManagerDelegate.java:33:
// TODO(crbug.com/635567): Fix this properly.
+rouslan

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/ManageSpaceActivity.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/ManageSpaceActivity.java:76:
// TODO(crbug.com/635567): Fix this properly.
Remove, this is doing the right thing comments why below

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProvider.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProvider.java:318:
@SuppressLint({
can this not all fit on a single line?

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/tab/TabStateBrowserControlsVisibilityDelegate.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/tab/TabStateBrowserControlsVisibilityDelegate.java:43:
// TODO(crbug.com/635567): Fix this properly.
No fix needed, mHandler.removeCallbacksAndMessages(null); is called below and
ensures that this doesn't dispatch things later than we expect.

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/widget/accessibility/AccessibilityTabModelListItem.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/widget/accessibility/AccessibilityTabModelListItem.java:407:
// TODO(crbug.com/635567): Fix this properly.
Let's just add @UsedByReflection and remove the todo.

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/javates...
File
chrome/android/javatests/src/org/chromium/chrome/browser/notifications/CustomNotificationBuilderTest.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/notifications/CustomNotificationBuilderTest.java:59:
// TODO(crbug.com/635567): Fix this properly.
this method checks api versions already...is there something that isn't caught
or is lint just be overly chatty here?

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/webapk/...
File
chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/DexOptimizer.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/webapk/...
chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/DexOptimizer.java:174:
// TODO(crbug.com/635567): Fix this properly.
the comment above implies this is likely not an issue

[Ted C, 2017-04-27 20:04:22]

tedchoc@chromium.org changed reviewers:
+ rouslan@chromium.org

[Ted C, 2017-04-27 20:04:23]

+rouslan for realz for PackageManagerDelegate.java

[please use gerrit instead, 2017-04-27 20:07:39]

PackageManagerDelegate LGTM.

Out of curiosity, why is lint complaining about reading signatures?

[Ted C, 2017-04-27 20:18:42]

On 2017/04/27 20:07:39, ಠ_ಠ wrote:
> PackageManagerDelegate LGTM.
> 
> Out of curiosity, why is lint complaining about reading signatures?

Easiest is to look here:
http://tools.android.com/tips/lint-checks

PackageManagerGetSignatures
---------------------------
Summary: Potential Multiple Certificate Exploit

Priority: 8 / 10
Severity: Information
Category: Security

Improper validation of app signatures could lead to issues where a malicious
app submits itself to the Play Store with both its real certificate and a fake
certificate and gains access to functionality or information it shouldn't have
due to another application only checking for the fake certificate and ignoring
the rest. Please make sure to validate all signatures returned by this
method.

More information: 
https://bluebox.com/technical/android-fake-id-vulnerability/

[please use gerrit instead, 2017-04-27 20:30:54]

Thank you for the link. FWIW, I'm aware of this exploit and our design mitigates
it.

[Ted C, 2017-04-27 22:49:51]

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PackageManagerDelegate.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PackageManagerDelegate.java:33:
// TODO(crbug.com/635567): Fix this properly.
On 2017/04/27 19:02:47, Ted C wrote:
> +rouslan

Sounds like we can remove this TODO based on Rouslan's comments

[F, 2017-04-28 15:34:19]

Description was changed from

==========
Update to newer Android Lint and suppress new Lint errors

This CL updates DEPS to enable updated Android Lint that would work for
Java 8. Also, it adds inline suppression for newly discovered Lint
errors. Additionally, it suppresses Lint deprecation warning for
"android:singleLine". Moreover, it suppresses Lint warnings "AuthLeak"
and "VisibleForTests" for tests, and Lint error "ByteOrderMark" in
generated android_chrome_strings.xml (crbug.com/715196).

BUG=635567,669999
==========

to

==========
Update to newer Android Lint and suppress new Lint errors

This CL updates DEPS to enable updated Android Lint that would work for
Java 8. Also, it adds inline suppression for newly discovered Lint
errors. Additionally, it suppresses Lint deprecation warning for
"android:singleLine". Moreover, it suppresses Lint warnings "AuthLeak",
"VisibleForTests", etc. for tests, and Lint error "ByteOrderMark" in
generated android_chrome_strings.xml (crbug.com/715196).

BUG=635567,669999
==========

[F, 2017-04-28 18:17:42]

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

[commit-bot: I haz the power, 2017-04-28 18:18:19]

Dry run: CQ is trying da patch.

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

[F, 2017-04-28 18:25:15]

Thanks Andrew, Rouslan, and Ted! PTAL

https://codereview.chromium.org/2830843004/diff/220001/base/android/java/src/...
File base/android/java/src/org/chromium/base/ApplicationStatus.java (right):

https://codereview.chromium.org/2830843004/diff/220001/base/android/java/src/...
base/android/java/src/org/chromium/base/ApplicationStatus.java:64: //
TODO(crbug.com/635567): Fix this properly.
On 2017/04/27 19:02:47, Ted C wrote:
> no fix needed, this is cleared when applicable already.

Done.

https://codereview.chromium.org/2830843004/diff/220001/base/android/java/src/...
base/android/java/src/org/chromium/base/ApplicationStatus.java:362: //
TODO(crbug.com/635567): Fix this properly.
On 2017/04/27 19:02:47, Ted C wrote:
> fix what?  activity.isDestroyed() is available in O and guarded by that check,
> so I don't think anything needs to be fixed here.

Done.

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/ChromeActivitySessionTracker.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/ChromeActivitySessionTracker.java:47:
// TODO(crbug.com/635567): Fix this properly.
On 2017/04/27 19:02:47, Ted C wrote:
> Again, this is a leaky singleton intentionally
> 
> So no fix should be needed here

Done.

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/DeferredStartupHandler.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/DeferredStartupHandler.java:73:
// TODO(crbug.com/635567): Fix this properly.
On 2017/04/27 19:02:47, Ted C wrote:
> same comment, this is correct and no fix is needed...I have a feeling that I'm
> going to say this on all of these.
> 
> For all the statics, let's remove these todos because I think there is no
action
> that would address it IMO.

Done.

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
File chrome/android/java/src/org/chromium/chrome/browser/appmenu/AppMenu.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/appmenu/AppMenu.java:154: //
TODO(crbug.com/635567): Fix this properly.
On 2017/04/27 19:02:47, Ted C wrote:
> What needs to be fixed?  Can we put the todo above the line(s) that lint is
> complaining about?

Done.

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java:639:
// TODO(crbug.com/635567): Fix this properly.
On 2017/04/27 19:02:47, Ted C wrote:
> move to the line that is wrong.

Done.

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/crash/MinidumpUploadRetry.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/crash/MinidumpUploadRetry.java:24:
// TODO(crbug.com/635567): Fix this properly.
On 2017/04/27 19:02:47, Ted C wrote:
> remove

Done.

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/identity/SettingsSecureBasedIdentificationGenerator.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/identity/SettingsSecureBasedIdentificationGenerator.java:43:
// TODO(crbug.com/635567): Fix this properly.
On 2017/04/27 19:02:47, Ted C wrote:
> This class was designed around ANDROID_ID.  Again, I don't think anything
> will/needs to be done here.  We could certainly have a discussion around this,
> but I don't think we should have any TODOs unless we know something is wrong
> (otherwise these TODOs will live on forever).

Done.

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
File chrome/android/java/src/org/chromium/chrome/browser/nfc/BeamCallback.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/nfc/BeamCallback.java:58: //
TODO(crbug.com/635567): Fix this properly.
On 2017/04/27 19:02:47, Ted C wrote:
> Here is a case where we should delete the code that uses this since we only
> support JB+...this is ok to leave in

Acknowledged.

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PackageManagerDelegate.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PackageManagerDelegate.java:33:
// TODO(crbug.com/635567): Fix this properly.
On 2017/04/27 22:49:51, Ted C wrote:
> On 2017/04/27 19:02:47, Ted C wrote:
> > +rouslan
> 
> Sounds like we can remove this TODO based on Rouslan's comments

Done.

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/ManageSpaceActivity.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/preferences/website/ManageSpaceActivity.java:76:
// TODO(crbug.com/635567): Fix this properly.
On 2017/04/27 19:02:47, Ted C wrote:
> Remove, this is doing the right thing comments why below

Done.

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProvider.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProvider.java:318:
@SuppressLint({
On 2017/04/27 19:02:47, Ted C wrote:
> can this not all fit on a single line?

Done.

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/tab/TabStateBrowserControlsVisibilityDelegate.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/tab/TabStateBrowserControlsVisibilityDelegate.java:43:
// TODO(crbug.com/635567): Fix this properly.
On 2017/04/27 19:02:47, Ted C wrote:
> No fix needed, mHandler.removeCallbacksAndMessages(null); is called below and
> ensures that this doesn't dispatch things later than we expect.

Done.

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/widget/accessibility/AccessibilityTabModelListItem.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/widget/accessibility/AccessibilityTabModelListItem.java:407:
// TODO(crbug.com/635567): Fix this properly.
On 2017/04/27 19:02:47, Ted C wrote:
> Let's just add @UsedByReflection and remove the todo.

Done.

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/javates...
File
chrome/android/javatests/src/org/chromium/chrome/browser/notifications/CustomNotificationBuilderTest.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/notifications/CustomNotificationBuilderTest.java:59:
// TODO(crbug.com/635567): Fix this properly.
On 2017/04/27 19:02:48, Ted C wrote:
> this method checks api versions already...is there something that isn't caught
> or is lint just be overly chatty here?

Done. Overly chatty. Thanks for pointing this out!

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/webapk/...
File
chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/DexOptimizer.java
(right):

https://codereview.chromium.org/2830843004/diff/220001/chrome/android/webapk/...
chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/DexOptimizer.java:174:
// TODO(crbug.com/635567): Fix this properly.
On 2017/04/27 19:02:48, Ted C wrote:
> the comment above implies this is likely not an issue

Done.

[F, 2017-04-28 18:45:22]

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

[commit-bot: I haz the power, 2017-04-28 18:46:47]

Dry run: CQ is trying da patch.

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

[Ted C, 2017-04-28 20:23:51]

lgtm

[F, 2017-04-28 20:49:43]

zpeng@chromium.org changed reviewers:
+ brettw@chromium.org

[F, 2017-04-28 20:49:44]

Hi Brett, PTAL. Thanks!

[commit-bot: I haz the power, 2017-04-28 21:25:52]

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

[commit-bot: I haz the power, 2017-04-28 21:25:53]

Dry run: This issue passed the CQ dry run.

[brettw, 2017-04-28 21:44:56]

Can you make clear what it is you need me to look at? I assume you don't need me
to review the whole thing today. if you don't hear back from me today you'll
probably need to find another reviewer as I won't be able to do much, if any,
code review stuff next week. Sorry!

[gone, 2017-04-30 02:15:56]

download/ui lgtm

[F, 2017-05-01 15:06:00]

zpeng@chromium.org changed reviewers:
- brettw@chromium.org

[F, 2017-05-01 15:53:23]

zpeng@chromium.org changed reviewers:
+ boliu@chromium.org, lambroslambrou@chromium.org, reillyg@chromium.org,
slan@chromium.org, yfriedman@chromium.org, yzshen@chromium.org,
zhongyi@chromium.org

[F, 2017-05-01 15:53:25]

Hi Bo, Cherie, Lambros, Reilly, Stephen, Yaron, and Yuzhu, PTAL. Thanks!

Please take a look at the changes in android_webview/, chromecast/, components/,
device/, mojo/, net/, remoting/, and tools/. New lint suppressions are added and
new TODOs are added above the offending lines, if fixes are recommended. Thanks!

[F, 2017-05-01 15:54:18]

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

[commit-bot: I haz the power, 2017-05-01 15:54:45]

Dry run: CQ is trying da patch.

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

[Reilly Grant (use Gerrit), 2017-05-01 16:06:43]

//device lgtm, I'll take a look at fixing this issues soon.

[boliu, 2017-05-01 16:20:24]

android_webview rs lgtm

[Zhongyi Shi, 2017-05-01 16:55:35]

zhongyi@chromium.org changed reviewers:
+ pauljensen@chromium.org
- zhongyi@chromium.org

[Zhongyi Shi, 2017-05-01 16:58:35]

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

[Zhongyi Shi, 2017-05-01 16:58:37]

The net changes generally look good to me, pulling Paul to take a look as I am
not actively working on android in net.

[Zhongyi Shi, 2017-05-01 16:59:07]

zhongyi@chromium.org changed reviewers:
- zhongyi@chromium.org

[pauljensen, 2017-05-01 17:12:30]

net/ lgtm

[Yaron, 2017-05-01 17:34:59]

lgtm

[commit-bot: I haz the power, 2017-05-01 17:39:30]

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

[commit-bot: I haz the power, 2017-05-01 17:39:32]

Dry run: This issue passed the CQ dry run.

[slan, 2017-05-01 17:49:40]

+tsunghung for FYI

cast lgtm

[Lambros, 2017-05-01 18:06:58]

remoting lgtm

[yzshen1, 2017-05-01 19:56:12]

mojo LGTM

[F, 2017-05-01 20:01:46]

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

[F, 2017-05-01 20:02:27]

The CQ bit was unchecked by zpeng@chromium.org

[commit-bot: I haz the power, 2017-05-01 20:02:53]

Dry run: CQ is trying da patch.

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

[F, 2017-05-01 20:03:03]

The CQ bit was checked by zpeng@chromium.org

[F, 2017-05-01 20:03:04]

The patchset sent to the CQ was uploaded after l-g-t-m from
agrieve@chromium.org, rouslan@chromium.org, slan@chromium.org,
reillyg@chromium.org, boliu@chromium.org, tedchoc@chromium.org,
lambroslambrou@chromium.org, yfriedman@chromium.org, yzshen@chromium.org,
pauljensen@chromium.org, dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/2830843004/#ps320001
(title: "rebase")

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

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-05-01 20:36:41]

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

[commit-bot: I haz the power, 2017-05-01 20:36:45]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[F, 2017-05-01 21:37:30]

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

[F, 2017-05-01 21:38:38]

The CQ bit was unchecked by zpeng@chromium.org

[commit-bot: I haz the power, 2017-05-01 21:38:46]

Dry run: CQ is trying da patch.

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

[F, 2017-05-01 21:38:53]

The CQ bit was checked by zpeng@chromium.org

[F, 2017-05-01 21:38:54]

The patchset sent to the CQ was uploaded after l-g-t-m from slan@chromium.org,
reillyg@chromium.org, rouslan@chromium.org, boliu@chromium.org,
agrieve@chromium.org, tedchoc@chromium.org, lambroslambrou@chromium.org,
yfriedman@chromium.org, yzshen@chromium.org, pauljensen@chromium.org,
dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/2830843004/#ps340001
(title: "rebase")

[commit-bot: I haz the power, 2017-05-01 21:39:35]

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-05-01 23:03:52]

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

[commit-bot: I haz the power, 2017-05-01 23:03:56]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[F, 2017-05-02 01:07:03]

The CQ bit was checked by zpeng@chromium.org

[F, 2017-05-02 01:07:04]

The patchset sent to the CQ was uploaded after l-g-t-m from slan@chromium.org,
reillyg@chromium.org, rouslan@chromium.org, boliu@chromium.org,
agrieve@chromium.org, tedchoc@chromium.org, lambroslambrou@chromium.org,
yfriedman@chromium.org, yzshen@chromium.org, pauljensen@chromium.org,
dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/2830843004/#ps360001
(title: "rebase")

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

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-05-02 03:18:20]

CQ is committing da patch.

Bot data: {"patchset_id": 360001, "attempt_start_ts": 1493687223322520,
"parent_rev": "aeae396182b48115df4170d4e6e61aea9b2e7416", "commit_rev":
"5512e732f9513b2e5b3f8c47f146dfa5c3582b6d"}

[commit-bot: I haz the power, 2017-05-02 03:18:59]

Description was changed from

==========
Update to newer Android Lint and suppress new Lint errors

This CL updates DEPS to enable updated Android Lint that would work for
Java 8. Also, it adds inline suppression for newly discovered Lint
errors. Additionally, it suppresses Lint deprecation warning for
"android:singleLine". Moreover, it suppresses Lint warnings "AuthLeak",
"VisibleForTests", etc. for tests, and Lint error "ByteOrderMark" in
generated android_chrome_strings.xml (crbug.com/715196).

BUG=635567,669999
==========

to

==========
Update to newer Android Lint and suppress new Lint errors

This CL updates DEPS to enable updated Android Lint that would work for
Java 8. Also, it adds inline suppression for newly discovered Lint
errors. Additionally, it suppresses Lint deprecation warning for
"android:singleLine". Moreover, it suppresses Lint warnings "AuthLeak",
"VisibleForTests", etc. for tests, and Lint error "ByteOrderMark" in
generated android_chrome_strings.xml (crbug.com/715196).

BUG=635567,669999

Review-Url: https://codereview.chromium.org/2830843004
Cr-Commit-Position: refs/heads/master@{#468545}
Committed:
https://chromium.googlesource.com/chromium/src/+/5512e732f9513b2e5b3f8c47f146...
==========

[commit-bot: I haz the power, 2017-05-02 03:19:02]

Committed patchset #18 (id:360001) as
https://chromium.googlesource.com/chromium/src/+/5512e732f9513b2e5b3f8c47f146...