Format remaining download time in java code

Format remaining download time in java code

When displaying the remaining download time, ui::TimeFormat::Simple() is
called.
It calls into third_party/icu, and is quite expensive, takes 70kB from memory dump.
This CL adds a simple java method to replace that call.
The Java method is similar to
android.java.text.Formatter.formatShortElapsedTime().
And it should generate the same output string as
ui::TimeFormat::Simple().

BUG=620479
Committed: https://crrev.com/9d2f27dd1d80ce3a4c295e9551d16d91329f3e9e
Cr-Commit-Position: refs/heads/master@{#400370}

[qinmin, 2016-06-15 22:32:45]

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

[qinmin, 2016-06-15 22:32:45]

PTAL

[Ted C, 2016-06-16 18:28:59]

Can you add some simple tests for this as well?

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

https://codereview.chromium.org/2070713002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java:571:
private static final int SECONDS_PER_MINUTE = 60;
these should go to the top of the file

https://codereview.chromium.org/2070713002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java:587:
int days = 0, hours = 0, minutes = 0;
we don't use the multi initialize on a single line pattern much if anywhere, so
let's split this here.

https://codereview.chromium.org/2070713002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java:604:
return context.getString(R.string.remaining_duration_days, days);
what should numbers look like in different languages?

In Arabic, should we show 3 or ٣?

Should we use the same logic we have in the tab switcher number:
String.format(Locale.getDefault(), "%d", mTabCount);

[qinmin, 2016-06-16 19:44:52]

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

https://codereview.chromium.org/2070713002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java:571:
private static final int SECONDS_PER_MINUTE = 60;
On 2016/06/16 18:28:59, Ted C wrote:
> these should go to the top of the file

Done.

https://codereview.chromium.org/2070713002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java:587:
int days = 0, hours = 0, minutes = 0;
On 2016/06/16 18:28:59, Ted C wrote:
> we don't use the multi initialize on a single line pattern much if anywhere,
so
> let's split this here.

Done.

https://codereview.chromium.org/2070713002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java:604:
return context.getString(R.string.remaining_duration_days, days);
On 2016/06/16 18:28:59, Ted C wrote:
> what should numbers look like in different languages?
> 
> In Arabic, should we show 3 or ٣?
> 
> Should we use the same logic we have in the tab switcher number:
> String.format(Locale.getDefault(), "%d", mTabCount);

The numbers will be in the device language. Tried it with Arabic, and verified
that the numbers are displayed in Arabic. Android will translate that for us.

[qinmin, 2016-06-16 19:45:43]

On 2016/06/16 18:28:59, Ted C wrote:
> Can you add some simple tests for this as well?

test is also added in the new patch 
> 
>
https://codereview.chromium.org/2070713002/diff/1/chrome/android/java/src/org...
> File
>
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java
> (right):
> 
>
https://codereview.chromium.org/2070713002/diff/1/chrome/android/java/src/org...
>
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java:571:
> private static final int SECONDS_PER_MINUTE = 60;
> these should go to the top of the file
> 
>
https://codereview.chromium.org/2070713002/diff/1/chrome/android/java/src/org...
>
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java:587:
> int days = 0, hours = 0, minutes = 0;
> we don't use the multi initialize on a single line pattern much if anywhere,
so
> let's split this here.
> 
>
https://codereview.chromium.org/2070713002/diff/1/chrome/android/java/src/org...
>
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java:604:
> return context.getString(R.string.remaining_duration_days, days);
> what should numbers look like in different languages?
> 
> In Arabic, should we show 3 or ٣?
> 
> Should we use the same logic we have in the tab switcher number:
> String.format(Locale.getDefault(), "%d", mTabCount);

[Ted C, 2016-06-16 20:06:31]

On 2016/06/16 19:45:43, qinmin wrote:
> On 2016/06/16 18:28:59, Ted C wrote:
> > Can you add some simple tests for this as well?
> 
> test is also added in the new patch 
> > 
> >
>
https://codereview.chromium.org/2070713002/diff/1/chrome/android/java/src/org...
> > File
> >
>
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java
> > (right):
> > 
> >
>
https://codereview.chromium.org/2070713002/diff/1/chrome/android/java/src/org...
> >
>
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java:571:
> > private static final int SECONDS_PER_MINUTE = 60;
> > these should go to the top of the file
> > 
> >
>
https://codereview.chromium.org/2070713002/diff/1/chrome/android/java/src/org...
> >
>
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java:587:
> > int days = 0, hours = 0, minutes = 0;
> > we don't use the multi initialize on a single line pattern much if anywhere,
> so
> > let's split this here.
> > 
> >
>
https://codereview.chromium.org/2070713002/diff/1/chrome/android/java/src/org...
> >
>
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java:604:
> > return context.getString(R.string.remaining_duration_days, days);
> > what should numbers look like in different languages?
> > 
> > In Arabic, should we show 3 or ٣?
> > 
> > Should we use the same logic we have in the tab switcher number:
> > String.format(Locale.getDefault(), "%d", mTabCount);

lgtm ... cool about numbers.  I guess the tab switcher wasn't using a
resource so we had to do it ourselves.

[qinmin, 2016-06-16 20:21:53]

The CQ bit was checked by qinmin@chromium.org

[commit-bot: I haz the power, 2016-06-16 20:22:39]

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

[commit-bot: I haz the power, 2016-06-16 21:30:04]

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

[commit-bot: I haz the power, 2016-06-16 21:30:05]

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

[qinmin, 2016-06-17 04:39:28]

The CQ bit was unchecked by qinmin@chromium.org

[qinmin, 2016-06-17 04:39:28]

The CQ bit was checked by qinmin@chromium.org

[qinmin, 2016-06-17 04:39:29]

The patchset sent to the CQ was uploaded after l-g-t-m from tedchoc@chromium.org
Link to the patchset: https://codereview.chromium.org/2070713002/#ps40001
(title: "fix findbugs warning")

[qinmin, 2016-06-17 04:39:29]

The CQ bit was checked by qinmin@chromium.org

[commit-bot: I haz the power, 2016-06-17 04:39:45]

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

[commit-bot: I haz the power, 2016-06-17 05:45:13]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-17 05:47:52]

Description was changed from

==========
Format remaining download time in java code

When displaying the remaining download time, ui::TimeFormat::Simple() is
called.
It calls into third_party/icu, and is quite expensive, takes 70kB from memory
dump.
This CL adds a simple java method to replace that call.
The Java method is similar to
android.java.text.Formatter.formatShortElapsedTime().
And it should generate the same output string as
ui::TimeFormat::Simple().

BUG=620479
==========

to

==========
Format remaining download time in java code

When displaying the remaining download time, ui::TimeFormat::Simple() is
called.
It calls into third_party/icu, and is quite expensive, takes 70kB from memory
dump.
This CL adds a simple java method to replace that call.
The Java method is similar to
android.java.text.Formatter.formatShortElapsedTime().
And it should generate the same output string as
ui::TimeFormat::Simple().

BUG=620479

Committed: https://crrev.com/9d2f27dd1d80ce3a4c295e9551d16d91329f3e9e
Cr-Commit-Position: refs/heads/master@{#400370}
==========

[commit-bot: I haz the power, 2016-06-17 05:47:53]

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