diff options
-rw-r--r-- | rlz/lib/financial_ping.cc | 68 | ||||
-rw-r--r-- | rlz/lib/financial_ping.h | 7 | ||||
-rw-r--r-- | rlz/lib/rlz_lib_test.cc | 67 |
3 files changed, 126 insertions, 16 deletions
diff --git a/rlz/lib/financial_ping.cc b/rlz/lib/financial_ping.cc index 5f15256..9c596a4 100644 --- a/rlz/lib/financial_ping.cc +++ b/rlz/lib/financial_ping.cc @@ -6,8 +6,10 @@ #include "rlz/lib/financial_ping.h" +#include "base/atomicops.h" #include "base/basictypes.h" #include "base/memory/scoped_ptr.h" +#include "base/memory/weak_ptr.h" #include "base/strings/string_util.h" #include "base/strings/stringprintf.h" #include "base/strings/utf_string_conversions.h" @@ -84,6 +86,8 @@ int64 GetSystemTimeAsInt64() { namespace rlz_lib { +using base::subtle::AtomicWord; + bool FinancialPing::FormRequest(Product product, const AccessPoint* access_points, const char* product_signature, const char* product_brand, const char* product_id, @@ -177,12 +181,15 @@ bool FinancialPing::FormRequest(Product product, } #if defined(RLZ_NETWORK_IMPLEMENTATION_CHROME_NET) -// The URLRequestContextGetter used by FinancialPing::PingServer(). -net::URLRequestContextGetter* g_context; +// The pointer to URLRequestContextGetter used by FinancialPing::PingServer(). +// It is atomic pointer because it can be accessed and modified by multiple +// threads. +AtomicWord g_context; bool FinancialPing::SetURLRequestContext( net::URLRequestContextGetter* context) { - g_context = context; + base::subtle::NoBarrier_Store( + &g_context, reinterpret_cast<AtomicWord>(context)); return true; } @@ -204,10 +211,33 @@ void FinancialPingUrlFetcherDelegate::OnURLFetchComplete( callback_.Run(); } +#if defined(RLZ_NETWORK_IMPLEMENTATION_CHROME_NET) +bool send_financial_ping_interrupted_for_test = false; +#endif + } // namespace #endif +#if defined(RLZ_NETWORK_IMPLEMENTATION_CHROME_NET) +void ShutdownCheck(base::WeakPtr<base::RunLoop> weak) { + if (!weak.get()) + return; + if (!base::subtle::NoBarrier_Load(&g_context)) { + send_financial_ping_interrupted_for_test = true; + weak->QuitClosure().Run(); + return; + } + // How frequently the financial ping thread should check + // the shutdown condition? + const base::TimeDelta kInterval = base::TimeDelta::FromMilliseconds(500); + base::MessageLoop::current()->PostDelayedTask( + FROM_HERE, + base::Bind(&ShutdownCheck, weak), + kInterval); +} +#endif + bool FinancialPing::PingServer(const char* request, std::string* response) { if (!response) return false; @@ -265,8 +295,15 @@ bool FinancialPing::PingServer(const char* request, std::string* response) { return true; #else + // Copy the pointer to stack because g_context may be set to NULL + // in different thread. The instance is guaranteed to exist while + // the method is running. + net::URLRequestContextGetter* context = + reinterpret_cast<net::URLRequestContextGetter*>( + base::subtle::NoBarrier_Load(&g_context)); + // Browser shutdown will cause the context to be reset to NULL. - if (!g_context) + if (!context) return false; // Run a blocking event loop to match the win inet implementation. @@ -292,13 +329,18 @@ bool FinancialPing::PingServer(const char* request, std::string* response) { // Ensure rlz_lib::SetURLRequestContext() has been called before sending // pings. - fetcher->SetRequestContext(g_context); + fetcher->SetRequestContext(context); + + base::WeakPtrFactory<base::RunLoop> weak(&loop); const base::TimeDelta kTimeout = base::TimeDelta::FromMinutes(5); base::MessageLoop::ScopedNestableTaskAllower allow_nested( base::MessageLoop::current()); base::MessageLoop::current()->PostTask( FROM_HERE, + base::Bind(&ShutdownCheck, weak.GetWeakPtr())); + base::MessageLoop::current()->PostTask( + FROM_HERE, base::Bind(&net::URLFetcher::Start, base::Unretained(fetcher.get()))); base::MessageLoop::current()->PostDelayedTask( FROM_HERE, loop.QuitClosure(), kTimeout); @@ -359,4 +401,18 @@ bool FinancialPing::ClearLastPingTime(Product product) { return store->ClearPingTime(product); } -} // namespace +#if defined(RLZ_NETWORK_IMPLEMENTATION_CHROME_NET) +namespace test { + +void ResetSendFinancialPingInterrupted() { + send_financial_ping_interrupted_for_test = false; +} + +bool WasSendFinancialPingInterrupted() { + return send_financial_ping_interrupted_for_test; +} + +} // namespace test +#endif + +} // namespace rlz_lib diff --git a/rlz/lib/financial_ping.h b/rlz/lib/financial_ping.h index 081ecba..63c20a1 100644 --- a/rlz/lib/financial_ping.h +++ b/rlz/lib/financial_ping.h @@ -58,6 +58,13 @@ class FinancialPing { ~FinancialPing() {} }; +#if defined(RLZ_NETWORK_IMPLEMENTATION_CHROME_NET) +namespace test { +void ResetSendFinancialPingInterrupted(); +bool WasSendFinancialPingInterrupted(); +} // namespace test +#endif + } // namespace rlz_lib diff --git a/rlz/lib/rlz_lib_test.cc b/rlz/lib/rlz_lib_test.cc index f21c540..10cf3de 100644 --- a/rlz/lib/rlz_lib_test.cc +++ b/rlz/lib/rlz_lib_test.cc @@ -19,6 +19,7 @@ #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" +#include "rlz/lib/financial_ping.h" #include "rlz/lib/rlz_lib.h" #include "rlz/lib/rlz_value_store.h" #include "rlz/test/rlz_test_helpers.h" @@ -416,6 +417,16 @@ TEST_F(RlzLibTest, ParsePingResponseWithStatefulEvents) { EXPECT_STREQ("events=W1I", value); } +class URLRequestRAII { + public: + URLRequestRAII(net::URLRequestContextGetter* context) { + rlz_lib::SetURLRequestContext(context); + } + ~URLRequestRAII() { + rlz_lib::SetURLRequestContext(NULL); + } +}; + TEST_F(RlzLibTest, SendFinancialPing) { // We don't really check a value or result in this test. All this does is // attempt to ping the financial server, which you can verify in Fiddler. @@ -437,16 +448,6 @@ TEST_F(RlzLibTest, SendFinancialPing) { io_thread.message_loop()->message_loop_proxy()); rlz_lib::SetURLRequestContext(context.get()); - class URLRequestRAII { - public: - URLRequestRAII(net::URLRequestContextGetter* context) { - rlz_lib::SetURLRequestContext(context); - } - ~URLRequestRAII() { - rlz_lib::SetURLRequestContext(NULL); - } - }; - URLRequestRAII set_context(context.get()); #endif @@ -474,6 +475,52 @@ TEST_F(RlzLibTest, SendFinancialPing) { /*skip_time_check=*/true); } +#if defined(RLZ_NETWORK_IMPLEMENTATION_CHROME_NET) + +void ResetContext() { + rlz_lib::SetURLRequestContext(NULL); +} + +TEST_F(RlzLibTest, SendFinancialPingDuringShutdown) { + // rlz_lib::SendFinancialPing fails when this is set. + if (!rlz_lib::SupplementaryBranding::GetBrand().empty()) + return; + +#if defined(OS_MACOSX) + base::mac::ScopedNSAutoreleasePool pool; +#endif + + base::Thread::Options options; + options.message_loop_type = base::MessageLoop::TYPE_IO; + + base::Thread io_thread("rlz_unittest_io_thread"); + ASSERT_TRUE(io_thread.StartWithOptions(options)); + + scoped_refptr<net::TestURLRequestContextGetter> context = + new net::TestURLRequestContextGetter( + io_thread.message_loop()->message_loop_proxy()); + rlz_lib::SetURLRequestContext(context.get()); + + URLRequestRAII set_context(context.get()); + + rlz_lib::AccessPoint points[] = + {rlz_lib::IETB_SEARCH_BOX, rlz_lib::NO_ACCESS_POINT, + rlz_lib::NO_ACCESS_POINT}; + rlz_lib::test::ResetSendFinancialPingInterrupted(); + EXPECT_FALSE(rlz_lib::test::WasSendFinancialPingInterrupted()); + + base::MessageLoop loop; + loop.PostTask(FROM_HERE, base::Bind(&ResetContext)); + std::string request; + EXPECT_FALSE(rlz_lib::SendFinancialPing(rlz_lib::TOOLBAR_NOTIFIER, points, + "swg", "GGLA", "SwgProductId1234", "en-UK", false, + /*skip_time_check=*/true)); + + EXPECT_TRUE(rlz_lib::test::WasSendFinancialPingInterrupted()); + rlz_lib::test::ResetSendFinancialPingInterrupted(); +} +#endif + TEST_F(RlzLibTest, ClearProductState) { MachineDealCodeHelper::Clear(); |