From cceb0bcb7358464a4a1c51731c3dc9b4c38c37b9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Thu, 7 Nov 2019 11:19:23 +0000
Subject: [PATCH] style: Simplify code for keeping alive shared memory until
 all sheets go away.

The existing code wasn't sound, as CSSOM objects also needed to go away before
the shared memory goes away (as they keep references to them).

This is sound assuming no presence of reference cycles introduced by CSSOM.

We may want to live with this and rely on chrome code not writing cycles like
this with UA stylesheet DOM objects.

We could explicitly drop all potentially-static objects... That seems pretty
error prone though.

Or we could also just leak the shared memory buffer, is there any reason why we
may not want to do that?

Differential Revision: https://phabricator.services.mozilla.com/D51870
---
 components/servo_arc/lib.rs                | 13 +++++++++----
 components/style/stylesheets/stylesheet.rs |  4 ++++
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/components/servo_arc/lib.rs b/components/servo_arc/lib.rs
index ad4489c10bc..bd6b5289504 100644
--- a/components/servo_arc/lib.rs
+++ b/components/servo_arc/lib.rs
@@ -485,6 +485,14 @@ impl<T: ?Sized> Arc<T> {
         }
     }
 
+    /// Whether or not the `Arc` is a static reference.
+    #[inline]
+    pub fn is_static(&self) -> bool {
+        // Using a relaxed ordering to check for STATIC_REFCOUNT is safe, since
+        // `count` never changes between STATIC_REFCOUNT and other values.
+        self.inner().count.load(Relaxed) == STATIC_REFCOUNT
+    }
+
     /// Whether or not the `Arc` is uniquely owned (is the refcount 1?) and not
     /// a static reference.
     #[inline]
@@ -501,10 +509,7 @@ impl<T: ?Sized> Drop for Arc<T> {
     fn drop(&mut self) {
         // NOTE(emilio): If you change anything here, make sure that the
         // implementation in layout/style/ServoStyleConstsInlines.h matches!
-        //
-        // Using a relaxed ordering to check for STATIC_REFCOUNT is safe, since
-        // `count` never changes between STATIC_REFCOUNT and other values.
-        if self.inner().count.load(Relaxed) == STATIC_REFCOUNT {
+        if self.is_static() {
             return;
         }
 
diff --git a/components/style/stylesheets/stylesheet.rs b/components/style/stylesheets/stylesheet.rs
index 15c60402be4..f5194d7d443 100644
--- a/components/style/stylesheets/stylesheet.rs
+++ b/components/style/stylesheets/stylesheet.rs
@@ -124,6 +124,7 @@ impl StylesheetContents {
         url_data: UrlExtraData,
         quirks_mode: QuirksMode,
     ) -> Self {
+        debug_assert!(rules.is_static());
         Self {
             rules,
             origin,
@@ -144,6 +145,9 @@ impl StylesheetContents {
     /// Measure heap usage.
     #[cfg(feature = "gecko")]
     pub fn size_of(&self, guard: &SharedRwLockReadGuard, ops: &mut MallocSizeOfOps) -> usize {
+        if self.rules.is_static() {
+            return 0;
+        }
         // Measurement of other fields may be added later.
         self.rules.unconditional_shallow_size_of(ops) +
             self.rules.read_with(guard).size_of(guard, ops)