[Offline Pages] Expanding event logger to be used by harness.

components/offline_pages/offline_event_logger.cc

 // Copyright 2016 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.
 
 #include "base/strings/stringprintf.h"
 #include "base/time/time.h"
 #include "components/offline_pages/offline_event_logger.h"
 
 namespace offline_pages {
 
 OfflineEventLogger::OfflineEventLogger()
     : activities_(kMaxLogCount), is_logging_(false) {}
 
 OfflineEventLogger::~OfflineEventLogger() {}
 
+void OfflineEventLogger::Clear() {

[fgorski, 2016/11/30 17:30:47]

Please explain why you are moving the code so much.

[romax, 2016/12/01 01:47:53]

I thought the order of the implementation should be the same with header file so
I'm moving a lot...

+  activities_.clear();
+}
+
 void OfflineEventLogger::SetIsLogging(bool is_logging) {
   is_logging_ = is_logging;
 }
 
 bool OfflineEventLogger::GetIsLogging() {
   return is_logging_;
 }
 
-void OfflineEventLogger::Clear() {
-  activities_.clear();
+void OfflineEventLogger::GetLogs(std::vector<std::string>* records) {

[fgorski, 2016/11/30 17:30:47]

I see you are moving this code, a few suggestions while you are at it.

[romax, 2016/12/01 01:47:53]

Acknowledged.

+  DCHECK(records);
+  for (std::deque<std::string>::iterator it = activities_.begin();

[fgorski, 2016/11/30 17:30:47]

you can probably use some form of auto here.

[romax, 2016/12/01 01:47:53]

Done.

+       it != activities_.end(); it++) {

[fgorski, 2016/11/30 17:30:47]

++it

[romax, 2016/12/01 01:47:53]

Done.

+    if (!it->empty())

[fgorski, 2016/11/30 17:30:47]

This guard would be more reasonable in record activity I believe, before we put
date in front of an empty activity, guaranteeing that the string here is not
empty. 

[romax, 2016/12/01 01:47:52]

Done.

+      records->push_back(*it);
+  }
 }
 
 void OfflineEventLogger::RecordActivity(const std::string& activity) {
   if (!is_logging_) {
     return;
   }
-  if (activities_.size() == kMaxLogCount)
-    activities_.pop_back();
 
   base::Time::Exploded current_time;
   base::Time::Now().LocalExplode(&current_time);
 
   std::string date_string = base::StringPrintf(
       "%d %02d %02d %02d:%02d:%02d",
       current_time.year,
       current_time.month,
       current_time.day_of_month,
       current_time.hour,
       current_time.minute,
       current_time.second);
 
+  if (client_)
+    client_->CustomLog(activity);
+
+  if (activities_.size() == kMaxLogCount)
+    activities_.pop_back();
+
   activities_.push_front(date_string + ": " + activity);
 }
 
-void OfflineEventLogger::GetLogs(std::vector<std::string>* records) {
-  DCHECK(records);
-  for (std::deque<std::string>::iterator it = activities_.begin();
-       it != activities_.end(); it++) {
-    if (!it->empty())
-      records->push_back(*it);
-  }
+void OfflineEventLogger::SetClient(Client* client) {
+  SetIsLogging(true);

[fgorski, 2016/11/30 17:30:47]

what if client == nullptr?

[romax, 2016/12/01 01:47:53]

then client_ would be set to nullptr. and nothing would be CustomLog'ed since it
is checked when logging.

+  client_ = client;
 }
 
 }  // namespace offline_pages