Add Promises to Service Worker Container

The purpose of the code changes is to enable the use of promises in the Service Worker container. I also modified the Service Worker test in order to support the promises.

components/script/dom/serviceworkercontainer.rs

@@ -4,18 +4,20 @@
 
 use dom::bindings::codegen::Bindings::ServiceWorkerContainerBinding::{ServiceWorkerContainerMethods, Wrap};
 use dom::bindings::codegen::Bindings::ServiceWorkerContainerBinding::RegistrationOptions;
-use dom::bindings::error::{Error, Fallible};
+use dom::bindings::error::Error;
 use dom::bindings::inheritance::Castable;
 use dom::bindings::js::{JS, MutNullableHeap, Root};
 use dom::bindings::reflector::{Reflectable, reflect_dom_object};
 use dom::bindings::str::USVString;
 use dom::eventtarget::EventTarget;
 use dom::globalscope::GlobalScope;
+use dom::promise::Promise;
 use dom::serviceworker::ServiceWorker;
 use dom::serviceworkerregistration::ServiceWorkerRegistration;
 use script_thread::ScriptThread;
 use std::ascii::AsciiExt;
 use std::default::Default;
+use std::rc::Rc;
 
 #[dom_struct]
 pub struct ServiceWorkerContainer {
@@ -53,26 +55,36 @@ impl ServiceWorkerContainerMethods for ServiceWorkerContainer {
         return self.controller.get()
     }
 
+    #[allow(unrooted_must_root)]
     // https://w3c.github.io/ServiceWorker/#service-worker-container-register-method
     fn Register(&self,
                 script_url: USVString,
-                options: &RegistrationOptions) -> Fallible<Root<ServiceWorkerRegistration>> {
+                options: &RegistrationOptions) -> Rc<Promise> {
+        let promise = Promise::new(&*self.global());
+        let ctx = self.global().get_cx();
         let USVString(ref script_url) = script_url;
         let api_base_url = self.global().api_base_url();
         // Step 3-4
         let script_url = match api_base_url.join(script_url) {
             Ok(url) => url,
-            Err(_) => return Err(Error::Type("Invalid script URL".to_owned()))
+            Err(_) => {
+                promise.reject_error(ctx, Error::Type("Invalid script URL".to_owned()));
+                return promise;
+            }
         };
         // Step 5
         match script_url.scheme() {
             "https" | "http" => {},
-            _ => return Err(Error::Type("Only secure origins are allowed".to_owned()))
+            _ => {
+                promise.reject_error(ctx, Error::Type("Only secure origins are allowed".to_owned()));
+                return promise;
+            }
         }
         // Step 6
         if script_url.path().to_ascii_lowercase().contains("%2f") ||
         script_url.path().to_ascii_lowercase().contains("%5c") {
-            return Err(Error::Type("Script URL contains forbidden characters".to_owned()));
+            promise.reject_error(ctx, Error::Type("Script URL contains forbidden characters".to_owned()));
+            return promise;
         }
         // Step 8-9
         let scope = match options.scope {
@@ -80,7 +92,10 @@ impl ServiceWorkerContainerMethods for ServiceWorkerContainer {
                 let &USVString(ref inner_scope) = scope;
                 match api_base_url.join(inner_scope) {
                     Ok(url) => url,
-                    Err(_) => return Err(Error::Type("Invalid scope URL".to_owned()))
+                    Err(_) => {
+                        promise.reject_error(ctx, Error::Type("Invalid scope URL".to_owned()));
+                        return promise;
+                    }
                 }
             },
             None => script_url.join("./").unwrap()
@@ -88,12 +103,16 @@ impl ServiceWorkerContainerMethods for ServiceWorkerContainer {
         // Step 11
         match scope.scheme() {
             "https" | "http" => {},
-            _ => return Err(Error::Type("Only secure origins are allowed".to_owned()))
+            _ => {
+                promise.reject_error(ctx, Error::Type("Only secure origins are allowed".to_owned()));
+                return promise;
+            }
         }
         // Step 12
         if scope.path().to_ascii_lowercase().contains("%2f") ||
         scope.path().to_ascii_lowercase().contains("%5c") {
-            return Err(Error::Type("Scope URL contains forbidden characters".to_owned()));
+            promise.reject_error(ctx, Error::Type("Scope URL contains forbidden characters".to_owned()));
+            return promise;
         }
 
         let global = self.global();
@@ -101,7 +120,9 @@ impl ServiceWorkerContainerMethods for ServiceWorkerContainer {
                                                                  script_url,
                                                                  scope.clone(),
                                                                  self);
-        ScriptThread::set_registration(scope, &*worker_registration, global.pipeline_id());
+        ScriptThread::set_registration(scope, &*worker_registration, self.global().pipeline_id());
-        Ok(worker_registration)
+
+        promise.resolve_native(ctx, &*worker_registration);
+        promise
     }
 }

components/script/dom/webidls/ServiceWorkerContainer.webidl

@@ -8,7 +8,7 @@ interface ServiceWorkerContainer : EventTarget {
   [Unforgeable] readonly attribute ServiceWorker? controller;
   //[SameObject] readonly attribute Promise<ServiceWorkerRegistration> ready;
 
-  [NewObject, Throws] ServiceWorkerRegistration register(USVString scriptURL, optional RegistrationOptions options);
+  [NewObject] Promise<ServiceWorkerRegistration> register(USVString scriptURL, optional RegistrationOptions options);
 
   //[NewObject] /*Promise<any>*/ any getRegistration(optional USVString clientURL = "");
   //[NewObject] /* Promise */<sequence<ServiceWorkerRegistration>> getRegistrations();

tests/html/service-worker/index.html

@@ -19,17 +19,22 @@
     <button onclick="post_message_root()">Msg to Root service worker</button>
 </div>
     <script>
-
+        navigator.serviceWorker.register('iframe_sw.js', { 'scope': '/demo_iframe.html' }).then(function(registration) {
-    var reg = navigator.serviceWorker.register('iframe_sw.js', { 'scope': '/demo_iframe.html' });
             console.log("From client worker registered: ", navigator.serviceWorker.controller);
-    console.log("Registered script source: "+ navigator.serviceWorker.controller.scriptURL);
+            console.log("Registered script source: iframe_sw.js");
             document.getElementById('iframe_sw').src = 'demo_iframe.html';
             post_message('/demo_iframe');
+        }).catch(function(error) {
+            console.log("Error in loading `iframe_sw.js` -->" + error);
+        });
 
     function register_zero() {
-        var reg = navigator.serviceWorker.register('sw.js', { 'scope': './*' });
+        navigator.serviceWorker.register('sw.js', { 'scope': './*' }).then(function(registration) {
             console.log("From client worker registered: ", navigator.serviceWorker.controller);
-        console.log("Registered script source: "+ navigator.serviceWorker.controller.scriptURL);
+            console.log("Registered script source: sw.js");
+        }).catch(function(error) {
+            console.log("Error in loading `sw.js` -->" + error);
+        });
     }
 
     function post_message_root() {
@@ -40,17 +45,23 @@
     }
 
     function register_one() {
-        var reg = navigator.serviceWorker.register('sw_dashboard.js', { 'scope': '/dashboard' });
+        navigator.serviceWorker.register('sw_dashboard.js', { 'scope': '/dashboard' }).then(function(registration) {
             console.log("From client worker registered: ", navigator.serviceWorker.controller);
-        console.log("Registered script source: "+ navigator.serviceWorker.controller.scriptURL);
+            console.log("Registered script source: sw_dashboard.js");
             post_message('/dashboard');
+        }).catch(function(error) {
+            console.log("Error in loading `sw_dashboard.js` -->" + error);
+        });
     }
 
     function register_two() {
-        var reg = navigator.serviceWorker.register('sw_profile.js', { 'scope': '/profile' });
+        navigator.serviceWorker.register('sw_profile.js', { 'scope': '/profile' }).then(function(registration) {
             console.log("From client worker registered: ", navigator.serviceWorker.controller);
-        console.log("Registered script source: "+ navigator.serviceWorker.controller.scriptURL);
+            console.log("Registered script source: sw_profile.js");
             post_message('/profile');
+        }).catch(function(error) {
+            console.log("Error in loading `sw_profile.js` --> " + error);
+        });
     }
 
     navigator.serviceWorker.addEventListener('controllerchange', function(e) {

tests/wpt/mozilla/tests/mozilla/service-workers/service-worker-registration.html

@@ -14,39 +14,39 @@ test(function (){
   assert_true('serviceWorker' in navigator);
 }, "Test: Asserts ServiceWorkerContainer in Navigator");
 
-test(function() {
+promise_test(function() {
-  var sw_reg = register_sw('sw.js');
+  return register_sw('sw.js').then(function(sw_reg) {
     assert_class_string(sw_reg, "ServiceWorkerRegistration");
     assert_class_string(sw_reg.active, "ServiceWorker");
     assert_class_string(navigator.serviceWorker.controller, "ServiceWorker");
+  });
 }, "Test: Asserts Active Service Worker and its Registration");
 
-test(function() {
+promise_test(function() {
-  var sw_reg = register_sw('resources/sw.js', './');
+  return register_sw('resources/sw.js', './').then(function(sw_reg) {
     assert_equals(sw_reg.scope, location.href.replace("service-worker-registration.html", ""));
+  });
 }, "Test: Service Worker Registration property scope Url when no scope");
 
-test(function() {
+promise_test(function() {
-  var sw_reg = register_sw('resources/sw.js', '/some/nested/cache/directory');
+  return register_sw('resources/sw.js', '/some/nested/cache/directory').then(function(sw_reg) {
     var expected_scope_url = location.protocol + '//' + location.host + '/some/nested/cache/directory';
     assert_equals(sw_reg.scope, expected_scope_url);
+  });
 }, "Test: Service Worker Registration property scope when provided a scope");
 
-test(function () {
+promise_test(function (p) {
-  assert_throws(new TypeError(),
+  promise_rejects(p, new TypeError(), register_sw('./in%5Csome%5fdir/sw.js'));
-              function() { var sw_reg = register_sw('./in%5Csome%5fdir/sw.js'); },
-              "Invalid URL Path");
 }, "Test: Throws Error when Invalid Url Path");
 
-test(function () {
+promise_test(function (p) {
-  assert_throws(new TypeError(),
+  return promise_rejects(p, new TypeError(), register_sw('sw.js', './mal%5fformed/sco%5Cpe/'));
-              function() { var sw_reg = register_sw('sw.js', './mal%5fformed/sco%5Cpe/'); },
-              "Invalid URL Path");
 }, "Test: Throws Error when Invalid Scope");
 
-test(function() {
+promise_test(function() {
-  var sw_reg = register_sw('resources/sw.js');
+  return register_sw('resources/sw.js').then(function(sw_reg) {
     assert_equals(sw_reg.active.scriptURL, location.href.replace("service-worker-registration.html", "resources/sw.js"));
+  });
 }, "Test: Active Service Worker ScriptURL property");
 
 </script>