Reland "[vm] Replaces fuchsia.deprecatedtimezone"

This is a reland of 16f09f2

The apparent break of internal tests was not caused by this change.

Original change's description:
> [vm] Replaces fuchsia.deprecatedtimezone
>
> (prior attempt was rolled back as it caused downstream tests to time
> out.  See prior attempt at: See:
> https://dart-review.googlesource.com/c/sdk/+/149206)
>
> The FIDL library fuchsia.deprecatedtimezone is going away.  There are
> different and better ways to obtain the same functionality.  This change
> removes the dependency on fuchsia.deprecatedtimezone from the Dart SDK.
>
> Adds inspect metrics that allow whitebox testing of the runners.  Here's
> a sample `fx iquery` excerpt from a running device, showing both a dart
> and a flutter runner exposing the same OS diagnostic metrics.
>
> ```
> /hub/c/dart_jit_runner.cmx/70981/out/diagnostics:
>   /hub/c/dart_jit_runner.cmx/70981/out/diagnostics#os:
>     dst_status = 0
>     get_profile_status = 0
>     timezone_content_status = 0
>     tz_data_close_status = 0
>     tz_data_status = 0
> /hub/c/flutter_jit_runner.cmx/29567/out/diagnostics:
>   /hub/c/flutter_jit_runner.cmx/29567/out/diagnostics#os:
>     dst_status = 0
>     get_profile_status = 0
>     timezone_content_status = 0
>     tz_data_close_status = 0
>     tz_data_status = 0
> ```
>
> Under nominal operation, all of the above values should be equal to 0.
> Nonzero values indicate an error.
>
> This functionality is guarded by Fuchsia integration tests at
> //src/tests/intl.
>
> Tested:
>   (compile locally for Fuchsia and deploy)
>   fx test //src/tests/intl
>
> See:
>   - #42245
>   - #39650
>
> Fixes #39650
>
> Change-Id: I97f6e17e57000f6eec71246aee670bca65b7e1d1
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/150662
> Commit-Queue: Filip Filmar <[email protected]>
> Reviewed-by: Martin Kustermann <[email protected]>

Change-Id: I5da6b0f481af0eb42c3b5e74c920588ac2ef5be9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/151862
Reviewed-by: Martin Kustermann <[email protected]>
Commit-Queue: Filip Filmar <[email protected]>

Author: filmil

runtime/vm/BUILD.gn

@@ -74,7 +74,7 @@ library_for_all_configs("libdart_vm") {
   if (is_fuchsia) {
     if (using_fuchsia_gn_sdk) {
       extra_deps = [
-        "$fuchsia_sdk_root/fidl/fuchsia.deprecatedtimezone",
+        "$fuchsia_sdk_root/fidl/fuchsia.intl",
         "$fuchsia_sdk_root/pkg/inspect",
         "$fuchsia_sdk_root/pkg/inspect_service_cpp",
         "$fuchsia_sdk_root/pkg/sys_cpp",
@@ -83,7 +83,7 @@ library_for_all_configs("libdart_vm") {
       ]
     } else if (using_fuchsia_sdk) {
       extra_deps = [
-        "$fuchsia_sdk_root/fidl:fuchsia.deprecatedtimezone",
+        "$fuchsia_sdk_root/fidl:fuchsia.intl",
         "$fuchsia_sdk_root/pkg:inspect",
         "$fuchsia_sdk_root/pkg:inspect_service_cpp",
         "$fuchsia_sdk_root/pkg:sys_cpp",
@@ -92,9 +92,7 @@ library_for_all_configs("libdart_vm") {
       ]
     } else {
       extra_deps = [
-        # TODO(US-399): Remove time_service specific code when it is no longer
-        # necessary.
-        "//sdk/fidl/fuchsia.deprecatedtimezone",
+        "//sdk/fidl/fuchsia.intl",
         "//sdk/lib/sys/cpp",
         "//sdk/lib/sys/inspect/cpp",
         "//zircon/public/lib/fbl",

runtime/vm/os_fuchsia.cc

@@ -11,7 +11,7 @@
 #include <fcntl.h>
 #include <stdint.h>
 
-#include <fuchsia/deprecatedtimezone/cpp/fidl.h>
+#include <fuchsia/intl/cpp/fidl.h>
 #include <lib/inspect/cpp/inspect.h>
 #include <lib/sys/cpp/component_context.h>
 #include <lib/sys/cpp/service_directory.h>
@@ -21,17 +21,31 @@
 #include <zircon/syscalls/object.h>
 #include <zircon/types.h>
 
+#include "third_party/icu/source/common/unicode/errorcode.h"
+#include "third_party/icu/source/i18n/unicode/timezone.h"
+
 #include "platform/assert.h"
+#include "platform/syslog.h"
 #include "platform/utils.h"
 #include "vm/zone.h"
 
 namespace {
 
+static constexpr int32_t kMsPerSec = 1000;
+
 // The data directory containing ICU timezone data files.
 static constexpr char kICUTZDataDir[] = "/config/data/tzdata/icu/44/le";
 
+// This is the general OK status.
+static constexpr int32_t kOk = 0;
+
+// This status means that the error code is not initialized yet ("set" was not
+// yet called).  Error codes are usually either 0 (kOk), or negative.
+static constexpr int32_t kUninitialized = 1;
+
 // The status codes for tzdata file open and read.
 enum class TZDataStatus {
+  // The operation completed without error.
   OK = 0,
   // The open call for the tzdata file did not succeed.
   COULD_NOT_OPEN = -1,
@@ -41,16 +55,23 @@ enum class TZDataStatus {
 
 // Adds a facility for introspecting timezone data errors.  Allows insight into
 // the internal state of the VM even if error reporting facilities fail.
+//
+// Under normal operation, all metric values below should be zero.
 class InspectMetrics {
  public:
   // Does not take ownership of inspector.
   explicit InspectMetrics(inspect::Inspector* inspector)
       : inspector_(inspector),
         root_(inspector_->GetRoot()),
         metrics_(root_.CreateChild("os")),
-        dst_status_(metrics_.CreateInt("dst_status", 0)),
-        tz_data_status_(metrics_.CreateInt("tz_data_status", 0)),
-        tz_data_close_status_(metrics_.CreateInt("tz_data_close_status", 0)) {}
+        dst_status_(metrics_.CreateInt("dst_status", kUninitialized)),
+        tz_data_status_(metrics_.CreateInt("tz_data_status", kUninitialized)),
+        tz_data_close_status_(
+            metrics_.CreateInt("tz_data_close_status", kUninitialized)),
+        get_profile_status_(
+            metrics_.CreateInt("get_profile_status", kUninitialized)),
+        profiles_timezone_content_status_(
+            metrics_.CreateInt("timezone_content_status", kOk)) {}
 
   // Sets the last status code for DST offset calls.
   void SetDSTOffsetStatus(zx_status_t status) {
@@ -64,6 +85,17 @@ class InspectMetrics {
     tz_data_close_status_.Set(status);
   }
 
+  // Sets the last status code for the call to PropertyProvider::GetProfile.
+  void SetProfileStatus(zx_status_t status) {
+    get_profile_status_.Set(static_cast<int32_t>(status));
+  }
+
+  // Sets the last status seen while examining timezones returned from
+  // PropertyProvider::GetProfile.
+  void SetTimeZoneContentStatus(zx_status_t status) {
+    profiles_timezone_content_status_.Set(static_cast<int32_t>(status));
+  }
+
  private:
   // The inspector that all metrics are being reported into.
   inspect::Inspector* inspector_;
@@ -82,6 +114,15 @@ class InspectMetrics {
 
   // The return code for the close() call for tzdata files.
   inspect::IntProperty tz_data_close_status_;
+
+  // The return code of the GetProfile call in GetTimeZoneName.  If this is
+  // nonzero, then os_fuchsia.cc reported a default timezone as a fallback.
+  inspect::IntProperty get_profile_status_;
+
+  // U_ILLEGAL_ARGUMENT_ERROR(=1) if timezones read from ProfileProvider were
+  // incorrect. Otherwise 0.  If this metric reports U_ILLEGAL_ARGUMENT_ERROR,
+  // the os_fuchsia.cc module reported a default timezone as a fallback.
+  inspect::IntProperty profiles_timezone_content_status_;
 };
 
 // Initialized on OS:Init(), deinitialized on OS::Cleanup.
@@ -132,50 +173,85 @@ intptr_t OS::ProcessId() {
   return static_cast<intptr_t>(getpid());
 }
 
+// This is the default timezone returned if it could not be obtained.  For
+// Fuchsia, the default device timezone is always UTC.
+static const char kDefaultTimezone[] = "UTC";
+
 // TODO(FL-98): Change this to talk to fuchsia.dart to get timezone service to
 // directly get timezone.
 //
 // Putting this hack right now due to CP-120 as I need to remove
 // component:ConnectToEnvironmentServices and this is the only thing that is
 // blocking it and FL-98 will take time.
-static fuchsia::deprecatedtimezone::TimezoneSyncPtr tz;
+static fuchsia::intl::PropertyProviderSyncPtr property_provider;
 
 static zx_status_t GetLocalAndDstOffsetInSeconds(int64_t seconds_since_epoch,
                                                  int32_t* local_offset,
                                                  int32_t* dst_offset) {
-  zx_status_t status = tz->GetTimezoneOffsetMinutes(seconds_since_epoch * 1000,
-                                                    local_offset, dst_offset);
-  metrics->SetDSTOffsetStatus(status);
-  if (status != ZX_OK) {
-    return status;
+  const char* timezone_id = OS::GetTimeZoneName(seconds_since_epoch);
+  std::unique_ptr<icu::TimeZone> timezone(
+      icu::TimeZone::createTimeZone(timezone_id));
+  UErrorCode error = U_ZERO_ERROR;
+  const auto ms_since_epoch =
+      static_cast<UDate>(kMsPerSec * seconds_since_epoch);
+  // The units of time that local_offset and dst_offset are returned from this
+  // function is, usefully, not documented, but it seems that the units are
+  // milliseconds.  Add these variables here for clarity.
+  int32_t local_offset_ms = 0;
+  int32_t dst_offset_ms = 0;
+  timezone->getOffset(ms_since_epoch, /*local_time=*/false, local_offset_ms,
+                      dst_offset_ms, error);
+  metrics->SetDSTOffsetStatus(error);
+  if (error != U_ZERO_ERROR) {
+    icu::ErrorCode icu_error;
+    icu_error.set(error);
+    Syslog::PrintErr("could not get DST offset: %s\n", icu_error.errorName());
+    return ZX_ERR_INTERNAL;
   }
-  *local_offset *= 60;
-  *dst_offset *= 60;
+  // We must return offset in seconds, so convert.
+  *local_offset = local_offset_ms / kMsPerSec;
+  *dst_offset = dst_offset_ms / kMsPerSec;
   return ZX_OK;
 }
 
 const char* OS::GetTimeZoneName(int64_t seconds_since_epoch) {
   // TODO(abarth): Handle time zone changes.
-  static const auto* tz_name = new std::string([] {
-    std::string result;
-    tz->GetTimezoneId(&result);
-    return result;
-  }());
+  static const std::unique_ptr<std::string> tz_name =
+      std::make_unique<std::string>([]() -> std::string {
+        fuchsia::intl::Profile profile;
+        const zx_status_t status = property_provider->GetProfile(&profile);
+        metrics->SetProfileStatus(status);
+        if (status != ZX_OK) {
+          return kDefaultTimezone;
+        }
+        const std::vector<fuchsia::intl::TimeZoneId>& timezones =
+            profile.time_zones();
+        if (timezones.empty()) {
+          metrics->SetTimeZoneContentStatus(U_ILLEGAL_ARGUMENT_ERROR);
+          // Empty timezone array is not up to fuchsia::intl spec.  The serving
+          // endpoint is broken and should be fixed.
+          Syslog::PrintErr("got empty timezone value\n");
+          return kDefaultTimezone;
+        }
+        return timezones[0].id;
+      }());
   return tz_name->c_str();
 }
 
 int OS::GetTimeZoneOffsetInSeconds(int64_t seconds_since_epoch) {
-  int32_t local_offset, dst_offset;
-  zx_status_t status = GetLocalAndDstOffsetInSeconds(
+  int32_t local_offset = 0;
+  int32_t dst_offset = 0;
+  const zx_status_t status = GetLocalAndDstOffsetInSeconds(
       seconds_since_epoch, &local_offset, &dst_offset);
   return status == ZX_OK ? local_offset + dst_offset : 0;
 }
 
 int OS::GetLocalTimeZoneAdjustmentInSeconds() {
-  int32_t local_offset, dst_offset;
   zx_time_t now = 0;
   zx_clock_get(ZX_CLOCK_UTC, &now);
-  zx_status_t status = GetLocalAndDstOffsetInSeconds(
+  int32_t local_offset = 0;
+  int32_t dst_offset = 0;
+  const zx_status_t status = GetLocalAndDstOffsetInSeconds(
       now / ZX_SEC(1), &local_offset, &dst_offset);
   return status == ZX_OK ? local_offset : 0;
 }
@@ -199,7 +275,7 @@ int64_t OS::GetCurrentMonotonicFrequency() {
 }
 
 int64_t OS::GetCurrentMonotonicMicros() {
-  int64_t ticks = GetCurrentMonotonicTicks();
+  const int64_t ticks = GetCurrentMonotonicTicks();
   ASSERT(GetCurrentMonotonicFrequency() == kNanosecondsPerSecond);
   return ticks / kNanosecondsPerMicrosecond;
 }
@@ -347,7 +423,8 @@ void OS::Init() {
   metrics = std::make_unique<InspectMetrics>(component_inspector->inspector());
 
   InitializeTZData();
-  context->svc()->Connect(tz.NewRequest());
+  auto services = sys::ServiceDirectory::CreateFromNamespace();
+  services->Connect(property_provider.NewRequest());
 }
 
 void OS::Cleanup() {