Skip to content

Commit

Permalink
Auto merge of #13680 - frewsxcv:user-agent-cow, r=nox
Browse files Browse the repository at this point in the history
Migrate user agent string to `Cow<'static, str>`.

In most scenarios, where the user of Servo will not override the default
user agent, the user agent can be a `&'static str`. But since we allow
for customization, we currently use a `String` to represent the user
agent. This commit migrates the user agent to be represented as a
`Cow<'static, str`, which (at the cost of ergonomics) prevents
unnecessary allocations whenever cloning the user agent string in the
scenario the user doesn't override the user agent.

<!-- 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/13680)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed Oct 11, 2016
2 parents cad5a4e + 60afad1 commit 4dcd223
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 63 deletions.
6 changes: 4 additions & 2 deletions components/net/fetch/methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use net_traits::request::{Type, Origin, Window};
use net_traits::response::{HttpsState, TerminationReason};
use net_traits::response::{Response, ResponseBody, ResponseType};
use resource_thread::CancellationListener;
use std::borrow::Cow;
use std::collections::HashSet;
use std::fs::File;
use std::io::Read;
Expand All @@ -51,7 +52,7 @@ enum Data {

pub struct FetchContext {
pub state: HttpState,
pub user_agent: String,
pub user_agent: Cow<'static, str>,
pub devtools_chan: Option<Sender<DevtoolsControlMsg>>,
}

Expand Down Expand Up @@ -763,7 +764,8 @@ fn http_network_or_cache_fetch(request: Rc<Request>,

// Step 8
if !http_request.headers.borrow().has::<UserAgent>() {
http_request.headers.borrow_mut().set(UserAgent(context.user_agent.clone()));
let user_agent = context.user_agent.clone().into_owned();
http_request.headers.borrow_mut().set(UserAgent(user_agent));
}

match http_request.cache_mode.get() {
Expand Down
8 changes: 4 additions & 4 deletions components/net/http_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ use openssl::ssl::error::{OpensslError, SslError};
use profile_traits::time::{ProfilerCategory, ProfilerChan, TimerMetadata, profile};
use profile_traits::time::{TimerMetadataFrameType, TimerMetadataReflowType};
use resource_thread::{AuthCache, AuthCacheEntry, CancellationListener, send_error, start_sending_sniffed_opt};
use std::borrow::ToOwned;
use std::borrow::{Cow, ToOwned};
use std::boxed::FnBox;
use std::collections::HashSet;
use std::error::Error;
Expand All @@ -57,7 +57,7 @@ use util::prefs::PREFS;
use util::thread::spawn_named;
use uuid;

pub fn factory(user_agent: String,
pub fn factory(user_agent: Cow<'static, str>,
http_state: HttpState,
devtools_chan: Option<Sender<DevtoolsControlMsg>>,
profiler_chan: ProfilerChan,
Expand Down Expand Up @@ -137,7 +137,7 @@ fn load_for_consumer(load_data: LoadData,
devtools_chan: Option<Sender<DevtoolsControlMsg>>,
swmanager_chan: Option<IpcSender<CustomResponseMediator>>,
cancel_listener: CancellationListener,
user_agent: String) {
user_agent: Cow<'static, str>) {
let factory = NetworkHttpRequestFactory {
connector: connector,
};
Expand Down Expand Up @@ -865,7 +865,7 @@ pub fn load<A, B>(load_data: &LoadData,
http_state: &HttpState,
devtools_chan: Option<Sender<DevtoolsControlMsg>>,
request_factory: &HttpRequestFactory<R=A>,
user_agent: String,
user_agent: Cow<'static, str>,
cancel_listener: &CancellationListener,
swmanager_chan: Option<IpcSender<CustomResponseMediator>>)
-> Result<StreamedResponse, LoadError> where A: HttpRequest + 'static, B: UIProvider {
Expand Down
10 changes: 5 additions & 5 deletions components/net/resource_thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use net_traits::storage_thread::StorageThreadMsg;
use profile_traits::time::ProfilerChan;
use rustc_serialize::{Decodable, Encodable};
use rustc_serialize::json;
use std::borrow::ToOwned;
use std::borrow::{Cow, ToOwned};
use std::boxed::FnBox;
use std::cell::Cell;
use std::collections::HashMap;
Expand Down Expand Up @@ -164,7 +164,7 @@ fn start_sending_opt(start_chan: LoadConsumer, metadata: Metadata,
}

/// Returns a tuple of (public, private) senders to the new threads.
pub fn new_resource_threads(user_agent: String,
pub fn new_resource_threads(user_agent: Cow<'static, str>,
devtools_chan: Option<Sender<DevtoolsControlMsg>>,
profiler_chan: ProfilerChan,
config_dir: Option<PathBuf>)
Expand All @@ -181,7 +181,7 @@ pub fn new_resource_threads(user_agent: String,


/// Create a CoreResourceThread
pub fn new_core_resource_thread(user_agent: String,
pub fn new_core_resource_thread(user_agent: Cow<'static, str>,
devtools_chan: Option<Sender<DevtoolsControlMsg>>,
profiler_chan: ProfilerChan,
config_dir: Option<PathBuf>)
Expand Down Expand Up @@ -470,7 +470,7 @@ pub struct AuthCache {
}

pub struct CoreResourceManager {
user_agent: String,
user_agent: Cow<'static, str>,
mime_classifier: Arc<MimeClassifier>,
devtools_chan: Option<Sender<DevtoolsControlMsg>>,
swmanager_chan: Option<IpcSender<CustomResponseMediator>>,
Expand All @@ -481,7 +481,7 @@ pub struct CoreResourceManager {
}

impl CoreResourceManager {
pub fn new(user_agent: String,
pub fn new(user_agent: Cow<'static, str>,
devtools_channel: Option<Sender<DevtoolsControlMsg>>,
profiler_chan: ProfilerChan) -> CoreResourceManager {
CoreResourceManager {
Expand Down
2 changes: 1 addition & 1 deletion components/servo/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ fn create_constellation(opts: opts::Opts,
let bluetooth_thread: IpcSender<BluetoothMethodMsg> = BluetoothThreadFactory::new();

let (public_resource_threads, private_resource_threads) =
new_resource_threads(opts.user_agent.clone(),
new_resource_threads(opts.user_agent,
devtools_chan.clone(),
time_profiler_chan.clone(),
opts.config_dir.map(Into::into));
Expand Down
17 changes: 9 additions & 8 deletions components/util/opts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use getopts::Options;
use num_cpus;
use prefs::{self, PrefValue, PREFS};
use resource_files::set_resources_path;
use std::borrow::Cow;
use std::cmp;
use std::default::Default;
use std::env;
Expand Down Expand Up @@ -139,7 +140,7 @@ pub struct Opts {
pub initial_window_size: TypedSize2D<u32, ScreenPx>,

/// An optional string allowing the user agent to be set for testing.
pub user_agent: String,
pub user_agent: Cow<'static, str>,

/// Whether we're running in multiprocess mode.
pub multiprocess: bool,
Expand Down Expand Up @@ -460,7 +461,7 @@ pub enum RenderApi {

const DEFAULT_RENDER_API: RenderApi = RenderApi::GL;

fn default_user_agent_string(agent: UserAgent) -> String {
fn default_user_agent_string(agent: UserAgent) -> &'static str {
#[cfg(all(target_os = "linux", target_arch = "x86_64"))]
const DESKTOP_UA_STRING: &'static str =
"Mozilla/5.0 (X11; Linux x86_64; rv:37.0) Servo/1.0 Firefox/37.0";
Expand Down Expand Up @@ -488,7 +489,7 @@ fn default_user_agent_string(agent: UserAgent) -> String {
UserAgent::Android => {
"Mozilla/5.0 (Android; Mobile; rv:37.0) Servo/1.0 Firefox/37.0"
}
}.to_owned()
}
}

#[cfg(target_os = "android")]
Expand Down Expand Up @@ -529,7 +530,7 @@ pub fn default_opts() -> Opts {
devtools_port: None,
webdriver_port: None,
initial_window_size: TypedSize2D::new(1024, 740),
user_agent: default_user_agent_string(DEFAULT_USER_AGENT),
user_agent: default_user_agent_string(DEFAULT_USER_AGENT).into(),
multiprocess: false,
random_pipeline_closure_probability: None,
random_pipeline_closure_seed: None,
Expand Down Expand Up @@ -778,10 +779,10 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
}

let user_agent = match opt_match.opt_str("u") {
Some(ref ua) if ua == "android" => default_user_agent_string(UserAgent::Android),
Some(ref ua) if ua == "desktop" => default_user_agent_string(UserAgent::Desktop),
Some(ua) => ua,
None => default_user_agent_string(DEFAULT_USER_AGENT),
Some(ref ua) if ua == "android" => default_user_agent_string(UserAgent::Android).into(),
Some(ref ua) if ua == "desktop" => default_user_agent_string(UserAgent::Desktop).into(),
Some(ua) => ua.into(),
None => default_user_agent_string(DEFAULT_USER_AGENT).into(),
};

let user_stylesheets = opt_match.opt_strs("user-stylesheet").iter().map(|filename| {
Expand Down

0 comments on commit 4dcd223

Please sign in to comment.