From ef02196dd8015d38fa5d30f0947be6d8e990b528 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fernando=20Jim=C3=A9nez=20Moreno?= <ferjmoreno@gmail.com>
Date: Tue, 18 Jun 2019 12:36:25 +0200
Subject: [PATCH] More detach shadow changes

---
 components/script/dom/document.rs         |  3 ---
 components/script/dom/element.rs          |  1 -
 components/script/dom/htmlmediaelement.rs |  4 ++--
 components/script/dom/shadowroot.rs       | 19 ++++++++++++++++---
 resources/media-controls.js               | 14 ++++++++++++++
 5 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs
index 20f29de2f42..5b17c8ea004 100644
--- a/components/script/dom/document.rs
+++ b/components/script/dom/document.rs
@@ -2478,9 +2478,6 @@ impl Document {
         if let Some(ref media_controls) = self.media_controls.borrow_mut().remove(id) {
             let media_controls = DomRoot::from_ref(&**media_controls);
             media_controls.Host().detach_shadow();
-            media_controls
-                .upcast::<Node>()
-                .dirty(NodeDamage::OtherNodeDamage);
         } else {
             debug_assert!(false, "Trying to unregister unknown media controls");
         }
diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs
index a2ce8d0678c..91d7a8aa450 100644
--- a/components/script/dom/element.rs
+++ b/components/script/dom/element.rs
@@ -505,7 +505,6 @@ impl Element {
     pub fn detach_shadow(&self) {
         if let Some(ref shadow_root) = self.shadow_root() {
             shadow_root.detach();
-            self.node.owner_doc().unregister_shadow_root(shadow_root);
             self.ensure_rare_data().shadow_root = None;
         } else {
             debug_assert!(false, "Trying to detach a non-attached shadow root");
diff --git a/components/script/dom/htmlmediaelement.rs b/components/script/dom/htmlmediaelement.rs
index 007e1c8d954..1b6f8ab1e3c 100644
--- a/components/script/dom/htmlmediaelement.rs
+++ b/components/script/dom/htmlmediaelement.rs
@@ -1787,8 +1787,8 @@ impl HTMLMediaElement {
     }
 
     fn remove_controls(&self) {
-        if let Some(ref id) = *self.media_controls_id.borrow() {
-            document_from_node(self).unregister_media_controls(id);
+        if let Some(id) = self.media_controls_id.borrow_mut().take() {
+            document_from_node(self).unregister_media_controls(&id);
         }
     }
 }
diff --git a/components/script/dom/shadowroot.rs b/components/script/dom/shadowroot.rs
index 11db6416f9f..d7fda201342 100644
--- a/components/script/dom/shadowroot.rs
+++ b/components/script/dom/shadowroot.rs
@@ -3,6 +3,7 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 use crate::dom::bindings::cell::DomRefCell;
+use crate::dom::bindings::codegen::Bindings::NodeBinding::NodeMethods;
 use crate::dom::bindings::codegen::Bindings::ShadowRootBinding::ShadowRootBinding::ShadowRootMethods;
 use crate::dom::bindings::codegen::Bindings::ShadowRootBinding::{self, ShadowRootMode};
 use crate::dom::bindings::inheritance::Castable;
@@ -78,13 +79,25 @@ impl ShadowRoot {
     }
 
     pub fn detach(&self) {
-        self.host.set(None);
+        self.document.unregister_shadow_root(&self);
         let node = self.upcast::<Node>();
         node.set_containing_shadow_root(None);
-        node.set_flag(NodeFlags::IS_CONNECTED, false);
         for child in node.traverse_preorder(ShadowIncluding::No) {
-            child.set_flag(NodeFlags::IS_CONNECTED, false);
+            if node.RemoveChild(&child).is_err() {
+                warn!("Could not remove shadow root child");
+            }
         }
+        if self
+            .host
+            .get()
+            .unwrap()
+            .upcast::<Node>()
+            .RemoveChild(&node)
+            .is_err()
+        {
+            warn!("Could not detach shadow root");
+        }
+        self.host.set(None);
     }
 
     pub fn get_focused_element(&self) -> Option<DomRoot<Element>> {
diff --git a/resources/media-controls.js b/resources/media-controls.js
index f921b40b0f5..bfb0fb5edf0 100644
--- a/resources/media-controls.js
+++ b/resources/media-controls.js
@@ -74,11 +74,22 @@
 
   class MediaControls {
     constructor() {
+      this.nonce = Date.now();
       // Get the instance of the shadow root where these controls live.
       this.controls = document.servoGetMediaControls("@@@id@@@");
       // Get the instance of the host of these controls.
       this.media = this.controls.host;
 
+      this.shutthingDown = false;
+      this.mutationObserver = new MutationObserver(() => {
+        // We can only get here if the `controls` attribute is removed.
+        this.shutthingDown = true;
+        this.mutationObserver.disconnect();
+      });
+      this.mutationObserver.observe(this.media, {
+        attributeFilter: ["controls"]
+      });
+
       // Create root element and load markup.
       this.root = document.createElement("div");
       this.root.classList.add("root");
@@ -203,6 +214,9 @@
     }
 
     render(from = this.state) {
+      if (this.shutthingDown) {
+        return;
+      }
       const isAudioOnly = this.media.localName == "audio";
       if (!isAudioOnly) {
         // XXX This should ideally use clientHeight/clientWidth,