metrics: fix memory leaks in unittest.
This fixes the memory leaks exposed in unittest.
BUG=chromium:408309
TEST=The memory leak related to |sender_| is gone.
Change-Id: I92970d0560f6ccd1ccd7f5022ece8cc5eba4a674
Reviewed-on: https://chromium-review.googlesource.com/214578
Reviewed-by: Yunlian Jiang <yunlian@chromium.org>
Tested-by: Yunlian Jiang <yunlian@chromium.org>
Commit-Queue: Yunlian Jiang <yunlian@chromium.org>
diff --git a/metrics/uploader/curl_sender.h b/metrics/uploader/curl_sender.h
index fc5b0f4..b7e9585 100644
--- a/metrics/uploader/curl_sender.h
+++ b/metrics/uploader/curl_sender.h
@@ -14,7 +14,7 @@
class CurlSender : public Sender {
public:
explicit CurlSender(std::string server_url);
-
+ virtual ~CurlSender() {}
// Sends |content| whose SHA1 hash is |hash| to server_url with a synchronous
// POST request to server_url.
bool Send(const std::string& content, const std::string& hash) override;
diff --git a/metrics/uploader/sender.h b/metrics/uploader/sender.h
index 70d29cb..5211834 100644
--- a/metrics/uploader/sender.h
+++ b/metrics/uploader/sender.h
@@ -10,6 +10,7 @@
// Abstract class for a Sender that uploads a metrics message.
class Sender {
public:
+ virtual ~Sender() {}
// Sends a message |content| with its sha1 hash |hash|
virtual bool Send(const std::string& content, const std::string& hash) = 0;
};
diff --git a/metrics/uploader/upload_service.h b/metrics/uploader/upload_service.h
index ed5ab85..80b1bf3 100644
--- a/metrics/uploader/upload_service.h
+++ b/metrics/uploader/upload_service.h
@@ -128,7 +128,7 @@
SystemProfileSetter* system_profile_setter_;
base::HistogramSnapshotManager histogram_snapshot_manager_;
- Sender* sender_;
+ scoped_ptr<Sender> sender_;
int failed_upload_count_;
scoped_ptr<MetricsLog> current_log_;
scoped_ptr<MetricsLog> staged_log_;
diff --git a/metrics/uploader/upload_service_test.cc b/metrics/uploader/upload_service_test.cc
index 9e5bd8d..7a82624 100644
--- a/metrics/uploader/upload_service_test.cc
+++ b/metrics/uploader/upload_service_test.cc
@@ -23,7 +23,8 @@
protected:
UploadServiceTest()
: upload_service_(), exit_manager_(new base::AtExitManager()) {
- upload_service_.sender_ = &sender_;
+ sender_ = new SenderMock;
+ upload_service_.sender_.reset(sender_);
upload_service_.system_profile_setter_ = new MockSystemProfileSetter();
upload_service_.Init();
}
@@ -32,7 +33,7 @@
CHECK(dir_.CreateUniqueTempDir());
upload_service_.GatherHistograms();
upload_service_.Reset();
- sender_.Reset();
+ sender_->Reset();
cache_.is_testing_ = true;
chromeos_metrics::PersistentInteger::SetTestingMode(true);
@@ -45,7 +46,7 @@
}
base::ScopedTempDir dir_;
- SenderMock sender_;
+ SenderMock *sender_;
SystemProfileCache cache_;
UploadService upload_service_;
@@ -90,20 +91,20 @@
}
TEST_F(UploadServiceTest, FailedSendAreRetried) {
- sender_.set_should_succeed(false);
+ sender_->set_should_succeed(false);
upload_service_.AddSample(*Crash("user"));
upload_service_.UploadEvent();
- EXPECT_EQ(1, sender_.send_call_count());
- std::string sent_string = sender_.last_message();
+ EXPECT_EQ(1, sender_->send_call_count());
+ std::string sent_string = sender_->last_message();
upload_service_.UploadEvent();
- EXPECT_EQ(2, sender_.send_call_count());
- EXPECT_EQ(sent_string, sender_.last_message());
+ EXPECT_EQ(2, sender_->send_call_count());
+ EXPECT_EQ(sent_string, sender_->last_message());
}
TEST_F(UploadServiceTest, DiscardLogsAfterTooManyFailedUpload) {
- sender_.set_should_succeed(false);
+ sender_->set_should_succeed(false);
upload_service_.AddSample(*Crash("user"));
for (int i = 0; i < UploadService::kMaxFailedUpload; i++) {
@@ -118,7 +119,7 @@
TEST_F(UploadServiceTest, EmptyLogsAreNotSent) {
upload_service_.UploadEvent();
EXPECT_FALSE(upload_service_.current_log_);
- EXPECT_EQ(0, sender_.send_call_count());
+ EXPECT_EQ(0, sender_->send_call_count());
}
TEST_F(UploadServiceTest, LogEmptyByDefault) {
@@ -133,12 +134,12 @@
upload_service_.AddSample(*Crash("user"));
upload_service_.UploadEvent();
- std::string first_message = sender_.last_message();
+ std::string first_message = sender_->last_message();
upload_service_.AddSample(*Crash("kernel"));
upload_service_.UploadEvent();
- EXPECT_NE(first_message, sender_.last_message());
+ EXPECT_NE(first_message, sender_->last_message());
}
TEST_F(UploadServiceTest, LogEmptyAfterUpload) {
@@ -197,16 +198,17 @@
upload_service_.AddSample(*histogram.get());
upload_service_.UploadEvent();
- EXPECT_EQ(1, sender_.send_call_count());
- EXPECT_TRUE(sender_.is_good_proto());
- EXPECT_EQ(1, sender_.last_message_proto().histogram_event().size());
+ EXPECT_EQ(1, sender_->send_call_count());
+ EXPECT_TRUE(sender_->is_good_proto());
+ EXPECT_EQ(1, sender_->last_message_proto().histogram_event().size());
- EXPECT_EQ(name, sender_.last_message_proto().system_profile().os().name());
+ EXPECT_EQ(name, sender_->last_message_proto().system_profile().os().name());
EXPECT_EQ(metrics::SystemProfileProto::CHANNEL_BETA,
- sender_.last_message_proto().system_profile().channel());
- EXPECT_NE(0, sender_.last_message_proto().client_id());
- EXPECT_NE(0, sender_.last_message_proto().system_profile().build_timestamp());
- EXPECT_NE(0, sender_.last_message_proto().session_id());
+ sender_->last_message_proto().system_profile().channel());
+ EXPECT_NE(0, sender_->last_message_proto().client_id());
+ EXPECT_NE(0,
+ sender_->last_message_proto().system_profile().build_timestamp());
+ EXPECT_NE(0, sender_->last_message_proto().session_id());
}
TEST_F(UploadServiceTest, PersistentGUID) {