google_apis: Make EventLogger thread-safe

Along the way, remove the parameter from the constructor so that
the class can be created using base::LazyInstance.

BUG=233448
TEST=none

Review URL: https://codereview.chromium.org/14046044

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@196047 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 58 insertions, 31 deletions

diff --git a/chrome/browser/chromeos/drive/drive_prefetcher_unittest.cc b/chrome/browser/chromeos/drive/drive_prefetcher_unittest.cc
index a7911cb..c0e4ad3 100644
--- a/chrome/browser/chromeos/drive/drive_prefetcher_unittest.cc
+++ b/chrome/browser/chromeos/drive/drive_prefetcher_unittest.cc
@@ -137,8 +137,7 @@ const char* kAllRegularFiles[] = {
 class DrivePrefetcherTest : public testing::Test {
  public:
   DrivePrefetcherTest()
-      : dummy_event_logger_(0),
-        ui_thread_(content::BrowserThread::UI, &message_loop_) {}
+      : ui_thread_(content::BrowserThread::UI, &message_loop_) {}
 
   virtual void SetUp() OVERRIDE {
     mock_file_system_.reset(new StrictMock<MockDriveFileSystem>);
diff --git a/chrome/browser/chromeos/drive/drive_system_service.cc b/chrome/browser/chromeos/drive/drive_system_service.cc
index 46dbf9c..3f06d0d 100644
--- a/chrome/browser/chromeos/drive/drive_system_service.cc
+++ b/chrome/browser/chromeos/drive/drive_system_service.cc
@@ -47,8 +47,6 @@ using content::BrowserThread;
 namespace drive {
 namespace {
 
-static const size_t kEventLogHistorySize = 100;
-
 // The sync invalidation object ID for Google Drive.
 const char kDriveInvalidationObjectId[] = "CHANGELOG";
 
@@ -113,7 +111,7 @@ DriveSystemService::DriveSystemService(
   blocking_task_runner_ = blocking_pool->GetSequencedTaskRunner(
       blocking_pool->GetSequenceToken());
 
-  event_logger_.reset(new google_apis::EventLogger(kEventLogHistorySize));
+  event_logger_.reset(new google_apis::EventLogger);
   if (test_drive_service) {
     drive_service_.reset(test_drive_service);
   } else if (google_apis::util::IsDriveV2ApiEnabled()) {
diff --git a/chrome/browser/google_apis/event_logger.cc b/chrome/browser/google_apis/event_logger.cc
index d00801f..c90a710 100644
--- a/chrome/browser/google_apis/event_logger.cc
+++ b/chrome/browser/google_apis/event_logger.cc
@@ -14,8 +14,8 @@ EventLogger::Event::Event(int id, const std::string& what)
       what(what) {
 }
 
-EventLogger::EventLogger(size_t history_size)
-    : history_size_(history_size),
+EventLogger::EventLogger()
+    : history_size_(kDefaultHistorySize),
       next_event_id_(0) {
 }
 
@@ -30,10 +30,25 @@ void EventLogger::Log(const char* format, ...) {
   base::StringAppendV(&what, format, args);
   va_end(args);
 
+  base::AutoLock auto_lock(lock_);
   history_.push_back(Event(next_event_id_, what));
   ++next_event_id_;
   if (history_.size() > history_size_)
     history_.pop_front();
 }
 
+void EventLogger::SetHistorySize(size_t history_size) {
+  base::AutoLock auto_lock(lock_);
+  history_.clear();
+  history_size_ = history_size;
+}
+
+std::vector<EventLogger::Event> EventLogger::GetHistory() {
+  base::AutoLock auto_lock(lock_);
+  std::vector<Event> output;
+  output.assign(history_.begin(), history_.end());
+  return output;
+}
+
+
 }  // namespace google_apis
diff --git a/chrome/browser/google_apis/event_logger.h b/chrome/browser/google_apis/event_logger.h
index 1826ecd..251fddf 100644
--- a/chrome/browser/google_apis/event_logger.h
+++ b/chrome/browser/google_apis/event_logger.h
@@ -8,13 +8,18 @@
 #include <stdarg.h>   // va_list
 #include <deque>
 #include <string>
+#include <vector>
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
+#include "base/synchronization/lock.h"
 #include "base/time.h"
 
 namespace google_apis {
 
+// The default history size used by EventLogger.
+const int kDefaultHistorySize = 100;
+
 // EventLogger is used to collect and expose text messages for diagnosing
 // behaviors of Google APIs stuff. For instance, the collected messages are
 // exposed to chrome:drive-internals.
@@ -28,22 +33,29 @@ class EventLogger {
     std::string what;  // What happened.
   };
 
-  // Creates an event logger that keeps the latest |history_size| events.
-  explicit EventLogger(size_t history_size);
+  // Creates an event logger that keeps the latest kDefaultHistorySize events.
+  EventLogger();
   ~EventLogger();
 
   // Logs a message using printf format.
+  // Can be called from any thread as long as the object is alive.
   // Note that PRINTF_FORMAT should be (2, 3) instead of (1, 2) as this is a
   // C++ member function.
   void Log(const char* format, ...) PRINTF_FORMAT(2, 3);
 
+  // Sets the history size. The existing history is cleared.
+  // Can be called from any thread as long as the object is alive.
+  void SetHistorySize(size_t history_size);
+
   // Gets the list of latest events (the oldest event comes first).
-  const std::deque<Event>& history() const { return history_; }
+  // Can be called from any thread as long as the object is alive.
+  std::vector<Event> GetHistory();
 
  private:
-  std::deque<Event> history_;
-  const size_t history_size_;
-  int next_event_id_;
+  std::deque<Event> history_;  // guarded by lock_.
+  size_t history_size_;  // guarded by lock_.
+  int next_event_id_;  // guarded by lock_.
+  base::Lock lock_;
 
   DISALLOW_COPY_AND_ASSIGN(EventLogger);
 };
diff --git a/chrome/browser/google_apis/event_logger_unittest.cc b/chrome/browser/google_apis/event_logger_unittest.cc
index 2fa5d74..e4c5308 100644
--- a/chrome/browser/google_apis/event_logger_unittest.cc
+++ b/chrome/browser/google_apis/event_logger_unittest.cc
@@ -9,32 +9,35 @@
 namespace google_apis {
 
 TEST(EventLoggerTest, BasicLogging) {
-  EventLogger logger(3);  // At most 3 events are kept.
-  EXPECT_EQ(0U, logger.history().size());
+  EventLogger logger;
+  logger.SetHistorySize(3);  // At most 3 events are kept.
+  EXPECT_EQ(0U, logger.GetHistory().size());
 
   logger.Log("first");
   logger.Log("%dnd", 2);
   logger.Log("third");
 
   // Events are recorded in the chronological order with sequential IDs.
-  ASSERT_EQ(3U, logger.history().size());
-  EXPECT_EQ(0, logger.history()[0].id);
-  EXPECT_EQ("first", logger.history()[0].what);
-  EXPECT_EQ(1, logger.history()[1].id);
-  EXPECT_EQ("2nd", logger.history()[1].what);
-  EXPECT_EQ(2, logger.history()[2].id);
-  EXPECT_EQ("third", logger.history()[2].what);
+  std::vector<EventLogger::Event> history = logger.GetHistory();
+  ASSERT_EQ(3U, history.size());
+  EXPECT_EQ(0, history[0].id);
+  EXPECT_EQ("first", history[0].what);
+  EXPECT_EQ(1, history[1].id);
+  EXPECT_EQ("2nd", history[1].what);
+  EXPECT_EQ(2, history[2].id);
+  EXPECT_EQ("third", history[2].what);
 
   logger.Log("fourth");
   // It does not log events beyond the specified.
-  ASSERT_EQ(3U, logger.history().size());
+  history = logger.GetHistory();
+  ASSERT_EQ(3U, history.size());
   // The oldest events is pushed out.
-  EXPECT_EQ(1, logger.history()[0].id);
-  EXPECT_EQ("2nd", logger.history()[0].what);
-  EXPECT_EQ(2, logger.history()[1].id);
-  EXPECT_EQ("third", logger.history()[1].what);
-  EXPECT_EQ(3, logger.history()[2].id);
-  EXPECT_EQ("fourth", logger.history()[2].what);
+  EXPECT_EQ(1, history[0].id);
+  EXPECT_EQ("2nd", history[0].what);
+  EXPECT_EQ(2, history[1].id);
+  EXPECT_EQ("third", history[1].what);
+  EXPECT_EQ(3, history[2].id);
+  EXPECT_EQ("fourth", history[2].what);
 }
 
 }   // namespace google_apis
diff --git a/chrome/browser/ui/webui/chromeos/drive_internals_ui.cc b/chrome/browser/ui/webui/chromeos/drive_internals_ui.cc
index 6b4b10b..ca8f920 100644
--- a/chrome/browser/ui/webui/chromeos/drive_internals_ui.cc
+++ b/chrome/browser/ui/webui/chromeos/drive_internals_ui.cc
@@ -631,8 +631,8 @@ void DriveInternalsWebUIHandler::UpdateCacheContentsSection(
 
 void DriveInternalsWebUIHandler::UpdateEventLogSection(
     google_apis::EventLogger* event_logger) {
-  const std::deque<google_apis::EventLogger::Event>& log =
-      event_logger->history();
+  const std::vector<google_apis::EventLogger::Event> log =
+      event_logger->GetHistory();
 
   base::ListValue list;
   for (size_t i = 0; i < log.size(); ++i) {