Document HTMLMediaElement fields and cleanup some algos

components/script/dom/htmlmediaelement.rs

@@ -45,14 +45,28 @@ use time::{self, Timespec, Duration};
 #[dom_struct]
 pub struct HTMLMediaElement {
     htmlelement: HTMLElement,
+    /// https://html.spec.whatwg.org/multipage/#dom-media-networkstate-2
+    // FIXME(nox): Use an enum.
     network_state: Cell<u16>,
+    /// https://html.spec.whatwg.org/multipage/#dom-media-readystate-2
+    // FIXME(nox): Use an enum.
     ready_state: Cell<u16>,
+    /// https://html.spec.whatwg.org/multipage/#dom-media-currentsrc-2
     current_src: DOMRefCell<String>,
+    // FIXME(nox): Document this one, I have no idea what it is used for.
     generation_id: Cell<u32>,
-    first_data_load: Cell<bool>,
+    /// https://html.spec.whatwg.org/multipage/#fire-loadeddata
+    ///
+    /// Reset to false every time the load algorithm is invoked.
+    fired_loadeddata_event: Cell<bool>,
+    /// https://html.spec.whatwg.org/multipage/#dom-media-error-2
     error: MutNullableJS<MediaError>,
+    /// https://html.spec.whatwg.org/multipage/#dom-media-paused-2
     paused: Cell<bool>,
+    /// https://html.spec.whatwg.org/multipage/#attr-media-autoplay
     autoplaying: Cell<bool>,
+    /// The details of the video currently related to this media element.
+    // FIXME(nox): Why isn't this in HTMLVideoElement?
     video: DOMRefCell<Option<VideoMedia>>,
 }
 
