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>
This commit is contained in:
parent
0b8cc1c952
commit
b1640eee93
4 changed files with 25 additions and 22 deletions
|
@ -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;
|
||||
|
|
|
@ -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;
|
||||
};
|
||||
|
|
|
@ -128,7 +128,7 @@ class UploadService : public base::HistogramFlattener {
|
|||
|
||||
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_;
|
||||
|
|
|
@ -23,7 +23,8 @@ class UploadServiceTest : public testing::Test {
|
|||
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 @@ class UploadServiceTest : public testing::Test {
|
|||
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 @@ class UploadServiceTest : public testing::Test {
|
|||
}
|
||||
|
||||
base::ScopedTempDir dir_;
|
||||
SenderMock sender_;
|
||||
SenderMock *sender_;
|
||||
SystemProfileCache cache_;
|
||||
UploadService upload_service_;
|
||||
|
||||
|
@ -90,20 +91,20 @@ TEST_F(UploadServiceTest, UnknownCrashIgnored) {
|
|||
}
|
||||
|
||||
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, DiscardLogsAfterTooManyFailedUpload) {
|
|||
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 @@ TEST_F(UploadServiceTest, CanSendMultipleTimes) {
|
|||
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 @@ TEST_F(UploadServiceTest, ValuesInConfigFileAreSent) {
|
|||
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) {
|
||||
|
|
Loading…
Reference in a new issue