diff options
-rw-r--r-- | base/message_loop_proxy.h | 3 | ||||
-rw-r--r-- | chrome/browser/chrome_plugin_unittest.cc | 6 | ||||
-rw-r--r-- | chrome/browser/chrome_thread.cc | 3 | ||||
-rw-r--r-- | chrome/browser/net/chrome_url_request_context.cc | 6 | ||||
-rw-r--r-- | chrome/browser/net/chrome_url_request_context.h | 5 | ||||
-rw-r--r-- | chrome/browser/net/url_fetcher.cc | 32 | ||||
-rw-r--r-- | chrome/browser/net/url_fetcher_unittest.cc | 39 | ||||
-rw-r--r-- | chrome/browser/net/url_request_context_getter.cc | 17 | ||||
-rw-r--r-- | chrome/browser/net/url_request_context_getter.h | 22 | ||||
-rw-r--r-- | chrome/browser/sync/glue/http_bridge.cc | 7 | ||||
-rw-r--r-- | chrome/browser/sync/glue/http_bridge.h | 3 | ||||
-rw-r--r-- | chrome/browser/sync/glue/http_bridge_unittest.cc | 37 | ||||
-rw-r--r-- | chrome/test/testing_profile.cc | 9 |
13 files changed, 144 insertions, 45 deletions
diff --git a/base/message_loop_proxy.h b/base/message_loop_proxy.h index 0cfe7ee..4b6abce 100644 --- a/base/message_loop_proxy.h +++ b/base/message_loop_proxy.h @@ -28,6 +28,9 @@ class MessageLoopProxy : public base::RefCountedThreadSafe<MessageLoopProxy> { const tracked_objects::Location& from_here, Task* task, int64 delay_ms) = 0; + // A method which checks if the caller is currently running in the thread that + // this proxy represents. + virtual bool BelongsToCurrentThread() = 0; template <class T> bool DeleteSoon(const tracked_objects::Location& from_here, diff --git a/chrome/browser/chrome_plugin_unittest.cc b/chrome/browser/chrome_plugin_unittest.cc index e515966..84d739a 100644 --- a/chrome/browser/chrome_plugin_unittest.cc +++ b/chrome/browser/chrome_plugin_unittest.cc @@ -4,6 +4,7 @@ // Tests exercising the Chrome Plugin API. #include "base/file_util.h" +#include "base/message_loop_proxy.h" #include "base/path_service.h" #include "base/string_util.h" #include "chrome/browser/chrome_plugin_host.h" @@ -31,6 +32,10 @@ class TestURLRequestContextGetter : public URLRequestContextGetter { context_ = new TestURLRequestContext(); return context_; } + virtual scoped_refptr<MessageLoopProxy> GetIOMessageLoopProxy() { + return ChromeThread::GetMessageLoopProxyForThread(ChromeThread::IO); + } + private: ~TestURLRequestContextGetter() {} scoped_refptr<URLRequestContext> context_; @@ -299,4 +304,3 @@ TEST_F(ChromePluginTest, DoesNotInterceptOwnRequest) { } } // namespace - diff --git a/chrome/browser/chrome_thread.cc b/chrome/browser/chrome_thread.cc index 6421a79..e6f03cc 100644 --- a/chrome/browser/chrome_thread.cc +++ b/chrome/browser/chrome_thread.cc @@ -51,6 +51,9 @@ class ChromeThreadMessageLoopProxy : public MessageLoopProxy { return ChromeThread::PostNonNestableDelayedTask(id_, from_here, task, delay_ms); } + virtual bool BelongsToCurrentThread() { + return ChromeThread::CurrentlyOn(id_); + } private: ChromeThread::ID id_; diff --git a/chrome/browser/net/chrome_url_request_context.cc b/chrome/browser/net/chrome_url_request_context.cc index 0dad578..97c202c 100644 --- a/chrome/browser/net/chrome_url_request_context.cc +++ b/chrome/browser/net/chrome_url_request_context.cc @@ -6,6 +6,7 @@ #include "base/command_line.h" #include "base/message_loop.h" +#include "base/message_loop_proxy.h" #include "base/string_util.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/chrome_thread.h" @@ -550,6 +551,11 @@ net::CookieStore* ChromeURLRequestContextGetter::GetCookieStore() { return result; } +scoped_refptr<MessageLoopProxy> +ChromeURLRequestContextGetter::GetIOMessageLoopProxy() { + return ChromeThread::GetMessageLoopProxyForThread(ChromeThread::IO); +} + // static ChromeURLRequestContextGetter* ChromeURLRequestContextGetter::CreateOriginal( Profile* profile, const FilePath& cookie_store_path, diff --git a/chrome/browser/net/chrome_url_request_context.h b/chrome/browser/net/chrome_url_request_context.h index 261743d..b22ee68 100644 --- a/chrome/browser/net/chrome_url_request_context.h +++ b/chrome/browser/net/chrome_url_request_context.h @@ -254,12 +254,13 @@ class ChromeURLRequestContextGetter : public URLRequestContextGetter, ChromeURLRequestContextFactory* factory); // Note that GetURLRequestContext() can only be called from the IO - // thread (it will assert otherwise). GetCookieStore() however can - // be called from any thread. + // thread (it will assert otherwise). GetCookieStore() and + // GetIOMessageLoopProxy however can be called from any thread. // // URLRequestContextGetter implementation. virtual URLRequestContext* GetURLRequestContext(); virtual net::CookieStore* GetCookieStore(); + virtual scoped_refptr<MessageLoopProxy> GetIOMessageLoopProxy(); // Convenience overload of GetURLRequestContext() that returns a // ChromeURLRequestContext* rather than a URLRequestContext*. diff --git a/chrome/browser/net/url_fetcher.cc b/chrome/browser/net/url_fetcher.cc index 0d1fbac..93a4834 100644 --- a/chrome/browser/net/url_fetcher.cc +++ b/chrome/browser/net/url_fetcher.cc @@ -5,10 +5,9 @@ #include "chrome/browser/net/url_fetcher.h" #include "base/compiler_specific.h" +#include "base/message_loop_proxy.h" #include "base/string_util.h" #include "base/thread.h" -#include "chrome/browser/browser_process.h" -#include "chrome/browser/chrome_thread.h" #include "chrome/browser/net/url_fetcher_protect.h" #include "chrome/browser/net/url_request_context_getter.h" #include "googleurl/src/gurl.h" @@ -70,6 +69,9 @@ class URLFetcher::Core RequestType request_type_; // What type of request is this? URLFetcher::Delegate* delegate_; // Object to notify on completion MessageLoop* delegate_loop_; // Message loop of the creating thread + scoped_refptr<MessageLoopProxy> io_message_loop_proxy_; + // The message loop proxy for the thread + // on which the request IO happens. URLRequest* request_; // The actual request this wraps int load_flags_; // Flags for the load operation int response_code_; // HTTP status code for the request @@ -147,8 +149,10 @@ URLFetcher::Core::Core(URLFetcher* fetcher, void URLFetcher::Core::Start() { DCHECK(delegate_loop_); CHECK(request_context_getter_) << "We need an URLRequestContext!"; - ChromeThread::PostDelayedTask( - ChromeThread::IO, FROM_HERE, + io_message_loop_proxy_ = request_context_getter_->GetIOMessageLoopProxy(); + CHECK(io_message_loop_proxy_.get()) << "We need an IO message loop proxy"; + io_message_loop_proxy_->PostDelayedTask( + FROM_HERE, NewRunnableMethod(this, &Core::StartURLRequest), protect_entry_->UpdateBackoff(URLFetcherProtectEntry::SEND)); } @@ -157,14 +161,15 @@ void URLFetcher::Core::Stop() { DCHECK_EQ(MessageLoop::current(), delegate_loop_); delegate_ = NULL; fetcher_ = NULL; - ChromeThread::PostTask( - ChromeThread::IO, FROM_HERE, - NewRunnableMethod(this, &Core::CancelURLRequest)); + if (io_message_loop_proxy_.get()) { + io_message_loop_proxy_->PostTask( + FROM_HERE, NewRunnableMethod(this, &Core::CancelURLRequest)); + } } void URLFetcher::Core::OnResponseStarted(URLRequest* request) { DCHECK(request == request_); - DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); + DCHECK(io_message_loop_proxy_->BelongsToCurrentThread()); if (request_->status().is_success()) { response_code_ = request_->GetResponseCode(); response_headers_ = request_->response_headers(); @@ -182,7 +187,7 @@ void URLFetcher::Core::OnResponseStarted(URLRequest* request) { void URLFetcher::Core::OnReadCompleted(URLRequest* request, int bytes_read) { DCHECK(request == request_); - DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); + DCHECK(io_message_loop_proxy_->BelongsToCurrentThread()); url_ = request->url(); @@ -205,7 +210,7 @@ void URLFetcher::Core::OnReadCompleted(URLRequest* request, int bytes_read) { } void URLFetcher::Core::StartURLRequest() { - DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); + DCHECK(io_message_loop_proxy_->BelongsToCurrentThread()); if (was_cancelled_) { // Since StartURLRequest() is posted as a *delayed* task, it may @@ -256,7 +261,8 @@ void URLFetcher::Core::StartURLRequest() { } void URLFetcher::Core::CancelURLRequest() { - DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); + DCHECK(io_message_loop_proxy_->BelongsToCurrentThread()); + if (request_) { request_->Cancel(); delete request_; @@ -288,8 +294,8 @@ void URLFetcher::Core::OnCompletedURLRequest(const URLRequestStatus& status) { if (delegate_) { if (fetcher_->automatically_retry_on_5xx_ && num_retries_ <= protect_entry_->max_retries()) { - ChromeThread::PostDelayedTask( - ChromeThread::IO, FROM_HERE, + io_message_loop_proxy_->PostDelayedTask( + FROM_HERE, NewRunnableMethod(this, &Core::StartURLRequest), back_off_time); } else { delegate_->OnURLFetchComplete(fetcher_, url_, status, response_code_, diff --git a/chrome/browser/net/url_fetcher_unittest.cc b/chrome/browser/net/url_fetcher_unittest.cc index 6136c7e..3445028 100644 --- a/chrome/browser/net/url_fetcher_unittest.cc +++ b/chrome/browser/net/url_fetcher_unittest.cc @@ -2,9 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "base/message_loop_proxy.h" #include "base/thread.h" -#include "base/time.h" -#include "base/timer.h" +#include "base/waitable_event.h" #include "chrome/browser/chrome_thread.h" #include "chrome/browser/net/url_fetcher.h" #include "chrome/browser/net/url_fetcher_protect.h" @@ -31,6 +31,10 @@ class TestURLRequestContextGetter : public URLRequestContextGetter { context_ = new TestURLRequestContext(); return context_; } + virtual scoped_refptr<MessageLoopProxy> GetIOMessageLoopProxy() { + return ChromeThread::GetMessageLoopProxyForThread(ChromeThread::IO); + } + private: ~TestURLRequestContextGetter() {} @@ -159,9 +163,6 @@ class URLFetcherCancelTest : public URLFetcherTest { const std::string& data); void CancelRequest(); - - private: - base::OneShotTimer<URLFetcherCancelTest> timer_; }; // Version of TestURLRequestContext that posts a Quit task to the IO @@ -175,15 +176,26 @@ class CancelTestURLRequestContext : public TestURLRequestContext { class CancelTestURLRequestContextGetter : public URLRequestContextGetter { public: + CancelTestURLRequestContextGetter() : context_created_(false, false) { + } virtual URLRequestContext* GetURLRequestContext() { - if (!context_) + if (!context_) { context_ = new CancelTestURLRequestContext(); + context_created_.Signal(); + } return context_; } + virtual scoped_refptr<MessageLoopProxy> GetIOMessageLoopProxy() { + return ChromeThread::GetMessageLoopProxyForThread(ChromeThread::IO); + } + void WaitForContextCreation() { + context_created_.Wait(); + } private: ~CancelTestURLRequestContextGetter() {} + base::WaitableEvent context_created_; scoped_refptr<URLRequestContext> context_; }; @@ -374,16 +386,14 @@ void URLFetcherBadHTTPSTest::OnURLFetchComplete( void URLFetcherCancelTest::CreateFetcher(const GURL& url) { fetcher_ = new URLFetcher(url, URLFetcher::GET, this); - // We need to force the creation of the URLRequestContext, since we - // rely on it being destroyed as a signal to end the test. - URLRequestContextGetter* context_getter = - new CancelTestURLRequestContextGetter(); - context_getter->GetURLRequestContext(); + CancelTestURLRequestContextGetter* context_getter = + new CancelTestURLRequestContextGetter; fetcher_->set_request_context(context_getter); fetcher_->Start(); - // Make sure we give the IO thread a chance to run. - timer_.Start(TimeDelta::FromMilliseconds(300), this, - &URLFetcherCancelTest::CancelRequest); + // We need to wait for the creation of the URLRequestContext, since we + // rely on it being destroyed as a signal to end the test. + context_getter->WaitForContextCreation(); + CancelRequest(); } void URLFetcherCancelTest::OnURLFetchComplete(const URLFetcher* source, @@ -401,7 +411,6 @@ void URLFetcherCancelTest::OnURLFetchComplete(const URLFetcher* source, void URLFetcherCancelTest::CancelRequest() { delete fetcher_; - timer_.Stop(); // The URLFetcher's test context will post a Quit task once it is // deleted. So if this test simply hangs, it means cancellation // did not work. diff --git a/chrome/browser/net/url_request_context_getter.cc b/chrome/browser/net/url_request_context_getter.cc index 23f9226..4970a1b 100644 --- a/chrome/browser/net/url_request_context_getter.cc +++ b/chrome/browser/net/url_request_context_getter.cc @@ -2,9 +2,26 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "base/message_loop_proxy.h" #include "chrome/browser/net/url_request_context_getter.h" #include "net/url_request/url_request_context.h" net::CookieStore* URLRequestContextGetter::GetCookieStore() { return GetURLRequestContext()->cookie_store(); } + +void URLRequestContextGetter::OnDestruct() { + scoped_refptr<MessageLoopProxy> io_message_loop_proxy = + GetIOMessageLoopProxy(); + DCHECK(io_message_loop_proxy); + if (io_message_loop_proxy) { + if (io_message_loop_proxy->BelongsToCurrentThread()) { + delete this; + } else { + io_message_loop_proxy->DeleteSoon(FROM_HERE, this); + } + } + // If no IO message loop proxy was available, we will just leak memory. + // This is also true if the IO thread is gone. +} + diff --git a/chrome/browser/net/url_request_context_getter.h b/chrome/browser/net/url_request_context_getter.h index 51c2ca9..5568192 100644 --- a/chrome/browser/net/url_request_context_getter.h +++ b/chrome/browser/net/url_request_context_getter.h @@ -6,30 +6,46 @@ #define CHROME_BROWSER_NET_URL_REQUEST_CONTEXT_GETTER_H_ #include "base/ref_counted.h" -#include "chrome/browser/chrome_thread.h" +#include "base/task.h" namespace net { class CookieStore; } +class MessageLoopProxy; class URLRequestContext; +struct URLRequestContextGetterTraits; // Interface for retrieving an URLRequestContext. class URLRequestContextGetter : public base::RefCountedThreadSafe<URLRequestContextGetter, - ChromeThread::DeleteOnIOThread> { + URLRequestContextGetterTraits> { public: virtual URLRequestContext* GetURLRequestContext() = 0; // Defaults to GetURLRequestContext()->cookie_store(), but // implementations can override it. virtual net::CookieStore* GetCookieStore(); + // Returns a MessageLoopProxy corresponding to the thread on which the + // request IO happens (the thread on which the returned URLRequestContext + // may be used). + virtual scoped_refptr<MessageLoopProxy> GetIOMessageLoopProxy() = 0; protected: - friend class ChromeThread; friend class DeleteTask<URLRequestContextGetter>; + friend struct URLRequestContextGetterTraits; virtual ~URLRequestContextGetter() {} + private: + // OnDestruct is meant to ensure deletion on the thread on which the request + // IO happens. + void OnDestruct(); +}; + +struct URLRequestContextGetterTraits { + static void Destruct(URLRequestContextGetter* context_getter) { + context_getter->OnDestruct(); + } }; #endif // CHROME_BROWSER_NET_URL_REQUEST_CONTEXT_GETTER_H_ diff --git a/chrome/browser/sync/glue/http_bridge.cc b/chrome/browser/sync/glue/http_bridge.cc index d22e780..d3becf65 100644 --- a/chrome/browser/sync/glue/http_bridge.cc +++ b/chrome/browser/sync/glue/http_bridge.cc @@ -5,6 +5,7 @@ #include "chrome/browser/sync/glue/http_bridge.h" #include "base/message_loop.h" +#include "base/message_loop_proxy.h" #include "base/string_util.h" #include "chrome/browser/chrome_thread.h" #include "chrome/browser/profile.h" @@ -42,6 +43,11 @@ URLRequestContext* HttpBridge::RequestContextGetter::GetURLRequestContext() { return context_; } +scoped_refptr<MessageLoopProxy> +HttpBridge::RequestContextGetter::GetIOMessageLoopProxy() { + return ChromeThread::GetMessageLoopProxyForThread(ChromeThread::IO); +} + HttpBridgeFactory::HttpBridgeFactory( URLRequestContextGetter* baseline_context_getter) { DCHECK(baseline_context_getter != NULL); @@ -98,6 +104,7 @@ HttpBridge::RequestContext::RequestContext(URLRequestContext* baseline_context) } HttpBridge::RequestContext::~RequestContext() { + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); delete http_transaction_factory_; } diff --git a/chrome/browser/sync/glue/http_bridge.h b/chrome/browser/sync/glue/http_bridge.h index 216544d..fc585ac 100644 --- a/chrome/browser/sync/glue/http_bridge.h +++ b/chrome/browser/sync/glue/http_bridge.h @@ -64,6 +64,7 @@ class HttpBridge : public base::RefCountedThreadSafe<HttpBridge>, } private: + // The destructor MUST be called on the IO thread. ~RequestContext(); std::string user_agent_; @@ -83,6 +84,7 @@ class HttpBridge : public base::RefCountedThreadSafe<HttpBridge>, // URLRequestContextGetter implementation. virtual URLRequestContext* GetURLRequestContext(); + virtual scoped_refptr<MessageLoopProxy> GetIOMessageLoopProxy(); private: ~RequestContextGetter() {} @@ -206,3 +208,4 @@ class HttpBridgeFactory } // namespace browser_sync #endif // CHROME_BROWSER_SYNC_GLUE_HTTP_BRIDGE_H_ + diff --git a/chrome/browser/sync/glue/http_bridge_unittest.cc b/chrome/browser/sync/glue/http_bridge_unittest.cc index 1f30c38..003e660 100644 --- a/chrome/browser/sync/glue/http_bridge_unittest.cc +++ b/chrome/browser/sync/glue/http_bridge_unittest.cc @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "base/message_loop_proxy.h" #include "base/thread.h" #include "chrome/browser/chrome_thread.h" #include "chrome/browser/net/test_url_fetcher_factory.h" @@ -24,6 +25,10 @@ class TestURLRequestContextGetter : public URLRequestContextGetter { context_ = new TestURLRequestContext; return context_; } + virtual scoped_refptr<MessageLoopProxy> GetIOMessageLoopProxy() { + return ChromeThread::GetMessageLoopProxyForThread(ChromeThread::IO); + } + private: ~TestURLRequestContextGetter() {} @@ -61,6 +66,20 @@ class HttpBridgeTest : public testing::Test { return bridge; } + static void TestSameHttpNetworkSession(MessageLoop* main_message_loop, + HttpBridgeTest* test) { + scoped_refptr<HttpBridge> http_bridge(test->BuildBridge()); + EXPECT_TRUE(test->GetTestRequestContextGetter()); + net::HttpNetworkSession* test_session = + test->GetTestRequestContextGetter()->GetURLRequestContext()-> + http_transaction_factory()->GetSession(); + EXPECT_EQ(test_session, + http_bridge->GetRequestContextGetter()-> + GetURLRequestContext()-> + http_transaction_factory()->GetSession()); + main_message_loop->PostTask(FROM_HERE, new MessageLoop::QuitTask); + } + MessageLoop* io_thread_loop() { return io_thread_.message_loop(); } // Note this is lazy created, so don't call this before your bridge. @@ -75,7 +94,7 @@ class HttpBridgeTest : public testing::Test { // Separate thread for IO used by the HttpBridge. ChromeThread io_thread_; - + MessageLoop loop_; }; class DummyURLFetcher : public TestURLFetcher { @@ -121,15 +140,13 @@ class ShuntedHttpBridge : public HttpBridge { }; TEST_F(HttpBridgeTest, TestUsesSameHttpNetworkSession) { - scoped_refptr<HttpBridge> http_bridge(this->BuildBridge()); - EXPECT_TRUE(GetTestRequestContextGetter()); - net::HttpNetworkSession* test_session = - GetTestRequestContextGetter()->GetURLRequestContext()-> - http_transaction_factory()->GetSession(); - EXPECT_EQ(test_session, - http_bridge->GetRequestContextGetter()-> - GetURLRequestContext()-> - http_transaction_factory()->GetSession()); + // Run this test on the IO thread because we can only call + // URLRequestContextGetter::GetURLRequestContext on the IO thread. + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableFunction(&HttpBridgeTest::TestSameHttpNetworkSession, + MessageLoop::current(), this)); + MessageLoop::current()->Run(); } // Test the HttpBridge without actually making any network requests. diff --git a/chrome/test/testing_profile.cc b/chrome/test/testing_profile.cc index c6f2a87..25207e9 100644 --- a/chrome/test/testing_profile.cc +++ b/chrome/test/testing_profile.cc @@ -6,6 +6,7 @@ #include "build/build_config.h" #include "base/command_line.h" +#include "base/message_loop_proxy.h" #include "base/string_util.h" #include "chrome/common/url_constants.h" #include "chrome/browser/bookmarks/bookmark_model.h" @@ -99,7 +100,7 @@ class TestURLRequestContext : public URLRequestContext { // The one here can be run on the main test thread. Note that this can lead to // a leak if your test does not have a ChromeThread::IO in it because // URLRequestContextGetter is defined as a ReferenceCounted object with a -// DeleteOnIOThread trait. +// special trait that deletes it on the IO thread. class TestURLRequestContextGetter : public URLRequestContextGetter { public: virtual URLRequestContext* GetURLRequestContext() { @@ -107,6 +108,9 @@ class TestURLRequestContextGetter : public URLRequestContextGetter { context_ = new TestURLRequestContext(); return context_.get(); } + virtual scoped_refptr<MessageLoopProxy> GetIOMessageLoopProxy() { + return ChromeThread::GetMessageLoopProxyForThread(ChromeThread::IO); + } private: scoped_refptr<URLRequestContext> context_; @@ -129,6 +133,9 @@ class TestExtensionURLRequestContextGetter : public URLRequestContextGetter { context_ = new TestExtensionURLRequestContext(); return context_.get(); } + virtual scoped_refptr<MessageLoopProxy> GetIOMessageLoopProxy() { + return ChromeThread::GetMessageLoopProxyForThread(ChromeThread::IO); + } private: scoped_refptr<URLRequestContext> context_; |