Fixes for a couple of ChromeFrame crashes seen in the latest dev channel buil...

chrome_frame/metrics_service.cc

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 
 //------------------------------------------------------------------------------
 // Description of the life cycle of a instance of MetricsService.
 //
 //  OVERVIEW
 //
@@ skipping 34 matching lines @@
 #include <windows.h>
 #include <objbase.h>
 
 #if defined(USE_SYSTEM_LIBBZ2)
 #include <bzlib.h>
 #else
 #include "third_party/bzip2/bzlib.h"
 #endif
 
 #include "base/file_version_info.h"
+#include "base/lock.h"
 #include "base/string_util.h"
 #include "base/thread.h"
 #include "base/string_number_conversions.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/common/net/url_fetcher.h"
 #include "chrome/common/net/url_fetcher_protect.h"
 #include "chrome/common/net/url_request_context_getter.h"
 #include "chrome/installer/util/browser_distribution.h"
 #include "chrome/installer/util/chrome_frame_distribution.h"
@@ skipping 18 matching lines @@
 using base::TimeDelta;
 
 static const char kMetricsType[] = "application/vnd.mozilla.metrics.bz2";
 
 // The first UMA upload occurs after this interval.
 static const int kInitialUMAUploadTimeoutMilliSeconds = 30000;
 
 // Default to one UMA upload per 10 mins.
 static const int kMinMilliSecondsPerUMAUpload = 600000;
 
