Auto merge of #23564 - mmiecz:clipboard-refactoring, r=jdm

Clipboard refactoring

<!-- Please describe your changes on the following line: -->
This PR removes clipboard handling from the constellation. Instead, now embedder handles it.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #23440 (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because it is enough to test manually in input box, if copying and pasting still works .

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

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

Cargo.lock

@@ -4019,6 +4019,7 @@ dependencies = [
  "backtrace 0.3.26 (registry+https://github.com/rust-lang/crates.io-index)",
  "bitflags 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "cc 1.0.35 (registry+https://github.com/rust-lang/crates.io-index)",
+ "clipboard 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "crossbeam-channel 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "euclid 0.19.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "gleam 0.6.16 (registry+https://github.com/rust-lang/crates.io-index)",

components/constellation/constellation.rs

@@ -107,7 +107,6 @@ use canvas::canvas_paint_thread::CanvasPaintThread;
 use canvas::webgl_thread::WebGLThreads;
 use canvas_traits::canvas::CanvasId;
 use canvas_traits::canvas::CanvasMsg;
-use clipboard::{ClipboardContext, ClipboardProvider};
 use compositing::compositor_thread::CompositorProxy;
 use compositing::compositor_thread::Msg as ToCompositorMsg;
 use compositing::SendableFrameTree;
@@ -361,9 +360,6 @@ pub struct Constellation<Message, LTF, STF> {
     /// The size of the top-level window.
     window_size: WindowSizeData,
 
-    /// Means of accessing the clipboard
-    clipboard_ctx: Option<ClipboardContext>,
-
     /// Bits of state used to interact with the webdriver implementation
     webdriver: WebDriverData,
 
@@ -734,13 +730,6 @@ where
                         device_pixel_ratio: TypedScale::new(device_pixels_per_px.unwrap_or(1.0)),
                     },
                     phantom: PhantomData,
-                    clipboard_ctx: match ClipboardContext::new() {
-                        Ok(c) => Some(c),
-                        Err(e) => {
-                            warn!("Error creating clipboard context ({})", e);
-                            None
-                        },
-                    },
                     webdriver: WebDriverData::new(),
                     scheduler_chan: TimerScheduler::start(),
                     document_states: HashMap::new(),
@@ -1524,30 +1513,6 @@ where
             FromScriptMsg::Focus => {
                 self.handle_focus_msg(source_pipeline_id);
             },
-            FromScriptMsg::GetClipboardContents(sender) => {
-                let contents = match self.clipboard_ctx {
-                    Some(ref mut ctx) => {
-                        match ctx.get_contents() {
-                            Ok(c) => c,
-                            Err(e) => {
-                                warn!("Error getting clipboard contents ({}), defaulting to empty string", e);
-                                "".to_owned()
-                            },
-                        }
-                    },
-                    None => "".to_owned(),
-                };
-                if let Err(e) = sender.send(contents) {
-                    warn!("Failed to send clipboard ({})", e);
-                }
-            },
-            FromScriptMsg::SetClipboardContents(s) => {
-                if let Some(ref mut ctx) = self.clipboard_ctx {
-                    if let Err(e) = ctx.set_contents(s) {
-                        warn!("Error setting clipboard contents ({})", e);
-                    }
-                }
-            },
             FromScriptMsg::VisibilityChangeComplete(is_visible) => {
                 self.handle_visibility_change_complete(source_pipeline_id, is_visible);
             },

components/embedder_traits/lib.rs

@@ -132,6 +132,10 @@ pub enum EmbedderMsg {
     AllowUnload(IpcSender<bool>),
     /// Sends an unconsumed key event back to the embedder.
     Keyboard(KeyboardEvent),
+    /// Gets system clipboard contents
+    GetClipboardContents(IpcSender<String>),
+    /// Sets system clipboard contents
+    SetClipboardContents(String),
     /// Changes the cursor.
     SetCursor(Cursor),
     /// A favicon was detected
@@ -175,6 +179,8 @@ impl Debug for EmbedderMsg {
             EmbedderMsg::AllowUnload(..) => write!(f, "AllowUnload"),
             EmbedderMsg::AllowNavigationRequest(..) => write!(f, "AllowNavigationRequest"),
             EmbedderMsg::Keyboard(..) => write!(f, "Keyboard"),
+            EmbedderMsg::GetClipboardContents(..) => write!(f, "GetClipboardContents"),
+            EmbedderMsg::SetClipboardContents(..) => write!(f, "SetClipboardContents"),
             EmbedderMsg::SetCursor(..) => write!(f, "SetCursor"),
             EmbedderMsg::NewFavicon(..) => write!(f, "NewFavicon"),
             EmbedderMsg::HeadParsed => write!(f, "HeadParsed"),

components/script/clipboard_provider.rs

@@ -2,9 +2,9 @@
  * 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 embedder_traits::EmbedderMsg;
 use ipc_channel::ipc::channel;
 use script_traits::{ScriptMsg, ScriptToConstellationChan};
-use std::borrow::ToOwned;
 
 pub trait ClipboardProvider {
     // blocking method to get the clipboard contents
@@ -16,31 +16,16 @@ pub trait ClipboardProvider {
 impl ClipboardProvider for ScriptToConstellationChan {
     fn clipboard_contents(&mut self) -> String {
         let (tx, rx) = channel().unwrap();
-        self.send(ScriptMsg::GetClipboardContents(tx)).unwrap();
+        self.send(ScriptMsg::ForwardToEmbedder(
+            EmbedderMsg::GetClipboardContents(tx),
+        ))
+        .unwrap();
         rx.recv().unwrap()
     }
     fn set_clipboard_contents(&mut self, s: String) {
-        self.send(ScriptMsg::SetClipboardContents(s)).unwrap();
+        self.send(ScriptMsg::ForwardToEmbedder(
-    }
+            EmbedderMsg::SetClipboardContents(s),
-}
+        ))
-
+        .unwrap();
-pub struct DummyClipboardContext {
-    content: String,
-}
-
-impl DummyClipboardContext {
-    pub fn new(s: &str) -> DummyClipboardContext {
-        DummyClipboardContext {
-            content: s.to_owned(),
-        }
-    }
-}
-
-impl ClipboardProvider for DummyClipboardContext {
-    fn clipboard_contents(&mut self) -> String {
-        self.content.clone()
-    }
-    fn set_clipboard_contents(&mut self, s: String) {
-        self.content = s;
     }
 }

components/script_traits/script_msg.rs

@@ -121,8 +121,6 @@ pub enum ScriptMsg {
     CreateCanvasPaintThread(Size2D<u64>, IpcSender<(IpcSender<CanvasMsg>, CanvasId)>),
     /// Notifies the constellation that this frame has received focus.
     Focus,
-    /// Requests that the constellation retrieve the current contents of the clipboard
-    GetClipboardContents(IpcSender<String>),
     /// Get the top-level browsing context info for a given browsing context.
     GetTopForBrowsingContext(
         BrowsingContextId,
@@ -183,8 +181,6 @@ pub enum ScriptMsg {
         AuxiliaryBrowsingContextLoadInfo,
         IpcSender<LayoutControlMsg>,
     ),
-    /// Requests that the constellation set the contents of the clipboard
-    SetClipboardContents(String),
     /// Mark a new document as active
     ActivateDocument,
     /// Set the document state for a pipeline (used by screenshot / reftests)
@@ -224,7 +220,6 @@ impl fmt::Debug for ScriptMsg {
             ChangeRunningAnimationsState(..) => "ChangeRunningAnimationsState",
             CreateCanvasPaintThread(..) => "CreateCanvasPaintThread",
             Focus => "Focus",
-            GetClipboardContents(..) => "GetClipboardContents",
             GetBrowsingContextInfo(..) => "GetBrowsingContextInfo",
             GetTopForBrowsingContext(..) => "GetParentBrowsingContext",
             GetChildBrowsingContextId(..) => "GetChildBrowsingContextId",
@@ -242,7 +237,6 @@ impl fmt::Debug for ScriptMsg {
             ScriptLoadedURLInIFrame(..) => "ScriptLoadedURLInIFrame",
             ScriptNewIFrame(..) => "ScriptNewIFrame",
             ScriptNewAuxiliary(..) => "ScriptNewAuxiliary",
-            SetClipboardContents(..) => "SetClipboardContents",
             ActivateDocument => "ActivateDocument",
             SetDocumentState(..) => "SetDocumentState",
             SetFinalUrl(..) => "SetFinalUrl",

ports/glutin/Cargo.toml

@@ -55,6 +55,7 @@ log = "0.4"
 rust-webvr = { version = "0.13", features = ["glwindow"] }
 webxr =  { git = "https://github.com/servo/webxr", features = ["glwindow"] }
 tinyfiledialogs = "3.0"
+clipboard = "0.5"
 
 [target.'cfg(any(target_os = "linux", target_os = "windows"))'.dependencies]
 image = "0.21"

ports/glutin/browser.rs

@@ -16,6 +16,7 @@ use servo::servo_config::opts;
 use servo::servo_config::pref;
 use servo::servo_url::ServoUrl;
 use servo::webrender_api::ScrollLocation;
+use clipboard::{ClipboardContext, ClipboardProvider};
 use std::env;
 use std::fs::File;
 use std::io::Write;
@@ -43,6 +44,7 @@ pub struct Browser<Window: WindowPortsMethods + ?Sized> {
     loading_state: Option<LoadingState>,
     window: Rc<Window>,
     event_queue: Vec<WindowEvent>,
+    clipboard_ctx: Option<ClipboardContext>,
     shutdown_requested: bool,
 }
 
@@ -66,6 +68,13 @@ where
             favicon: None,
             loading_state: None,
             window: window,
+            clipboard_ctx: match ClipboardContext::new() {
+                Ok(c) => Some(c),
+                Err(e) => {
+                    warn!("Error creating clipboard context ({})", e);
+                    None
+                },
+            },
             event_queue: Vec::new(),
             shutdown_requested: false,
         }
@@ -344,6 +353,30 @@ where
                 EmbedderMsg::Keyboard(key_event) => {
                     self.handle_key_from_servo(browser_id, key_event);
                 },
+                EmbedderMsg::GetClipboardContents(sender) => {
+                    let contents = match self.clipboard_ctx {
+                        Some(ref mut ctx) => {
+                            match ctx.get_contents() {
+                                Ok(c) => c,
+                                Err(e) => {
+                                    warn!("Error getting clipboard contents ({}), defaulting to empty string", e);
+                                    "".to_owned()
+                                },
+                            }
+                        },
+                        None => "".to_owned(),
+                    };
+                    if let Err(e) = sender.send(contents) {
+                        warn!("Failed to send clipboard ({})", e);
+                    }
+                }
+                EmbedderMsg::SetClipboardContents(text) => {
+                    if let Some(ref mut ctx) = self.clipboard_ctx {
+                        if let Err(e) = ctx.set_contents(text) {
+                            warn!("Error setting clipboard contents ({})", e);
+                        }
+                    }
+                }
                 EmbedderMsg::SetCursor(cursor) => {
                     self.window.set_cursor(cursor);
                 },

ports/libmlservo/src/lib.rs

@@ -378,6 +378,12 @@ impl HostTrait for HostCallbacks {
             keyboard(self.app, show)
         }
     }
+
+    fn get_clipboard_contents(&self) -> Option<String> {
+        None
+    }
+
+    fn set_clipboard_contents(&self, _contents: String) {}
 }
 
 pub struct ServoInstance {

ports/libsimpleservo/api/src/lib.rs

@@ -25,6 +25,7 @@ use servo::servo_url::ServoUrl;
 use servo::webrender_api::{DevicePixel, FramebufferPixel, ScrollLocation};
 use servo::webvr::{VRExternalShmemPtr, VRMainThreadHeartbeat, VRService, VRServiceManager};
 use servo::{self, gl, BrowserId, Servo};
+
 use std::cell::RefCell;
 use std::mem;
 use std::os::raw::c_void;
@@ -118,6 +119,10 @@ pub trait HostTrait {
     fn on_shutdown_complete(&self);
     /// A text input is focused.
     fn on_ime_state_changed(&self, show: bool);
+    /// Gets sytem clipboard contents
+    fn get_clipboard_contents(&self) -> Option<String>;
+    /// Sets system clipboard contents
+    fn set_clipboard_contents(&self, contents: String);
 }
 
 pub struct ServoGlue {
@@ -528,6 +533,13 @@ impl ServoGlue {
                     }
                     self.events.push(WindowEvent::SelectBrowser(new_browser_id));
                 },
+                EmbedderMsg::GetClipboardContents(sender) => {
+                    let contents = self.callbacks.host_callbacks.get_clipboard_contents();
+                    let _ = sender.send(contents.unwrap_or("".to_owned()));
+                },
+                EmbedderMsg::SetClipboardContents(text) => {
+                    self.callbacks.host_callbacks.set_clipboard_contents(text);
+                },
                 EmbedderMsg::CloseBrowser => {
                     // TODO: close the appropriate "tab".
                     let _ = self.browsers.pop();

ports/libsimpleservo/capi/src/lib.rs

@@ -43,6 +43,8 @@ pub struct CHostCallbacks {
     pub on_animating_changed: extern "C" fn(animating: bool),
     pub on_shutdown_complete: extern "C" fn(),
     pub on_ime_state_changed: extern "C" fn(show: bool),
+    pub get_clipboard_contents: extern "C" fn() -> *const c_char,
+    pub set_clipboard_contents: extern "C" fn(contents: *const c_char),
 }
 
 /// Servo options
@@ -358,4 +360,23 @@ impl HostTrait for HostCallbacks {
         debug!("on_ime_state_changed");
         (self.0.on_ime_state_changed)(show);
     }
+
+    fn get_clipboard_contents(&self) -> Option<String> {
+        debug!("get_clipboard_contents");
+        let raw_contents = (self.0.get_clipboard_contents)();
+        if raw_contents.is_null() {
+            return None;
+        }
+        let c_str = unsafe { CStr::from_ptr(raw_contents) };
+        let contents_str = c_str.to_str().expect("Can't create str");
+        Some(contents_str.to_owned())
+    }
+
+    fn set_clipboard_contents(&self, contents: String) {
+        debug!("set_clipboard_contents");
+        let contents = CString::new(contents).expect("Can't create string");
+        let contents_ptr = contents.as_ptr();
+        mem::forget(contents);
+        (self.0.set_clipboard_contents)(contents_ptr);
+    }
 }

ports/libsimpleservo/jniapi/src/lib.rs

@@ -476,6 +476,12 @@ impl HostTrait for HostCallbacks {
     }
 
     fn on_ime_state_changed(&self, _show: bool) {}
+
+    fn get_clipboard_contents(&self) -> Option<String> {
+        None
+    }
+
+    fn set_clipboard_contents(&self, _contents: String) {}
 }
 
 fn initialize_android_glue(env: &JNIEnv, activity: JObject) {

tests/unit/script/textinput.rs

@@ -8,13 +8,34 @@
 // except according to those terms.
 
 use keyboard_types::{Key, Modifiers};
-use script::clipboard_provider::DummyClipboardContext;
+use script::clipboard_provider::ClipboardProvider;
 use script::test::DOMString;
 use script::textinput::{
     Direction, Lines, Selection, SelectionDirection, TextInput, TextPoint, UTF16CodeUnits,
     UTF8Bytes,
 };
 
+pub struct DummyClipboardContext {
+    content: String,
+}
+
+impl DummyClipboardContext {
+    pub fn new(s: &str) -> DummyClipboardContext {
+        DummyClipboardContext {
+            content: s.to_owned(),
+        }
+    }
+}
+
+impl ClipboardProvider for DummyClipboardContext {
+    fn clipboard_contents(&mut self) -> String {
+        self.content.clone()
+    }
+    fn set_clipboard_contents(&mut self, s: String) {
+        self.content = s;
+    }
+}
+
 fn text_input(lines: Lines, s: &str) -> TextInput<DummyClipboardContext> {
     TextInput::new(
         lines,