From 9d01994498c14fbb7a84ef7c423ea8be9501cd31 Mon Sep 17 00:00:00 2001 From: andresantoso Date: Wed, 27 May 2015 10:01:08 -0700 Subject: [PATCH] Run load state update timer only when needed Run the load state update timer only when there is any tab that is loading. Otherwise, the timer would run indefinitely on pages using a hanging GET request. BUG=483287 Review URL: https://codereview.chromium.org/1117923004 Cr-Commit-Position: refs/heads/master@{#331588} --- .../browser/loader/resource_dispatcher_host_impl.cc | 19 +++++++++++-------- .../browser/loader/resource_dispatcher_host_impl.h | 4 ++-- content/browser/loader/resource_scheduler.cc | 8 ++++++++ content/browser/loader/resource_scheduler.h | 3 +++ content/public/browser/web_contents.h | 1 + 5 files changed, 25 insertions(+), 10 deletions(-) diff --git a/content/browser/loader/resource_dispatcher_host_impl.cc b/content/browser/loader/resource_dispatcher_host_impl.cc index d741d6757399..ade6e5d1648e 100644 --- a/content/browser/loader/resource_dispatcher_host_impl.cc +++ b/content/browser/loader/resource_dispatcher_host_impl.cc @@ -115,7 +115,7 @@ namespace { static ResourceDispatcherHostImpl* g_resource_dispatcher_host; // The interval for calls to ResourceDispatcherHostImpl::UpdateLoadStates -const int kUpdateLoadStatesIntervalMsec = 100; +const int kUpdateLoadStatesIntervalMsec = 250; // Maximum byte "cost" of all the outstanding requests for a renderer. // See delcaration of |max_outstanding_requests_cost_per_process_| for details. @@ -788,8 +788,9 @@ bool ResourceDispatcherHostImpl::HandleExternalProtocol(ResourceLoader* loader, } void ResourceDispatcherHostImpl::DidStartRequest(ResourceLoader* loader) { - // Make sure we have the load state monitor running - if (!update_load_states_timer_->IsRunning()) { + // Make sure we have the load state monitor running. + if (!update_load_states_timer_->IsRunning() && + scheduler_->HasLoadingClients()) { update_load_states_timer_->Start( FROM_HERE, TimeDelta::FromMilliseconds(kUpdateLoadStatesIntervalMsec), this, &ResourceDispatcherHostImpl::UpdateLoadInfo); @@ -1779,10 +1780,6 @@ void ResourceDispatcherHostImpl::RemovePendingLoader( IncrementOutstandingRequestsMemory(-1, *info); pending_loaders_.erase(iter); - - // If we have no more pending requests, then stop the load state monitor - if (pending_loaders_.empty() && update_load_states_timer_) - update_load_states_timer_->Stop(); } void ResourceDispatcherHostImpl::CancelRequest(int child_id, @@ -2200,8 +2197,14 @@ ResourceDispatcherHostImpl::GetLoadInfoForAllRoutes() { void ResourceDispatcherHostImpl::UpdateLoadInfo() { scoped_ptr info_map(GetLoadInfoForAllRoutes()); - if (info_map->empty()) + // Stop the timer if there are no more pending requests. Future new requests + // will restart it as necessary. + // Also stop the timer if there are no loading clients, to avoid waking up + // unnecessarily when there is a long running (hanging get) request. + if (info_map->empty() || !scheduler_->HasLoadingClients()) { + update_load_states_timer_->Stop(); return; + } BrowserThread::PostTask( BrowserThread::UI, FROM_HERE, diff --git a/content/browser/loader/resource_dispatcher_host_impl.h b/content/browser/loader/resource_dispatcher_host_impl.h index 152cbbd77e46..51d5b7386acf 100644 --- a/content/browser/loader/resource_dispatcher_host_impl.h +++ b/content/browser/loader/resource_dispatcher_host_impl.h @@ -502,8 +502,8 @@ class CONTENT_EXPORT ResourceDispatcherHostImpl RegisteredTempFiles; // key is child process id RegisteredTempFiles registered_temp_files_; - // A timer that periodically calls UpdateLoadStates while pending_requests_ - // is not empty. + // A timer that periodically calls UpdateLoadInfo while pending_loaders_ is + // not empty and at least one RenderViewHost is loading. scoped_ptr > update_load_states_timer_; diff --git a/content/browser/loader/resource_scheduler.cc b/content/browser/loader/resource_scheduler.cc index a3d3d649ac2d..7bc6c0458b86 100644 --- a/content/browser/loader/resource_scheduler.cc +++ b/content/browser/loader/resource_scheduler.cc @@ -1005,6 +1005,14 @@ bool ResourceScheduler::IsClientVisibleForTesting(int child_id, int route_id) { return client->is_visible(); } +bool ResourceScheduler::HasLoadingClients() const { + for (const auto& client : client_map_) { + if (!client.second->is_loaded()) + return true; + } + return false; +} + ResourceScheduler::Client* ResourceScheduler::GetClient(int child_id, int route_id) { ClientId client_id = MakeClientId(child_id, route_id); diff --git a/content/browser/loader/resource_scheduler.h b/content/browser/loader/resource_scheduler.h index 2b8d7373b4f2..659487122d60 100644 --- a/content/browser/loader/resource_scheduler.h +++ b/content/browser/loader/resource_scheduler.h @@ -150,6 +150,9 @@ class CONTENT_EXPORT ResourceScheduler : public base::NonThreadSafe { bool IsClientVisibleForTesting(int child_id, int route_id); + // Returns true if at least one client is currently loading. + bool HasLoadingClients() const; + private: enum ClientState { // Observable client. diff --git a/content/public/browser/web_contents.h b/content/public/browser/web_contents.h index 86ac67516f24..c68407d8e09a 100644 --- a/content/public/browser/web_contents.h +++ b/content/public/browser/web_contents.h @@ -305,6 +305,7 @@ class WebContents : public PageNavigator, virtual bool IsWaitingForResponse() const = 0; // Returns the current load state and the URL associated with it. + // The load state is only updated while IsLoading() is true. virtual const net::LoadStateWithParam& GetLoadState() const = 0; virtual const base::string16& GetLoadStateHost() const = 0; -- 2.11.4.GIT