Add periodic check to see if context has been reset

rlz/lib/financial_ping.cc

Index: rlz/lib/financial_ping.cc
diff --git a/rlz/lib/financial_ping.cc b/rlz/lib/financial_ping.cc
index 5f152569dd6cc5bddb6537a28cb84e1c1b33b2e7..e8ff2d0d45e54401b2f874d8b492020133fadd0e 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,16 @@ 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(
+      reinterpret_cast<AtomicWord*>(&g_context),
+      reinterpret_cast<AtomicWord>(context));

[Roger Tawa OOO till Jul 10th, 2013/10/17 01:03:22]

Do you need the first reinterpret_cast<>?

[oshima, 2013/10/17 01:09:06]

Done.

   return true;
 }
 
@@ -204,10 +212,34 @@ 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 (!g_context) {

[Roger Tawa OOO till Jul 10th, 2013/10/17 01:03:22]

Should use nobarrier_load here too.

[oshima, 2013/10/17 01:09:06]

Done.

+    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 +297,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 +331,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 +403,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