Transfer date/time value to chooser as double

content/public/android/java/src/org/chromium/content/browser/input/InputDialogContainer.java

Index: content/public/android/java/src/org/chromium/content/browser/input/InputDialogContainer.java
diff --git a/content/public/android/java/src/org/chromium/content/browser/input/InputDialogContainer.java b/content/public/android/java/src/org/chromium/content/browser/input/InputDialogContainer.java
index ac610461f19a6b2f3602b707e2831bf2edb4c5af..b9ed58c9022486613dc091f3ad140038d7df029b 100644
--- a/content/public/android/java/src/org/chromium/content/browser/input/InputDialogContainer.java
+++ b/content/public/android/java/src/org/chromium/content/browser/input/InputDialogContainer.java
@@ -21,13 +21,13 @@ import org.chromium.content.browser.input.DateTimePickerDialog.OnDateTimeSetList
 import org.chromium.content.browser.input.MultiFieldTimePickerDialog.OnMultiFieldTimeSetListener;
 
 import java.util.Calendar;
+import java.util.TimeZone;
 
 public class InputDialogContainer {
 
     interface InputActionDelegate {
         void cancelDateTimeDialog();
-        void replaceDateTime(int dialogType,
-            int year, int month, int day, int hour, int minute, int second, int milli, int week);
+        void replaceDateTime(double value);
     }
 
     // Default values used in Time representations of selected date/time before formatting.
@@ -39,16 +39,6 @@ public class InputDialogContainer {
     private static final int MINUTE_DEFAULT = 0;
     private static final int WEEK_DEFAULT = 0;
 
-    // Date formats as accepted by Time.format.
-    private static final String HTML_DATE_FORMAT = "%Y-%m-%d";
-    private static final String HTML_TIME_FORMAT = "%H:%M";
-    // For datetime we always send selected time as UTC, as we have no timezone selector.
-    // This is consistent with other browsers.
-    private static final String HTML_DATE_TIME_FORMAT = "%Y-%m-%dT%H:%MZ";
-    private static final String HTML_DATE_TIME_LOCAL_FORMAT = "%Y-%m-%dT%H:%M";
-    private static final String HTML_MONTH_FORMAT = "%Y-%m";
-    private static final String HTML_WEEK_FORMAT = "%Y-%w";
-
     private static int sTextInputTypeDate;
     private static int sTextInputTypeDateTime;
     private static int sTextInputTypeDateTimeLocal;
@@ -87,24 +77,8 @@ public class InputDialogContainer {
         mInputActionDelegate = inputActionDelegate;
     }
 
-    private Time normalizeTime(int year, int month, int monthDay,
-                               int hour, int minute, int second) {
-        Time result = new Time();
-        if (year == 0 && month == 0 && monthDay == 0 && hour == 0 &&
-                minute == 0 && second == 0) {
-            Calendar cal = Calendar.getInstance();
-            result.set(cal.get(Calendar.SECOND), cal.get(Calendar.MINUTE),
-                    cal.get(Calendar.HOUR_OF_DAY), cal.get(Calendar.DATE),
-                    cal.get(Calendar.MONTH), cal.get(Calendar.YEAR));
-        } else {
-            result.set(second, minute, hour, monthDay, month, year);
-        }
-        return result;
-    }
-
-    void showDialog(final int dialogType, int year, int month, int monthDay,
-                    int hour, int minute, int second, int milli, int week,
-                    double min, double max, double step) {
+    void showDialog(final int dialogType, double dialogValue,
+        double min, double max, double step) {
         if (isDialogShowing()) mDialog.dismiss();
 
         // Java Date dialogs like longs but Blink prefers doubles..
@@ -116,23 +90,40 @@ public class InputDialogContainer {
         long maxTime = (long) max;
         int stepTime = (int) step;
 
-        if (milli > 1000) {
-            second += milli / 1000;
-            milli %= 1000;
+        Calendar cal;
+        // If |dialogValue| is NaN it means an empty value. We will show the current time.
+        if (Double.isNaN(dialogValue)) {

[Miguel Garcia, 2013/11/26 16:07:08]

I would really like to see a test that checks that for a given dialog type and
dialog value the right YEAR MONTH DAY HOUR... combination is generated. We used
to a similar thing in date_time_formatter_unittest.cc but this has been removed
now.

[keishi, 2013/11/27 06:58:37]

I separated out the double to year/month/date... conversion so we can unit test
it. I couldn't find examples of Java unit tests so I'm not sure how to write
one.

[Miguel Garcia, 2013/11/27 11:17:24]

Here is an example you can base the test on

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/android/jav...

I think it'd be important to test the edge cases (max value, min value...) this
way.




On 2013/11/27 06:58:37, keishi1 wrote:

[keishi, 2013/11/29 03:30:14]

Thanks I was able to add a test (InputDialogContainerTest.java). 

+            cal = Calendar.getInstance();

[Miguel Garcia, 2013/11/26 16:07:08]

How do you pass Nan via JNI BTW?

[keishi, 2013/11/27 06:58:37]

I'm not sure what you mean. NaN is passed from C++ to Java using double.

[Miguel Garcia, 2013/11/27 11:17:24]

Nevermind this works out of the box as you say

On 2013/11/27 06:58:37, keishi1 wrote:

+            cal.set(Calendar.MILLISECOND, 0);
+        } else {
+            cal =  Calendar.getInstance(TimeZone.getTimeZone("UTC"));
+            if (dialogType == sTextInputTypeMonth) {

[Miguel Garcia, 2013/11/26 16:07:08]

please document why month is special.

[keishi, 2013/11/27 06:58:37]

Done.

+                cal.set(Calendar.YEAR, (int) (dialogValue / 12));
+                cal.set(Calendar.MONTH, (int) (dialogValue % 12));
+            } else {
+                cal.setTimeInMillis((long) dialogValue);
+            }
         }
-        Time time = normalizeTime(year, month, monthDay, hour, minute, second);
         if (dialogType == sTextInputTypeDate) {
             DatePickerDialog dialog = new DatePickerDialog(mContext,
-                    new DateListener(dialogType), time.year, time.month, time.monthDay);
+                    new DateListener(dialogType),
+                    cal.get(Calendar.YEAR),
+                    cal.get(Calendar.MONTH),
+                    cal.get(Calendar.DAY_OF_MONTH));
             DateDialogNormalizer.normalize(dialog.getDatePicker(), dialog,
-                    time.year, time.month, time.monthDay, 0, 0, minTime, maxTime);
+                    cal.get(Calendar.YEAR),
+                    cal.get(Calendar.MONTH),
+                    cal.get(Calendar.DAY_OF_MONTH),
+                    0, 0,
+                    minTime, maxTime);
 
             dialog.setTitle(mContext.getText(R.string.date_picker_dialog_title));
             mDialog = dialog;
         } else if (dialogType == sTextInputTypeTime) {
             mDialog = new MultiFieldTimePickerDialog(
                 mContext, 0 /* theme */ ,
-                time.hour, time.minute, time.second, milli,
+                cal.get(Calendar.HOUR_OF_DAY), cal.get(Calendar.MINUTE),
+                cal.get(Calendar.SECOND), cal.get(Calendar.MILLISECOND),
                 (int) minTime, (int) maxTime, stepTime,
                 DateFormat.is24HourFormat(mContext),
                 new FullTimeListener(dialogType));
@@ -140,18 +131,18 @@ public class InputDialogContainer {
                 dialogType == sTextInputTypeDateTimeLocal) {
             mDialog = new DateTimePickerDialog(mContext,
                     new DateTimeListener(dialogType),
-                    time.year, time.month, time.monthDay,
-                    time.hour, time.minute, DateFormat.is24HourFormat(mContext),
-                    minTime, maxTime);
+                    cal.get(Calendar.YEAR),
+                    cal.get(Calendar.MONTH),
+                    cal.get(Calendar.DAY_OF_MONTH),
+                    cal.get(Calendar.HOUR_OF_DAY),
+                    cal.get(Calendar.MINUTE),
+                    DateFormat.is24HourFormat(mContext), minTime, maxTime);
         } else if (dialogType == sTextInputTypeMonth) {
             mDialog = new MonthPickerDialog(mContext, new MonthOrWeekListener(dialogType),
-                    time.year, time.month, minTime, maxTime);
+                    cal.get(Calendar.YEAR), cal.get(Calendar.MONTH), minTime, maxTime);
         } else if (dialogType == sTextInputTypeWeek) {
-            if (week == 0) {
-                Calendar cal = Calendar.getInstance();
-                year = WeekPicker.getISOWeekYearForDate(cal);
-                week = WeekPicker.getWeekForDate(cal);
-            }
+            int year = WeekPicker.getISOWeekYearForDate(cal);
+            int week = WeekPicker.getWeekForDate(cal);
             mDialog = new WeekPickerDialog(mContext, new MonthOrWeekListener(dialogType),
                     year, week, minTime, maxTime);
         }
@@ -170,7 +161,7 @@ public class InputDialogContainer {
                     @Override
                     public void onClick(DialogInterface dialog, int which) {
                         mDialogAlreadyDismissed = true;
-                        mInputActionDelegate.replaceDateTime(dialogType, 0, 0, 0, 0, 0, 0, 0, 0);
+                        mInputActionDelegate.replaceDateTime(Double.NaN);
                     }
                 });
 
@@ -207,27 +198,12 @@ public class InputDialogContainer {
         @Override
         public void onDateSet(DatePicker view, int year, int month, int monthDay) {
             if (!mDialogAlreadyDismissed) {
-                setFieldDateTimeValue(mDialogType,
-                        year, month, monthDay,
-                        HOUR_DEFAULT, MINUTE_DEFAULT, WEEK_DEFAULT,
-                        HTML_DATE_FORMAT);
-            }
-        }
-    }
-
-    private class TimeListener implements OnTimeSetListener {
-        private final int mDialogType;
-
-        TimeListener(int dialogType) {
-            mDialogType = dialogType;
-        }
-
-        @Override
-        public void onTimeSet(TimePicker view, int hourOfDay, int minute) {
-            if (!mDialogAlreadyDismissed) {
-                setFieldDateTimeValue(mDialogType,
-                        YEAR_DEFAULT, MONTH_DEFAULT, MONTHDAY_DEFAULT,
-                        hourOfDay, minute, WEEK_DEFAULT, HTML_TIME_FORMAT);
+                Calendar cal = Calendar.getInstance(TimeZone.getTimeZone("UTC"));
+                cal.clear();
+                cal.set(Calendar.YEAR, year);
+                cal.set(Calendar.MONTH, month);
+                cal.set(Calendar.DAY_OF_MONTH, monthDay);
+                setFieldDateTimeValue((double) cal.getTimeInMillis());
             }
         }
     }
@@ -241,9 +217,8 @@ public class InputDialogContainer {
         @Override
         public void onTimeSet(int hourOfDay, int minute, int second, int milli) {
             if (!mDialogAlreadyDismissed) {
-                setFieldDateTimeValue(mDialogType,
-                        YEAR_DEFAULT, MONTH_DEFAULT, MONTHDAY_DEFAULT,
-                        hourOfDay, minute, second, milli, WEEK_DEFAULT, HTML_TIME_FORMAT);
+                setFieldDateTimeValue(
+                        (double) ((hourOfDay * 60 + minute) * 60 + second) * 1000 + milli);

[Miguel Garcia, 2013/11/26 16:07:08]

Use TimeUnit(HOUR).toMillis() + TimeUnit.MINUTE(minute).toMillis +
TimeUnit.SECOND(second).toMillis() + millis

[keishi, 2013/11/27 06:58:37]

Done.

             }

[Miguel Garcia, 2013/11/26 16:07:08]

You are not setting mDialogAlreadyDismissed in these cases now since you don't
call setFieldDateTimeValue. 

 I recommend you always call setFieldDateTimeValue and modify that methods so it
takes a dialog type. Then you can generate the right double. If you make the
method static and public and make it return the right double you can actually
unittest it as well.

[keishi, 2013/11/27 06:58:37]

Done.

         }
     }
@@ -262,9 +237,14 @@ public class InputDialogContainer {
                 int year, int month, int monthDay,
                 int hourOfDay, int minute) {
             if (!mDialogAlreadyDismissed) {
-                setFieldDateTimeValue(mDialogType, year, month, monthDay,
-                        hourOfDay, minute, WEEK_DEFAULT,
-                        mLocal ? HTML_DATE_TIME_LOCAL_FORMAT : HTML_DATE_TIME_FORMAT);
+                Calendar cal = Calendar.getInstance(TimeZone.getTimeZone("UTC"));
+                cal.clear();
+                cal.set(Calendar.YEAR, year);
+                cal.set(Calendar.MONTH, month);
+                cal.set(Calendar.DAY_OF_MONTH, monthDay);
+                cal.set(Calendar.HOUR_OF_DAY, hourOfDay);
+                cal.set(Calendar.MINUTE, minute);
+                setFieldDateTimeValue((double) cal.getTimeInMillis());
             }
         }
     }
@@ -277,39 +257,18 @@ public class InputDialogContainer {
         }
 
         @Override
-        public void onValueSet(int year, int positionInYear) {
+        public void onValueSet(double value) {
             if (!mDialogAlreadyDismissed) {
-                if (mDialogType == sTextInputTypeMonth) {
-                    setFieldDateTimeValue(mDialogType, year, positionInYear, MONTHDAY_DEFAULT,
-                            HOUR_DEFAULT, MINUTE_DEFAULT, WEEK_DEFAULT,
-                            HTML_MONTH_FORMAT);
-                } else {
-                    setFieldDateTimeValue(mDialogType, year, MONTH_DEFAULT, MONTHDAY_DEFAULT,
-                            HOUR_DEFAULT, MINUTE_DEFAULT, positionInYear, HTML_WEEK_FORMAT);
-                }
+                setFieldDateTimeValue(value);
             }
         }
     }
 
-    private void setFieldDateTimeValue(int dialogType,
-            int year, int month, int monthDay, int hourOfDay,
-            int minute, int week, String dateFormat) {
-        // Prevents more than one callback being sent to the native
-        // side when the dialog triggers multiple events.
-        mDialogAlreadyDismissed = true;
-
-        mInputActionDelegate.replaceDateTime(dialogType,
-            year, month, monthDay, hourOfDay, minute, 0 /* second */, 0 /* milli */, week);
-    }
-
-    private void setFieldDateTimeValue(int dialogType,
-            int year, int month, int monthDay, int hourOfDay,
-            int minute, int second, int milli, int week, String dateFormat) {
+    private void setFieldDateTimeValue(double value) {
         // Prevents more than one callback being sent to the native
         // side when the dialog triggers multiple events.
         mDialogAlreadyDismissed = true;
 
-        mInputActionDelegate.replaceDateTime(
-            dialogType, year, month, monthDay, hourOfDay, minute, second, milli, week);
+        mInputActionDelegate.replaceDateTime(value);
     }
 }