QIcuTimeZonePrivate::displayName(): tidy and critique

Made the code simpler and amended comments. Computing the offset costs
less than computing full data at the given moment, especially when
we're going to ignore the daylight-offset that reports and call
another ICU function for it, if needed. Explain why the result is
unreliable and why, when that matters, we probably came via a
different code path that gets it right. Note the potential
inconsistency in locale between the UCalendar's construction and the
request for localized display name.

Change-Id: I2203c0bb053fa0e658373e4a818b77739be50a23
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

1 files changed, 13 insertions, 7 deletions

diff --git a/src/corelib/time/qtimezoneprivate_icu.cpp b/src/corelib/time/qtimezoneprivate_icu.cpp
index fb8c620f32..2a2770d3ea 100644
--- a/src/corelib/time/qtimezoneprivate_icu.cpp
+++ b/src/corelib/time/qtimezoneprivate_icu.cpp
@@ -301,16 +301,22 @@ QString QIcuTimeZonePrivate::displayName(QTimeZone::TimeType timeType,
                                          QTimeZone::NameType nameType,
                                          const QLocale &locale) const
 {
-    // Return standard offset format name as ICU C api doesn't support it yet
+    // Base class has handled OffsetName if we came via the other overload.
     if (nameType == QTimeZone::OffsetName) {
-        const Data nowData = data(QDateTime::currentMSecsSinceEpoch());
-        // We can't use transitions reliably to find out right dst offset
-        // Instead use dst offset api to try get it if needed
+        int offset = standardTimeOffset(QDateTime::currentMSecsSinceEpoch());
+        // We can't use transitions reliably to find out right DST offset.
+        // Instead use DST offset API to try to get it, when needed:
         if (timeType == QTimeZone::DaylightTime)
-            return isoOffsetFormat(nowData.standardTimeOffset + ucalDaylightOffset(m_id));
-        else
-            return isoOffsetFormat(nowData.standardTimeOffset);
+            offset += ucalDaylightOffset(m_id);
+        // This is only valid for times since the most recent standard offset
+        // change; for earlier times, caller must use the other overload.
+
+        // Use our own formating for offset names (ICU C API doesn't support it
+        // and we may as well be self-consistent anyway).
+        return isoOffsetFormat(offset);
     }
+    // Technically this may be suspect, if locale isn't QLocale(), since that's
+    // what we used when constructing m_ucal; does ICU cope with inconsistency ?
     return ucalTimeZoneDisplayName(m_ucal, timeType, nameType, locale.name());
 }