0

Avoid time zone changes from TimeZoneMonitor in test

Fixes test failure when machine is not in "America/Los_Angeles" time
zone.

Before this CL, ScopedTimeZone was changing the ICU's time zone to
America/Los_Angeles, and was adding kTimeZoneForTesting to command-line
so time zone was propagated to newly spawned renderer processes.
But TimeZoneMonitor is platform-specific and reads the current time
zone from the OS. It sends that time zone to each its client (renderer
process) and it was replacing the testing time zone
(America/Los_Angeles) with the OS time zone in the renderer process.

This CL introduces FakeTimeZoneMonitor which doesn't read the time zone
from the OS. It doesn't change the current ICU's time zone but still
sends it to newly spawned renderer processes. This makes
kTimeZoneForTesting switch redundant.

ScopedTimeZone changes the time zone but now also overrides the
TimeZoneMonitor binder in DeviceService so FakeTimeZoneMonitor is
created instead of the real TimeZoneMonitor. DeviceService and
TimeZoneMonitor are created very early during the test setup and we
need to override binder before that happens. It's too late to override
TimeZoneMonitor binder in ViewerPropertiesDialog test case because the
real TimeZoneMonitor already exists at this point and
FakeTimeZoneMonitor won't be created and used.

TEST=All/PDFExtensionPacificTimeZoneJSTest.ViewerPropertiesDialog/*

Bug: 340983077
Change-Id: I499763675d879b764e848e5358813c111adce9e8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5542444
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Andy Phan <andyphan@chromium.org>
Commit-Queue: Tomasz Moniuszko <tmoniuszko@opera.com>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1305535}
This commit is contained in:
Tomasz Moniuszko
2024-05-24 06:42:07 +00:00
committed by Chromium LUCI CQ
parent fde33b7794
commit 3b61e51438
20 changed files with 107 additions and 54 deletions

@ -273,15 +273,6 @@ IN_PROC_BROWSER_TEST_P(PDFExtensionJSTest, Fullscreen) {
RunTestsInJsModule("fullscreen_test.js", "test-bookmarks.pdf");
}
IN_PROC_BROWSER_TEST_P(PDFExtensionJSTest, ViewerPropertiesDialog) {
// The properties dialog formats some values based on locale.
base::test::ScopedRestoreICUDefaultLocale scoped_locale{"en_US"};
// This will apply to the new processes spawned within RunTestsInJsModule(),
// thus consistently running the test in a well known time zone.
content::ScopedTimeZone scoped_time_zone{"America/Los_Angeles"};
RunTestsInJsModule("viewer_properties_dialog_test.js", "document_info.pdf");
}
IN_PROC_BROWSER_TEST_P(PDFExtensionJSTest, PostMessageProxy) {
// Although this test file does not require a PDF to be loaded, loading the
// elements without loading a PDF is difficult.
@ -330,6 +321,23 @@ IN_PROC_BROWSER_TEST_P(PDFExtensionJSTest, DISABLED_PdfOcrToolbar) {
}
#endif // BUILDFLAG(ENABLE_SCREEN_AI_SERVICE)
// PDFExtensionJSTest with forced Pacific Time Zone.
class PDFExtensionPacificTimeZoneJSTest : public PDFExtensionJSTest {
// This will apply to the new processes spawned within RunTestsInJsModule(),
// thus consistently running the test in a well known time zone.
// ScopedTimeZone needs to be created before the test setup. ScopedTimeZone
// overrides TimeZoneMonitor binder in DeviceService so the test setup creates
// FakeTimeZoneMonitor instead of the real TimeZoneMonitor implementation.
content::ScopedTimeZone scoped_time_zone_{"America/Los_Angeles"};
};
IN_PROC_BROWSER_TEST_P(PDFExtensionPacificTimeZoneJSTest,
ViewerPropertiesDialog) {
// The properties dialog formats some values based on locale.
base::test::ScopedRestoreICUDefaultLocale scoped_locale{"en_US"};
RunTestsInJsModule("viewer_properties_dialog_test.js", "document_info.pdf");
}
class PDFExtensionContentSettingJSTest : public PDFExtensionJSTest {
protected:
// When blocking JavaScript, block the exact query from pdf/main.js while
@ -485,6 +493,7 @@ IN_PROC_BROWSER_TEST_P(PDFExtensionJSInk2Test, Ink2AnnotationBar) {
// TODO(crbug.com/40268279): Stop testing both modes after OOPIF PDF viewer
// launches.
INSTANTIATE_FEATURE_OVERRIDE_TEST_SUITE(PDFExtensionJSTest);
INSTANTIATE_FEATURE_OVERRIDE_TEST_SUITE(PDFExtensionPacificTimeZoneJSTest);
INSTANTIATE_FEATURE_OVERRIDE_TEST_SUITE(PDFExtensionContentSettingJSTest);
INSTANTIATE_FEATURE_OVERRIDE_TEST_SUITE(PDFExtensionWebUICodeCacheJSTest);
INSTANTIATE_FEATURE_OVERRIDE_TEST_SUITE(PDFExtensionServiceWorkerJSTest);

@ -253,7 +253,6 @@ bool PpapiPluginProcessHost::Init(const ContentPluginInfo& info) {
sandbox::policy::switches::kEnableSandboxLogging,
#endif
switches::kPpapiStartupDialog,
switches::kTimeZoneForTesting,
};
cmd_line->CopySwitchesFrom(browser_command_line, kPluginForwardSwitches);

@ -3501,7 +3501,6 @@ void RenderProcessHostImpl::PropagateBrowserCommandLineToRenderer(
switches::kSkiaFontCacheLimitMb,
switches::kSkiaResourceCacheLimitMb,
switches::kTestType,
switches::kTimeZoneForTesting,
switches::kTouchEventFeatureDetection,
switches::kTraceToConsole,
switches::kUseCmdDecoder,

@ -337,7 +337,6 @@ bool UtilityProcessHost::StartProcess() {
switches::kUseFileForFakeVideoCapture,
switches::kUseMockCertVerifierForTesting,
switches::kMockCertVerifierDefaultResultForTesting,
switches::kTimeZoneForTesting,
switches::kUtilityStartupDialog,
switches::kUseANGLE,
switches::kUseGL,

@ -26,8 +26,6 @@
#include "ipc/ipc_sender.h"
#include "ppapi/proxy/plugin_globals.h"
#include "services/tracing/public/cpp/trace_startup.h"
#include "third_party/icu/source/common/unicode/unistr.h"
#include "third_party/icu/source/i18n/unicode/timezone.h"
#include "ui/base/ui_base_switches.h"
#if BUILDFLAG(IS_WIN)
@ -124,13 +122,6 @@ int PpapiPluginMain(MainFunctionParams parameters) {
#endif
}
if (command_line.HasSwitch(switches::kTimeZoneForTesting)) {
std::string time_zone =
command_line.GetSwitchValueASCII(switches::kTimeZoneForTesting);
icu::TimeZone::adoptDefault(
icu::TimeZone::createTimeZone(icu::UnicodeString(time_zone.c_str())));
}
#if BUILDFLAG(IS_CHROMEOS)
// Specifies $HOME explicitly because some plugins rely on $HOME but
// no other part of Chrome OS uses that. See crbug.com/335290.

@ -759,9 +759,6 @@ const char kSkiaResourceCacheLimitMb[] = "skia-resource-cache-limit-mb";
// Type of the current test harness ("browser" or "ui" or "gpu").
const char kTestType[] = "test-type";
// The time zone to use for testing. Passed to renderers and plugins on startup.
const char kTimeZoneForTesting[] = "time-zone-for-testing";
// Enable support for touch event feature detection.
const char kTouchEventFeatureDetection[] = "touch-events";

@ -206,7 +206,6 @@ extern const char kSkiaFontCacheLimitMb[];
extern const char kSkiaResourceCacheLimitMb[];
CONTENT_EXPORT extern const char kTestType[];
CONTENT_EXPORT extern const char kTimeTicksAtUnixEpoch[];
CONTENT_EXPORT extern const char kTimeZoneForTesting[];
CONTENT_EXPORT extern const char kTouchEventFeatureDetection[];
CONTENT_EXPORT extern const char kTouchEventFeatureDetectionAuto[];
CONTENT_EXPORT extern const char kTouchEventFeatureDetectionEnabled[];

@ -87,13 +87,19 @@ specific_include_rules = {
"mock_video_picture_in_picture_window_controller_impl.h": [
"+content/browser/picture_in_picture/video_picture_in_picture_window_controller_impl.h"
],
"scoped_time_zone.cc": [
"+services/device/device_service.h"
],
"scoped_time_zone.h": [
"+services/device/time_zone_monitor/fake_time_zone_monitor.h"
],
"shared_storage_test_utils.h": [
"+content/browser/private_aggregation/private_aggregation_host.h"
],
"test_image_transport_factory.h": [
"+content/browser/compositor/image_transport_factory.h"
],
"test_shared_storage_header_observer.h": [
"+content/browser/shared_storage/shared_storage_header_observer.h"
],
"shared_storage_test_utils.h": [
"+content/browser/private_aggregation/private_aggregation_host.h"
]
}

@ -4,17 +4,20 @@
#include "content/public/test/scoped_time_zone.h"
#include "base/command_line.h"
#include "content/public/common/content_switches.h"
#include "services/device/device_service.h"
namespace content {
ScopedTimeZone::ScopedTimeZone(const char* new_zoneid)
: icu_time_zone_(new_zoneid) {
base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
switches::kTimeZoneForTesting, new_zoneid);
device::DeviceService::OverrideTimeZoneMonitorBinderForTesting(
base::BindRepeating(&device::FakeTimeZoneMonitor::Bind,
base::Unretained(&time_zone_monitor_)));
}
ScopedTimeZone::~ScopedTimeZone() = default;
ScopedTimeZone::~ScopedTimeZone() {
device::DeviceService::OverrideTimeZoneMonitorBinderForTesting(
base::NullCallback());
}
} // namespace content

@ -6,7 +6,7 @@
#define CONTENT_PUBLIC_TEST_SCOPED_TIME_ZONE_H_
#include "base/test/icu_test_util.h"
#include "base/test/scoped_command_line.h"
#include "services/device/time_zone_monitor/fake_time_zone_monitor.h"
namespace content {
@ -20,8 +20,12 @@ class ScopedTimeZone {
~ScopedTimeZone();
private:
base::test::ScopedCommandLine command_line_;
// Must be created before TimeZoneMonitor. ScopedRestoreDefaultTimezone sets
// the new time zone in ICU which is then picked by TimeZoneMonitor
// constructor.
base::test::ScopedRestoreDefaultTimezone icu_time_zone_;
device::FakeTimeZoneMonitor time_zone_monitor_;
};
} // namespace content

@ -49,8 +49,6 @@
#include "services/tracing/public/cpp/trace_startup.h"
#include "third_party/blink/public/platform/platform.h"
#include "third_party/blink/public/platform/scheduler/web_thread_scheduler.h"
#include "third_party/icu/source/common/unicode/unistr.h"
#include "third_party/icu/source/i18n/unicode/timezone.h"
#include "third_party/webrtc_overrides/init_webrtc.h" // nogncheck
#include "ui/base/ui_base_switches.h"
@ -198,13 +196,6 @@ int RendererMain(MainFunctionParams parameters) {
#endif // defined(ARCH_CPU_X86_64)
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
if (command_line.HasSwitch(switches::kTimeZoneForTesting)) {
std::string time_zone =
command_line.GetSwitchValueASCII(switches::kTimeZoneForTesting);
icu::TimeZone::adoptDefault(
icu::TimeZone::createTimeZone(icu::UnicodeString(time_zone.c_str())));
}
InitializeSkia();
// This function allows pausing execution using the --renderer-startup-dialog

@ -585,8 +585,10 @@ static_library("test_support") {
"//services/cert_verifier:lib",
"//services/data_decoder/public/cpp:test_support",
"//services/data_decoder/public/mojom",
"//services/device:lib",
"//services/device/public/mojom",
"//services/device/public/mojom:usb",
"//services/device/time_zone_monitor:test_support",
"//services/metrics/public/cpp:metrics_cpp",
"//storage/browser/quota:mojo_bindings",

@ -35,8 +35,6 @@
#include "services/on_device_model/on_device_model_service.h"
#include "services/screen_ai/buildflags/buildflags.h"
#include "services/tracing/public/cpp/trace_startup.h"
#include "third_party/icu/source/common/unicode/unistr.h"
#include "third_party/icu/source/i18n/unicode/timezone.h"
#if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS)
#include "base/file_descriptor_store.h"
@ -225,13 +223,6 @@ int UtilityMain(MainFunctionParams parameters) {
message_pump_type = base::MessagePumpType::IO;
#endif // BUILDFLAG(IS_FUCHSIA)
if (parameters.command_line->HasSwitch(switches::kTimeZoneForTesting)) {
std::string time_zone = parameters.command_line->GetSwitchValueASCII(
switches::kTimeZoneForTesting);
icu::TimeZone::adoptDefault(
icu::TimeZone::createTimeZone(icu::UnicodeString(time_zone.c_str())));
}
// The main task executor of the utility process.
base::SingleThreadTaskExecutor main_thread_task_executor(message_pump_type);
const std::string utility_sub_type =

@ -23,6 +23,7 @@ source_set("lib") {
":test_support",
":tests",
"//content/browser",
"//content/test:test_support",
"//services/device/public/cpp:test_support",
]
sources = [

@ -20,6 +20,11 @@ PressureManagerBinder& GetPressureManagerBinderOverride() {
return *binder;
}
TimeZoneMonitorBinder& GetTimeZoneMonitorBinderOverride() {
static base::NoDestructor<TimeZoneMonitorBinder> binder;
return *binder;
}
#if BUILDFLAG(IS_ANDROID)
NFCProviderBinder& GetNFCProviderBinderOverride() {
static base::NoDestructor<NFCProviderBinder> binder;

@ -11,6 +11,7 @@
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "services/device/public/mojom/geolocation_context.mojom.h"
#include "services/device/public/mojom/pressure_manager.mojom.h"
#include "services/device/public/mojom/time_zone_monitor.mojom.h"
#if BUILDFLAG(IS_ANDROID)
#include "services/device/public/mojom/nfc_provider.mojom.h"
@ -29,6 +30,11 @@ using PressureManagerBinder = base::RepeatingCallback<void(
COMPONENT_EXPORT(DEVICE_SERVICE_BINDER_OVERRIDES)
PressureManagerBinder& GetPressureManagerBinderOverride();
using TimeZoneMonitorBinder = base::RepeatingCallback<void(
mojo::PendingReceiver<device::mojom::TimeZoneMonitor>)>;
COMPONENT_EXPORT(DEVICE_SERVICE_BINDER_OVERRIDES)
TimeZoneMonitorBinder& GetTimeZoneMonitorBinderOverride();
#if BUILDFLAG(IS_ANDROID)
using NFCProviderBinder = base::RepeatingCallback<void(
mojo::PendingReceiver<device::mojom::NFCProvider>)>;

@ -175,6 +175,12 @@ void DeviceService::OverridePressureManagerBinderForTesting(
internal::GetPressureManagerBinderOverride() = std::move(binder);
}
// static
void DeviceService::OverrideTimeZoneMonitorBinderForTesting(
TimeZoneMonitorBinder binder) {
internal::GetTimeZoneMonitorBinderOverride() = std::move(binder);
}
void DeviceService::BindBatteryMonitor(
mojo::PendingReceiver<mojom::BatteryMonitor> receiver) {
#if BUILDFLAG(IS_ANDROID)
@ -343,6 +349,12 @@ void DeviceService::BindSerialPortManager(
void DeviceService::BindTimeZoneMonitor(
mojo::PendingReceiver<mojom::TimeZoneMonitor> receiver) {
const auto& binder_override = internal::GetTimeZoneMonitorBinderOverride();
if (binder_override) {
binder_override.Run(std::move(receiver));
return;
}
if (!time_zone_monitor_)
time_zone_monitor_ = TimeZoneMonitor::Create(file_task_runner_);
time_zone_monitor_->Bind(std::move(receiver));

@ -137,6 +137,12 @@ class DeviceService : public mojom::DeviceService {
static void OverridePressureManagerBinderForTesting(
PressureManagerBinder binder);
// Supports global override of TimeZoneMonitor binding within the service.
using TimeZoneMonitorBinder = base::RepeatingCallback<void(
mojo::PendingReceiver<mojom::TimeZoneMonitor>)>;
static void OverrideTimeZoneMonitorBinderForTesting(
TimeZoneMonitorBinder binder);
#if BUILDFLAG(IS_ANDROID)
// Allows tests to override how frame hosts bind NFCProvider receivers.
using NFCProviderBinder = base::RepeatingCallback<void(

@ -11,7 +11,10 @@ if (is_android) {
}
source_set("time_zone_monitor") {
visibility = [ "//services/device:lib" ]
visibility = [
":test_support",
"//services/device:lib",
]
sources = [
"time_zone_monitor.cc",
@ -74,6 +77,14 @@ source_set("time_zone_monitor") {
}
}
source_set("test_support") {
testonly = true
sources = [ "fake_time_zone_monitor.h" ]
public_deps = [ ":time_zone_monitor" ]
}
if (is_android) {
generate_jni("time_zone_monitor_jni_headers") {
visibility = [ ":*" ]

@ -0,0 +1,22 @@
// Copyright 2024 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef SERVICES_DEVICE_TIME_ZONE_MONITOR_FAKE_TIME_ZONE_MONITOR_H_
#define SERVICES_DEVICE_TIME_ZONE_MONITOR_FAKE_TIME_ZONE_MONITOR_H_
#include "services/device/time_zone_monitor/time_zone_monitor.h"
namespace device {
// Empty TimeZoneMonitor implementation that doesn't observe time zone changes
// in the OS so it never changes ICU's time zone during its lifetime.
class FakeTimeZoneMonitor : public device::TimeZoneMonitor {
public:
FakeTimeZoneMonitor() = default;
~FakeTimeZoneMonitor() override = default;
};
} // namespace device
#endif // SERVICES_DEVICE_TIME_ZONE_MONITOR_FAKE_TIME_ZONE_MONITOR_H_