diff options
author | cbentzel@chromium.org <cbentzel@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-04-22 18:04:09 +0000 |
---|---|---|
committer | cbentzel@chromium.org <cbentzel@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-04-22 18:04:09 +0000 |
commit | a4fa83b987f37cc28f5af761655fe6c6e69c4359 (patch) | |
tree | c9f1db17cea17c756ec56b42af1810f51413d9c6 /chrome | |
parent | 5f7a75d708b6c4d6257a1e4f88ec8abaf7b39242 (diff) | |
download | chromium_src-a4fa83b987f37cc28f5af761655fe6c6e69c4359.zip chromium_src-a4fa83b987f37cc28f5af761655fe6c6e69c4359.tar.gz chromium_src-a4fa83b987f37cc28f5af761655fe6c6e69c4359.tar.bz2 |
PrerenderManager is no longer refcounted.
The PrerenderManager should be destroyed on the UI thread, along with
the PrerenderContents that it owns.
I originally did this by using the DeleteOnUIThread trait, but this
leaked PrerenderManager's during browser shutdown, since the last
reference was held by an object on the IO thread, and the DeleteTask
was never processed.
Now, the Profile owns the PrerenderManager's lifetime. All other
references to the PrerenderManager are weak references. With the
current thread-safety restrictions of WeakPtr's, this means that the
presence of a PrererenderManager can not be checked on the IO thread,
so all LOAD_PREFETCH requests will have a PrerenderResourceHandler
constructed, regardless of whether the current profile supports
prerendering.
BUG=77930
TEST=Existing unit tests and browser tests.
Review URL: http://codereview.chromium.org/6877085
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@82669 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome')
-rw-r--r-- | chrome/browser/net/chrome_url_request_context.h | 14 | ||||
-rw-r--r-- | chrome/browser/net/net_pref_observer.h | 7 | ||||
-rw-r--r-- | chrome/browser/prerender/prerender_manager.cc | 55 | ||||
-rw-r--r-- | chrome/browser/prerender/prerender_manager.h | 18 | ||||
-rw-r--r-- | chrome/browser/prerender/prerender_manager_unittest.cc | 216 | ||||
-rw-r--r-- | chrome/browser/prerender/prerender_resource_handler.cc | 18 | ||||
-rw-r--r-- | chrome/browser/prerender/prerender_resource_handler.h | 13 | ||||
-rw-r--r-- | chrome/browser/profiles/profile_impl.cc | 8 | ||||
-rw-r--r-- | chrome/browser/profiles/profile_impl.h | 2 | ||||
-rw-r--r-- | chrome/browser/profiles/profile_io_data.cc | 8 | ||||
-rw-r--r-- | chrome/browser/profiles/profile_io_data.h | 3 | ||||
-rw-r--r-- | chrome/test/testing_profile.cc | 8 | ||||
-rw-r--r-- | chrome/test/testing_profile.h | 2 |
13 files changed, 219 insertions, 153 deletions
diff --git a/chrome/browser/net/chrome_url_request_context.h b/chrome/browser/net/chrome_url_request_context.h index 95bc71b..b8f1582 100644 --- a/chrome/browser/net/chrome_url_request_context.h +++ b/chrome/browser/net/chrome_url_request_context.h @@ -10,12 +10,12 @@ #include <vector> #include "base/file_path.h" +#include "base/memory/weak_ptr.h" #include "chrome/browser/content_settings/host_content_settings_map.h" #include "chrome/browser/extensions/extension_info_map.h" #include "chrome/browser/extensions/extension_webrequest_api.h" #include "chrome/browser/prefs/pref_change_registrar.h" #include "chrome/browser/prefs/pref_service.h" -#include "chrome/browser/prerender/prerender_manager.h" #include "chrome/common/extensions/extension_icon_set.h" #include "content/browser/appcache/chrome_appcache_service.h" #include "content/browser/chrome_blob_storage_context.h" @@ -33,6 +33,9 @@ class DnsCertProvenanceChecker; class NetworkDelegate; } class PrefService; +namespace prerender { +class PrerenderManager; +} class Profile; class ProfileIOData; @@ -85,8 +88,8 @@ class ChromeURLRequestContext : public net::URLRequestContext { return extension_info_map_; } - prerender::PrerenderManager* prerender_manager() { - return prerender_manager_.get(); + base::WeakPtr<prerender::PrerenderManager> prerender_manager() const { + return prerender_manager_; } ChromeURLDataManagerBackend* GetChromeURLDataManagerBackend(); @@ -117,7 +120,8 @@ class ChromeURLRequestContext : public net::URLRequestContext { void set_extension_info_map(ExtensionInfoMap* map) { extension_info_map_ = map; } - void set_prerender_manager(prerender::PrerenderManager* prerender_manager) { + void set_prerender_manager( + const base::WeakPtr<prerender::PrerenderManager>& prerender_manager) { prerender_manager_ = prerender_manager; } @@ -147,7 +151,7 @@ class ChromeURLRequestContext : public net::URLRequestContext { scoped_refptr<fileapi::FileSystemContext> file_system_context_; // TODO(aa): This should use chrome/common/extensions/extension_set.h. scoped_refptr<ExtensionInfoMap> extension_info_map_; - scoped_refptr<prerender::PrerenderManager> prerender_manager_; + base::WeakPtr<prerender::PrerenderManager> prerender_manager_; scoped_ptr<ChromeURLDataManagerBackend> chrome_url_data_manager_backend_; bool is_incognito_; diff --git a/chrome/browser/net/net_pref_observer.h b/chrome/browser/net/net_pref_observer.h index a03129e..d023572 100644 --- a/chrome/browser/net/net_pref_observer.h +++ b/chrome/browser/net/net_pref_observer.h @@ -23,8 +23,9 @@ class PrerenderManager; // Must be used only on the UI thread. class NetPrefObserver : public NotificationObserver { public: - // |prefs| must outlive this NetPrefObserver. A reference is - // held to |prerender_manager| if it is non-NULL. + // |prefs| must be non-NULL and |*prefs| must outlive this. + // |prerender_manager| may be NULL. If not, |*prerender_manager| must + // outlive this. NetPrefObserver(PrefService* prefs, prerender::PrerenderManager* prerender_manager); ~NetPrefObserver(); @@ -43,7 +44,7 @@ class NetPrefObserver : public NotificationObserver { BooleanPrefMember network_prediction_enabled_; BooleanPrefMember spdy_disabled_; BooleanPrefMember http_throttling_enabled_; - scoped_refptr<prerender::PrerenderManager> prerender_manager_; + prerender::PrerenderManager* prerender_manager_; DISALLOW_COPY_AND_ASSIGN(NetPrefObserver); }; diff --git a/chrome/browser/prerender/prerender_manager.cc b/chrome/browser/prerender/prerender_manager.cc index cd0d681..de6fd21 100644 --- a/chrome/browser/prerender/prerender_manager.cc +++ b/chrome/browser/prerender/prerender_manager.cc @@ -138,6 +138,10 @@ PrerenderManager::PrerenderManager(Profile* profile) prerender_contents_factory_(PrerenderContents::CreateFactory()), last_prerender_start_time_(GetCurrentTimeTicks() - base::TimeDelta::FromMilliseconds(kMinTimeBetweenPrerendersMs)) { + // There are some assumptions that the PrerenderManager is on the UI thread. + // Any other checks simply make sure that the PrerenderManager is accessed on + // the same thread that it was created on. + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); } PrerenderManager::~PrerenderManager() { @@ -151,6 +155,7 @@ PrerenderManager::~PrerenderManager() { void PrerenderManager::SetPrerenderContentsFactory( PrerenderContents::Factory* prerender_contents_factory) { + DCHECK(CalledOnValidThread()); prerender_contents_factory_.reset(prerender_contents_factory); } @@ -159,7 +164,7 @@ bool PrerenderManager::AddPreload( const GURL& url, const std::vector<GURL>& alias_urls, const GURL& referrer) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + DCHECK(CalledOnValidThread()); DeleteOldEntries(); if (FindEntry(url)) return false; @@ -240,6 +245,7 @@ void PrerenderManager::AddPendingPreload( const GURL& url, const std::vector<GURL>& alias_urls, const GURL& referrer) { + DCHECK(CalledOnValidThread()); // Check if this is coming from a valid prerender rvh. bool is_valid_prerender = false; for (std::list<PrerenderContentsData>::iterator it = prerender_list_.begin(); @@ -282,6 +288,7 @@ void PrerenderManager::AddPendingPreload( } void PrerenderManager::DeleteOldEntries() { + DCHECK(CalledOnValidThread()); while (!prerender_list_.empty()) { PrerenderContentsData data = prerender_list_.front(); if (IsPrerenderElementFresh(data.start_time_)) @@ -295,6 +302,7 @@ void PrerenderManager::DeleteOldEntries() { } PrerenderContents* PrerenderManager::GetEntry(const GURL& url) { + DCHECK(CalledOnValidThread()); DeleteOldEntries(); for (std::list<PrerenderContentsData>::iterator it = prerender_list_.begin(); it != prerender_list_.end(); @@ -310,7 +318,7 @@ PrerenderContents* PrerenderManager::GetEntry(const GURL& url) { } bool PrerenderManager::MaybeUsePreloadedPage(TabContents* tc, const GURL& url) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + DCHECK(CalledOnValidThread()); scoped_ptr<PrerenderContents> pc(GetEntry(url)); if (pc.get() == NULL) return false; @@ -391,7 +399,7 @@ bool PrerenderManager::MaybeUsePreloadedPage(TabContents* tc, const GURL& url) { } void PrerenderManager::RemoveEntry(PrerenderContents* entry) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + DCHECK(CalledOnValidThread()); for (std::list<PrerenderContentsData>::iterator it = prerender_list_.begin(); it != prerender_list_.end(); ++it) { @@ -413,6 +421,7 @@ base::TimeTicks PrerenderManager::GetCurrentTimeTicks() const { } bool PrerenderManager::IsPrerenderElementFresh(const base::Time start) const { + DCHECK(CalledOnValidThread()); base::Time now = GetCurrentTime(); return (now - start < max_prerender_age_); } @@ -421,6 +430,7 @@ PrerenderContents* PrerenderManager::CreatePrerenderContents( const GURL& url, const std::vector<GURL>& alias_urls, const GURL& referrer) { + DCHECK(CalledOnValidThread()); return prerender_contents_factory_->CreatePrerenderContents( this, profile_, url, alias_urls, referrer); } @@ -440,6 +450,7 @@ PrerenderContents* PrerenderManager::CreatePrerenderContents( void PrerenderManager::RecordPerceivedPageLoadTime( base::TimeDelta perceived_page_load_time, TabContents* tab_contents) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); bool within_window = WithinWindow(); PrerenderManager* prerender_manager = tab_contents->profile()->GetPrerenderManager(); @@ -463,6 +474,7 @@ void PrerenderManager::RecordPerceivedPageLoadTime( } void PrerenderManager::RecordTimeUntilUsed(base::TimeDelta time_until_used) { + DCHECK(CalledOnValidThread()); UMA_HISTOGRAM_CUSTOM_TIMES( "Prerender.TimeUntilUsed", time_until_used, @@ -471,17 +483,38 @@ void PrerenderManager::RecordTimeUntilUsed(base::TimeDelta time_until_used) { 50); } +base::TimeDelta PrerenderManager::max_prerender_age() const { + DCHECK(CalledOnValidThread()); + return max_prerender_age_; +} + +void PrerenderManager::set_max_prerender_age(base::TimeDelta max_age) { + DCHECK(CalledOnValidThread()); + max_prerender_age_ = max_age; +} + +unsigned int PrerenderManager::max_elements() const { + DCHECK(CalledOnValidThread()); + return max_elements_; +} + +void PrerenderManager::set_max_elements(unsigned int max_elements) { + DCHECK(CalledOnValidThread()); + max_elements_ = max_elements; +} + bool PrerenderManager::is_enabled() const { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + DCHECK(CalledOnValidThread()); return enabled_; } void PrerenderManager::set_enabled(bool enabled) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + DCHECK(CalledOnValidThread()); enabled_ = enabled; } PrerenderContents* PrerenderManager::FindEntry(const GURL& url) { + DCHECK(CalledOnValidThread()); for (std::list<PrerenderContentsData>::iterator it = prerender_list_.begin(); it != prerender_list_.end(); ++it) { @@ -494,6 +527,7 @@ PrerenderContents* PrerenderManager::FindEntry(const GURL& url) { PrerenderManager::PendingContentsData* PrerenderManager::FindPendingEntry(const GURL& url) { + DCHECK(CalledOnValidThread()); for (PendingPrerenderList::iterator map_it = pending_prerender_list_.begin(); map_it != pending_prerender_list_.end(); ++map_it) { @@ -537,6 +571,7 @@ void PrerenderManager::RecordPrefetchTagObservedOnUIThread() { } void PrerenderManager::RemovePendingPreload(PrerenderContents* entry) { + DCHECK(CalledOnValidThread()); int child_id; int route_id; bool has_child_id = entry->GetChildId(&child_id); @@ -561,7 +596,7 @@ bool PrerenderManager::WithinWindow() { } bool PrerenderManager::DoesRateLimitAllowPrerender() const { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + DCHECK(CalledOnValidThread()); base::TimeDelta elapsed_time = GetCurrentTimeTicks() - last_prerender_start_time_; UMA_HISTOGRAM_TIMES("Prerender.TimeBetweenPrerenderRequests", @@ -573,6 +608,7 @@ bool PrerenderManager::DoesRateLimitAllowPrerender() const { } void PrerenderManager::StartSchedulingPeriodicCleanups() { + DCHECK(CalledOnValidThread()); if (repeating_timer_.IsRunning()) return; repeating_timer_.Start( @@ -582,10 +618,12 @@ void PrerenderManager::StartSchedulingPeriodicCleanups() { } void PrerenderManager::StopSchedulingPeriodicCleanups() { + DCHECK(CalledOnValidThread()); repeating_timer_.Stop(); } void PrerenderManager::PeriodicCleanup() { + DCHECK(CalledOnValidThread()); DeleteOldEntries(); // Grab a copy of the current PrerenderContents pointers, so that we // will not interfere with potential deletions of the list. @@ -604,23 +642,28 @@ void PrerenderManager::PeriodicCleanup() { } void PrerenderManager::MarkTabContentsAsPrerendered(TabContents* tc) { + DCHECK(CalledOnValidThread()); prerendered_tc_set_.insert(tc); } void PrerenderManager::MarkTabContentsAsWouldBePrerendered(TabContents* tc) { + DCHECK(CalledOnValidThread()); would_be_prerendered_tc_set_.insert(tc); } void PrerenderManager::MarkTabContentsAsNotPrerendered(TabContents* tc) { + DCHECK(CalledOnValidThread()); prerendered_tc_set_.erase(tc); would_be_prerendered_tc_set_.erase(tc); } bool PrerenderManager::IsTabContentsPrerendered(TabContents* tc) const { + DCHECK(CalledOnValidThread()); return prerendered_tc_set_.count(tc) > 0; } bool PrerenderManager::WouldTabContentsBePrerendered(TabContents* tc) const { + DCHECK(CalledOnValidThread()); return would_be_prerendered_tc_set_.count(tc) > 0; } diff --git a/chrome/browser/prerender/prerender_manager.h b/chrome/browser/prerender/prerender_manager.h index 7cde14c..214475b 100644 --- a/chrome/browser/prerender/prerender_manager.h +++ b/chrome/browser/prerender/prerender_manager.h @@ -11,8 +11,9 @@ #include <vector> #include "base/hash_tables.h" -#include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" +#include "base/memory/weak_ptr.h" +#include "base/threading/non_thread_safe.h" #include "base/time.h" #include "base/timer.h" #include "chrome/browser/prerender/prerender_contents.h" @@ -38,7 +39,8 @@ namespace prerender { // PrerenderManager is responsible for initiating and keeping prerendered // views of webpages. -class PrerenderManager : public base::RefCountedThreadSafe<PrerenderManager> { +class PrerenderManager : public base::SupportsWeakPtr<PrerenderManager>, + public base::NonThreadSafe { public: // PrerenderManagerMode is used in a UMA_HISTOGRAM, so please do not // add in the middle. @@ -53,6 +55,8 @@ class PrerenderManager : public base::RefCountedThreadSafe<PrerenderManager> { // Owned by a Profile object for the lifetime of the profile. explicit PrerenderManager(Profile* profile); + virtual ~PrerenderManager(); + // Preloads |url| if valid. |alias_urls| indicates URLs that redirect // to the same URL to be preloaded. |child_route_id_pair| identifies the // RenderViewHost that the prerender request came from and is used to @@ -97,10 +101,10 @@ class PrerenderManager : public base::RefCountedThreadSafe<PrerenderManager> { // navigates to it. This must be called on the UI thread. void RecordTimeUntilUsed(base::TimeDelta time_until_used); - base::TimeDelta max_prerender_age() const { return max_prerender_age_; } - void set_max_prerender_age(base::TimeDelta td) { max_prerender_age_ = td; } - unsigned int max_elements() const { return max_elements_; } - void set_max_elements(unsigned int num) { max_elements_ = num; } + base::TimeDelta max_prerender_age() const; + void set_max_prerender_age(base::TimeDelta max_age); + unsigned int max_elements() const; + void set_max_elements(unsigned int num); // Returns whether prerendering is currently enabled for this manager. // Must be called on the UI thread. @@ -138,8 +142,6 @@ class PrerenderManager : public base::RefCountedThreadSafe<PrerenderManager> { protected: struct PendingContentsData; - virtual ~PrerenderManager(); - void SetPrerenderContentsFactory( PrerenderContents::Factory* prerender_contents_factory); bool rate_limit_enabled_; diff --git a/chrome/browser/prerender/prerender_manager_unittest.cc b/chrome/browser/prerender/prerender_manager_unittest.cc index 6ea9625..f5d81ab 100644 --- a/chrome/browser/prerender/prerender_manager_unittest.cc +++ b/chrome/browser/prerender/prerender_manager_unittest.cc @@ -65,6 +65,21 @@ class TestPrerenderManager : public PrerenderManager { rate_limit_enabled_ = false; } + virtual ~TestPrerenderManager() { + if (next_pc()) { + next_pc()->set_final_status( + FINAL_STATUS_MANAGER_SHUTDOWN); + } + // Set the final status for all PrerenderContents with an expected final + // status of FINAL_STATUS_USED. These values are normally set when the + // prerendered RVH is swapped into a tab, which doesn't happen in these + // unit tests. + for (ScopedVector<PrerenderContents>::iterator it = used_pcs_.begin(); + it != used_pcs_.end(); ++it) { + (*it)->set_final_status(FINAL_STATUS_USED); + } + } + void AdvanceTime(base::TimeDelta delta) { time_ += delta; } @@ -114,22 +129,6 @@ class TestPrerenderManager : public PrerenderManager { PrerenderContents* next_pc() { return next_pc_.get(); } - protected: - virtual ~TestPrerenderManager() { - if (next_pc()) { - next_pc()->set_final_status( - FINAL_STATUS_MANAGER_SHUTDOWN); - } - // Set the final status for all PrerenderContents with an expected final - // status of FINAL_STATUS_USED. These values are normally set when the - // prerendered RVH is swapped into a tab, which doesn't happen in these - // unit tests. - for (ScopedVector<PrerenderContents>::iterator it = used_pcs_.begin(); - it != used_pcs_.end(); ++it) { - (*it)->set_final_status(FINAL_STATUS_USED); - } - } - private: void SetNextPrerenderContents(DummyPrerenderContents* pc) { DCHECK(!next_pc_.get()); @@ -176,33 +175,35 @@ class RestorePrerenderMode { class PrerenderManagerTest : public testing::Test { public: - PrerenderManagerTest() : prerender_manager_(new TestPrerenderManager()), - ui_thread_(BrowserThread::UI, &message_loop_) { + PrerenderManagerTest() : ui_thread_(BrowserThread::UI, &message_loop_), + prerender_manager_(new TestPrerenderManager()) { } - protected: - scoped_refptr<TestPrerenderManager> prerender_manager_; + TestPrerenderManager* prerender_manager() { + return prerender_manager_.get(); + } private: // Needed to pass PrerenderManager's DCHECKs. MessageLoop message_loop_; BrowserThread ui_thread_; + scoped_ptr<TestPrerenderManager> prerender_manager_; }; TEST_F(PrerenderManagerTest, EmptyTest) { GURL url("http://www.google.com/"); - EXPECT_FALSE(prerender_manager_->MaybeUsePreloadedPage(NULL, url)); + EXPECT_FALSE(prerender_manager()->MaybeUsePreloadedPage(NULL, url)); } TEST_F(PrerenderManagerTest, FoundTest) { GURL url("http://www.google.com/"); DummyPrerenderContents* pc = - prerender_manager_->CreateNextPrerenderContents( + prerender_manager()->CreateNextPrerenderContents( url, FINAL_STATUS_USED); - EXPECT_TRUE(prerender_manager_->AddSimplePreload(url)); + EXPECT_TRUE(prerender_manager()->AddSimplePreload(url)); EXPECT_TRUE(pc->has_started()); - ASSERT_EQ(pc, prerender_manager_->GetEntry(url)); + ASSERT_EQ(pc, prerender_manager()->GetEntry(url)); } // Make sure that if queue a request, and a second prerender request for the @@ -210,39 +211,39 @@ TEST_F(PrerenderManagerTest, FoundTest) { TEST_F(PrerenderManagerTest, DropSecondRequestTest) { GURL url("http://www.google.com/"); DummyPrerenderContents* pc = - prerender_manager_->CreateNextPrerenderContents( + prerender_manager()->CreateNextPrerenderContents( url, FINAL_STATUS_USED); DummyPrerenderContents* null = NULL; - EXPECT_TRUE(prerender_manager_->AddSimplePreload(url)); - EXPECT_EQ(null, prerender_manager_->next_pc()); + EXPECT_TRUE(prerender_manager()->AddSimplePreload(url)); + EXPECT_EQ(null, prerender_manager()->next_pc()); EXPECT_TRUE(pc->has_started()); DummyPrerenderContents* pc1 = - prerender_manager_->CreateNextPrerenderContents( + prerender_manager()->CreateNextPrerenderContents( url, FINAL_STATUS_MANAGER_SHUTDOWN); - EXPECT_FALSE(prerender_manager_->AddSimplePreload(url)); - EXPECT_EQ(pc1, prerender_manager_->next_pc()); + EXPECT_FALSE(prerender_manager()->AddSimplePreload(url)); + EXPECT_EQ(pc1, prerender_manager()->next_pc()); EXPECT_FALSE(pc1->has_started()); - ASSERT_EQ(pc, prerender_manager_->GetEntry(url)); + ASSERT_EQ(pc, prerender_manager()->GetEntry(url)); } // Ensure that we expire a prerendered page after the max. permitted time. TEST_F(PrerenderManagerTest, ExpireTest) { GURL url("http://www.google.com/"); DummyPrerenderContents* pc = - prerender_manager_->CreateNextPrerenderContents( + prerender_manager()->CreateNextPrerenderContents( url, FINAL_STATUS_TIMED_OUT); DummyPrerenderContents* null = NULL; - EXPECT_TRUE(prerender_manager_->AddSimplePreload(url)); - EXPECT_EQ(null, prerender_manager_->next_pc()); + EXPECT_TRUE(prerender_manager()->AddSimplePreload(url)); + EXPECT_EQ(null, prerender_manager()->next_pc()); EXPECT_TRUE(pc->has_started()); - prerender_manager_->AdvanceTime(prerender_manager_->max_prerender_age() + prerender_manager()->AdvanceTime(prerender_manager()->max_prerender_age() + base::TimeDelta::FromSeconds(1)); - ASSERT_EQ(null, prerender_manager_->GetEntry(url)); + ASSERT_EQ(null, prerender_manager()->GetEntry(url)); } // LRU Test. Make sure that if we prerender more than one request, that @@ -250,62 +251,62 @@ TEST_F(PrerenderManagerTest, ExpireTest) { TEST_F(PrerenderManagerTest, DropOldestRequestTest) { GURL url("http://www.google.com/"); DummyPrerenderContents* pc = - prerender_manager_->CreateNextPrerenderContents( + prerender_manager()->CreateNextPrerenderContents( url, FINAL_STATUS_EVICTED); DummyPrerenderContents* null = NULL; - EXPECT_TRUE(prerender_manager_->AddSimplePreload(url)); - EXPECT_EQ(null, prerender_manager_->next_pc()); + EXPECT_TRUE(prerender_manager()->AddSimplePreload(url)); + EXPECT_EQ(null, prerender_manager()->next_pc()); EXPECT_TRUE(pc->has_started()); GURL url1("http://news.google.com/"); DummyPrerenderContents* pc1 = - prerender_manager_->CreateNextPrerenderContents( + prerender_manager()->CreateNextPrerenderContents( url1, FINAL_STATUS_USED); - EXPECT_TRUE(prerender_manager_->AddSimplePreload(url1)); - EXPECT_EQ(null, prerender_manager_->next_pc()); + EXPECT_TRUE(prerender_manager()->AddSimplePreload(url1)); + EXPECT_EQ(null, prerender_manager()->next_pc()); EXPECT_TRUE(pc1->has_started()); - ASSERT_EQ(null, prerender_manager_->GetEntry(url)); - ASSERT_EQ(pc1, prerender_manager_->GetEntry(url1)); + ASSERT_EQ(null, prerender_manager()->GetEntry(url)); + ASSERT_EQ(pc1, prerender_manager()->GetEntry(url1)); } // Two element prerender test. Ensure that the LRU operates correctly if we // permit 2 elements to be kept prerendered. TEST_F(PrerenderManagerTest, TwoElementPrerenderTest) { - prerender_manager_->set_max_elements(2); + prerender_manager()->set_max_elements(2); GURL url("http://www.google.com/"); DummyPrerenderContents* pc = - prerender_manager_->CreateNextPrerenderContents( + prerender_manager()->CreateNextPrerenderContents( url, FINAL_STATUS_EVICTED); DummyPrerenderContents* null = NULL; - EXPECT_TRUE(prerender_manager_->AddSimplePreload(url)); - EXPECT_EQ(null, prerender_manager_->next_pc()); + EXPECT_TRUE(prerender_manager()->AddSimplePreload(url)); + EXPECT_EQ(null, prerender_manager()->next_pc()); EXPECT_TRUE(pc->has_started()); GURL url1("http://news.google.com/"); DummyPrerenderContents* pc1 = - prerender_manager_->CreateNextPrerenderContents( + prerender_manager()->CreateNextPrerenderContents( url1, FINAL_STATUS_USED); - EXPECT_TRUE(prerender_manager_->AddSimplePreload(url1)); - EXPECT_EQ(null, prerender_manager_->next_pc()); + EXPECT_TRUE(prerender_manager()->AddSimplePreload(url1)); + EXPECT_EQ(null, prerender_manager()->next_pc()); EXPECT_TRUE(pc1->has_started()); GURL url2("http://images.google.com/"); DummyPrerenderContents* pc2 = - prerender_manager_->CreateNextPrerenderContents( + prerender_manager()->CreateNextPrerenderContents( url2, FINAL_STATUS_USED); - EXPECT_TRUE(prerender_manager_->AddSimplePreload(url2)); - EXPECT_EQ(null, prerender_manager_->next_pc()); + EXPECT_TRUE(prerender_manager()->AddSimplePreload(url2)); + EXPECT_EQ(null, prerender_manager()->next_pc()); EXPECT_TRUE(pc2->has_started()); - ASSERT_EQ(null, prerender_manager_->GetEntry(url)); - ASSERT_EQ(pc1, prerender_manager_->GetEntry(url1)); - ASSERT_EQ(pc2, prerender_manager_->GetEntry(url2)); + ASSERT_EQ(null, prerender_manager()->GetEntry(url)); + ASSERT_EQ(pc1, prerender_manager()->GetEntry(url1)); + ASSERT_EQ(pc2, prerender_manager()->GetEntry(url2)); } TEST_F(PrerenderManagerTest, AliasURLTest) { @@ -319,86 +320,91 @@ TEST_F(PrerenderManagerTest, AliasURLTest) { // Test that all of the aliases work, but nont_an_alias_url does not. DummyPrerenderContents* pc = - prerender_manager_->CreateNextPrerenderContents( + prerender_manager()->CreateNextPrerenderContents( url, alias_urls, FINAL_STATUS_USED); - EXPECT_TRUE(prerender_manager_->AddSimplePreloadWithAliases(url, alias_urls)); - ASSERT_EQ(NULL, prerender_manager_->GetEntry(not_an_alias_url)); - ASSERT_EQ(pc, prerender_manager_->GetEntry(alias_url1)); - pc = prerender_manager_->CreateNextPrerenderContents( + EXPECT_TRUE( + prerender_manager()->AddSimplePreloadWithAliases(url, alias_urls)); + ASSERT_EQ(NULL, prerender_manager()->GetEntry(not_an_alias_url)); + ASSERT_EQ(pc, prerender_manager()->GetEntry(alias_url1)); + pc = prerender_manager()->CreateNextPrerenderContents( url, alias_urls, FINAL_STATUS_USED); - EXPECT_TRUE(prerender_manager_->AddSimplePreloadWithAliases(url, alias_urls)); - ASSERT_EQ(pc, prerender_manager_->GetEntry(alias_url2)); - pc = prerender_manager_->CreateNextPrerenderContents( + EXPECT_TRUE( + prerender_manager()->AddSimplePreloadWithAliases(url, alias_urls)); + ASSERT_EQ(pc, prerender_manager()->GetEntry(alias_url2)); + pc = prerender_manager()->CreateNextPrerenderContents( url, alias_urls, FINAL_STATUS_USED); - EXPECT_TRUE(prerender_manager_->AddSimplePreloadWithAliases(url, alias_urls)); - ASSERT_EQ(pc, prerender_manager_->GetEntry(url)); + EXPECT_TRUE( + prerender_manager()->AddSimplePreloadWithAliases(url, alias_urls)); + ASSERT_EQ(pc, prerender_manager()->GetEntry(url)); // Test that alias URLs can not be added. - pc = prerender_manager_->CreateNextPrerenderContents( + pc = prerender_manager()->CreateNextPrerenderContents( url, alias_urls, FINAL_STATUS_USED); - EXPECT_TRUE(prerender_manager_->AddSimplePreloadWithAliases(url, alias_urls)); - EXPECT_FALSE(prerender_manager_->AddSimplePreload(url)); - EXPECT_FALSE(prerender_manager_->AddSimplePreload(alias_url1)); - EXPECT_FALSE(prerender_manager_->AddSimplePreload(alias_url2)); - ASSERT_EQ(pc, prerender_manager_->GetEntry(url)); + EXPECT_TRUE( + prerender_manager()->AddSimplePreloadWithAliases(url, alias_urls)); + EXPECT_FALSE(prerender_manager()->AddSimplePreload(url)); + EXPECT_FALSE(prerender_manager()->AddSimplePreload(alias_url1)); + EXPECT_FALSE(prerender_manager()->AddSimplePreload(alias_url2)); + ASSERT_EQ(pc, prerender_manager()->GetEntry(url)); } // Ensure that we ignore prerender requests within the rate limit. TEST_F(PrerenderManagerTest, RateLimitInWindowTest) { GURL url("http://www.google.com/"); DummyPrerenderContents* pc = - prerender_manager_->CreateNextPrerenderContents( + prerender_manager()->CreateNextPrerenderContents( url, FINAL_STATUS_MANAGER_SHUTDOWN); DummyPrerenderContents* null = NULL; - EXPECT_TRUE(prerender_manager_->AddSimplePreload(url)); - EXPECT_EQ(null, prerender_manager_->next_pc()); + EXPECT_TRUE(prerender_manager()->AddSimplePreload(url)); + EXPECT_EQ(null, prerender_manager()->next_pc()); EXPECT_TRUE(pc->has_started()); - prerender_manager_->set_rate_limit_enabled(true); - prerender_manager_->AdvanceTimeTicks(base::TimeDelta::FromMilliseconds(1)); + prerender_manager()->set_rate_limit_enabled(true); + prerender_manager()->AdvanceTimeTicks(base::TimeDelta::FromMilliseconds(1)); GURL url1("http://news.google.com/"); - prerender_manager_->CreateNextPrerenderContents( + prerender_manager()->CreateNextPrerenderContents( url, FINAL_STATUS_MANAGER_SHUTDOWN); - EXPECT_FALSE(prerender_manager_->AddSimplePreload(url1)); - prerender_manager_->set_rate_limit_enabled(false); + EXPECT_FALSE(prerender_manager()->AddSimplePreload(url1)); + prerender_manager()->set_rate_limit_enabled(false); } // Ensure that we don't ignore prerender requests outside the rate limit. TEST_F(PrerenderManagerTest, RateLimitOutsideWindowTest) { GURL url("http://www.google.com/"); DummyPrerenderContents* pc = - prerender_manager_->CreateNextPrerenderContents( + prerender_manager()->CreateNextPrerenderContents( url, FINAL_STATUS_EVICTED); DummyPrerenderContents* null = NULL; - EXPECT_TRUE(prerender_manager_->AddSimplePreload(url)); - EXPECT_EQ(null, prerender_manager_->next_pc()); + EXPECT_TRUE(prerender_manager()->AddSimplePreload(url)); + EXPECT_EQ(null, prerender_manager()->next_pc()); EXPECT_TRUE(pc->has_started()); - prerender_manager_->set_rate_limit_enabled(true); - prerender_manager_->AdvanceTimeTicks(base::TimeDelta::FromMilliseconds(2000)); + prerender_manager()->set_rate_limit_enabled(true); + prerender_manager()->AdvanceTimeTicks( + base::TimeDelta::FromMilliseconds(2000)); GURL url1("http://news.google.com/"); DummyPrerenderContents* rate_limit_pc = - prerender_manager_->CreateNextPrerenderContents( + prerender_manager()->CreateNextPrerenderContents( url1, FINAL_STATUS_MANAGER_SHUTDOWN); - EXPECT_TRUE(prerender_manager_->AddSimplePreload(url1)); - EXPECT_EQ(null, prerender_manager_->next_pc()); + EXPECT_TRUE(prerender_manager()->AddSimplePreload(url1)); + EXPECT_EQ(null, prerender_manager()->next_pc()); EXPECT_TRUE(rate_limit_pc->has_started()); - prerender_manager_->set_rate_limit_enabled(false); + prerender_manager()->set_rate_limit_enabled(false); } TEST_F(PrerenderManagerTest, PendingPreloadTest) { GURL url("http://www.google.com/"); DummyPrerenderContents* pc = - prerender_manager_->CreateNextPrerenderContents( + prerender_manager()->CreateNextPrerenderContents( url, FINAL_STATUS_USED); - EXPECT_TRUE(prerender_manager_->AddSimplePreload(url)); + EXPECT_TRUE(prerender_manager()->AddSimplePreload(url)); int child_id; int route_id; @@ -407,20 +413,20 @@ TEST_F(PrerenderManagerTest, PendingPreloadTest) { GURL pending_url("http://news.google.com/"); - prerender_manager_->AddPendingPreload(std::make_pair(child_id, route_id), + prerender_manager()->AddPendingPreload(std::make_pair(child_id, route_id), pending_url, std::vector<GURL>(), url); - EXPECT_TRUE(prerender_manager_->IsPendingEntry(pending_url)); + EXPECT_TRUE(prerender_manager()->IsPendingEntry(pending_url)); EXPECT_TRUE(pc->has_started()); - ASSERT_EQ(pc, prerender_manager_->GetEntry(url)); + ASSERT_EQ(pc, prerender_manager()->GetEntry(url)); } TEST_F(PrerenderManagerTest, PendingPreloadSkippedTest) { GURL url("http://www.google.com/"); DummyPrerenderContents* pc = - prerender_manager_->CreateNextPrerenderContents( + prerender_manager()->CreateNextPrerenderContents( url, FINAL_STATUS_TIMED_OUT); @@ -429,19 +435,19 @@ TEST_F(PrerenderManagerTest, PendingPreloadSkippedTest) { ASSERT_TRUE(pc->GetChildId(&child_id)); ASSERT_TRUE(pc->GetRouteId(&route_id)); - EXPECT_TRUE(prerender_manager_->AddSimplePreload(url)); - prerender_manager_->AdvanceTime(prerender_manager_->max_prerender_age() + EXPECT_TRUE(prerender_manager()->AddSimplePreload(url)); + prerender_manager()->AdvanceTime(prerender_manager()->max_prerender_age() + base::TimeDelta::FromSeconds(1)); // GetEntry will cull old entries which should now include pc. - ASSERT_EQ(NULL, prerender_manager_->GetEntry(url)); + ASSERT_EQ(NULL, prerender_manager()->GetEntry(url)); GURL pending_url("http://news.google.com/"); - prerender_manager_->AddPendingPreload(std::make_pair(child_id, route_id), + prerender_manager()->AddPendingPreload(std::make_pair(child_id, route_id), pending_url, std::vector<GURL>(), url); - EXPECT_FALSE(prerender_manager_->IsPendingEntry(pending_url)); + EXPECT_FALSE(prerender_manager()->IsPendingEntry(pending_url)); } // Ensure that extracting a urlencoded URL in the url= query string component @@ -470,10 +476,10 @@ TEST_F(PrerenderManagerTest, ControlGroup) { PrerenderManager::PRERENDER_MODE_EXPERIMENT_CONTROL_GROUP); GURL url("http://www.google.com/"); DummyPrerenderContents* pc = - prerender_manager_->CreateNextPrerenderContents( + prerender_manager()->CreateNextPrerenderContents( url, FINAL_STATUS_CONTROL_GROUP); - EXPECT_TRUE(prerender_manager_->AddSimplePreload(url)); + EXPECT_TRUE(prerender_manager()->AddSimplePreload(url)); EXPECT_FALSE(pc->has_started()); } @@ -482,10 +488,10 @@ TEST_F(PrerenderManagerTest, ControlGroup) { // triggered. TEST_F(PrerenderManagerTest, SourceRenderViewClosed) { GURL url("http://www.google.com/"); - prerender_manager_->CreateNextPrerenderContents( + prerender_manager()->CreateNextPrerenderContents( url, FINAL_STATUS_MANAGER_SHUTDOWN); - EXPECT_FALSE(prerender_manager_->AddPreload( + EXPECT_FALSE(prerender_manager()->AddPreload( std::pair<int, int>(100, 100), url, std::vector<GURL>(), GURL())); } diff --git a/chrome/browser/prerender/prerender_resource_handler.cc b/chrome/browser/prerender/prerender_resource_handler.cc index dc96fd2..bd33574 100644 --- a/chrome/browser/prerender/prerender_resource_handler.cc +++ b/chrome/browser/prerender/prerender_resource_handler.cc @@ -56,7 +56,7 @@ PrerenderResourceHandler* PrerenderResourceHandler::MaybeCreate( bool is_from_prerender, int child_id, int route_id) { - if (!context || !context->prerender_manager()) + if (!context) return NULL; if (!(request.load_flags() & net::LOAD_PREFETCH)) return NULL; @@ -76,7 +76,7 @@ PrerenderResourceHandler* PrerenderResourceHandler::MaybeCreate( PrerenderResourceHandler::PrerenderResourceHandler( const net::URLRequest& request, ResourceHandler* next_handler, - PrerenderManager* prerender_manager, + const base::WeakPtr<PrerenderManager>& prerender_manager, bool make_pending, int child_id, int route_id) @@ -90,7 +90,6 @@ PrerenderResourceHandler::PrerenderResourceHandler( route_id_(route_id), make_pending_(make_pending) { DCHECK(next_handler); - DCHECK(prerender_manager); } PrerenderResourceHandler::PrerenderResourceHandler( @@ -204,14 +203,17 @@ void PrerenderResourceHandler::StartPrerender( const GURL& referrer, bool make_pending) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - if (!prerender_manager_->is_enabled()) + // The PrerenderManager may have been destroyed. Only do prerendering + // if it is still alive and is enabled. + PrerenderManager* prerender_manager = prerender_manager_.get(); + if (!prerender_manager || !prerender_manager->is_enabled()) return; if (make_pending) { - prerender_manager_->AddPendingPreload(child_route_id_pair, - url, alias_urls, referrer); + prerender_manager->AddPendingPreload(child_route_id_pair, + url, alias_urls, referrer); } else { - prerender_manager_->AddPreload(child_route_id_pair, url, alias_urls, - referrer); + prerender_manager->AddPreload(child_route_id_pair, url, alias_urls, + referrer); } } diff --git a/chrome/browser/prerender/prerender_resource_handler.h b/chrome/browser/prerender/prerender_resource_handler.h index a517f64..c21f3e3 100644 --- a/chrome/browser/prerender/prerender_resource_handler.h +++ b/chrome/browser/prerender/prerender_resource_handler.h @@ -76,10 +76,13 @@ class PrerenderResourceHandler : public ResourceHandler { const GURL&, bool>::Type PrerenderCallback; - PrerenderResourceHandler(const net::URLRequest& request, - ResourceHandler* next_handler, - PrerenderManager* prerender_manager, - bool make_pending, int child_id, int route_id); + PrerenderResourceHandler( + const net::URLRequest& request, + ResourceHandler* next_handler, + const base::WeakPtr<PrerenderManager>& prerender_manager, + bool make_pending, + int child_id, + int route_id); // This constructor is only used from unit tests. PrerenderResourceHandler(const net::URLRequest& request, @@ -104,7 +107,7 @@ class PrerenderResourceHandler : public ResourceHandler { std::vector<GURL> alias_urls_; GURL url_; scoped_refptr<ResourceHandler> next_handler_; - scoped_refptr<PrerenderManager> prerender_manager_; + base::WeakPtr<PrerenderManager> prerender_manager_; scoped_ptr<PrerenderCallback> prerender_callback_; // Used to obtain the referrer, but only after any redirections occur, as they diff --git a/chrome/browser/profiles/profile_impl.cc b/chrome/browser/profiles/profile_impl.cc index 3fb658d..0b18cc2 100644 --- a/chrome/browser/profiles/profile_impl.cc +++ b/chrome/browser/profiles/profile_impl.cc @@ -1553,8 +1553,8 @@ PrefProxyConfigTracker* ProfileImpl::GetProxyConfigTracker() { prerender::PrerenderManager* ProfileImpl::GetPrerenderManager() { if (!prerender::PrerenderManager::IsPrerenderingPossible()) - return NULL; - if (!prerender_manager_) - prerender_manager_ = new prerender::PrerenderManager(this); - return prerender_manager_; + return base::WeakPtr<prerender::PrerenderManager>(); + if (!prerender_manager_.get()) + prerender_manager_.reset(new prerender::PrerenderManager(this)); + return prerender_manager_.get(); } diff --git a/chrome/browser/profiles/profile_impl.h b/chrome/browser/profiles/profile_impl.h index 8892080..c28204f 100644 --- a/chrome/browser/profiles/profile_impl.h +++ b/chrome/browser/profiles/profile_impl.h @@ -214,7 +214,7 @@ class ProfileImpl : public Profile, scoped_refptr<TransportSecurityPersister> transport_security_persister_; scoped_ptr<policy::ProfilePolicyConnector> profile_policy_connector_; - scoped_refptr<prerender::PrerenderManager> prerender_manager_; + scoped_ptr<prerender::PrerenderManager> prerender_manager_; scoped_ptr<NetPrefObserver> net_pref_observer_; scoped_ptr<TemplateURLFetcher> template_url_fetcher_; scoped_ptr<TemplateURLModel> template_url_model_; diff --git a/chrome/browser/profiles/profile_io_data.cc b/chrome/browser/profiles/profile_io_data.cc index 3419efb..16c3c88 100644 --- a/chrome/browser/profiles/profile_io_data.cc +++ b/chrome/browser/profiles/profile_io_data.cc @@ -24,6 +24,7 @@ #include "chrome/browser/net/pref_proxy_config_service.h" #include "chrome/browser/net/proxy_service_factory.h" #include "chrome/browser/prefs/pref_service.h" +#include "chrome/browser/prerender/prerender_manager.h" #include "chrome/browser/profiles/profile.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/pref_names.h" @@ -58,7 +59,7 @@ class ChromeCookieMonsterDelegate : public net::CookieMonster::Delegate { NewRunnableMethod(this, &ChromeCookieMonsterDelegate::OnCookieChangedAsyncHelper, cookie, - removed, + removed, cause)); } @@ -178,7 +179,10 @@ void ProfileIOData::InitializeProfileParams(Profile* profile) { params->blob_storage_context = profile->GetBlobStorageContext(); params->file_system_context = profile->GetFileSystemContext(); params->extension_info_map = profile->GetExtensionInfoMap(); - params->prerender_manager = profile->GetPrerenderManager(); + prerender::PrerenderManager* prerender_manager = + profile->GetPrerenderManager(); + if (prerender_manager) + params->prerender_manager = prerender_manager->AsWeakPtr(); params->protocol_handler_registry = profile->GetProtocolHandlerRegistry(); params->proxy_config_service.reset( diff --git a/chrome/browser/profiles/profile_io_data.h b/chrome/browser/profiles/profile_io_data.h index ab4c252..ce5f954 100644 --- a/chrome/browser/profiles/profile_io_data.h +++ b/chrome/browser/profiles/profile_io_data.h @@ -12,6 +12,7 @@ #include "base/file_path.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" +#include "base/memory/weak_ptr.h" #include "base/synchronization/lock.h" #include "chrome/browser/net/chrome_url_request_context.h" #include "chrome/browser/prefs/pref_member.h" @@ -111,7 +112,7 @@ class ProfileIOData : public base::RefCountedThreadSafe<ProfileIOData> { scoped_refptr<ChromeBlobStorageContext> blob_storage_context; scoped_refptr<fileapi::FileSystemContext> file_system_context; scoped_refptr<ExtensionInfoMap> extension_info_map; - scoped_refptr<prerender::PrerenderManager> prerender_manager; + base::WeakPtr<prerender::PrerenderManager> prerender_manager; scoped_refptr<ProtocolHandlerRegistry> protocol_handler_registry; // We need to initialize the ProxyConfigService from the UI thread // because on linux it relies on initializing things through gconf, diff --git a/chrome/test/testing_profile.cc b/chrome/test/testing_profile.cc index 3a4c3a7..67487db 100644 --- a/chrome/test/testing_profile.cc +++ b/chrome/test/testing_profile.cc @@ -788,10 +788,10 @@ ChromeURLDataManager* TestingProfile::GetChromeURLDataManager() { prerender::PrerenderManager* TestingProfile::GetPrerenderManager() { if (!prerender::PrerenderManager::IsPrerenderingPossible()) - return NULL; - if (!prerender_manager_) - prerender_manager_ = new prerender::PrerenderManager(this); - return prerender_manager_; + return base::WeakPtr<prerender::PrerenderManager>(); + if (!prerender_manager_.get()) + prerender_manager_.reset(new prerender::PrerenderManager(this)); + return prerender_manager_.get(); } PrefService* TestingProfile::GetOffTheRecordPrefs() { diff --git a/chrome/test/testing_profile.h b/chrome/test/testing_profile.h index 3e74b92..c8639e3 100644 --- a/chrome/test/testing_profile.h +++ b/chrome/test/testing_profile.h @@ -399,7 +399,7 @@ class TestingProfile : public Profile { scoped_ptr<ChromeURLDataManager> chrome_url_data_manager_; - scoped_refptr<prerender::PrerenderManager> prerender_manager_; + scoped_ptr<prerender::PrerenderManager> prerender_manager_; // We keep a weak pointer to the dependency manager we want to notify on our // death. Defaults to the Singleton implementation but overridable for |