Auto merge of #1037 - nical:arc-clone-syntax, r=glennw

Favor the function over the method syntax when cloning a reference counted pointer.

```data.clone()``` reads like performance hazard, due to the vague definition of Clone, which can be either very expensive if data is an uniquely owned large resource or very cheap if it is a reference counted pointer to such a resource. This has already confused people reading the code and even code reviews several times in webrender, so I would like to work around this unfortunate ambiguity.

I first tried to address this at the [standard library level](rust-lang/rfcs#1954) but the decision of the rust team appears to be that we should use the function syntax instead of the method syntax to address the problem.

See also the [clippy issue](https://github.com/Manishearth/rust-clippy/issues/1645) about providing a lint for this.

I decided to take a systematic approach and use the function call syntax for all of the outstanding reference counted pointer clones that I came across (I would not be surprised that I missed some of them). Rather than try to separate the obvious from the ambiguous ones. I would rather make this a default policy than have to debate about where the line is in each code review.

The three commits are independent and can be reviewed separately (although they are small and simple enough to look at as one diff if you prefer).

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1037)
<!-- Reviewable:end -->

Author: bors-servo

webrender/src/device.rs

@@ -676,10 +676,10 @@ impl<T> GpuProfiler<T> {
         GpuProfiler {
             next_frame: 0,
             frames: [
-                      GpuFrameProfile::new(gl.clone()),
-                      GpuFrameProfile::new(gl.clone()),
-                      GpuFrameProfile::new(gl.clone()),
-                      GpuFrameProfile::new(gl.clone()),
+                      GpuFrameProfile::new(Rc::clone(&gl)),
+                      GpuFrameProfile::new(Rc::clone(&gl)),
+                      GpuFrameProfile::new(Rc::clone(&gl)),
+                      GpuFrameProfile::new(Rc::clone(&gl)),
                     ],
         }
     }
@@ -721,12 +721,12 @@ impl GpuMarker {
             gl::GlType::Gl =>  {
                 gl.push_group_marker_ext(message);
                 GpuMarker{
-                    gl: gl.clone(),
+                    gl: Rc::clone(&gl),
                 }
             }
             gl::GlType::Gles => {
                 GpuMarker{
-                    gl: gl.clone(),
+                    gl: Rc::clone(&gl),
                 }
             }
         }
@@ -1108,7 +1108,7 @@ impl Device {
             };
 
             let texture = Texture {
-                gl: self.gl.clone(),
+                gl: Rc::clone(&self.gl),
                 id: id,
                 width: 0,
                 height: 0,
@@ -1451,7 +1451,7 @@ impl Device {
         }
 
         let program = Program {
-            gl: self.gl.clone(),
+            gl: Rc::clone(&self.gl),
             name: base_filename.to_owned(),
             id: pid,
             u_transform: -1,
@@ -1795,7 +1795,7 @@ impl Device {
         ibo_id.bind(self.gl()); // force it to be a part of VAO
 
         let vao = VAO {
-            gl: self.gl.clone(),
+            gl: Rc::clone(&self.gl),
             id: vao_id,
             ibo_id: ibo_id,
             main_vbo_id: main_vbo_id,

webrender/src/render_backend.rs

@@ -301,7 +301,7 @@ impl RenderBackend {
                             if let Some(ref wrapper) = self.webrender_context_handle {
                                 let dispatcher: Option<Box<GLContextDispatcher>> = if cfg!(target_os = "windows") {
                                     Some(Box::new(WebRenderGLDispatcher {
-                                        dispatcher: self.main_thread_dispatcher.clone()
+                                        dispatcher: Arc::clone(&self.main_thread_dispatcher)
                                     }))
                                 } else {
                                     None

webrender/src/renderer.rs

@@ -589,7 +589,7 @@ impl Renderer {
 
         let file_watch_handler = FileWatcher {
             result_tx: result_tx.clone(),
-            notifier: notifier.clone(),
+            notifier: Arc::clone(&notifier),
         };
 
         let mut device = Device::new(gl,
@@ -857,11 +857,11 @@ impl Renderer {
         device.end_frame();
 
         let main_thread_dispatcher = Arc::new(Mutex::new(None));
-        let backend_notifier = notifier.clone();
-        let backend_main_thread_dispatcher = main_thread_dispatcher.clone();
+        let backend_notifier = Arc::clone(&notifier);
+        let backend_main_thread_dispatcher = Arc::clone(&main_thread_dispatcher);
 
         let vr_compositor = Arc::new(Mutex::new(None));
-        let backend_vr_compositor = vr_compositor.clone();
+        let backend_vr_compositor = Arc::clone(&vr_compositor);
 
         // We need a reference to the webrender context from the render backend in order to share
         // texture ids

webrender/src/resource_cache.rs

@@ -406,7 +406,7 @@ impl ResourceCache {
                 if !same_epoch && self.blob_image_requests.insert(request) {
                     renderer.request_blob_image(
                         key,
-                        data.clone(),
+                        Arc::clone(&data),
                         &BlobImageDescriptor {
                             width: template.descriptor.width,
                             height: template.descriptor.height,
@@ -846,7 +846,7 @@ fn spawn_glyph_cache_thread(workers: Arc<Mutex<ThreadPool>>) -> (Sender<GlyphCac
 
         let barrier = Arc::new(Barrier::new(worker_count));
         for i in 0..worker_count {
-            let barrier = barrier.clone();
+            let barrier = Arc::clone(&barrier);
             workers.lock().unwrap().execute(move || {
                 register_thread_with_profiler(format!("Glyph Worker {}", i));
                 barrier.wait();
@@ -882,7 +882,7 @@ fn spawn_glyph_cache_thread(workers: Arc<Mutex<ThreadPool>>) -> (Sender<GlyphCac
                     // added to each worker thread.
                     let barrier = Arc::new(Barrier::new(worker_count));
                     for _ in 0..worker_count {
-                        let barrier = barrier.clone();
+                        let barrier = Arc::clone(&barrier);
                         let font_template = font_template.clone();
                         workers.lock().unwrap().execute(move || {
                             FONT_CONTEXT.with(|font_context| {
@@ -908,7 +908,7 @@ fn spawn_glyph_cache_thread(workers: Arc<Mutex<ThreadPool>>) -> (Sender<GlyphCac
                     // Delete a font from the font context in each worker thread.
                     let barrier = Arc::new(Barrier::new(worker_count));
                     for _ in 0..worker_count {
-                        let barrier = barrier.clone();
+                        let barrier = Arc::clone(&barrier);
                         workers.lock().unwrap().execute(move || {
                             FONT_CONTEXT.with(|font_context| {
                                 let mut font_context = font_context.borrow_mut();