Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(80)

Issue 1622773002: Fix date picker performance issues, crash, and incorrect pre-1582 dates. (Closed)

Created:
4 years, 11 months ago by newt (away)
Modified:
4 years, 10 months ago
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix date picker performance issues, crash, and incorrect pre-1582 dates. This cleans up the DateDialogNormalizer, fixing three issues in the process: 1. The date picker is now way faster on L devices. This works around a performance issue with the Android DatePicker in L, where large min/max ranges consume tons of memory and time (many seconds). The workaround is simply to limit the date picker's range to at most 5000 years before and after the initial selected date. In practice, this doesn't affect users since manually scrolling through 5000 years is infeasible. 2. This fixes a crash when the min date is after 2100 on pre-L devices. On KK and earlier, calling DatePicker.setMinDate() with a date that's greater than the current max date (which defaults to 2100) causes a crash. The fix is to set the DatePicker's min and max dates in such an order that the min is always less than the max. 3. This fixes an issue where min/max dates before 1582 were off by 10 days due to incorrect handling of the Gregorian/Julian switch. This also revamps DateDialogNormalizerTest and converts it to a Robolectric test. BUG=441060, 580385, 580388 Committed: https://crrev.com/9e226039ff46dbc40f8529e2b70a577230d4d9dd Cr-Commit-Position: refs/heads/master@{#371404}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+293 lines, -150 lines) Patch
M BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M build/all.gyp View 1 chunk +1 line, -0 lines 0 comments Download
M build/gn_migration.gypi View 1 chunk +1 line, -0 lines 0 comments Download
M ui/android/BUILD.gn View 1 chunk +11 lines, -0 lines 0 comments Download
M ui/android/java/src/org/chromium/ui/picker/DateDialogNormalizer.java View 1 chunk +99 lines, -66 lines 0 comments Download
M ui/android/java/src/org/chromium/ui/picker/DateTimePickerDialog.java View 1 chunk +1 line, -1 line 0 comments Download
M ui/android/java/src/org/chromium/ui/picker/InputDialogContainer.java View 1 chunk +0 lines, -1 line 0 comments Download
D ui/android/javatests/src/org/chromium/ui/picker/DateDialogNormalizerTest.java View 1 chunk +0 lines, -82 lines 0 comments Download
A ui/android/junit/src/org/chromium/ui/picker/DateDialogNormalizerTest.java View 1 chunk +159 lines, -0 lines 0 comments Download
M ui/android/ui_android.gyp View 1 chunk +20 lines, -0 lines 0 comments Download

Messages

Total messages: 24 (10 generated)
newt (away)
miguelg: PTAL
4 years, 11 months ago (2016-01-22 22:57:41 UTC) #2
newt (away)
Mike: PTAL gyp/gn changes for the new ui_junit_tests target
4 years, 11 months ago (2016-01-22 22:58:13 UTC) #4
mikecase (-- gone --)
gyp and gn, lgtm
4 years, 11 months ago (2016-01-22 23:04:17 UTC) #5
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1622773002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1622773002/1
4 years, 11 months ago (2016-01-22 23:08:45 UTC) #7
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 11 months ago (2016-01-23 00:46:09 UTC) #9
Miguel Garcia
lgtm Thanks for the thorough change!
4 years, 11 months ago (2016-01-23 16:52:47 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1622773002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1622773002/1
4 years, 10 months ago (2016-01-25 18:00:14 UTC) #12
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/138624)
4 years, 10 months ago (2016-01-25 18:10:24 UTC) #14
newt (away)
dpranke: PTAL at src/BUILD.gn, all.gyp, and gn_migration.gypi.
4 years, 10 months ago (2016-01-25 18:19:39 UTC) #15
newt (away)
dpranke: PTAL at src/BUILD.gn, all.gyp, and gn_migration.gypi. Thanks!
4 years, 10 months ago (2016-01-26 00:46:19 UTC) #17
Dirk Pranke
lgtm
4 years, 10 months ago (2016-01-26 00:53:05 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1622773002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1622773002/1
4 years, 10 months ago (2016-01-26 00:55:15 UTC) #21
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 10 months ago (2016-01-26 01:30:21 UTC) #22
commit-bot: I haz the power
4 years, 10 months ago (2016-01-26 01:31:34 UTC) #24
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/9e226039ff46dbc40f8529e2b70a577230d4d9dd
Cr-Commit-Position: refs/heads/master@{#371404}

Powered by Google App Engine
This is Rietveld 408576698