Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement postMessage for ServiceWorkers #12910

Merged
merged 3 commits into from Sep 15, 2016
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

store senders instead of buffering messages

  • Loading branch information
creativcoder committed Sep 7, 2016
commit 9dcb7348a2c69be69ae8284d56715177713091d2
@@ -738,12 +738,6 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
// store service worker manager for communicating with it.
self.swmanager_chan = Some(sw_sender);
}
SWManagerMsg::ConnectServiceWorker(scope_url, pipeline_id, msg_chan) => {
if let Some(ref parent_info) = self.pipelines.get(&pipeline_id) {
let from_cons_msg = ConstellationControlMsg::ConnectServiceWorker(scope_url, msg_chan);
let _ = parent_info.script_chan.send(from_cons_msg);
}
}
}
}

@@ -1006,6 +1000,13 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
debug!("constellation got store registration scope message");
self.handle_register_serviceworker(scope_things, scope);
}
FromScriptMsg::ForwardDOMMessage(msg_vec, scope_url) => {
if let Some(ref mgr) = self.swmanager_chan {
let _ = mgr.send(ServiceWorkerMsg::ForwardDOMMessage(msg_vec, scope_url));
} else {
warn!("Unable to forward DOMMessage for postMessage call");
}
}
}
}

@@ -11,6 +11,7 @@ use js::jsapi::{HandleValue, MutableHandleValue};
use js::jsapi::{JSContext, JS_ReadStructuredClone, JS_STRUCTURED_CLONE_VERSION};
use js::jsapi::{JS_ClearPendingException, JS_WriteStructuredClone};
use libc::size_t;
use script_traits::DOMMessage;
use std::ptr;
use std::slice;

@@ -46,15 +47,18 @@ impl StructuredCloneData {
})
}

