Auto merge of #8043 - eefriedman:async-listener-mut, r=jdm

Make AsyncResponseListener methods take `&mut self`

Gets rid of a bunch of useless `Cell`/`RefCell` types.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8043)
<!-- Reviewable:end -->

components/net_traits/lib.rs

@@ -163,13 +163,13 @@ pub trait AsyncFetchListener {
 /// A listener for asynchronous network events. Cancelling the underlying request is unsupported.
 pub trait AsyncResponseListener {
     /// The response headers for a request have been received.
-    fn headers_available(&self, metadata: Metadata);
+    fn headers_available(&mut self, metadata: Metadata);
     /// A portion of the response body has been received. This data is unavailable after
     /// this method returned, and must be stored accordingly.
-    fn data_available(&self, payload: Vec<u8>);
+    fn data_available(&mut self, payload: Vec<u8>);
     /// The response is complete. If the provided status is an Err value, there is no guarantee
     /// that the response body was completely read.
-    fn response_complete(&self, status: Result<(), String>);
+    fn response_complete(&mut self, status: Result<(), String>);
 }
 
 /// Data for passing between threads/processes to indicate a particular action to
@@ -186,7 +186,7 @@ pub enum ResponseAction {
 
 impl ResponseAction {
     /// Execute the default action on a provided listener.
-    pub fn process(self, listener: &AsyncResponseListener) {
+    pub fn process(self, listener: &mut AsyncResponseListener) {
         match self {
             ResponseAction::HeadersAvailable(m) => listener.headers_available(m),
             ResponseAction::DataAvailable(d) => listener.data_available(d),

components/script/cors.rs

@@ -23,7 +23,6 @@ use network_listener::{NetworkListener, PreInvoke};
 use script_task::ScriptChan;
 use std::ascii::AsciiExt;
 use std::borrow::ToOwned;
-use std::cell::RefCell;
 use std::sync::{Arc, Mutex};
 use time::{self, Timespec, now};
 use unicase::UniCase;
@@ -106,20 +105,20 @@ impl CORSRequest {
                             script_chan: Box<ScriptChan + Send>) {
         struct CORSContext {
             listener: Box<AsyncCORSResponseListener + Send>,
-            response: RefCell<Option<CORSResponse>>,
+            response: Option<CORSResponse>,
         }
 
         // This is shoe-horning the CORSReponse stuff into the rest of the async network
         // framework right now. It would be worth redesigning http_fetch to do this properly.
         impl AsyncResponseListener for CORSContext {
-            fn headers_available(&self, _metadata: Metadata) {
+            fn headers_available(&mut self, _metadata: Metadata) {
             }
 
-            fn data_available(&self, _payload: Vec<u8>) {
+            fn data_available(&mut self, _payload: Vec<u8>) {
             }
 
-            fn response_complete(&self, _status: Result<(), String>) {
+            fn response_complete(&mut self, _status: Result<(), String>) {
-                let response = self.response.borrow_mut().take().unwrap();
+                let response = self.response.take().unwrap();
                 self.listener.response_available(response);
             }
         }
@@ -127,7 +126,7 @@ impl CORSRequest {
 
         let context = CORSContext {
             listener: listener,
-            response: RefCell::new(None),
+            response: None,
         };
         let listener = NetworkListener {
             context: Arc::new(Mutex::new(context)),
@@ -141,7 +140,7 @@ impl CORSRequest {
             let response = req.http_fetch();
             let mut context = listener.context.lock();
             let context = context.as_mut().unwrap();
-            *context.response.borrow_mut() = Some(response);
+            context.response = Some(response);
             listener.notify(ResponseAction::ResponseComplete(Ok(())));
         });
     }

components/script/dom/htmlscriptelement.rs

@@ -38,7 +38,7 @@ use network_listener::{NetworkListener, PreInvoke};
 use script_task::ScriptTaskEventCategory::ScriptEvent;
 use script_task::{CommonScriptMsg, Runnable, ScriptChan};
 use std::ascii::AsciiExt;
-use std::cell::{Cell, RefCell};
+use std::cell::Cell;
 use std::mem;
 use std::sync::{Arc, Mutex};
 use url::{Url, UrlParser};
@@ -128,9 +128,9 @@ struct ScriptContext {
     /// The element that initiated the request.
     elem: Trusted<HTMLScriptElement>,
     /// The response body received to date.
-    data: RefCell<Vec<u8>>,
+    data: Vec<u8>,
     /// The response metadata received to date.
-    metadata: RefCell<Option<Metadata>>,
+    metadata: Option<Metadata>,
     /// Whether the owning document's parser should resume once the response completes.
     resume_on_completion: bool,
     /// The initial URL requested.
@@ -138,19 +138,19 @@ struct ScriptContext {
 }
 
 impl AsyncResponseListener for ScriptContext {
-    fn headers_available(&self, metadata: Metadata) {
+    fn headers_available(&mut self, metadata: Metadata) {
-        *self.metadata.borrow_mut() = Some(metadata);
+        self.metadata = Some(metadata);
     }
 
-    fn data_available(&self, payload: Vec<u8>) {
+    fn data_available(&mut self, payload: Vec<u8>) {
         let mut payload = payload;
-        self.data.borrow_mut().append(&mut payload);
+        self.data.append(&mut payload);
     }
 
-    fn response_complete(&self, status: Result<(), String>) {
+    fn response_complete(&mut self, status: Result<(), String>) {
         let load = status.map(|_| {
-            let data = mem::replace(&mut *self.data.borrow_mut(), vec!());
+            let data = mem::replace(&mut self.data, vec!());
-            let metadata = self.metadata.borrow_mut().take().unwrap();
+            let metadata = self.metadata.take().unwrap();
             (metadata, data)
         });
         let elem = self.elem.root();
@@ -283,8 +283,8 @@ impl HTMLScriptElement {
 
                         let context = Arc::new(Mutex::new(ScriptContext {
                             elem: elem,
-                            data: RefCell::new(vec!()),
+                            data: vec!(),
-                            metadata: RefCell::new(None),
+                            metadata: None,
                             resume_on_completion: self.parser_inserted.get(),
                             url: url.clone(),
                         }));

components/script/dom/servohtmlparser.rs

@@ -31,7 +31,7 @@ use net_traits::{AsyncResponseListener, Metadata};
 use network_listener::PreInvoke;
 use parse::Parser;
 use script_task::{ScriptChan, ScriptTask};
-use std::cell::{Cell, RefCell};
+use std::cell::Cell;
 use std::default::Default;
 use url::Url;
 
@@ -69,9 +69,9 @@ pub type Tokenizer = tokenizer::Tokenizer<TreeBuilder<JS<Node>, Sink>>;
 /// The context required for asynchronously fetching a document and parsing it progressively.
 pub struct ParserContext {
     /// The parser that initiated the request.
-    parser: RefCell<Option<Trusted<ServoHTMLParser>>>,
+    parser: Option<Trusted<ServoHTMLParser>>,
     /// Is this document a synthesized document for a single image?
-    is_image_document: Cell<bool>,
+    is_image_document: bool,
     /// The pipeline associated with this document.
     id: PipelineId,
     /// The subpage associated with this document.
@@ -86,8 +86,8 @@ impl ParserContext {
     pub fn new(id: PipelineId, subpage: Option<SubpageId>, script_chan: Box<ScriptChan + Send>,
                url: Url) -> ParserContext {
         ParserContext {
-            parser: RefCell::new(None),
+            parser: None,
-            is_image_document: Cell::new(false),
+            is_image_document: false,
             id: id,
             subpage: subpage,
             script_chan: script_chan,
@@ -97,7 +97,7 @@ impl ParserContext {
 }
 
 impl AsyncResponseListener for ParserContext {
-    fn headers_available(&self, metadata: Metadata) {
+    fn headers_available(&mut self, metadata: Metadata) {
         let content_type = metadata.content_type.clone();
 
         let parser = ScriptTask::page_fetch_complete(self.id.clone(), self.subpage.clone(),
@@ -109,12 +109,11 @@ impl AsyncResponseListener for ParserContext {
 
         let parser = parser.r();
         let win = parser.window();
-        *self.parser.borrow_mut() = Some(Trusted::new(win.r().get_cx(), parser,
+        self.parser = Some(Trusted::new(win.r().get_cx(), parser, self.script_chan.clone()));
-                                                      self.script_chan.clone()));
 
         match content_type {
             Some(ContentType(Mime(TopLevel::Image, _, _))) => {
-                self.is_image_document.set(true);
+                self.is_image_document = true;
                 let page = format!("<html><body><img src='{}' /></body></html>",
                                    self.url.serialize());
                 parser.pending_input.borrow_mut().push(page);
@@ -137,11 +136,11 @@ impl AsyncResponseListener for ParserContext {
         }
     }
 
-    fn data_available(&self, payload: Vec<u8>) {
+    fn data_available(&mut self, payload: Vec<u8>) {
-        if !self.is_image_document.get() {
+        if !self.is_image_document {
             // FIXME: use Vec<u8> (html5ever #34)
             let data = UTF_8.decode(&payload, DecoderTrap::Replace).unwrap();
-            let parser = match self.parser.borrow().as_ref() {
+            let parser = match self.parser.as_ref() {
                 Some(parser) => parser.root(),
                 None => return,
             };
@@ -149,8 +148,8 @@ impl AsyncResponseListener for ParserContext {
         }
     }
 
-    fn response_complete(&self, status: Result<(), String>) {
+    fn response_complete(&mut self, status: Result<(), String>) {
-        let parser = match self.parser.borrow().as_ref() {
+        let parser = match self.parser.as_ref() {
             Some(parser) => parser.root(),
             None => return,
         };

components/script/dom/xmlhttprequest.rs

@@ -237,7 +237,7 @@ impl XMLHttpRequest {
                           resource_task: ResourceTask,
                           load_data: LoadData) {
         impl AsyncResponseListener for XHRContext {
-            fn headers_available(&self, metadata: Metadata) {
+            fn headers_available(&mut self, metadata: Metadata) {
                 let xhr = self.xhr.root();
                 let rv = xhr.r().process_headers_available(self.cors_request.clone(),
                                                            self.gen_id,
@@ -247,13 +247,13 @@ impl XMLHttpRequest {
                 }
             }
 
-            fn data_available(&self, payload: Vec<u8>) {
+            fn data_available(&mut self, payload: Vec<u8>) {
                 self.buf.borrow_mut().push_all(&payload);
                 let xhr = self.xhr.root();
                 xhr.r().process_data_available(self.gen_id, self.buf.borrow().clone());
             }
 
-            fn response_complete(&self, status: Result<(), String>) {
+            fn response_complete(&mut self, status: Result<(), String>) {
                 let xhr = self.xhr.root();
                 let rv = xhr.r().process_response_complete(self.gen_id, status);
                 *self.sync_status.borrow_mut() = Some(rv);

components/script/network_listener.rs

@@ -43,9 +43,9 @@ struct ListenerRunnable<T: AsyncResponseListener + PreInvoke + Send> {
 impl<T: AsyncResponseListener + PreInvoke + Send> Runnable for ListenerRunnable<T> {
     fn handler(self: Box<ListenerRunnable<T>>) {
         let this = *self;
-        let context = this.context.lock().unwrap();
+        let mut context = this.context.lock().unwrap();
         if context.should_invoke() {
-            this.action.process(&*context);
+            this.action.process(&mut *context);
         }
     }
 }