Stop using time@0.1 in Servo (#33394)

This removes the last few uses of `time@0.1` in Servo. There are still
dependencies from `style` and `webrender`, but they will be removed soon
as well. The uses of this version of `time` are replaced with
`std::time` types and `time@0.3` when negative `Duration` is necessary.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>

Cargo.lock

@@ -915,7 +915,6 @@ dependencies = [
  "servo_url",
  "style_traits",
  "surfman",
- "time 0.1.45",
  "tracing",
  "webrender",
  "webrender_api",
@@ -1352,7 +1351,6 @@ dependencies = [
  "malloc_size_of_derive",
  "serde",
  "servo_url",
- "time 0.1.45",
  "uuid",
 ]
 
@@ -4281,7 +4279,6 @@ dependencies = [
  "metrics",
  "profile_traits",
  "servo_url",
- "time 0.1.45",
 ]
 
 [[package]]
@@ -4560,7 +4557,7 @@ dependencies = [
  "servo_config",
  "servo_url",
  "sha2",
- "time 0.1.45",
+ "time 0.3.36",
  "tokio",
  "tokio-rustls",
  "tokio-stream",
@@ -5789,7 +5786,6 @@ dependencies = [
  "swapper",
  "tempfile",
  "tendril",
- "time 0.1.45",
  "time 0.3.36",
  "unicode-bidi",
  "unicode-segmentation",

Cargo.toml

@@ -122,8 +122,7 @@ surfman = { version = "0.9.8", features = ["chains"] }
 syn = { version = "2", default-features = false, features = ["clone-impls", "derive", "parsing"] }
 synstructure = "0.13"
 thin-vec = "0.2.13"
-time = "0.1.41"
-time_03 = { package = "time", version = "0.3", features = ["large-dates", "serde"] }
+time_03 = { package = "time", version = "0.3", features = ["large-dates", "local-offset", "serde"] }
 to_shmem = { git = "https://github.com/servo/stylo", branch = "2024-07-16" }
 tokio = "1"
 tokio-rustls = "0.24"

components/compositing/Cargo.toml

@@ -40,7 +40,6 @@ servo-media = { workspace = true }
 servo_geometry = { path = "../geometry" }
 servo_url = { path = "../url" }
 style_traits = { workspace = true }
-time = { workspace = true }
 tracing = { workspace = true }
 webrender = { workspace = true }
 webrender_api = { workspace = true }

components/devtools/actor.rs

@@ -9,7 +9,7 @@ use std::mem;
 use std::net::TcpStream;
 use std::sync::{Arc, Mutex};
 
-use devtools_traits::PreciseTime;
+use base::cross_process_instant::CrossProcessInstant;
 use log::{debug, warn};
 use serde_json::{Map, Value};
 
@@ -60,7 +60,7 @@ pub struct ActorRegistry {
     script_actors: RefCell<HashMap<String, String>>,
     shareable: Option<Arc<Mutex<ActorRegistry>>>,
     next: Cell<u32>,
-    start_stamp: PreciseTime,
+    start_stamp: CrossProcessInstant,
 }
 
 impl ActorRegistry {
@@ -73,7 +73,7 @@ impl ActorRegistry {
             script_actors: RefCell::new(HashMap::new()),
             shareable: None,
             next: Cell::new(0),
-            start_stamp: PreciseTime::now(),
+            start_stamp: CrossProcessInstant::now(),
         }
     }
 
@@ -104,7 +104,7 @@ impl ActorRegistry {
     }
 
     /// Get start stamp when registry was started
-    pub fn start_stamp(&self) -> PreciseTime {
+    pub fn start_stamp(&self) -> CrossProcessInstant {
         self.start_stamp
     }
 

components/devtools/actors/timeline.rs

@@ -12,9 +12,10 @@ use std::sync::{Arc, Mutex};
 use std::thread;
 use std::time::Duration;
 
+use base::cross_process_instant::CrossProcessInstant;
 use base::id::PipelineId;
 use devtools_traits::DevtoolScriptControlMsg::{DropTimelineMarkers, SetTimelineMarkers};
-use devtools_traits::{DevtoolScriptControlMsg, PreciseTime, TimelineMarker, TimelineMarkerType};
+use devtools_traits::{DevtoolScriptControlMsg, TimelineMarker, TimelineMarkerType};
 use ipc_channel::ipc::{self, IpcReceiver, IpcSender};
 use serde::{Serialize, Serializer};
 use serde_json::{Map, Value};
@@ -41,7 +42,7 @@ struct Emitter {
     from: String,
     stream: TcpStream,
     registry: Arc<Mutex<ActorRegistry>>,
-    start_stamp: PreciseTime,
+    start_stamp: CrossProcessInstant,
 
     framerate_actor: Option<String>,
     memory_actor: Option<String>,
@@ -110,8 +111,8 @@ struct FramerateEmitterReply {
 pub struct HighResolutionStamp(f64);
 
 impl HighResolutionStamp {
-    pub fn new(start_stamp: PreciseTime, time: PreciseTime) -> HighResolutionStamp {
-        let duration = start_stamp.to(time).as_micros();
+    pub fn new(start_stamp: CrossProcessInstant, time: CrossProcessInstant) -> HighResolutionStamp {
+        let duration = (time - start_stamp).whole_microseconds();
         HighResolutionStamp(duration as f64 / 1000_f64)
     }
 
@@ -242,7 +243,10 @@ impl Actor for TimelineActor {
 
                 let msg = StartReply {
                     from: self.name(),
-                    value: HighResolutionStamp::new(registry.start_stamp(), PreciseTime::now()),
+                    value: HighResolutionStamp::new(
+                        registry.start_stamp(),
+                        CrossProcessInstant::now(),
+                    ),
                 };
                 let _ = stream.write_json_packet(&msg);
                 ActorMessageStatus::Processed
@@ -251,7 +255,10 @@ impl Actor for TimelineActor {
             "stop" => {
                 let msg = StopReply {
                     from: self.name(),
-                    value: HighResolutionStamp::new(registry.start_stamp(), PreciseTime::now()),
+                    value: HighResolutionStamp::new(
+                        registry.start_stamp(),
+                        CrossProcessInstant::now(),
+                    ),
                 };
 
                 let _ = stream.write_json_packet(&msg);
@@ -295,7 +302,7 @@ impl Emitter {
     pub fn new(
         name: String,
         registry: Arc<Mutex<ActorRegistry>>,
-        start_stamp: PreciseTime,
+        start_stamp: CrossProcessInstant,
         stream: TcpStream,
         memory_actor_name: Option<String>,
         framerate_actor_name: Option<String>,
@@ -322,7 +329,7 @@ impl Emitter {
     }
 
     fn send(&mut self, markers: Vec<TimelineMarkerReply>) -> Result<(), Box<dyn Error>> {
-        let end_time = PreciseTime::now();
+        let end_time = CrossProcessInstant::now();
         let reply = MarkersEmitterReply {
             type_: "markers".to_owned(),
             markers,

components/hyper_serde/README.md

@@ -19,7 +19,6 @@ The supported types are:
 * `hyper::method::Method`
 * `hyper::Uri`
 * `mime::Mime`
-* `time::Tm`
 
 For more details, see the crate documentation.
 

components/net/Cargo.toml

@@ -56,8 +56,8 @@ servo_arc = { workspace = true }
 servo_config = { path = "../config" }
 servo_url = { path = "../url" }
 sha2 = "0.10"
-time = { workspace = true }
 chrono = { workspace = true }
+time_03 = { workspace = true }
 tokio = { workspace = true, features = ["sync", "macros", "rt-multi-thread"] }
 tokio-rustls = { workspace = true }
 tokio-stream = "0.1"

components/net/filemanager_thread.rs

@@ -57,8 +57,6 @@ struct FileStoreEntry {
 #[derive(Clone)]
 struct FileMetaData {
     path: PathBuf,
-    /// Modified time in UNIX Epoch format
-    _modified: u64,
     size: u64,
 }
 
@@ -639,11 +637,6 @@ impl FileManagerStore {
         let modified = metadata
             .modified()
             .map_err(|e| FileSystemError(e.to_string()))?;
-        let elapsed = modified
-            .elapsed()
-            .map_err(|e| FileSystemError(e.to_string()))?;
-        // Unix Epoch: https://doc.servo.org/std/time/constant.UNIX_EPOCH.html
-        let modified_epoch = elapsed.as_secs() * 1000 + elapsed.subsec_nanos() as u64 / 1000000;
         let file_size = metadata.len();
         let file_name = file_path
             .file_name()
@@ -651,7 +644,6 @@ impl FileManagerStore {
 
         let file_impl = FileImpl::MetaDataOnly(FileMetaData {
             path: file_path.to_path_buf(),
-            _modified: modified_epoch,
             size: file_size,
         });
 
@@ -678,7 +670,7 @@ impl FileManagerStore {
         Ok(SelectedFile {
             id,
             filename: filename_path.to_path_buf(),
-            modified: modified_epoch,
+            modified,
             size: file_size,
             type_string,
         })

components/net/hsts.rs

@@ -4,7 +4,9 @@
 
 use std::collections::HashMap;
 use std::net::{Ipv4Addr, Ipv6Addr};
+use std::time::Duration;
 
+use base::cross_process_instant::CrossProcessInstant;
 use embedder_traits::resources::{self, Resource};
 use headers::{HeaderMapExt, StrictTransportSecurity};
 use http::HeaderMap;
@@ -19,15 +21,15 @@ use servo_url::{Host, ServoUrl};
 pub struct HstsEntry {
     pub host: String,
     pub include_subdomains: bool,
-    pub max_age: Option<u64>,
-    pub timestamp: Option<u64>,
+    pub max_age: Option<Duration>,
+    pub timestamp: Option<CrossProcessInstant>,
 }
 
 impl HstsEntry {
     pub fn new(
         host: String,
         subdomains: IncludeSubdomains,
-        max_age: Option<u64>,
+        max_age: Option<Duration>,
     ) -> Option<HstsEntry> {
         if host.parse::<Ipv4Addr>().is_ok() || host.parse::<Ipv6Addr>().is_ok() {
             None
@@ -36,16 +38,14 @@ impl HstsEntry {
                 host,
                 include_subdomains: (subdomains == IncludeSubdomains::Included),
                 max_age,
-                timestamp: Some(time::get_time().sec as u64),
+                timestamp: Some(CrossProcessInstant::now()),
             })
         }
     }
 
     pub fn is_expired(&self) -> bool {
         match (self.max_age, self.timestamp) {
-            (Some(max_age), Some(timestamp)) => {
-                (time::get_time().sec as u64) - timestamp >= max_age
-            },
+            (Some(max_age), Some(timestamp)) => CrossProcessInstant::now() - timestamp >= max_age,
 
             _ => false,
         }
@@ -187,11 +187,9 @@ impl HstsList {
                     IncludeSubdomains::NotIncluded
                 };
 
-                if let Some(entry) = HstsEntry::new(
-                    host.to_owned(),
-                    include_subdomains,
-                    Some(header.max_age().as_secs()),
-                ) {
+                if let Some(entry) =
+                    HstsEntry::new(host.to_owned(), include_subdomains, Some(header.max_age()))
+                {
                     info!("adding host {} to the strict transport security list", host);
                     info!("- max-age {}", header.max_age().as_secs());
                     if header.include_subdomains() {

components/net/tests/hsts.rs

@@ -3,16 +3,19 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 use std::collections::HashMap;
+use std::time::Duration as StdDuration;
 
+use base::cross_process_instant::CrossProcessInstant;
 use net::hsts::{HstsEntry, HstsList};
 use net_traits::IncludeSubdomains;
+use time_03::Duration;
 
 #[test]
 fn test_hsts_entry_is_not_expired_when_it_has_no_timestamp() {
     let entry = HstsEntry {
         host: "mozilla.org".to_owned(),
         include_subdomains: false,
-        max_age: Some(20),
+        max_age: Some(StdDuration::from_secs(20)),
         timestamp: None,
     };
 
@@ -25,7 +28,7 @@ fn test_hsts_entry_is_not_expired_when_it_has_no_max_age() {
         host: "mozilla.org".to_owned(),
         include_subdomains: false,
         max_age: None,
-        timestamp: Some(time::get_time().sec as u64),
+        timestamp: Some(CrossProcessInstant::now()),
     };
 
     assert!(!entry.is_expired());
@@ -36,8 +39,8 @@ fn test_hsts_entry_is_expired_when_it_has_reached_its_max_age() {
     let entry = HstsEntry {
         host: "mozilla.org".to_owned(),
         include_subdomains: false,
-        max_age: Some(10),
-        timestamp: Some(time::get_time().sec as u64 - 20u64),
+        max_age: Some(StdDuration::from_secs(10)),
+        timestamp: Some(CrossProcessInstant::now() - Duration::seconds(20)),
     };
 
     assert!(entry.is_expired());
@@ -106,7 +109,7 @@ fn test_push_entry_with_0_max_age_evicts_entry_from_list() {
         vec![HstsEntry::new(
             "mozilla.org".to_owned(),
             IncludeSubdomains::NotIncluded,
-            Some(500000u64),
+            Some(StdDuration::from_secs(500000)),
         )
         .unwrap()],
     );
@@ -118,7 +121,7 @@ fn test_push_entry_with_0_max_age_evicts_entry_from_list() {
         HstsEntry::new(
             "mozilla.org".to_owned(),
             IncludeSubdomains::NotIncluded,
-            Some(0),
+            Some(StdDuration::ZERO),
         )
         .unwrap(),
     );
@@ -367,8 +370,8 @@ fn test_hsts_list_with_expired_entry_is_not_is_host_secure() {
         vec![HstsEntry {
             host: "mozilla.org".to_owned(),
             include_subdomains: false,
-            max_age: Some(20),
-            timestamp: Some(time::get_time().sec as u64 - 100u64),
+            max_age: Some(StdDuration::from_secs(20)),
+            timestamp: Some(CrossProcessInstant::now() - Duration::seconds(100)),
         }],
     );
     let hsts_list = HstsList {

components/script/Cargo.toml

@@ -104,7 +104,6 @@ style_traits = { workspace = true }
 swapper = "0.1"
 tempfile = "3"
 tendril = { version = "0.4.1", features = ["encoding_rs"] }
-time = { workspace = true }
 time_03 = { workspace = true }
 unicode-bidi = { workspace = true }
 unicode-segmentation = { workspace = true }

components/script/dom/document.rs

@@ -17,6 +17,7 @@ use std::time::{Duration, Instant};
 use base::cross_process_instant::CrossProcessInstant;
 use base::id::BrowsingContextId;
 use canvas_traits::webgl::{self, WebGLContextId, WebGLMsg};
+use chrono::Local;
 use content_security_policy::{self as csp, CspList};
 use cookie::Cookie;
 use cssparser::match_ignore_ascii_case;
@@ -4625,15 +4626,14 @@ impl DocumentMethods for Document {
 
     // https://html.spec.whatwg.org/multipage/#dom-document-lastmodified
     fn LastModified(&self) -> DOMString {
-        match self.last_modified {
-            Some(ref t) => DOMString::from(t.clone()),
-            None => DOMString::from(
-                time::now()
-                    .strftime("%m/%d/%Y %H:%M:%S")
-                    .unwrap()
-                    .to_string(),
-            ),
-        }
+        DOMString::from(self.last_modified.as_ref().cloned().unwrap_or_else(|| {
+            // Ideally this would get the local time using `time`, but `time` always fails to get the local
+            // timezone on Unix unless the application is single threaded unless the library is explicitly
+            // set to "unsound" mode. Maybe that's fine, but it needs more investigation. see
+            // https://nvd.nist.gov/vuln/detail/CVE-2020-26235
+            // When `time` supports a thread-safe way of getting the local time zone we could use it here.
+            Local::now().format("%m/%d/%Y %H:%M:%S").to_string()
+        }))
     }
 
     // https://dom.spec.whatwg.org/#dom-document-createrange

components/script/dom/file.rs

@@ -2,10 +2,13 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
+use std::time::SystemTime;
+
 use dom_struct::dom_struct;
 use js::rust::HandleObject;
 use net_traits::filemanager_thread::SelectedFile;
 use script_traits::serializable::BlobImpl;
+use time_03::{Duration, OffsetDateTime};
 
 use crate::dom::bindings::codegen::Bindings::FileBinding;
 use crate::dom::bindings::codegen::Bindings::FileBinding::FileMethods;
@@ -24,23 +27,17 @@ use crate::script_runtime::CanGc;
 pub struct File {
     blob: Blob,
     name: DOMString,
-    modified: i64,
+    modified: SystemTime,
 }
 
 impl File {
     #[allow(crown::unrooted_must_root)]
-    fn new_inherited(blob_impl: &BlobImpl, name: DOMString, modified: Option<i64>) -> File {
+    fn new_inherited(blob_impl: &BlobImpl, name: DOMString, modified: Option<SystemTime>) -> File {
         File {
             blob: Blob::new_inherited(blob_impl),
             name,
             // https://w3c.github.io/FileAPI/#dfn-lastModified
-            modified: match modified {
-                Some(m) => m,
-                None => {
-                    let time = time::get_time();
-                    time.sec * 1000 + (time.nsec / 1000000) as i64
-                },
-            },
+            modified: modified.unwrap_or_else(SystemTime::now).into(),
         }
     }
 
@@ -48,7 +45,7 @@ impl File {
         global: &GlobalScope,
         blob_impl: BlobImpl,
         name: DOMString,
-        modified: Option<i64>,
+        modified: Option<SystemTime>,
     ) -> DomRoot<File> {
         Self::new_with_proto(global, None, blob_impl, name, modified, CanGc::note())
     }
@@ -59,7 +56,7 @@ impl File {
         proto: Option<HandleObject>,
         blob_impl: BlobImpl,
         name: DOMString,
-        modified: Option<i64>,
+        modified: Option<SystemTime>,
         can_gc: CanGc,
     ) -> DomRoot<File> {
         let file = reflect_dom_object_with_proto(
@@ -90,7 +87,7 @@ impl File {
                 normalize_type_string(&selected.type_string.to_string()),
             ),
             name,
-            Some(selected.modified as i64),
+            Some(selected.modified.into()),
         )
     }
 
@@ -110,8 +107,11 @@ impl File {
         };
 
         let blobPropertyBag = &filePropertyBag.parent;
+        let modified = filePropertyBag
+            .lastModified
+            .map(|modified| OffsetDateTime::UNIX_EPOCH + Duration::milliseconds(modified))
+            .map(Into::into);
 
-        let modified = filePropertyBag.lastModified;
         // NOTE: Following behaviour might be removed in future,
         // see https://github.com/w3c/FileAPI/issues/41
         let replaced_filename = DOMString::from_string(filename.replace('/', ":"));
@@ -139,6 +139,9 @@ impl FileMethods for File {
 
     // https://w3c.github.io/FileAPI/#dfn-lastModified
     fn LastModified(&self) -> i64 {
-        self.modified
+        // This is first converted to a `time::OffsetDateTime` because it might be from before the
+        // Unix epoch in which case we will need to return a negative duration to script.
+        (OffsetDateTime::from(self.modified) - OffsetDateTime::UNIX_EPOCH).whole_milliseconds()
+            as i64
     }
 }

components/shared/base/cross_process_instant.rs

@@ -61,6 +61,16 @@ impl Add<Duration> for CrossProcessInstant {
     }
 }
 
+impl Sub<Duration> for CrossProcessInstant {
+    type Output = Self;
+
+    fn sub(self, rhs: Duration) -> Self::Output {
+        Self {
+            value: self.value - rhs.whole_nanoseconds() as u64,
+        }
+    }
+}
+
 #[cfg(all(unix, not(any(target_os = "macos", target_os = "ios"))))]
 mod platform {
     use libc::timespec;

components/shared/devtools/Cargo.toml

@@ -19,5 +19,4 @@ malloc_size_of = { workspace = true }
 malloc_size_of_derive = { workspace = true }
 serde = { workspace = true }
 servo_url = { path = "../../url" }
-time = { workspace = true }
 uuid = { workspace = true, features = ["serde"] }

components/shared/devtools/lib.rs

@@ -14,6 +14,7 @@ use std::collections::HashMap;
 use std::net::TcpStream;
 use std::time::{Duration, SystemTime};
 
+use base::cross_process_instant::CrossProcessInstant;
 use base::id::{BrowsingContextId, PipelineId};
 use bitflags::bitflags;
 use http::{HeaderMap, Method};
@@ -134,16 +135,16 @@ pub struct NodeInfo {
 
 pub struct StartedTimelineMarker {
     name: String,
-    start_time: PreciseTime,
+    start_time: CrossProcessInstant,
     start_stack: Option<Vec<()>>,
 }
 
 #[derive(Debug, Deserialize, Serialize)]
 pub struct TimelineMarker {
     pub name: String,
-    pub start_time: PreciseTime,
+    pub start_time: CrossProcessInstant,
     pub start_stack: Option<Vec<()>>,
-    pub end_time: PreciseTime,
+    pub end_time: CrossProcessInstant,
     pub end_stack: Option<Vec<()>>,
 }
 
@@ -364,7 +365,7 @@ impl TimelineMarker {
     pub fn start(name: String) -> StartedTimelineMarker {
         StartedTimelineMarker {
             name,
-            start_time: PreciseTime::now(),
+            start_time: CrossProcessInstant::now(),
             start_stack: None,
         }
     }
@@ -376,31 +377,11 @@ impl StartedTimelineMarker {
             name: self.name,
             start_time: self.start_time,
             start_stack: self.start_stack,
-            end_time: PreciseTime::now(),
+            end_time: CrossProcessInstant::now(),
             end_stack: None,
         }
     }
 }
-
-/// A replacement for `time::PreciseTime` that isn't opaque, so we can serialize it.
-///
-/// The reason why this doesn't go upstream is that `time` is slated to be part of Rust's standard
-/// library, which definitely can't have any dependencies on `serde`. But `serde` can't implement
-/// `Deserialize` and `Serialize` itself, because `time::PreciseTime` is opaque! A Catch-22. So I'm
-/// duplicating the definition here.
-#[derive(Clone, Copy, Debug, Deserialize, Serialize)]
-pub struct PreciseTime(u64);
-
-impl PreciseTime {
-    pub fn now() -> PreciseTime {
-        PreciseTime(time::precise_time_ns())
-    }
-
-    pub fn to(&self, later: PreciseTime) -> Duration {
-        Duration::from_nanos(later.0 - self.0)
-    }
-}
-
 #[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, MallocSizeOf, PartialEq, Serialize)]
 pub struct WorkerId(pub Uuid);
 

components/shared/net/filemanager_thread.rs

@@ -5,6 +5,7 @@
 use std::cmp::{max, min};
 use std::ops::Range;
 use std::path::PathBuf;
+use std::time::SystemTime;
 
 use embedder_traits::FilterPattern;
 use ipc_channel::ipc::IpcSender;
@@ -115,7 +116,7 @@ impl RelativePos {
 pub struct SelectedFile {
     pub id: Uuid,
     pub filename: PathBuf,
-    pub modified: u64,
+    pub modified: SystemTime,
     pub size: u64,
     // https://w3c.github.io/FileAPI/#dfn-type
     pub type_string: String,

tests/unit/metrics/Cargo.toml

@@ -18,4 +18,3 @@ ipc-channel = { workspace = true }
 metrics = { path = "../../../components/metrics" }
 profile_traits = { workspace = true }
 servo_url = { path = "../../../components/url" }
-time = { workspace = true }