servo: Merge #17686 - remove the ability to request for the title (from paulrouget:rm_Title); r=jdm
authorPaul Rouget <me@paulrouget.com>
Wed, 12 Jul 2017 09:50:45 -0700
changeset 417222 b78777da01e8378072ea647da1bc29b95ac21232
parent 417221 835d645c14f001fc4ad1e350b656203bfa76fcca
child 417223 d982adfcfb188b4863e66f0a6811b7734c77a520
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdm
milestone56.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
servo: Merge #17686 - remove the ability to request for the title (from paulrouget:rm_Title); r=jdm If we offer the ability to request info like these, we need to expose a lot of other getters, like url, loading state, favicon, … It's up to the embedder to keep track of the state of a browser. So let's keep that consistent. <!-- Please describe your changes on the following line: --> --- <!-- 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 - [ ] These changes fix #__ (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- 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. --> Source-Repo: https://github.com/servo/servo Source-Revision: ee47fe192511558f69d12036cc9cab5a009b8b25
servo/components/compositing/compositor.rs
servo/components/constellation/constellation.rs
servo/components/script_traits/lib.rs
servo/components/servo/lib.rs
servo/ports/cef/browser.rs
servo/ports/cef/frame.rs
servo/ports/cef/window.rs
--- a/servo/components/compositing/compositor.rs
+++ b/servo/components/compositing/compositor.rs
@@ -458,17 +458,16 @@ impl<Window: WindowMethods> IOCompositor
             (Msg::ChangePageTitle(pipeline_id, title), ShutdownState::NotShuttingDown) => {
                 self.change_page_title(pipeline_id, title);
             }
 
             (Msg::SetFrameTree(frame_tree),
              ShutdownState::NotShuttingDown) => {
                 self.set_frame_tree(&frame_tree);
                 self.send_viewport_rects();
-                self.title_for_main_frame();
             }
 
             (Msg::ScrollFragmentPoint(scroll_root_id, point, _),
              ShutdownState::NotShuttingDown) => {
                 self.scroll_fragment_to_point(scroll_root_id, point);
             }
 
             (Msg::MoveTo(point),
@@ -1654,27 +1653,16 @@ impl<Window: WindowMethods> IOCompositor
             }
         }
     }
 
     pub fn pinch_zoom_level(&self) -> f32 {
         // TODO(gw): Access via WR.
         1.0
     }
-
-    pub fn title_for_main_frame(&self) {
-        let root_pipeline_id = match self.root_pipeline {
-            None => return,
-            Some(ref root_pipeline) => root_pipeline.id,
-        };
-        let msg = ConstellationMsg::GetPipelineTitle(root_pipeline_id);
-        if let Err(e) = self.constellation_chan.send(msg) {
-            warn!("Failed to send pipeline title ({}).", e);
-        }
-    }
 }
 
 
 /// Why we performed a composite. This is used for debugging.
 #[derive(Copy, Clone, PartialEq, Debug)]
 pub enum CompositingReason {
     /// We hit the delayed composition timeout. (See `delayed_composition.rs`.)
     DelayedCompositeTimeout,
--- a/servo/components/constellation/constellation.rs
+++ b/servo/components/constellation/constellation.rs
@@ -914,20 +914,16 @@ impl<Message, LTF, STF> Constellation<Me
             }
             FromCompositorMsg::GetFocusTopLevelBrowsingContext(resp_chan) => {
                 debug!("constellation got get focus browsing context message");
                 let focus_browsing_context = self.focus_pipeline_id
                     .and_then(|pipeline_id| self.pipelines.get(&pipeline_id))
                     .map(|pipeline| pipeline.top_level_browsing_context_id);
                 let _ = resp_chan.send(focus_browsing_context);
             }
-            FromCompositorMsg::GetPipelineTitle(pipeline_id) => {
-                debug!("constellation got get-pipeline-title message");
-                self.handle_get_pipeline_title_msg(pipeline_id);
-            }
             FromCompositorMsg::KeyEvent(ch, key, state, modifiers) => {
                 debug!("constellation got key event message");
                 self.handle_key_msg(ch, key, state, modifiers);
             }
             // Load a new page from a typed url
             // If there is already a pending page (self.pending_changes), it will not be overridden;
             // However, if the id is not encompassed by another change, it will be.
             FromCompositorMsg::LoadUrl(source_id, load_data) => {
@@ -1911,26 +1907,16 @@ impl<Message, LTF, STF> Constellation<Me
             None => return warn!("Pipeline {} got reload event after closure.", pipeline_id),
             Some(pipeline) => pipeline.event_loop.send(msg),
         };
         if let Err(e) = result {
             self.handle_send_error(pipeline_id, e);
         }
     }
 