@@ -68,90 +82,94 @@ pub struct VideoMedia {
 }
 
 impl HTMLMediaElement {
-    pub fn new_inherited(tag_name: LocalName,
+    pub fn new_inherited(
-                         prefix: Option<Prefix>, document: &Document)
+        tag_name: LocalName,
-                         -> HTMLMediaElement {
+        prefix: Option<Prefix>,
-        HTMLMediaElement {
+        document: &Document,
-            htmlelement:
+    ) -> Self {
-                HTMLElement::new_inherited(tag_name, prefix, document),
+        Self {
+            htmlelement: HTMLElement::new_inherited(tag_name, prefix, document),
             network_state: Cell::new(NETWORK_EMPTY),
             ready_state: Cell::new(HAVE_NOTHING),
             current_src: DOMRefCell::new("".to_owned()),
             generation_id: Cell::new(0),
-            first_data_load: Cell::new(true),
+            fired_loadeddata_event: Cell::new(false),
             error: Default::default(),
             paused: Cell::new(true),
+            // FIXME(nox): Why is this initialised to true?
             autoplaying: Cell::new(true),
             video: DOMRefCell::new(None),
         }
     }
 
-    // https://html.spec.whatwg.org/multipage/#internal-pause-steps
+    /// https://html.spec.whatwg.org/multipage/#internal-pause-steps
     fn internal_pause_steps(&self) {
-        // Step 1
+        // Step 1.
         self.autoplaying.set(false);
 
-        // Step 2
+        // Step 2.
         if !self.Paused() {
-            // 2.1
+            // Step 2.1.
             self.paused.set(true);
 
-            // 2.2
+            // Step 2.2.
-            self.queue_internal_pause_steps_task();
+            // FIXME(nox): Take pending play promises and let promises be the
+            // result.
 
-            // TODO 2.3 (official playback position)
+            // Step 2.3.
+            let window = window_from_node(self);
+            // FIXME(nox): Why are errors silenced here?
+            let _ = window.dom_manipulation_task_source().queue(
+                box InternalPauseStepsTask(Trusted::new(self.upcast())),
+                window.upcast(),
+            );
+            struct InternalPauseStepsTask(Trusted<EventTarget>);
+            impl Runnable for InternalPauseStepsTask {
+                fn handler(self: Box<Self>) {
+                    let target = self.0.root();
+
+                    // Step 2.3.1.
+                    target.fire_event(atom!("timeupdate"));
+
+                    // Step 2.3.2.
+                    target.fire_event(atom!("pause"));
+
+                    // Step 2.3.3.
+                    // FIXME(nox): Reject pending play promises with promises
+                    // and an "AbortError" DOMException.
+                }
             }
 
-        // TODO step 3 (media controller)
+            // Step 2.4.
+            // FIXME(nox): Set the official playback position to the current
+            // playback position.
+        }
     }
 
     // https://html.spec.whatwg.org/multipage/#notify-about-playing
     fn notify_about_playing(&self) {
-        // Step 1
+        // Step 1.
-        self.upcast::<EventTarget>().fire_event(atom!("playing"));
+        // TODO(nox): Take pending play promises and let promises be the result.
-        // TODO Step 2
-    }
 
-    fn queue_notify_about_playing(&self) {
+        // Step 2.
-        struct Task {
+        let window = window_from_node(self);
-            elem: Trusted<HTMLMediaElement>,
+        // FIXME(nox): Why are errors silenced here?
-        }
+        let _ = window.dom_manipulation_task_source().queue(
+            box NotifyAboutPlayingTask(Trusted::new(self.upcast())),
+            window.upcast(),
+        );
+        struct NotifyAboutPlayingTask(Trusted<EventTarget>);
+        impl Runnable for NotifyAboutPlayingTask {
+            fn handler(self: Box<Self>) {
+                let target = self.0.root();
 
-        impl Runnable for Task {
+                // Step 2.1.
-            fn handler(self: Box<Task>) {
+                target.fire_event(atom!("playing"));
-                self.elem.root().notify_about_playing();
-            }
-        }
 
-        let task = box Task {
+                // Step 2.2.
-            elem: Trusted::new(self),
+                // FIXME(nox): Resolve pending play promises with promises.
-        };
-        let win = window_from_node(self);
-        let _ = win.dom_manipulation_task_source().queue(task, win.upcast());
-    }
-
-    // https://html.spec.whatwg.org/multipage/#internal-pause-steps step 2.2
-    fn queue_internal_pause_steps_task(&self) {
-        struct Task {
-            elem: Trusted<HTMLMediaElement>,
-        }
-
-        impl Runnable for Task {
-            fn handler(self: Box<Task>) {
-                let target = Root::upcast::<EventTarget>(self.elem.root());
-                // 2.2.1
-                target.fire_event(atom!("timeupdate"));
-                // 2.2.2
-                target.fire_event(atom!("pause"));
-                // TODO 2.2.3
             }
         }
-
-        let task = box Task {
-            elem: Trusted::new(self),
-        };
-        let win = window_from_node(self);
-        let _ = win.dom_manipulation_task_source().queue(task, win.upcast());
     }
 
     // https://html.spec.whatwg.org/multipage/#ready-states
@@ -183,8 +201,8 @@ impl HTMLMediaElement {
             (HAVE_METADATA, HAVE_CURRENT_DATA) |
             (HAVE_METADATA, HAVE_FUTURE_DATA) |
             (HAVE_METADATA, HAVE_ENOUGH_DATA) => {
-                if self.first_data_load.get() {
+                if !self.fired_loadeddata_event.get() {
-                    self.first_data_load.set(false);
+                    self.fired_loadeddata_event.set(true);
                     task_source.queue_simple_event(
                         self.upcast(),
                         atom!("loadeddata"),
@@ -223,7 +241,7 @@ impl HTMLMediaElement {
                 );
 
                 if !self.Paused() {
-                    self.queue_notify_about_playing();
+                    self.notify_about_playing();
                 }
             }
 
@@ -237,7 +255,7 @@ impl HTMLMediaElement {
                     );
 
                     if !self.Paused() {
-                        self.queue_notify_about_playing();
+                        self.notify_about_playing();
                     }
                 }
 
@@ -255,7 +273,7 @@ impl HTMLMediaElement {
                         &window,
                     );
                     // Step 4
-                    self.queue_notify_about_playing();
+                    self.notify_about_playing();
                     // Step 5
                     self.autoplaying.set(false);
                 }
@@ -455,7 +473,9 @@ impl HTMLMediaElement {
 
     // https://html.spec.whatwg.org/multipage/#media-element-load-algorithm
     fn media_element_load_algorithm(&self) {
-        self.first_data_load.set(true);
+        // Reset the flag that signals whether loadeddata was ever fired for
+        // this invokation of the load algorithm.
+        self.fired_loadeddata_event.set(false);
 
         // TODO Step 1 (abort resource selection algorithm instances)
 
@@ -607,7 +627,7 @@ impl HTMLMediaElementMethods for HTMLMediaElement {
                     &window,
                 );
             } else {
-                self.queue_notify_about_playing();
+                self.notify_about_playing();
             }
         }
         // Step 8