TimeZoneMonitor: Move conditional code to platform files

Bug: 1013709
Test: There are separate tests for the render and browser process.
 Renderer process (adapted from https://crrev.com/2349963002):
  1. Load the file from http://crbug.com/288697#c12.
  2. Change the OS time zone.
  3. Click "recheck" and verify that the new time zone is displayed.
     Don't reload the page as this could result in a new renderer process.
  4. Change the OS time zone back to the original time zone.
  5. Repeat step 3.
 Browser process (adapted from https://crrev.com/697203006):
  1. Open History (chrome://history/)
  2. Change the OS time zone.
  3. Reload the History page and verify that times displayed are in
     the new time zone.

Change-Id: Ifefbb67efebda8a23200a3ba163ce961a7260807
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1845623Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Jungshik Shin <jshin@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Commit-Queue: David Dorwin <ddorwin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709068}

base/i18n/icu_util.cc

@@ -20,7 +20,7 @@
 #include "build/build_config.h"
 #include "third_party/icu/source/common/unicode/putil.h"
 #include "third_party/icu/source/common/unicode/udata.h"
-#if (defined(OS_LINUX) && !defined(OS_CHROMEOS)) || defined(OS_ANDROID)
+#if (defined(OS_LINUX) && !defined(IS_CHROMECAST)) || defined(OS_ANDROID)
 #include "third_party/icu/source/i18n/unicode/timezone.h"
 #endif
 
@@ -335,18 +335,19 @@ bool InitializeICU() {
   wcscpy_s(debug_icu_pf_filename, g_debug_icu_pf_filename);
   debug::Alias(&debug_icu_pf_filename);
   CHECK(result);  // TODO(brucedawson): http://crbug.com/445616
-#endif
-#endif
-
-// To respond to the timezone change properly, the default timezone
-// cache in ICU has to be populated on starting up.
-// TODO(jungshik): Some callers do not care about tz at all. If necessary,
-// add a boolean argument to this function to init'd the default tz only
-// when requested.
-#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
+#endif  // defined(OS_WIN)
+#endif  // (ICU_UTIL_DATA_IMPL == ICU_UTIL_DATA_STATIC)
+
+#if defined(OS_LINUX) && !defined(IS_CHROMECAST)
+  // To respond to the timezone change properly, the default timezone
+  // cache in ICU has to be populated on starting up.
+  // See TimeZoneMonitorLinux::NotifyClientsFromImpl().
+  // TODO(jungshik): Some callers do not care about tz at all. If necessary,
+  // add a boolean argument to this function to init the default tz only
+  // when requested.
   if (result)
     std::unique_ptr<icu::TimeZone> zone(icu::TimeZone::createDefault());
-#endif
+#endif  // defined(OS_LINUX) && !defined(IS_CHROMECAST)
   return result;
 }
 #endif  // !defined(OS_NACL)

services/device/time_zone_monitor/time_zone_monitor.cc

@@ -5,18 +5,14 @@
 #include "services/device/time_zone_monitor/time_zone_monitor.h"
 
 #include "base/logging.h"
-#include "build/build_config.h"
+#include "base/memory/ptr_util.h"
+#include "base/strings/string_piece.h"
 #include "third_party/icu/source/common/unicode/unistr.h"
 #include "third_party/icu/source/i18n/unicode/timezone.h"
 
-#if defined(OS_ANDROID)
-#include "base/android/timezone_utils.h"  // nogncheck
-#endif
-
 namespace device {
 
 TimeZoneMonitor::TimeZoneMonitor() {
-  DCHECK(thread_checker_.CalledOnValidThread());
 }
 
 TimeZoneMonitor::~TimeZoneMonitor() {
@@ -25,48 +21,45 @@ TimeZoneMonitor::~TimeZoneMonitor() {
 
 void TimeZoneMonitor::Bind(
     mojo::PendingReceiver<device::mojom::TimeZoneMonitor> receiver) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   receivers_.Add(this, std::move(receiver));
 }
 
-void TimeZoneMonitor::NotifyClients() {
+void TimeZoneMonitor::NotifyClients(base::StringPiece zone_id_str) {
   DCHECK(thread_checker_.CalledOnValidThread());
-#if defined(OS_CHROMEOS) || (defined(OS_LINUX) && defined(IS_CHROMECAST))
-  // On CrOS (and Chromecast), ICU's default tz is already set to a new zone. No
-  // need to redetect it with detectHostTimeZone().
-  std::unique_ptr<icu::TimeZone> new_zone(icu::TimeZone::createDefault());
-#else
-#if defined(OS_ANDROID)
-  base::string16 timezone_id = base::android::GetDefaultTimeZoneId();
-  std::unique_ptr<icu::TimeZone> new_zone(icu::TimeZone::createTimeZone(
-      icu::UnicodeString(FALSE, timezone_id.data(), timezone_id.length())));
-#else
-  std::unique_ptr<icu::TimeZone> new_zone(icu::TimeZone::detectHostTimeZone());
-#endif
-#if defined(OS_LINUX) && !defined(IS_CHROMECAST)
-  // We get here multiple times on Linux per a single tz change, but
-  // want to update the ICU default zone and notify renderer only once.
-  std::unique_ptr<icu::TimeZone> current_zone(icu::TimeZone::createDefault());
-  if (*current_zone == *new_zone) {
-    VLOG(1) << "timezone already updated";
-    return;
-  }
-#endif
-#endif  // OS_CHROMEOS
-  icu::UnicodeString zone_id;
-  std::string zone_id_str;
-  new_zone->getID(zone_id).toUTF8String(zone_id_str);
   VLOG(1) << "timezone reset to " << zone_id_str;
 
-#if !defined(OS_CHROMEOS)
+  for (auto& client : clients_)
+    client->OnTimeZoneChange(zone_id_str.as_string());
+}
+
+void TimeZoneMonitor::UpdateIcuAndNotifyClients(
+    std::unique_ptr<icu::TimeZone> new_zone) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  std::string zone_id_str = GetTimeZoneId(*new_zone);
+
   icu::TimeZone::adoptDefault(new_zone.release());
-#endif
 
-  for (auto& client : clients_)
-    client->OnTimeZoneChange(zone_id_str);
+  NotifyClients(zone_id_str);
+}
+
+// static
+std::unique_ptr<icu::TimeZone> TimeZoneMonitor::DetectHostTimeZoneFromIcu() {
+  return base::WrapUnique(icu::TimeZone::detectHostTimeZone());
+}
+
+// static
+std::string TimeZoneMonitor::GetTimeZoneId(const icu::TimeZone& zone) {
+  icu::UnicodeString zone_id;
+  std::string zone_id_str;
+  zone.getID(zone_id).toUTF8String(zone_id_str);
+  return zone_id_str;
 }
 
 void TimeZoneMonitor::AddClient(
     mojo::PendingRemote<device::mojom::TimeZoneMonitorClient> client) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   clients_.Add(std::move(client));
 }
 

services/device/time_zone_monitor/time_zone_monitor.h

@@ -6,14 +6,17 @@
 #define SERVICES_DEVICE_TIME_ZONE_MONITOR_TIME_ZONE_MONITOR_H_
 
 #include <memory>
+#include <string>
 
 #include "base/macros.h"
+#include "base/strings/string_piece_forward.h"
 #include "base/threading/thread_checker.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/bindings/receiver_set.h"
 #include "mojo/public/cpp/bindings/remote_set.h"
 #include "services/device/public/mojom/time_zone_monitor.mojom.h"
+#include "third_party/icu/source/common/unicode/uversion.h"
 
 template <class T>
 class scoped_refptr;
@@ -22,6 +25,10 @@ namespace base {
 class SequencedTaskRunner;
 }
 
+U_NAMESPACE_BEGIN
+class TimeZone;
+U_NAMESPACE_END
+
 namespace device {
 
 // TimeZoneMonitor watches the system time zone, and notifies renderers
@@ -37,7 +44,7 @@ namespace device {
 //  - Mac uses a sandbox hole defined in content/renderer/renderer.sb.
 //  - Linux-based platforms use ProxyLocaltimeCallToBrowser in
 //    content/zygote/zygote_main_linux.cc and HandleLocaltime in
-//    content/browser/renderer_host/sandbox_ipc_linux.cc to override
+//    content/browser/sandbox_ipc_linux.cc to override
 //    localtime in renderer processes with custom code that calls
 //    localtime in the browser process via Chrome IPC.
 
@@ -56,8 +63,18 @@ class TimeZoneMonitor : public device::mojom::TimeZoneMonitor {
  protected:
   TimeZoneMonitor();
 
-  // Notify all callbacks that the system time zone may have changed.
-  void NotifyClients();
+  // Notifies clients that the system time zone may have changed and is now
+  // zone_id_str.
+  void NotifyClients(base::StringPiece zone_id_str);
+
+  // Sets ICU's default TimeZone for the process and calls NotifyClients().
+  void UpdateIcuAndNotifyClients(std::unique_ptr<icu::TimeZone> new_zone);
+
+  // Detects the host time zone using the logic in ICU.
+  static std::unique_ptr<icu::TimeZone> DetectHostTimeZoneFromIcu();
+
+  // Converts |zone|'s ID string to UTF-8 and returns it.
+  static std::string GetTimeZoneId(const icu::TimeZone& zone);
 
  private:
   base::ThreadChecker thread_checker_;

services/device/time_zone_monitor/time_zone_monitor_android.cc

@@ -4,9 +4,15 @@
 
 #include "services/device/time_zone_monitor/time_zone_monitor_android.h"
 
+#include <memory>
+
 #include "base/android/jni_android.h"
+#include "base/android/timezone_utils.h"  // nogncheck
 #include "base/sequenced_task_runner.h"
+#include "base/strings/string16.h"
 #include "services/device/time_zone_monitor/time_zone_monitor_jni_headers/TimeZoneMonitor_jni.h"
+#include "third_party/icu/source/common/unicode/unistr.h"
+#include "third_party/icu/source/i18n/unicode/timezone.h"
 
 using base::android::JavaParamRef;
 
@@ -25,7 +31,10 @@ TimeZoneMonitorAndroid::~TimeZoneMonitorAndroid() {
 void TimeZoneMonitorAndroid::TimeZoneChangedFromJava(
     JNIEnv* env,
     const JavaParamRef<jobject>& caller) {
-  NotifyClients();
+  base::string16 timezone_id = base::android::GetDefaultTimeZoneId();
+  std::unique_ptr<icu::TimeZone> new_zone(icu::TimeZone::createTimeZone(
+      icu::UnicodeString(FALSE, timezone_id.data(), timezone_id.length())));
+  UpdateIcuAndNotifyClients(std::move(new_zone));
 }
 
 // static

services/device/time_zone_monitor/time_zone_monitor_chromeos.cc

@@ -4,8 +4,12 @@
 
 #include "services/device/time_zone_monitor/time_zone_monitor.h"
 
+#include <memory>
+
+#include "base/logging.h"
 #include "base/macros.h"
 #include "chromeos/settings/timezone_settings.h"
+#include "third_party/icu/source/i18n/unicode/timezone.h"
 
 namespace device {
 
@@ -23,7 +27,9 @@ class TimeZoneMonitorChromeOS
 
   // chromeos::system::TimezoneSettings::Observer implementation.
   void TimezoneChanged(const icu::TimeZone& time_zone) override {
-    NotifyClients();
+    // ICU's default time zone is already set to a new zone. No need to redetect
+    // it with detectHostTimeZone() or to update ICU.
+    NotifyClients(GetTimeZoneId(time_zone));
   }
 
  private:

services/device/time_zone_monitor/time_zone_monitor_linux.cc

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#if !defined(OS_CHROMEOS)
+
 #include "services/device/time_zone_monitor/time_zone_monitor.h"
 
 #include <stddef.h>
@@ -19,8 +21,7 @@
 #include "base/threading/scoped_blocking_call.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
-
-#if !defined(OS_CHROMEOS)
+#include "third_party/icu/source/i18n/unicode/timezone.h"
 
 namespace device {
 
@@ -34,7 +35,28 @@ class TimeZoneMonitorLinux : public TimeZoneMonitor {
       scoped_refptr<base::SequencedTaskRunner> file_task_runner);
   ~TimeZoneMonitorLinux() override;
 
-  void NotifyClientsFromImpl() { NotifyClients(); }
+  void NotifyClientsFromImpl() {
+#if defined(IS_CHROMECAST)
+    // On Chromecast, ICU's default time zone is already set to a new zone. No
+    // need to redetect it with detectHostTimeZone() or to update ICU.
+    // See http://b/112498903 and http://b/113344065.
+    std::unique_ptr<icu::TimeZone> new_zone(icu::TimeZone::createDefault());
+    NotifyClients(GetTimeZoneId(*new_zone));
+#else
+    std::unique_ptr<icu::TimeZone> new_zone(DetectHostTimeZoneFromIcu());
+
+    // We get here multiple times on Linux per a single tz change, but
+    // want to update the ICU default zone and notify renderer only once.
+    // The timezone must have previously been populated. See InitializeICU().
+    std::unique_ptr<icu::TimeZone> current_zone(icu::TimeZone::createDefault());
+    if (*current_zone == *new_zone) {
+      VLOG(1) << "timezone already updated";
+      return;
+    }
+
+    UpdateIcuAndNotifyClients(std::move(new_zone));
+#endif  // defined(IS_CHROMECAST)
+  }
 
  private:
   scoped_refptr<TimeZoneMonitorLinuxImpl> impl_;

services/device/time_zone_monitor/time_zone_monitor_mac.mm

@@ -4,8 +4,11 @@
 
 #import <Foundation/Foundation.h>
 
+#include <memory>
+
 #include "base/macros.h"
 #include "services/device/time_zone_monitor/time_zone_monitor.h"
+#include "third_party/icu/source/i18n/unicode/timezone.h"
 
 namespace device {
 
@@ -18,7 +21,7 @@ class TimeZoneMonitorMac : public TimeZoneMonitor {
                         object:nil
                          queue:nil
                     usingBlock:^(NSNotification* notification) {
-                      NotifyClients();
+                      UpdateIcuAndNotifyClients(DetectHostTimeZoneFromIcu());
                     }];
   }
 

services/device/time_zone_monitor/time_zone_monitor_win.cc

@@ -11,6 +11,7 @@
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/macros.h"
+#include "third_party/icu/source/i18n/unicode/timezone.h"
 #include "ui/gfx/win/singleton_hwnd_observer.h"
 
 namespace device {
@@ -31,7 +32,7 @@ class TimeZoneMonitorWin : public TimeZoneMonitor {
       return;
     }
 
-    NotifyClients();
+    UpdateIcuAndNotifyClients(DetectHostTimeZoneFromIcu());
   }
 
   std::unique_ptr<gfx::SingletonHwndObserver> singleton_hwnd_observer_;