Skip to content

Commit

Permalink
Reland "[vm] Replaces fuchsia.deprecatedtimezone"
Browse files Browse the repository at this point in the history 
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 <fmil@google.com>
> Reviewed-by: Martin Kustermann <kustermann@google.com>

Change-Id: I5da6b0f481af0eb42c3b5e74c920588ac2ef5be9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/151862
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Filip Filmar <fmil@google.com>
  • Loading branch information
@filmil
filmil authored and commit-bot@chromium.org committed Jul 9, 2020
1 parent 6f6b1f8 commit e3a6824
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 28 deletions.
8 changes: 3 additions & 5 deletions runtime/vm/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand Down
123 changes: 100 additions & 23 deletions runtime/vm/os_fuchsia.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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,
Expand All @@ -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) {
Expand All @@ -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_;
Expand All @@ -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.
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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() {
Expand Down

0 comments on commit e3a6824

Please sign in to comment.