Make callers of FromUTC(Local)Exploded in components/ use new time API.

components/ntp_snippets/ntp_snippets_service.cc

Index: components/ntp_snippets/ntp_snippets_service.cc
diff --git a/components/ntp_snippets/ntp_snippets_service.cc b/components/ntp_snippets/ntp_snippets_service.cc
index 48b3f0b11128729b6bde2273f06ad6b2b9e3d658..cd09266a2d376098cb37bf65521aa6ca4e4d23fd 100644
--- a/components/ntp_snippets/ntp_snippets_service.cc
+++ b/components/ntp_snippets/ntp_snippets_service.cc
@@ -140,7 +140,14 @@ base::Time GetRescheduleTime(const base::Time& now) {
   exploded.minute = 0;
   exploded.second = 0;
   exploded.millisecond = 0;
-  base::Time reschedule = base::Time::FromLocalExploded(exploded);
+  base::Time reschedule;
+  if (!base::Time::FromLocalExploded(exploded, &reschedule)) {

[sfiera, 2016/06/28 14:13:28]

Here, we are combining YMD of a valid date (from LocalExplode()) with HMSm of a
valid time (06:00:00.000 or 22:00:00.000).

Can this fail?

I suppose if a country were to enter DST at 6AM, then for example
2017/03/01-06:00:00 might not exist locally [1]. In this case, we probably want
7AM, but any approximation might be good enough. Can we approximate somehow? I
imagine the old behavior on posix/mac might have been what we wanted.

[1] Or maybe I have it backwards and this kind of thing happens when leaving
DST. I hate DST.

[maksims (do not use this acc), 2016/06/29 05:48:59]

[1] Me neither.

Well, it seems like it shouldn't fail in this case. If you read the comments in
time.h, it says the conversion is treated as failed when a day of month is set
to 31 on a 28-30 day month.

In other words, what is done there is after time has been converted, it is
converted back to the exploded structure and checked if both the original and
converted times are same. That is treated as correct converted time.

[sfiera, 2016/06/29 09:15:11]

TL;DR this code can fail in Western Greenland.

See http://crrev.com/2108823003. If we call FromLocalExploded() on a time that
we're skipping over due to DST, then it'll now fail. In these cases, we can
un-explode the time just fine (even on Windows, I think?), but when we explode
it again, we get a corresponding valid time, which doesn't match.

Most places don't enter DST at 10PM or 6AM (our times here), but Western
Greenland does. Previously, we would've gotten back 11PM. That's good enough for
us. Skipping the time is also probably good enough for us.

I guess the best thing to do here would be "return GetRescheduleTime(now +
base::TimeDelta::FromDays(1))". We might skip an update or two, but that's
better than rescheduling at Time(0).

[maksims (do not use this acc), 2016/06/30 07:33:14]

Check the latests patch. Is it fine now? Did I understand you right?

[sfiera, 2016/06/30 08:41:50]

I was suggesting:

  return GetRescheduleTime(now + base::TimeDelta::FromDays(1))

meaning, if we fail to get a reschedule time for today, get one for this time
tomorrow.

[maksims (do not use this acc), 2016/07/04 07:34:04]

Done.

[sfiera, 2016/07/04 07:48:20]

Thanks! I also looked at the other changes you've made, and I think this is the
only one that suffered from a DST problem.

+    // TODO(maksims): implement error handling here.
+    // We might just return |reschedule|, which is Time(0) here.
+    NOTIMPLEMENTED();
+    return reschedule;
+  }
+
   if (next_day)
     reschedule += base::TimeDelta::FromDays(1);