From 59689475af654c1fce0cb24cfdefac46a8fb97e7 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Tue, 4 Oct 2016 11:17:05 +0200 Subject: [PATCH 1/6] Avoid PendingAsyncLoad in fetch_async. --- components/net_traits/lib.rs | 7 ------- components/script/document_loader.rs | 12 +++++------- components/script/dom/document.rs | 5 ++--- components/script/dom/htmlscriptelement.rs | 2 +- 4 files changed, 8 insertions(+), 18 deletions(-) diff --git a/components/net_traits/lib.rs b/components/net_traits/lib.rs index 6317a7af1d3..b2d2456e211 100644 --- a/components/net_traits/lib.rs +++ b/components/net_traits/lib.rs @@ -536,13 +536,6 @@ impl PendingAsyncLoad { let consumer = LoadConsumer::Listener(listener); self.core_resource_thread.send(CoreResourceMsg::Load(load_data, consumer, None)).unwrap(); } - - /// Initiate the fetch associated with this pending load. - pub fn fetch_async(mut self, request: RequestInit, fetch_target: IpcSender) { - self.guard.neuter(); - - self.core_resource_thread.send(CoreResourceMsg::Fetch(request, fetch_target)).unwrap(); - } } /// Message sent in response to `Load`. Contains metadata, and a port diff --git a/components/script/document_loader.rs b/components/script/document_loader.rs index 5209b68b9ac..516a026e61d 100644 --- a/components/script/document_loader.rs +++ b/components/script/document_loader.rs @@ -9,8 +9,8 @@ use dom::bindings::js::JS; use dom::document::Document; use ipc_channel::ipc::IpcSender; use msg::constellation_msg::{PipelineId, ReferrerPolicy}; -use net_traits::{AsyncResponseTarget, PendingAsyncLoad, LoadContext}; -use net_traits::{FetchResponseMsg, ResourceThreads, IpcSend}; +use net_traits::{AsyncResponseTarget, CoreResourceMsg, PendingAsyncLoad}; +use net_traits::{FetchResponseMsg, LoadContext, ResourceThreads, IpcSend}; use net_traits::request::RequestInit; use std::thread; use url::Url; @@ -154,11 +154,9 @@ impl DocumentLoader { pub fn fetch_async(&mut self, load: LoadType, request: RequestInit, - fetch_target: IpcSender, - referrer: &Document, - referrer_policy: Option) { - let pending = self.prepare_async_load(load, referrer, referrer_policy); - pending.fetch_async(request, fetch_target); + fetch_target: IpcSender) { + self.add_blocking_load(load); + self.resource_threads.sender().send(CoreResourceMsg::Fetch(request, fetch_target)).unwrap(); } /// Mark an in-progress network request complete. diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 1b91e743a78..2d645a53775 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -1441,10 +1441,9 @@ impl Document { pub fn fetch_async(&self, load: LoadType, request: RequestInit, - fetch_target: IpcSender, - referrer_policy: Option) { + fetch_target: IpcSender) { let mut loader = self.loader.borrow_mut(); - loader.fetch_async(load, request, fetch_target, self, referrer_policy); + loader.fetch_async(load, request, fetch_target); } pub fn finish_load(&self, load: LoadType) { diff --git a/components/script/dom/htmlscriptelement.rs b/components/script/dom/htmlscriptelement.rs index ed550309a3c..a4bcbb97d52 100644 --- a/components/script/dom/htmlscriptelement.rs +++ b/components/script/dom/htmlscriptelement.rs @@ -271,7 +271,7 @@ fn fetch_a_classic_script(script: &HTMLScriptElement, ROUTER.add_route(action_receiver.to_opaque(), box move |message| { listener.notify_fetch(message.to().unwrap()); }); - doc.fetch_async(LoadType::Script(url), request, action_sender, None); + doc.fetch_async(LoadType::Script(url), request, action_sender); } impl HTMLScriptElement { From 8d01baaf44b3707878bba98a43f481dfc369579e Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Tue, 4 Oct 2016 11:27:45 +0200 Subject: [PATCH 2/6] Remove unused Document::prepare_async_load. --- components/script/dom/document.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 2d645a53775..a765f767f2d 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -96,7 +96,7 @@ use js::jsapi::JS_GetRuntime; use msg::constellation_msg::{ALT, CONTROL, SHIFT, SUPER}; use msg::constellation_msg::{Key, KeyModifiers, KeyState}; use msg::constellation_msg::{PipelineId, ReferrerPolicy}; -use net_traits::{AsyncResponseTarget, FetchResponseMsg, IpcSend, PendingAsyncLoad}; +use net_traits::{AsyncResponseTarget, FetchResponseMsg, IpcSend}; use net_traits::CookieSource::NonHTTP; use net_traits::CoreResourceMsg::{GetCookiesForUrl, SetCookiesForUrl}; use net_traits::request::RequestInit; @@ -1429,11 +1429,6 @@ impl Document { loader.add_blocking_load(load) } - pub fn prepare_async_load(&self, load: LoadType, referrer_policy: Option) -> PendingAsyncLoad { - let mut loader = self.loader.borrow_mut(); - loader.prepare_async_load(load, self, referrer_policy) - } - pub fn load_async(&self, load: LoadType, listener: AsyncResponseTarget, referrer_policy: Option) { let mut loader = self.loader.borrow_mut(); loader.load_async(load, listener, self, referrer_policy); From e4c0007d04fea15b912be1fbbe0dac7b295d6cc9 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Tue, 4 Oct 2016 13:04:09 +0200 Subject: [PATCH 3/6] Inline prepare_async_load. --- components/script/document_loader.rs | 29 ++++++++++------------------ 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/components/script/document_loader.rs b/components/script/document_loader.rs index 516a026e61d..65f5ddf55af 100644 --- a/components/script/document_loader.rs +++ b/components/script/document_loader.rs @@ -49,7 +49,7 @@ impl LoadType { } /// Canary value ensuring that manually added blocking loads (ie. ones that weren't -/// created via DocumentLoader::prepare_async_load) are always removed by the time +/// created via DocumentLoader::{load_async, fetch_async}) are always removed by the time /// that the owner is destroyed. #[derive(JSTraceable, HeapSizeOf)] #[must_root] @@ -123,30 +123,21 @@ impl DocumentLoader { self.blocking_loads.push(load); } - /// Create a new pending network request, which can be initiated at some point in - /// the future. - pub fn prepare_async_load(&mut self, - load: LoadType, - referrer: &Document, - referrer_policy: Option) -> PendingAsyncLoad { - let context = load.to_load_context(); - let url = load.url().clone(); - self.add_blocking_load(load); - PendingAsyncLoad::new(context, - self.resource_threads.sender(), - url, - self.pipeline, - referrer_policy.or(referrer.get_referrer_policy()), - Some(referrer.url().clone())) - } - /// Create and initiate a new network request. pub fn load_async(&mut self, load: LoadType, listener: AsyncResponseTarget, referrer: &Document, referrer_policy: Option) { - let pending = self.prepare_async_load(load, referrer, referrer_policy); + let context = load.to_load_context(); + let url = load.url().clone(); + self.add_blocking_load(load); + let pending = PendingAsyncLoad::new(context, + self.resource_threads.sender(), + url, + self.pipeline, + referrer_policy.or(referrer.get_referrer_policy()), + Some(referrer.url().clone())); pending.load_async(listener) } From aa48ec76287e6951a9a60089bf3d683d796d3af6 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Tue, 4 Oct 2016 13:11:03 +0200 Subject: [PATCH 4/6] Remove Document::add_blocking_load. --- components/script/document_loader.rs | 4 ++-- components/script/dom/document.rs | 6 ------ 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/components/script/document_loader.rs b/components/script/document_loader.rs index 65f5ddf55af..f56e5c54a9d 100644 --- a/components/script/document_loader.rs +++ b/components/script/document_loader.rs @@ -63,7 +63,7 @@ pub struct LoadBlocker { impl LoadBlocker { /// Mark the document's load event as blocked on this new load. pub fn new(doc: &Document, load: LoadType) -> LoadBlocker { - doc.add_blocking_load(load.clone()); + doc.mut_loader().add_blocking_load(load.clone()); LoadBlocker { doc: JS::from_ref(doc), load: Some(load), @@ -119,7 +119,7 @@ impl DocumentLoader { } /// Add a load to the list of blocking loads. - pub fn add_blocking_load(&mut self, load: LoadType) { + fn add_blocking_load(&mut self, load: LoadType) { self.blocking_loads.push(load); } diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index a765f767f2d..f8238d645e1 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -1423,12 +1423,6 @@ impl Document { ReflowReason::RequestAnimationFrame); } - /// Add a load to the list of loads blocking this document's load. - pub fn add_blocking_load(&self, load: LoadType) { - let mut loader = self.loader.borrow_mut(); - loader.add_blocking_load(load) - } - pub fn load_async(&self, load: LoadType, listener: AsyncResponseTarget, referrer_policy: Option) { let mut loader = self.loader.borrow_mut(); loader.load_async(load, listener, self, referrer_policy); From 1a2db0b32711f2e52951c33f8f2074faeccee2d1 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Tue, 4 Oct 2016 13:24:03 +0200 Subject: [PATCH 5/6] Privatize PendingAsyncLoad. --- components/gfx/font_cache_thread.rs | 16 ++++++------- components/net_traits/lib.rs | 35 +++++++++++++++++++++------- components/script/document_loader.rs | 16 ++++++------- 3 files changed, 42 insertions(+), 25 deletions(-) diff --git a/components/gfx/font_cache_thread.rs b/components/gfx/font_cache_thread.rs index 26fcd6b98b7..c3fcf7dfc07 100644 --- a/components/gfx/font_cache_thread.rs +++ b/components/gfx/font_cache_thread.rs @@ -7,7 +7,7 @@ use fontsan; use ipc_channel::ipc::{self, IpcReceiver, IpcSender}; use ipc_channel::router::ROUTER; use mime::{TopLevel, SubLevel}; -use net_traits::{AsyncResponseTarget, LoadContext, PendingAsyncLoad, CoreResourceThread, ResponseAction}; +use net_traits::{AsyncResponseTarget, LoadContext, CoreResourceThread, ResponseAction, load_async}; use platform::font_context::FontContextHandle; use platform::font_list::SANS_SERIF_FONT_FAMILY; use platform::font_list::for_each_available_family; @@ -208,17 +208,17 @@ impl FontCache { match src { Source::Url(ref url_source) => { let url = &url_source.url; - let load = PendingAsyncLoad::new(LoadContext::Font, - self.core_resource_thread.clone(), - url.clone(), - None, - None, - None); let (data_sender, data_receiver) = ipc::channel().unwrap(); let data_target = AsyncResponseTarget { sender: data_sender, }; - load.load_async(data_target); + load_async(LoadContext::Font, + self.core_resource_thread.clone(), + url.clone(), + None, + None, + None, + data_target); let channel_to_self = self.channel_to_self.clone(); let url = (*url).clone(); let bytes = Mutex::new(Vec::new()); diff --git a/components/net_traits/lib.rs b/components/net_traits/lib.rs index b2d2456e211..d91c855df86 100644 --- a/components/net_traits/lib.rs +++ b/components/net_traits/lib.rs @@ -467,7 +467,7 @@ pub enum CoreResourceMsg { /// Initialized but unsent request. Encapsulates everything necessary to instruct /// the resource thread to make a new request. The `load` method *must* be called before /// destruction or the thread will panic. -pub struct PendingAsyncLoad { +struct PendingAsyncLoad { core_resource_thread: CoreResourceThread, url: Url, pipeline: Option, @@ -508,13 +508,13 @@ impl LoadOrigin for PendingAsyncLoad { } impl PendingAsyncLoad { - pub fn new(context: LoadContext, - core_resource_thread: CoreResourceThread, - url: Url, - pipeline: Option, - referrer_policy: Option, - referrer_url: Option) - -> PendingAsyncLoad { + fn new(context: LoadContext, + core_resource_thread: CoreResourceThread, + url: Url, + pipeline: Option, + referrer_policy: Option, + referrer_url: Option) + -> PendingAsyncLoad { PendingAsyncLoad { core_resource_thread: core_resource_thread, url: url, @@ -527,7 +527,7 @@ impl PendingAsyncLoad { } /// Initiate the network request associated with this pending load, using the provided target. - pub fn load_async(mut self, listener: AsyncResponseTarget) { + fn load_async(mut self, listener: AsyncResponseTarget) { self.guard.neuter(); let load_data = LoadData::new(self.context.clone(), @@ -538,6 +538,23 @@ impl PendingAsyncLoad { } } +/// Instruct the resource thread to make a new request. +pub fn load_async(context: LoadContext, + core_resource_thread: CoreResourceThread, + url: Url, + pipeline: Option, + referrer_policy: Option, + referrer_url: Option, + listener: AsyncResponseTarget) { + let load = PendingAsyncLoad::new(context, + core_resource_thread, + url, + pipeline, + referrer_policy, + referrer_url); + load.load_async(listener); +} + /// Message sent in response to `Load`. Contains metadata, and a port /// for receiving the data. /// diff --git a/components/script/document_loader.rs b/components/script/document_loader.rs index f56e5c54a9d..cfb04f614fa 100644 --- a/components/script/document_loader.rs +++ b/components/script/document_loader.rs @@ -9,7 +9,7 @@ use dom::bindings::js::JS; use dom::document::Document; use ipc_channel::ipc::IpcSender; use msg::constellation_msg::{PipelineId, ReferrerPolicy}; -use net_traits::{AsyncResponseTarget, CoreResourceMsg, PendingAsyncLoad}; +use net_traits::{AsyncResponseTarget, CoreResourceMsg, load_async}; use net_traits::{FetchResponseMsg, LoadContext, ResourceThreads, IpcSend}; use net_traits::request::RequestInit; use std::thread; @@ -132,13 +132,13 @@ impl DocumentLoader { let context = load.to_load_context(); let url = load.url().clone(); self.add_blocking_load(load); - let pending = PendingAsyncLoad::new(context, - self.resource_threads.sender(), - url, - self.pipeline, - referrer_policy.or(referrer.get_referrer_policy()), - Some(referrer.url().clone())); - pending.load_async(listener) + load_async(context, + self.resource_threads.sender(), + url, + self.pipeline, + referrer_policy.or(referrer.get_referrer_policy()), + Some(referrer.url().clone()), + listener); } /// Initiate a new fetch. From 86fdab2edc366e4c3a71b14f414a3123c6521c47 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Tue, 4 Oct 2016 13:33:28 +0200 Subject: [PATCH 6/6] Refactor away PendingAsyncLoad. --- components/net_traits/lib.rs | 73 +++++------------------------------- 1 file changed, 10 insertions(+), 63 deletions(-) diff --git a/components/net_traits/lib.rs b/components/net_traits/lib.rs index d91c855df86..7f4a3abd164 100644 --- a/components/net_traits/lib.rs +++ b/components/net_traits/lib.rs @@ -44,7 +44,6 @@ use msg::constellation_msg::{PipelineId, ReferrerPolicy}; use request::{Request, RequestInit}; use response::{HttpsState, Response}; use std::io::Error as IOError; -use std::thread; use storage_thread::StorageThreadMsg; use url::Url; use websocket::header; @@ -467,35 +466,13 @@ pub enum CoreResourceMsg { /// Initialized but unsent request. Encapsulates everything necessary to instruct /// the resource thread to make a new request. The `load` method *must* be called before /// destruction or the thread will panic. -struct PendingAsyncLoad { - core_resource_thread: CoreResourceThread, - url: Url, +struct LoadOriginData { pipeline: Option, - guard: PendingLoadGuard, - context: LoadContext, referrer_policy: Option, referrer_url: Option } -struct PendingLoadGuard { - loaded: bool, -} - -impl PendingLoadGuard { - fn neuter(&mut self) { - self.loaded = true; - } -} - -impl Drop for PendingLoadGuard { - fn drop(&mut self) { - if !thread::panicking() { - assert!(self.loaded) - } - } -} - -impl LoadOrigin for PendingAsyncLoad { +impl LoadOrigin for LoadOriginData { fn referrer_url(&self) -> Option { self.referrer_url.clone() } @@ -507,37 +484,6 @@ impl LoadOrigin for PendingAsyncLoad { } } -impl PendingAsyncLoad { - fn new(context: LoadContext, - core_resource_thread: CoreResourceThread, - url: Url, - pipeline: Option, - referrer_policy: Option, - referrer_url: Option) - -> PendingAsyncLoad { - PendingAsyncLoad { - core_resource_thread: core_resource_thread, - url: url, - pipeline: pipeline, - guard: PendingLoadGuard { loaded: false, }, - context: context, - referrer_policy: referrer_policy, - referrer_url: referrer_url - } - } - - /// Initiate the network request associated with this pending load, using the provided target. - fn load_async(mut self, listener: AsyncResponseTarget) { - self.guard.neuter(); - - let load_data = LoadData::new(self.context.clone(), - self.url.clone(), - &self); - let consumer = LoadConsumer::Listener(listener); - self.core_resource_thread.send(CoreResourceMsg::Load(load_data, consumer, None)).unwrap(); - } -} - /// Instruct the resource thread to make a new request. pub fn load_async(context: LoadContext, core_resource_thread: CoreResourceThread, @@ -546,13 +492,14 @@ pub fn load_async(context: LoadContext, referrer_policy: Option, referrer_url: Option, listener: AsyncResponseTarget) { - let load = PendingAsyncLoad::new(context, - core_resource_thread, - url, - pipeline, - referrer_policy, - referrer_url); - load.load_async(listener); + let load = LoadOriginData { + pipeline: pipeline, + referrer_policy: referrer_policy, + referrer_url: referrer_url + }; + let load_data = LoadData::new(context, url, &load); + let consumer = LoadConsumer::Listener(listener); + core_resource_thread.send(CoreResourceMsg::Load(load_data, consumer, None)).unwrap(); } /// Message sent in response to `Load`. Contains metadata, and a port