-base::LazyInstance<base::ThreadLocalPointer<MetricsService> >
-    MetricsService::g_metrics_instance_(base::LINKER_INITIALIZED);
+base::LazyInstance<MetricsService>
+    g_metrics_instance_(base::LINKER_INITIALIZED);
+
+// Traits to create an instance of the ChromeFrame upload thread.
+struct UploadThreadInstanceTraits
+    : public base::DefaultLazyInstanceTraits<base::Thread> {
+  static base::Thread* New(void* instance) {
+    // Use placement new to initialize our instance in our preallocated space.
+    // The parenthesis is very important here to force POD type initialization.
+    base::Thread* upload_thread =
+        new (instance) base::Thread("ChromeFrameUploadThread");
+    base::Thread::Options options;
+    options.message_loop_type = MessageLoop::TYPE_IO;
+    bool ret = upload_thread->StartWithOptions(options);
+    if (!ret) {
+      NOTREACHED() << "Failed to start upload thread";
+    }
+    return upload_thread;
+  }
+};
+
+// ChromeFrame UMA uploads occur on this thread. This thread is started on the
+// IE UI thread. This thread needs to be stopped on the same thread it was
+// started on. We don't have a good way of achieving this at this point. This
+// thread object is currently leaked.
+// TODO(ananta)
+// Fix this.
+base::LazyInstance<base::Thread, UploadThreadInstanceTraits>
+    g_metrics_upload_thread_(base::LINKER_INITIALIZED);
+
+Lock g_metrics_service_lock;
 
 extern base::LazyInstance<StatisticsRecorder> g_statistics_recorder_;
 
 // This class provides HTTP request context information for metrics upload
 // requests initiated by ChromeFrame.
 class ChromeFrameUploadRequestContext : public URLRequestContext {
  public:
   ChromeFrameUploadRequestContext(MessageLoop* io_loop)
       : io_loop_(io_loop) {
     Initialize();
@@ skipping 97 matching lines @@
       public CWindowImpl<ChromeFrameMetricsDataUploader>,
       public TaskMarshallerThroughWindowsMessages<
           ChromeFrameMetricsDataUploader> {
  public:
   BEGIN_MSG_MAP(ChromeFrameMetricsDataUploader)
     CHAIN_MSG_MAP(
         TaskMarshallerThroughWindowsMessages<ChromeFrameMetricsDataUploader>)
   END_MSG_MAP()
 
   ChromeFrameMetricsDataUploader()
-      : fetcher_(NULL),
-        upload_thread_("ChromeFrameMetricsUploadThread") {
+      : fetcher_(NULL) {
     DLOG(INFO) << __FUNCTION__;
     creator_thread_id_ = PlatformThread::CurrentId();
   }
 
   ~ChromeFrameMetricsDataUploader() {
     DLOG(INFO) << __FUNCTION__;
     DCHECK(creator_thread_id_ == PlatformThread::CurrentId());
   }
 
   virtual void OnFinalMessage(HWND wnd) {
     Release();
   }
 
   bool Initialize() {
+    bool ret = false;
+
     if (!Create(NULL, NULL, NULL, WS_OVERLAPPEDWINDOW)) {
       NOTREACHED() << "Failed to create window";
-      return false;
+      return ret;
     }
     DCHECK(IsWindow());
 
+    if (!g_metrics_upload_thread_.Get().IsRunning()) {
+      NOTREACHED() << "Upload thread is not running";
+      return ret;
+    }
+
+    ret = true;
     // Grab a reference to the current instance which ensures that it stays
     // around until the HTTP request initiated below completes.
     // Corresponding Release is in OnFinalMessage.
     AddRef();
-
-    base::Thread::Options options;
-    options.message_loop_type = MessageLoop::TYPE_IO;
-    return upload_thread_.StartWithOptions(options);
+    return ret;
   }
 
   bool Uninitialize() {
     DestroyWindow();
     return true;
   }
 
   static HRESULT ChromeFrameMetricsDataUploader::UploadDataHelper(
       const std::string& upload_data) {
     scoped_refptr<ChromeFrameMetricsDataUploader> data_uploader =
         new ChromeFrameMetricsDataUploader();
 
-    data_uploader->Initialize();
-    DCHECK(data_uploader->upload_thread_.message_loop());
+    if (!data_uploader->Initialize()) {
+      NOTREACHED() << "Failed to initialize ChromeFrameMetricsDataUploader";
+      return E_FAIL;
+    }
 
-    data_uploader->upload_thread_.message_loop()->PostTask(FROM_HERE,
+    MessageLoop* io_loop = g_metrics_upload_thread_.Get().message_loop();
+    if (!io_loop) {
+      NOTREACHED() << "Failed to initialize ChromeFrame UMA upload thread";
+      return E_FAIL;
+    }
+
+    io_loop->PostTask(
+        FROM_HERE,
         NewRunnableMethod(data_uploader.get(),
                           &ChromeFrameMetricsDataUploader::UploadData,
-                          upload_data));
+                          upload_data, io_loop));
     return S_OK;
   }
 
-  void UploadData(const std::string& upload_data) {
+  void UploadData(const std::string& upload_data, MessageLoop* message_loop) {
     DCHECK(fetcher_ == NULL);
+    DCHECK(message_loop != NULL);
 
     BrowserDistribution* dist = ChromeFrameDistribution::GetDistribution();
     DCHECK(dist != NULL);
 
     fetcher_ = new URLFetcher(GURL(WideToUTF8(dist->GetStatsServerURL())),
                               URLFetcher::POST, this);
 
     fetcher_->set_request_context(new ChromeFrameUploadRequestContextGetter(
-        upload_thread_.message_loop()));
+        message_loop));
     fetcher_->set_upload_data(kMetricsType, upload_data);
     fetcher_->Start();
   }
 
   // URLFetcher::Delegate
   virtual void OnURLFetchComplete(const URLFetcher* source,
                                   const GURL& url,
                                   const URLRequestStatus& status,
                                   int response_code,
                                   const ResponseCookies& cookies,
                                   const std::string& data) {
     DLOG(INFO) << __FUNCTION__
                << StringPrintf(": url : %hs, status:%d, response code: %d\n",
                                url.spec().c_str(), status.status(),
                                response_code);
     delete fetcher_;
     fetcher_ = NULL;
 
     PostTask(FROM_HERE,
              NewRunnableMethod(this,
                                &ChromeFrameMetricsDataUploader::Uninitialize));
   }
 
  private:
-  base::Thread upload_thread_;
   URLFetcher* fetcher_;
   PlatformThreadId creator_thread_id_;
 };
 
 MetricsService* MetricsService::GetInstance() {
-  if (g_metrics_instance_.Pointer()->Get())
-    return g_metrics_instance_.Pointer()->Get();
-
-  g_metrics_instance_.Pointer()->Set(new MetricsService);
-  return g_metrics_instance_.Pointer()->Get();
+  AutoLock lock(g_metrics_service_lock);
+  return &g_metrics_instance_.Get();
 }
 
 MetricsService::MetricsService()
     : recording_active_(false),
       reporting_active_(false),
       user_permits_upload_(false),
       state_(INITIALIZED),
       thread_(NULL),
       initial_uma_upload_(true),
       transmission_timer_id_(0) {
@@ skipping 16 matching lines @@
 
   thread_ = PlatformThread::CurrentId();
 
   user_permits_upload_ = GoogleUpdateSettings::GetCollectStatsConsent();
   // Update session ID
   session_id_ = CrashMetricsReporter::GetInstance()->IncrementMetric(
       CrashMetricsReporter::SESSION_ID);
 
   // Ensure that an instance of the StatisticsRecorder object is created.
   g_statistics_recorder_.Get();
+
+  if (user_permits_upload_) {
+    // Ensure that an instance of the metrics upload thread is created.
+    g_metrics_upload_thread_.Get();
+  }
+
   CrashMetricsReporter::GetInstance()->set_active(true);
 }
 
 // static
 void MetricsService::Start() {
   if (GetInstance()->state_ == ACTIVE)
     return;
 
   GetInstance()->InitializeMetricsState();
   GetInstance()->SetRecording(true);
   GetInstance()->SetReporting(true);
 }
 
 // static
 void MetricsService::Stop() {
   GetInstance()->SetReporting(false);
   GetInstance()->SetRecording(false);
 }
 
 void MetricsService::SetRecording(bool enabled) {
-  DCHECK_EQ(thread_, PlatformThread::CurrentId());
   if (enabled == recording_active_)
     return;
 
   if (enabled) {
     if (client_id_.empty()) {
       client_id_ = GenerateClientID();
       // Save client id somewhere.
     }
     StartRecording();
 
@@ skipping 173 matching lines @@
     version += "-F";
     if (!version_info.IsOfficialBuild())
       version.append("-devel");
     return version;
   } else {
     NOTREACHED() << "Unable to retrieve version string.";
   }
 
   return std::string();
 }