Revert "Reland "[vm] Replaces fuchsia.deprecatedtimezone""

This reverts commit e3a6824.

Reason for revert: Broke time reporting on new
devices.

Original change's description:
> 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]>

[email protected],[email protected],[email protected]

Change-Id: I6e590cf22347f9153e5203b255f37872dbd91fa6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/155505
Commit-Queue: Filip Filmar <[email protected]>
Reviewed-by: 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.intl",
+        "$fuchsia_sdk_root/fidl/fuchsia.deprecatedtimezone",
         "$fuchsia_sdk_root/pkg/async",
         "$fuchsia_sdk_root/pkg/async-default",
         "$fuchsia_sdk_root/pkg/async-loop",
@@ -87,7 +87,7 @@ library_for_all_configs("libdart_vm") {
       ]
     } else if (using_fuchsia_sdk) {
       extra_deps = [
-        "$fuchsia_sdk_root/fidl:fuchsia.intl",
+        "$fuchsia_sdk_root/fidl:fuchsia.deprecatedtimezone",
         "$fuchsia_sdk_root/pkg:async-loop",
         "$fuchsia_sdk_root/pkg:async-loop-default",
         "$fuchsia_sdk_root/pkg:inspect",
@@ -98,7 +98,9 @@ library_for_all_configs("libdart_vm") {
       ]
     } else {
       extra_deps = [
-        "//sdk/fidl/fuchsia.intl",
+        # TODO(US-399): Remove time_service specific code when it is no longer
+        # necessary.
+        "//sdk/fidl/fuchsia.deprecatedtimezone",
         "//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/intl/cpp/fidl.h>
+#include <fuchsia/deprecatedtimezone/cpp/fidl.h>
 #include <lib/async/default.h>
 #include <lib/async-loop/loop.h>
 #include <lib/async-loop/default.h>
@@ -24,31 +24,17 @@
 #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,
@@ -58,23 +44,16 @@ 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", 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)) {}
+        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)) {}
 
   // Sets the last status code for DST offset calls.
   void SetDSTOffsetStatus(zx_status_t status) {
@@ -88,17 +67,6 @@ 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_;
@@ -117,15 +85,6 @@ 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.
@@ -177,85 +136,50 @@ 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::intl::PropertyProviderSyncPtr property_provider;
+static fuchsia::deprecatedtimezone::TimezoneSyncPtr tz;
 
 static zx_status_t GetLocalAndDstOffsetInSeconds(int64_t seconds_since_epoch,
                                                  int32_t* local_offset,
                                                  int32_t* dst_offset) {
-  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;
+  zx_status_t status = tz->GetTimezoneOffsetMinutes(seconds_since_epoch * 1000,
+                                                    local_offset, dst_offset);
+  metrics->SetDSTOffsetStatus(status);
+  if (status != ZX_OK) {
+    return status;
   }
-  // We must return offset in seconds, so convert.
-  *local_offset = local_offset_ms / kMsPerSec;
-  *dst_offset = dst_offset_ms / kMsPerSec;
+  *local_offset *= 60;
+  *dst_offset *= 60;
   return ZX_OK;
 }
 
 const char* OS::GetTimeZoneName(int64_t seconds_since_epoch) {
   // TODO(abarth): Handle time zone changes.
-  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;
-      }());
+  static const auto* tz_name = new std::string([] {
+    std::string result;
+    tz->GetTimezoneId(&result);
+    return result;
+  }());
   return tz_name->c_str();
 }
 
 int OS::GetTimeZoneOffsetInSeconds(int64_t seconds_since_epoch) {
-  int32_t local_offset = 0;
-  int32_t dst_offset = 0;
-  const zx_status_t status = GetLocalAndDstOffsetInSeconds(
+  int32_t local_offset, dst_offset;
+  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);
-  int32_t local_offset = 0;
-  int32_t dst_offset = 0;
-  const zx_status_t status = GetLocalAndDstOffsetInSeconds(
+  zx_status_t status = GetLocalAndDstOffsetInSeconds(
       now / ZX_SEC(1), &local_offset, &dst_offset);
   return status == ZX_OK ? local_offset : 0;
 }
@@ -279,7 +203,7 @@ int64_t OS::GetCurrentMonotonicFrequency() {
 }
 
 int64_t OS::GetCurrentMonotonicMicros() {
-  const int64_t ticks = GetCurrentMonotonicTicks();
+  int64_t ticks = GetCurrentMonotonicTicks();
   ASSERT(GetCurrentMonotonicFrequency() == kNanosecondsPerSecond);
   return ticks / kNanosecondsPerMicrosecond;
 }
@@ -433,8 +357,7 @@ void OS::Init() {
   metrics = std::make_unique<InspectMetrics>(component_inspector->inspector());
 
   InitializeTZData();
-  auto services = sys::ServiceDirectory::CreateFromNamespace();
-  services->Connect(property_provider.NewRequest());
+  context->svc()->Connect(tz.NewRequest());
 }
 
 void OS::Cleanup() {