summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--rlz/lib/financial_ping.cc68
-rw-r--r--rlz/lib/financial_ping.h7
-rw-r--r--rlz/lib/rlz_lib_test.cc67
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();