Gamepad: Align closer to spec and implement missing slots (#31385)

* Implement missing gamepad slots, align to spec more

- Fixes TODO's from initial gamepad implementation
- Adds some missing spec steps

* Only handle gamepad events when pref is enabled

* Return empty list in getGamepads if document not active

* ./mach fmt

* Update getGamepads to return an array instead of GamepadList

* Add spec link for [[exposed]] slot

* Remove failing test expectations for not-fully-active

* A few fixes

- Change should_notify to has_gesture
- Add spec links and TODO to navigator
- Remove unneeded clone from GamepadList::list
- Move gamepadconnected event firing into has_gesture block

* Use queue_with_canceller for tasks and add expects

* Explicitly check for gamepad user gesture

* Move user gesture check into separate function

* Change contains_user_gesture to be a gamepad function

* mach fmt

* Change axis/button threshold constants to be private to module

components/script/dom/gamepad.rs

@@ -6,6 +6,7 @@ use std::cell::Cell;
 
 use dom_struct::dom_struct;
 use js::typedarray::{Float64, Float64Array};
+use script_traits::GamepadUpdateType;
 
 use super::bindings::buffer_source::HeapBufferSource;
 use crate::dom::bindings::codegen::Bindings::GamepadBinding::{GamepadHand, GamepadMethods};
@@ -23,6 +24,9 @@ use crate::dom::gamepadpose::GamepadPose;
 use crate::dom::globalscope::GlobalScope;
 use crate::script_runtime::JSContext;
 
+// This value is for determining when to consider a gamepad as having a user gesture
+// from an axis tilt. This matches the threshold in Chromium.
+const AXIS_TILT_THRESHOLD: f64 = 0.5;
 // This value is for determining when to consider a non-digital button "pressed".
 // Like Gecko and Chromium it derives from the XInput trigger threshold.
 const BUTTON_PRESS_THRESHOLD: f64 = 30.0 / 255.0;
@@ -44,6 +48,7 @@ pub struct Gamepad {
     hand: GamepadHand,
     axis_bounds: (f64, f64),
     button_bounds: (f64, f64),
+    exposed: Cell<bool>,
 }
 
 impl Gamepad {
@@ -74,6 +79,7 @@ impl Gamepad {
             hand: hand,
             axis_bounds: axis_bounds,
             button_bounds: button_bounds,
+            exposed: Cell::new(false),
         }
     }
 
@@ -105,7 +111,7 @@ impl Gamepad {
                 gamepad_id,
                 id,
                 0,
-                false,
+                true,
                 0.,
                 String::from("standard"),
                 &button_list,
@@ -175,7 +181,7 @@ impl Gamepad {
         self.gamepad_id
     }
 
-    pub fn update_connected(&self, connected: bool) {
+    pub fn update_connected(&self, connected: bool, has_gesture: bool) {
         if self.connected.get() == connected {
             return;
         }
@@ -187,7 +193,9 @@ impl Gamepad {
             GamepadEventType::Disconnected
         };
 
-        self.notify_event(event_type);
+        if has_gesture {
+            self.notify_event(event_type);
+        }
     }
 
     pub fn update_index(&self, index: i32) {
@@ -263,4 +271,26 @@ impl Gamepad {
             warn!("Button bounds difference is either 0 or non-finite!");
         }
     }
+
+    /// <https://www.w3.org/TR/gamepad/#dfn-exposed>
+    pub fn exposed(&self) -> bool {
+        self.exposed.get()
+    }
+
+    /// <https://www.w3.org/TR/gamepad/#dfn-exposed>
+    pub fn set_exposed(&self, exposed: bool) {
+        self.exposed.set(exposed);
+    }
+}
+
+/// <https://www.w3.org/TR/gamepad/#dfn-gamepad-user-gesture>
+pub fn contains_user_gesture(update_type: GamepadUpdateType) -> bool {
+    match update_type {
+        GamepadUpdateType::Axis(_, value) => {
+            return value.abs() > AXIS_TILT_THRESHOLD;
+        },
+        GamepadUpdateType::Button(_, value) => {
+            return value > BUTTON_PRESS_THRESHOLD;
+        },
+    };
 }

components/script/dom/gamepadlist.rs

@@ -48,6 +48,14 @@ impl GamepadList {
     pub fn remove_gamepad(&self, index: usize) {
         self.list.borrow_mut().remove(index);
     }
+
+    pub fn list(&self) -> Vec<Option<DomRoot<Gamepad>>> {
+        self.list
+            .borrow()
+            .iter()
+            .map(|gamepad| Some(DomRoot::from_ref(&**gamepad)))
+            .collect()
+    }
 }
 
 impl GamepadListMethods for GamepadList {

components/script/dom/globalscope.rs

@@ -67,7 +67,6 @@ use crate::dom::bindings::codegen::Bindings::GamepadListBinding::GamepadList_Bin
 use crate::dom::bindings::codegen::Bindings::ImageBitmapBinding::{
     ImageBitmapOptions, ImageBitmapSource,
 };
-use crate::dom::bindings::codegen::Bindings::NavigatorBinding::Navigator_Binding::NavigatorMethods;
 use crate::dom::bindings::codegen::Bindings::PerformanceBinding::Performance_Binding::PerformanceMethods;
 use crate::dom::bindings::codegen::Bindings::PermissionStatusBinding::PermissionState;
 use crate::dom::bindings::codegen::Bindings::VoidFunctionBinding::VoidFunction;
@@ -95,7 +94,8 @@ use crate::dom::event::{Event, EventBubbles, EventCancelable, EventStatus};
 use crate::dom::eventsource::EventSource;
 use crate::dom::eventtarget::EventTarget;
 use crate::dom::file::File;
-use crate::dom::gamepad::Gamepad;
+use crate::dom::gamepad::{contains_user_gesture, Gamepad};
+use crate::dom::gamepadevent::GamepadEventType;
 use crate::dom::gpudevice::GPUDevice;
 use crate::dom::htmlscriptelement::{ScriptId, SourceCode};
 use crate::dom::identityhub::Identities;
@@ -3136,42 +3136,43 @@ impl GlobalScope {
         // TODO: 2. If document is not null and is not allowed to use the "gamepad" permission,
         //          then abort these steps.
         let this = Trusted::new(&*self);
-        self.gamepad_task_source().queue(
+        self.gamepad_task_source().queue_with_canceller(
             task!(gamepad_connected: move || {
                 let global = this.root();
                 let gamepad = Gamepad::new(&global, index as u32, name, axis_bounds, button_bounds);
 
                 if let Some(window) = global.downcast::<Window>() {
-                    let gamepad_list = window.Navigator().GetGamepads();
+                    let has_gesture = window.Navigator().has_gamepad_gesture();
+                    if has_gesture {
+                        gamepad.set_exposed(true);
+                        if window.Document().is_fully_active() {
+                            gamepad.update_connected(true, has_gesture);
+                        }
+                    }
+                    let gamepad_list = window.Navigator().gamepads();
                     let gamepad_arr: [DomRoot<Gamepad>; 1] = [gamepad.clone()];
                     gamepad_list.add_if_not_exists(&gamepad_arr);
-
-                    // TODO: 3.4 If navigator.[[hasGamepadGesture]] is true:
-                    // TODO: 3.4.1 Set gamepad.[[exposed]] to true.
-
-                    if window.Document().is_fully_active() {
-                        gamepad.update_connected(true);
-                    }
                 }
             }),
-            &self,
+            &self.task_canceller(TaskSourceName::Gamepad)
         )
-        .unwrap();
+        .expect("Failed to queue gamepad connected task.");
     }
 
     /// <https://www.w3.org/TR/gamepad/#dfn-gamepaddisconnected>
     pub fn handle_gamepad_disconnect(&self, index: usize) {
         let this = Trusted::new(&*self);
         self.gamepad_task_source()
-            .queue(
+            .queue_with_canceller(
                 task!(gamepad_disconnected: move || {
                     let global = this.root();
                     if let Some(window) = global.downcast::<Window>() {
-                        let gamepad_list = window.Navigator().GetGamepads();
+                        let gamepad_list = window.Navigator().gamepads();
                         if let Some(gamepad) = gamepad_list.Item(index as u32) {
-                            // TODO: If gamepad.[[exposed]]
+                            if window.Document().is_fully_active() {
-                            gamepad.update_connected(false);
+                                gamepad.update_connected(false, gamepad.exposed());
-                            gamepad_list.remove_gamepad(index);
+                                gamepad_list.remove_gamepad(index);
+                            }
                         }
                         for i in (0..gamepad_list.Length()).rev() {
                             if gamepad_list.Item(i as u32).is_none() {
@@ -3182,9 +3183,9 @@ impl GlobalScope {
                         }
                     }
                 }),
-                &self,
+                &self.task_canceller(TaskSourceName::Gamepad),
             )
-            .unwrap();
+            .expect("Failed to queue gamepad disconnected task.");
     }
 
     /// <https://www.w3.org/TR/gamepad/#receiving-inputs>
@@ -3193,12 +3194,12 @@ impl GlobalScope {
 
         // <https://w3c.github.io/gamepad/#dfn-update-gamepad-state>
         self.gamepad_task_source()
-            .queue(
+            .queue_with_canceller(
                 task!(update_gamepad_state: move || {
                     let global = this.root();
                     if let Some(window) = global.downcast::<Window>() {
-                        let gamepad_list = window.Navigator().GetGamepads();
+                        let gamepad_list = window.Navigator().gamepads();
-                        if let Some(gamepad) = gamepad_list.IndexedGetter(index as u32) {
+                        if let Some(gamepad) = gamepad_list.Item(index as u32) {
                             let current_time = global.performance().Now();
                             gamepad.update_timestamp(*current_time);
 
@@ -3211,14 +3212,32 @@ impl GlobalScope {
                                 }
                             };
 
-                            // TODO: 6. If navigator.[[hasGamepadGesture]] is false
+                            if !window.Navigator().has_gamepad_gesture() && contains_user_gesture(update_type) {
-                            //          and gamepad contains a gamepad user gesture:
+                                window.Navigator().set_has_gamepad_gesture(true);
+                                for i in 0..gamepad_list.Length() {
+                                    if let Some(gamepad) = gamepad_list.Item(i as u32) {
+                                        gamepad.set_exposed(true);
+                                        gamepad.update_timestamp(*current_time);
+                                        let new_gamepad = Trusted::new(&*gamepad);
+                                        if window.Document().is_fully_active() {
+                                            window.task_manager().gamepad_task_source().queue_with_canceller(
+                                                task!(update_gamepad_connect: move || {
+                                                    let gamepad = new_gamepad.root();
+                                                    gamepad.notify_event(GamepadEventType::Connected);
+                                                }),
+                                                &window.upcast::<GlobalScope>().task_canceller(TaskSourceName::Gamepad),
+                                            )
+                                            .expect("Failed to queue update gamepad connect task.");
+                                        }
+                                    }
+                                }
+                            }
                         }
                     }
                 }),
-                &self,
+                &self.task_canceller(TaskSourceName::Gamepad),
             )
-            .unwrap();
+            .expect("Failed to queue update gamepad state task.");
     }
 
     pub(crate) fn current_group_label(&self) -> Option<DOMString> {

components/script/dom/navigator.rs

@@ -2,6 +2,7 @@
  * 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::cell::Cell;
 use std::convert::TryInto;
 
 use dom_struct::dom_struct;
@@ -9,11 +10,13 @@ use js::jsval::JSVal;
 use lazy_static::lazy_static;
 
 use crate::dom::bindings::codegen::Bindings::NavigatorBinding::NavigatorMethods;
+use crate::dom::bindings::codegen::Bindings::WindowBinding::Window_Binding::WindowMethods;
 use crate::dom::bindings::reflector::{reflect_dom_object, DomObject, Reflector};
 use crate::dom::bindings::root::{DomRoot, MutNullableDom};
 use crate::dom::bindings::str::DOMString;
 use crate::dom::bindings::utils::to_frozen_array;
 use crate::dom::bluetooth::Bluetooth;
+use crate::dom::gamepad::Gamepad;
 use crate::dom::gamepadlist::GamepadList;
 use crate::dom::gpu::GPU;
 use crate::dom::mediadevices::MediaDevices;
@@ -43,10 +46,13 @@ pub struct Navigator {
     service_worker: MutNullableDom<ServiceWorkerContainer>,
     xr: MutNullableDom<XRSystem>,
     mediadevices: MutNullableDom<MediaDevices>,
+    /// <https://www.w3.org/TR/gamepad/#dfn-gamepads>
     gamepads: MutNullableDom<GamepadList>,
     permissions: MutNullableDom<Permissions>,
     mediasession: MutNullableDom<MediaSession>,
     gpu: MutNullableDom<GPU>,
+    /// <https://www.w3.org/TR/gamepad/#dfn-hasgamepadgesture>
+    has_gamepad_gesture: Cell<bool>,
 }
 
 impl Navigator {
@@ -63,6 +69,7 @@ impl Navigator {
             permissions: Default::default(),
             mediasession: Default::default(),
             gpu: Default::default(),
+            has_gamepad_gesture: Cell::new(false),
         }
     }
 
@@ -73,6 +80,21 @@ impl Navigator {
     pub fn xr(&self) -> Option<DomRoot<XRSystem>> {
         self.xr.get()
     }
+
+    pub fn gamepads(&self) -> DomRoot<GamepadList> {
+        let gamepads = self
+            .gamepads
+            .or_init(|| GamepadList::new(&self.global(), &[]));
+        gamepads
+    }
+
+    pub fn has_gamepad_gesture(&self) -> bool {
+        self.has_gamepad_gesture.get()
+    }
+
+    pub fn set_has_gamepad_gesture(&self, has_gamepad_gesture: bool) {
+        self.has_gamepad_gesture.set(has_gamepad_gesture);
+    }
 }
 
 impl NavigatorMethods for Navigator {
@@ -169,14 +191,20 @@ impl NavigatorMethods for Navigator {
         true
     }
 
-    // https://www.w3.org/TR/gamepad/#navigator-interface-extension
+    /// <https://www.w3.org/TR/gamepad/#dom-navigator-getgamepads>
-    fn GetGamepads(&self) -> DomRoot<GamepadList> {
+    fn GetGamepads(&self) -> Vec<Option<DomRoot<Gamepad>>> {
-        let root = self
+        let global = self.global();
-            .gamepads
+        let window = global.as_window();
-            .or_init(|| GamepadList::new(&self.global(), &[]));
+        let doc = window.Document();
 
-        // TODO: Add gamepads
+        // TODO: Handle permissions policy once implemented
-        root
+        if !doc.is_fully_active() || !self.has_gamepad_gesture.get() {
+            return Vec::new();
+        }
+
+        let root = self.gamepads.or_init(|| GamepadList::new(&global, &[]));
+
+        root.list()
     }
     // https://w3c.github.io/permissions/#navigator-and-workernavigator-extension
     fn Permissions(&self) -> DomRoot<Permissions> {

components/script/dom/webidls/Navigator.webidl

@@ -69,7 +69,7 @@ partial interface Navigator {
 
 // https://w3c.github.io/gamepad/#navigator-interface-extension
 partial interface Navigator {
-    [Pref="dom.gamepad.enabled"] GamepadList getGamepads();
+  [Pref="dom.gamepad.enabled"] sequence<Gamepad?> getGamepads();
 };
 
 // https://html.spec.whatwg.org/multipage/#navigatorconcurrenthardware

ports/servoshell/app.rs

@@ -436,7 +436,7 @@ impl App {
 
         // Catch some keyboard events, and push the rest onto the WebViewManager event queue.
         webviews.handle_window_events(embedder_events);
-        if webviews.webview_id().is_some() {
+        if pref!(dom.gamepad.enabled) && webviews.webview_id().is_some() {
             webviews.handle_gamepad_events();
         }
 

tests/wpt/meta-legacy-layout/gamepad/not-fully-active.html.ini

@@ -1,3 +0,0 @@
-[not-fully-active.html]
-  [calling getGamepads() in a non-fully-active document]
-    expected: FAIL

tests/wpt/meta/gamepad/not-fully-active.html.ini

@@ -1,3 +0,0 @@
-[not-fully-active.html]
-  [calling getGamepads() in a non-fully-active document]
-    expected: FAIL