Work round ICU disagreeing with TZDB about zone transitions

Since the TZ backend resorts to ICU for display names, we got
inconsistent results if they disagree about when transitions happened.
Also, ICU uses the current (or only recent history) names for the
zone, so one currently not doing DST (Africa/Tripoli) doesn't get a
report of its DST name at a historical time when it did DST (but ICU
doesn't know about it). Since the ICU backend, in any case, doesn't
override the displayName(qint64, ...), we can simply use the QTZP base
version instead of overloading in the TZ back-end, so we only delegate
to ICU when it might actually help. This also saves duplicating some
locking and lazy-initialization code.

In the process, turn a unique lock into a scoped lock within a
suitable scope. Also, make a comment more grammatical and less
verbose.

Change-Id: Iaaed1fb6a380be8b4258c43cbc4bacd5784345fb
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>

3 files changed, 16 insertions, 34 deletions

diff --git a/src/corelib/time/qtimezoneprivate_p.h b/src/corelib/time/qtimezoneprivate_p.h
index 51e093e002..456a1db606 100644
--- a/src/corelib/time/qtimezoneprivate_p.h
+++ b/src/corelib/time/qtimezoneprivate_p.h
@@ -290,9 +290,6 @@ public:
     QLocale::Territory territory() const override;
     QString comment() const override;
 
-    QString displayName(qint64 atMSecsSinceEpoch,
-                        QTimeZone::NameType nameType,
-                        const QLocale &locale) const override;
     QString displayName(QTimeZone::TimeType timeType,
                         QTimeZone::NameType nameType,
                         const QLocale &locale) const override;
diff --git a/src/corelib/time/qtimezoneprivate_tz.cpp b/src/corelib/time/qtimezoneprivate_tz.cpp
index 0046782fc9..ab5978e0a6 100644
--- a/src/corelib/time/qtimezoneprivate_tz.cpp
+++ b/src/corelib/time/qtimezoneprivate_tz.cpp
@@ -1022,46 +1022,27 @@ QString QTzTimeZonePrivate::comment() const
     return QString::fromUtf8(tzZones->value(m_id).comment);
 }
 
-QString QTzTimeZonePrivate::displayName(qint64 atMSecsSinceEpoch,
-                                        QTimeZone::NameType nameType,
-                                        const QLocale &locale) const
-{
-#if QT_CONFIG(icu)
-    auto lock = qt_unique_lock(s_icu_mutex);
-    if (!m_icu)
-        m_icu = new QIcuTimeZonePrivate(m_id);
-    // TODO small risk may not match if tran times differ due to outdated files
-    // TODO Some valid TZ names are not valid ICU names, use translation table?
-    if (m_icu->isValid())
-        return m_icu->displayName(atMSecsSinceEpoch, nameType, locale);
-    lock.unlock();
-#else
-    Q_UNUSED(nameType);
-    Q_UNUSED(locale);
-#endif
-    // Fall back to base-class:
-    return QTimeZonePrivate::displayName(atMSecsSinceEpoch, nameType, locale);
-}
-
 QString QTzTimeZonePrivate::displayName(QTimeZone::TimeType timeType,
                                         QTimeZone::NameType nameType,
                                         const QLocale &locale) const
 {
+    // TZ DB lacks localized names (it only has IANA IDs), so delegate to ICU
+    // for those, when available.
 #if QT_CONFIG(icu)
-    auto lock = qt_unique_lock(s_icu_mutex);
-    if (!m_icu)
-        m_icu = new QIcuTimeZonePrivate(m_id);
-    // TODO small risk may not match if tran times differ due to outdated files
-    // TODO Some valid TZ names are not valid ICU names, use translation table?
-    if (m_icu->isValid())
-        return m_icu->displayName(timeType, nameType, locale);
-    lock.unlock();
+    {
+        auto lock = qt_scoped_lock(s_icu_mutex);
+        // TODO Some valid TZ names are not valid ICU names, use translation table?
+        if (!m_icu)
+            m_icu = new QIcuTimeZonePrivate(m_id);
+        if (m_icu->isValid())
+            return m_icu->displayName(timeType, nameType, locale);
+    }
 #else
     Q_UNUSED(timeType);
     Q_UNUSED(nameType);
     Q_UNUSED(locale);
 #endif
-    // If no ICU available then have to use abbreviations instead
+    // If ICU is unavailable, fall back to abbreviations.
     // Abbreviations don't have GenericTime
     if (timeType == QTimeZone::GenericTime)
         timeType = QTimeZone::StandardTime;
diff --git a/tests/auto/corelib/text/qlocale/tst_qlocale.cpp b/tests/auto/corelib/text/qlocale/tst_qlocale.cpp
index 53b4f10447..1b0fc57ce5 100644
--- a/tests/auto/corelib/text/qlocale/tst_qlocale.cpp
+++ b/tests/auto/corelib/text/qlocale/tst_qlocale.cpp
@@ -173,7 +173,11 @@ tst_QLocale::tst_QLocale()
     // Some tests are specific to CET, test if it applies:
     : europeanTimeZone(
         QDate(2013, 1, 1).startOfDay().offsetFromUtc() == 3600
-        && QDate(2013, 6, 1).startOfDay().offsetFromUtc() == 7200)
+        && QDate(2013, 6, 1).startOfDay().offsetFromUtc() == 7200
+        // ICU in a zone not currently doing DST may ignore any historical DST
+        // excursions in its display-names (Africa/Tripoli).
+        && QDate(QDate::currentDate().year(), 1, 1).startOfDay().offsetFromUtc() == 3600
+        && QDate(QDate::currentDate().year(), 7, 1).startOfDay().offsetFromUtc() == 7200)
 {
     qRegisterMetaType<QLocale::FormatType>("QLocale::FormatType");
 }