-    fn handle_get_pipeline_title_msg(&mut self, pipeline_id: PipelineId) {
-        let result = match self.pipelines.get(&pipeline_id) {
-            None => return self.compositor_proxy.send(ToCompositorMsg::ChangePageTitle(pipeline_id, None)),
-            Some(pipeline) => pipeline.event_loop.send(ConstellationControlMsg::GetTitle(pipeline_id)),
-        };
-        if let Err(e) = result {
-            self.handle_send_error(pipeline_id, e);
-        }
-    }
-
     fn handle_post_message_msg(&mut self,
                                browsing_context_id: BrowsingContextId,
                                origin: Option<ImmutableOrigin>,
                                data: Vec<u8>)
     {
         let pipeline_id = match self.browsing_contexts.get(&browsing_context_id) {
             None => return warn!("postMessage to closed browsing_context {}.", browsing_context_id),
             Some(browsing_context) => browsing_context.pipeline_id,
--- a/servo/components/script_traits/lib.rs
+++ b/servo/components/script_traits/lib.rs
@@ -742,19 +742,16 @@ pub enum ConstellationMsg {
     /// with the provided pipeline id
     GetBrowsingContext(PipelineId, IpcSender<Option<BrowsingContextId>>),
     /// Request that the constellation send the current pipeline id for the provided
     /// browsing context id, over a provided channel.
     GetPipeline(BrowsingContextId, IpcSender<Option<PipelineId>>),
     /// Request that the constellation send the current focused top-level browsing context id,
     /// over a provided channel.
     GetFocusTopLevelBrowsingContext(IpcSender<Option<TopLevelBrowsingContextId>>),
-    /// Requests that the constellation inform the compositor of the title of the pipeline
-    /// immediately.
-    GetPipelineTitle(PipelineId),
     /// Request to load the initial page.
     InitLoadUrl(ServoUrl),
     /// Query the constellation to see if the current compositor output is stable
     IsReadyToSaveImage(HashMap<PipelineId, Epoch>),
     /// Inform the constellation of a key event.
     KeyEvent(Option<char>, Key, KeyState, KeyModifiers),
     /// Request to load a page.
     LoadUrl(PipelineId, LoadData),
--- a/servo/components/servo/lib.rs
+++ b/servo/components/servo/lib.rs
@@ -247,20 +247,16 @@ impl<Window> Browser<Window> where Windo
     pub fn repaint_synchronously(&mut self) {
         self.compositor.repaint_synchronously()
     }
 
     pub fn pinch_zoom_level(&self) -> f32 {
         self.compositor.pinch_zoom_level()
     }
 
-    pub fn request_title_for_main_frame(&self) {
-        self.compositor.title_for_main_frame()
-    }
-
     pub fn setup_logging(&self) {
         let constellation_chan = self.constellation_chan.clone();
         log::set_logger(|max_log_level| {
             let env_logger = EnvLogger::new();
             let con_logger = FromCompositorLogger::new(constellation_chan);
             let filter = max(env_logger.filter(), con_logger.filter());
             let logger = BothLogger(env_logger, con_logger);
             max_log_level.set(filter);
--- a/servo/ports/cef/browser.rs
+++ b/servo/ports/cef/browser.rs
@@ -36,24 +36,16 @@ impl ServoBrowser {
     fn handle_event(&mut self, event: WindowEvent) {
         match *self {
             ServoBrowser::OnScreen(ref mut browser) => { browser.handle_events(vec![event]); }
             ServoBrowser::OffScreen(ref mut browser) => { browser.handle_events(vec![event]); }
             ServoBrowser::Invalid => {}
         }
     }
 
-    pub fn request_title_for_main_frame(&self) {
-        match *self {
-            ServoBrowser::OnScreen(ref browser) => browser.request_title_for_main_frame(),
-            ServoBrowser::OffScreen(ref browser) => browser.request_title_for_main_frame(),
-            ServoBrowser::Invalid => {}
-        }
-    }
-
     pub fn pinch_zoom_level(&self) -> f32 {
         match *self {
             ServoBrowser::OnScreen(ref browser) => browser.pinch_zoom_level(),
             ServoBrowser::OffScreen(ref browser) => browser.pinch_zoom_level(),
             ServoBrowser::Invalid => 1.0,
         }
     }
 }
@@ -158,17 +150,16 @@ impl ServoCefBrowser {
             window_handle: window_handle,
         }
     }
 }
 
 pub trait ServoCefBrowserExtensions {
     fn init(&self, window_info: &cef_window_info_t);
     fn send_window_event(&self, event: WindowEvent);
-    fn request_title_for_main_frame(&self);
     fn pinch_zoom_level(&self) -> f32;
 }
 
 impl ServoCefBrowserExtensions for CefBrowser {
     fn init(&self, window_info: &cef_window_info_t) {
         if window_info.windowless_rendering_enabled != 0 {
             let window = window::Window::new(window_info.width, window_info.height);
             window.set_browser(self.clone());
@@ -195,20 +186,16 @@ impl ServoCefBrowserExtensions for CefBr
         } else {
             // If we fail to borrow mutably, this means we're trying to send an
             // event while processing another one. This will cause general badness,
             // we just queue up that event instead of immediately processing it.
             browser.message_queue.borrow_mut().push(event);
         }
     }
 
-    fn request_title_for_main_frame(&self) {
-        self.downcast().servo_browser.borrow().request_title_for_main_frame()
-    }
-
     fn pinch_zoom_level(&self) -> f32 {
         self.downcast().servo_browser.borrow().pinch_zoom_level()
     }
 }
 
 #[cfg(target_os="macos")]
 pub fn get_null_window_handle() -> cef_window_handle_t {
     ptr::null_mut()
--- a/servo/ports/cef/frame.rs
+++ b/servo/ports/cef/frame.rs
@@ -6,28 +6,28 @@ use browser::ServoCefBrowserExtensions;
 use eutil::Downcast;
 use interfaces::{CefBrowser, CefFrame, CefStringVisitor, cef_frame_t, cef_string_visitor_t};
 use types::{cef_string_t, cef_string_userfree_t};
 
 use compositing::windowing::WindowEvent;
 use std::cell::RefCell;
 
 pub struct ServoCefFrame {
-    pub title_visitor: RefCell<Option<CefStringVisitor>>,
     pub url: RefCell<String>,
+    pub title: RefCell<Vec<u16>>,
 
     /// A reference to the browser.
     pub browser: RefCell<Option<CefBrowser>>,
 }
 
 impl ServoCefFrame {
     pub fn new() -> ServoCefFrame {
         ServoCefFrame {
-            title_visitor: RefCell::new(None),
             url: RefCell::new(String::new()),
+            title: RefCell::new(vec![]),
             browser: RefCell::new(None),
         }
     }
 }
 
 full_cef_class_impl! {
     ServoCefFrame : CefFrame, cef_frame_t {
         fn load_url(&this, url: *const cef_string_t [&[u16]],) -> () {{
@@ -39,33 +39,28 @@ full_cef_class_impl! {
         }}
         fn get_url(&this,) -> cef_string_userfree_t {{
             // FIXME(https://github.com/rust-lang/rust/issues/23338)
             let url = this.downcast().url.borrow();
             (*url).clone()
         }}
         fn get_text(&this, visitor: *mut cef_string_visitor_t [CefStringVisitor],) -> () {{
             let this = this.downcast();
-            *this.title_visitor.borrow_mut() = Some(visitor);
-            this.browser.borrow().as_ref().unwrap().request_title_for_main_frame();
+            let str = &*this.title.borrow();
+            visitor.visit(str)
         }}
     }
 }
 
 pub trait ServoCefFrameExtensions {
     fn set_browser(&self, browser: CefBrowser);
-    fn set_url(&self, url: &[u16]);
     fn load(&self);
 }
 
 impl ServoCefFrameExtensions for CefFrame {
     fn set_browser(&self, browser: CefBrowser) {
         *self.downcast().browser.borrow_mut() = Some(browser)
     }
-    fn set_url(&self, url: &[u16]) {
-        let frame = self.downcast();
-        *frame.url.borrow_mut() = String::from_utf16(url).unwrap();
-    }
     fn load(&self) {
         let event = WindowEvent::LoadUrl(self.downcast().url.borrow().clone());
         self.downcast().browser.borrow_mut().as_mut().unwrap().send_window_event(event);
     }
 }
--- a/servo/ports/cef/window.rs
+++ b/servo/ports/cef/window.rs
@@ -428,30 +428,27 @@ impl WindowMethods for Window {
     fn set_page_title(&self, string: Option<String>) {
         let browser = self.cef_browser.borrow();
         let browser = match *browser {
             None => return,
             Some(ref browser) => browser,
         };
         let frame = browser.get_main_frame();
         let frame = frame.downcast();
-        let mut title_visitor = frame.title_visitor.borrow_mut();
         let str = match string {
             Some(s) => s.encode_utf16().collect(),
             None => vec![]
         };
 
         if check_ptr_exist!(browser.get_host().get_client(), get_display_handler) &&
            check_ptr_exist!(browser.get_host().get_client().get_display_handler(), on_title_change) {
             browser.get_host().get_client().get_display_handler().on_title_change((*browser).clone(), str.as_slice());
         }
 
-        if let Some(ref mut visitor) = *title_visitor {
-            visitor.visit(&str);
-        }
+        *frame.title.borrow_mut() = str;
     }
 
     fn history_changed(&self, history: Vec<LoadData>, current: usize) {
         let browser = self.cef_browser.borrow();
         let browser = match *browser {
             None => return,
             Some(ref browser) => browser,
         };