/// Converts a StructuredCloneData to Vec<u64>
pub fn move_to_arraybuffer(self) -> Vec<u64> {
/// Converts a StructuredCloneData to Vec<u8> for inter-thread sharing
pub fn move_to_arraybuffer(self) -> DOMMessage {
unsafe {
slice::from_raw_parts(self.data, self.nbytes).to_vec()
DOMMessage(slice::from_raw_parts(self.data as *mut u8, self.nbytes).to_vec())
}
}

/// Converts back to StructuredCloneData using a pointer and no of bytes
pub fn make_structured_clone(data: *mut u64, nbytes: size_t) -> StructuredCloneData {
/// Converts back to StructuredCloneData
pub fn make_structured_clone(data: DOMMessage) -> StructuredCloneData {
let DOMMessage(mut data) = data;
let nbytes = data.len();
let data = data.as_mut_ptr() as *mut u64;
StructuredCloneData {
data: data,

This comment has been minimized.

@jdm

jdm Sep 6, 2016

Member

This is unsafe - the actual Vec value goes out of scope when we return, so the StructuredCloneData contains a pointer to freed memory. We should make StructuredCloneData an enum that can store a backing Vec if necessary.

This comment has been minimized.

@creativcoder

creativcoder Sep 7, 2016

Author Contributor

Ah i see. DOMMessage is local to this method, and we are referencing a pointer to something local. Thanks for pointing out, as it didn't came to my thought in the first place. Probably my first meet with use-after-free issue.

I have switched to an enum and we should be fine now. :)

nbytes: nbytes
@@ -6,7 +6,7 @@ use dom::abstractworker::SimpleWorkerErrorHandler;
use dom::bindings::cell::DOMRefCell;
use dom::bindings::codegen::Bindings::EventHandlerBinding::EventHandlerNonNull;
use dom::bindings::codegen::Bindings::ServiceWorkerBinding::{ServiceWorkerMethods, ServiceWorkerState, Wrap};
use dom::bindings::error::ErrorResult;
use dom::bindings::error::{ErrorResult, Error};
use dom::bindings::global::GlobalRef;
use dom::bindings::inheritance::Castable;
use dom::bindings::js::Root;
@@ -15,10 +15,9 @@ use dom::bindings::reflector::reflect_dom_object;
use dom::bindings::str::USVString;
use dom::bindings::structuredclone::StructuredCloneData;
use dom::eventtarget::EventTarget;
use ipc_channel::ipc::IpcSender;
use js::jsapi::{HandleValue, JSContext};
use script_thread::Runnable;
use script_traits::DOMMessage;
use script_traits::ScriptMsg;
use std::cell::Cell;
use url::Url;

@@ -28,28 +27,29 @@ pub type TrustedServiceWorkerAddress = Trusted<ServiceWorker>;
pub struct ServiceWorker {
eventtarget: EventTarget,
script_url: DOMRefCell<String>,
scope_url: DOMRefCell<String>,
state: Cell<ServiceWorkerState>,
#[ignore_heap_size_of = "Defined in std"]
msg_sender: DOMRefCell<Option<IpcSender<DOMMessage>>>,
skip_waiting: Cell<bool>
}

impl ServiceWorker {
fn new_inherited(script_url: &str,
skip_waiting: bool) -> ServiceWorker {
skip_waiting: bool,
scope_url: &str) -> ServiceWorker {
ServiceWorker {
eventtarget: EventTarget::new_inherited(),
script_url: DOMRefCell::new(String::from(script_url)),
state: Cell::new(ServiceWorkerState::Installing),
skip_waiting: Cell::new(skip_waiting),
msg_sender: DOMRefCell::new(None)
scope_url: DOMRefCell::new(String::from(scope_url)),
skip_waiting: Cell::new(skip_waiting)
}
}

pub fn new(global: GlobalRef,
script_url: &str,
scope_url: &str,
skip_waiting: bool) -> Root<ServiceWorker> {
reflect_dom_object(box ServiceWorker::new_inherited(script_url, skip_waiting), global, Wrap)
reflect_dom_object(box ServiceWorker::new_inherited(script_url, skip_waiting, scope_url), global, Wrap)
}

pub fn dispatch_simple_error(address: TrustedServiceWorkerAddress) {
@@ -68,18 +68,13 @@ impl ServiceWorker {

pub fn install_serviceworker(global: GlobalRef,
script_url: Url,
scope_url: &str,
skip_waiting: bool) -> Root<ServiceWorker> {
ServiceWorker::new(global,
script_url.as_str(),
scope_url,

This comment has been minimized.

@creativcoder

creativcoder Sep 7, 2016

Author Contributor

Refactored this into a single method as this was just an unneccessary indirection.

skip_waiting)
}

pub fn store_sender(trusted_worker: TrustedServiceWorkerAddress, sender: IpcSender<DOMMessage>) {
let worker = trusted_worker.root();
// This channel is used for sending message from the ServiceWorker object to its
// corresponding ServiceWorkerGlobalScope
*worker.msg_sender.borrow_mut() = Some(sender);
}
}

impl ServiceWorkerMethods for ServiceWorker {
@@ -95,13 +90,15 @@ impl ServiceWorkerMethods for ServiceWorker {

// https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#service-worker-postmessage
fn PostMessage(&self, cx: *mut JSContext, message: HandleValue) -> ErrorResult {
let data = try!(StructuredCloneData::write(cx, message));
let msg_vec = DOMMessage(data.move_to_arraybuffer());
if let Some(ref sender) = *self.msg_sender.borrow() {
let _ = sender.send(msg_vec);
} else {
warn!("Could not communicate message to ServiceWorkerGlobalScope");
// Step 1
if let ServiceWorkerState::Redundant = self.state.get() {
return Err(Error::InvalidState);
}
// Step 7
let data = try!(StructuredCloneData::write(cx, message));
let msg_vec = data.move_to_arraybuffer();
let scope_url = Url::parse(&*self.scope_url.borrow()).unwrap();

This comment has been minimized.

@jdm

jdm Sep 6, 2016

Member

We should probably store a Url value in scope_url instead.

let _ = self.global().r().constellation_chan().send(ScriptMsg::ForwardDOMMessage(msg_vec, scope_url));
Ok(())
}

@@ -13,7 +13,6 @@ use dom::bindings::inheritance::Castable;
use dom::bindings::js::{Root, RootCollection};
use dom::bindings::reflector::Reflectable;
use dom::bindings::str::DOMString;
use dom::bindings::structuredclone::StructuredCloneData;
use dom::eventtarget::EventTarget;
use dom::messageevent::MessageEvent;
use dom::workerglobalscope::WorkerGlobalScope;
@@ -26,12 +25,11 @@ use msg::constellation_msg::PipelineId;
use net_traits::{LoadContext, load_whole_resource, IpcSend, CustomResponseMediator};
use rand::random;
use script_runtime::{CommonScriptMsg, StackRootTLS, get_reports, new_rt_and_cx, ScriptChan};
use script_traits::{TimerEvent, WorkerGlobalScopeInit, ScopeThings, ServiceWorkerMsg, DOMMessage};
use script_traits::{TimerEvent, WorkerGlobalScopeInit, ScopeThings, ServiceWorkerMsg};
use std::sync::mpsc::{Receiver, RecvError, Select, Sender, channel};
use std::thread;
use std::time::Duration;
use style::thread_state;
use style::thread_state::{IN_WORKER, SCRIPT};
use style::thread_state::{self, IN_WORKER, SCRIPT};
use url::Url;
use util::prefs::PREFS;
use util::thread::spawn_named;
@@ -47,11 +45,9 @@ pub enum ServiceWorkerScriptMsg {
pub enum MixedMessage {
FromServiceWorker(ServiceWorkerScriptMsg),
FromDevtools(DevtoolScriptControlMsg),
FromTimeoutThread(()),
PostMessage(DOMMessage)
FromTimeoutThread(())
}

// Required for run_with_memory_reporting
#[derive(JSTraceable, Clone)]
pub struct ServiceWorkerChan {
pub sender: Sender<ServiceWorkerScriptMsg>
@@ -83,9 +79,6 @@ pub struct ServiceWorkerGlobalScope {
timer_event_port: Receiver<()>,
#[ignore_heap_size_of = "Defined in std"]
swmanager_sender: IpcSender<ServiceWorkerMsg>,
#[ignore_heap_size_of = "Defined in std"]
msg_port: Receiver<DOMMessage>,
#[ignore_heap_size_of = "Defined in std"]
scope_url: Url,
}

@@ -100,8 +93,7 @@ impl ServiceWorkerGlobalScope {
timer_event_chan: IpcSender<TimerEvent>,
timer_event_port: Receiver<()>,
swmanager_sender: IpcSender<ServiceWorkerMsg>,
scope_url: Url,
msg_port: Receiver<DOMMessage>)
scope_url: Url)
-> ServiceWorkerGlobalScope {
ServiceWorkerGlobalScope {
workerglobalscope: WorkerGlobalScope::new_inherited(init,
@@ -115,8 +107,7 @@ impl ServiceWorkerGlobalScope {
timer_event_port: timer_event_port,
own_sender: own_sender,
swmanager_sender: swmanager_sender,
scope_url: scope_url,
msg_port: msg_port
scope_url: scope_url
}
}

@@ -130,8 +121,7 @@ impl ServiceWorkerGlobalScope {
timer_event_chan: IpcSender<TimerEvent>,
timer_event_port: Receiver<()>,
swmanager_sender: IpcSender<ServiceWorkerMsg>,
scope_url: Url,
msg_port: Receiver<DOMMessage>)
scope_url: Url)
-> Root<ServiceWorkerGlobalScope> {
let cx = runtime.cx();
let scope = box ServiceWorkerGlobalScope::new_inherited(init,
@@ -144,8 +134,7 @@ impl ServiceWorkerGlobalScope {
timer_event_chan,
timer_event_port,
swmanager_sender,
scope_url,
msg_port);
scope_url);
ServiceWorkerGlobalScopeBinding::Wrap(cx, scope)
}

@@ -155,8 +144,7 @@ impl ServiceWorkerGlobalScope {
receiver: Receiver<ServiceWorkerScriptMsg>,
devtools_receiver: IpcReceiver<DevtoolScriptControlMsg>,
swmanager_sender: IpcSender<ServiceWorkerMsg>,
scope_url: Url,
msg_port: Receiver<DOMMessage>) {
scope_url: Url) {
let ScopeThings { script_url,
pipeline_id,
init,
@@ -191,7 +179,7 @@ impl ServiceWorkerGlobalScope {
let global = ServiceWorkerGlobalScope::new(
init, url, pipeline_id, devtools_mpsc_port, runtime,
own_sender, receiver,
timer_ipc_chan, timer_port, swmanager_sender, scope_url, msg_port);
timer_ipc_chan, timer_port, swmanager_sender, scope_url);
let scope = global.upcast::<WorkerGlobalScope>();

unsafe {
@@ -238,17 +226,6 @@ impl ServiceWorkerGlobalScope {
self.handle_script_event(msg);
true
}
MixedMessage::PostMessage(data) => {
let DOMMessage(mut data) = data;
let data = StructuredCloneData::make_structured_clone(data.as_mut_ptr(), data.len());
let scope = self.upcast::<WorkerGlobalScope>();
let target = self.upcast();
let _ac = JSAutoCompartment::new(scope.get_cx(), scope.reflector().get_jsobject().get());
rooted!(in(scope.get_cx()) let mut message = UndefinedValue());
data.read(GlobalRef::Worker(scope), message.handle_mut());
MessageEvent::dispatch_jsval(target, GlobalRef::Worker(scope), message.handle());
true
}
MixedMessage::FromTimeoutThread(_) => {
let _ = self.swmanager_sender.send(ServiceWorkerMsg::Timeout(self.scope_url.clone()));
false
@@ -260,7 +237,14 @@ impl ServiceWorkerGlobalScope {
use self::ServiceWorkerScriptMsg::*;

match msg {
CommonWorker(WorkerScriptMsg::DOMMessage(_)) => { },
CommonWorker(WorkerScriptMsg::DOMMessage(data)) => {
let scope = self.upcast::<WorkerGlobalScope>();
let target = self.upcast();
let _ac = JSAutoCompartment::new(scope.get_cx(), scope.reflector().get_jsobject().get());
rooted!(in(scope.get_cx()) let mut message = UndefinedValue());
data.read(GlobalRef::Worker(scope), message.handle_mut());
MessageEvent::dispatch_jsval(target, GlobalRef::Worker(scope), message.handle());
},
CommonWorker(WorkerScriptMsg::Common(CommonScriptMsg::RunnableMsg(_, runnable))) => {
runnable.handler()
},
@@ -287,20 +271,17 @@ impl ServiceWorkerGlobalScope {
let worker_port = &self.receiver;
let devtools_port = scope.from_devtools_receiver();
let timer_event_port = &self.timer_event_port;
let msg_port = &self.msg_port;

let sel = Select::new();
let mut worker_handle = sel.handle(worker_port);
let mut devtools_handle = sel.handle(devtools_port);
let mut timer_port_handle = sel.handle(timer_event_port);
let mut msg_port_handle = sel.handle(msg_port);
unsafe {
worker_handle.add();
if scope.from_devtools_sender().is_some() {
devtools_handle.add();
}
timer_port_handle.add();
msg_port_handle.add();
}

let ret = sel.wait();
@@ -310,8 +291,6 @@ impl ServiceWorkerGlobalScope {
Ok(MixedMessage::FromDevtools(try!(devtools_port.recv())))
} else if ret == timer_port_handle.id() {
Ok(MixedMessage::FromTimeoutThread(try!(timer_event_port.recv())))
} else if ret == msg_port_handle.id() {
Ok(MixedMessage::PostMessage(try!(msg_port.recv())))
} else {
panic!("unexpected select result!")
}
@@ -6,11 +6,10 @@ use dom::bindings::codegen::Bindings::ServiceWorkerBinding::ServiceWorkerState;
use dom::bindings::codegen::Bindings::ServiceWorkerRegistrationBinding::{ServiceWorkerRegistrationMethods, Wrap};
use dom::bindings::global::GlobalRef;
use dom::bindings::js::{JS, Root};
use dom::bindings::refcounted::Trusted;
use dom::bindings::reflector::reflect_dom_object;
use dom::bindings::str::USVString;
use dom::eventtarget::EventTarget;
use dom::serviceworker::{ServiceWorker, TrustedServiceWorkerAddress};
use dom::serviceworker::ServiceWorker;
use dom::serviceworkercontainer::Controllable;
use dom::workerglobalscope::prepare_workerscope_init;
use script_traits::{WorkerScriptLoadOrigin, ScopeThings};
@@ -40,7 +39,7 @@ impl ServiceWorkerRegistration {
script_url: Url,
scope: String,
container: &Controllable) -> Root<ServiceWorkerRegistration> {
let active_worker = ServiceWorker::install_serviceworker(global, script_url.clone(), true);
let active_worker = ServiceWorker::install_serviceworker(global, script_url.clone(), &scope, true);
active_worker.set_transition_state(ServiceWorkerState::Installed);
container.set_controller(&*active_worker.clone());
reflect_dom_object(box ServiceWorkerRegistration::new_inherited(&*active_worker, scope), global, Wrap)
@@ -50,10 +49,6 @@ impl ServiceWorkerRegistration {
self.active.as_ref().unwrap()
}

pub fn get_trusted_worker(&self) -> TrustedServiceWorkerAddress {
Trusted::new(self.active.as_ref().unwrap())
}

pub fn create_scope_things(global: GlobalRef, script_url: Url) -> ScopeThings {
let worker_load_origin = WorkerScriptLoadOrigin {
referrer_url: None,
@@ -399,7 +399,7 @@ impl WorkerGlobalScope {
} else if let Some(service_worker) = service_worker {
return service_worker.script_chan();
} else {
panic!("need to implement a sender for SharedWorker/ServiceWorker")
panic!("need to implement a sender for SharedWorker")
}
}

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.