Bulk Reports: Fix an infinite loop in ChildTraceMessageFilter::OnSetU…

…MACallback.

Also added some unit testing for this file.

BUG=555690

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

Cr-Commit-Position: refs/heads/master@{#359736}
(cherry picked from commit 5ee4455)

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

Cr-Commit-Position: refs/branch-heads/2564@{crosswalk-project#9}
Cr-Branched-From: 1283eca-refs/heads/master@{#359700}

components/components_tests.gyp

@@ -97,6 +97,9 @@
     'certificate_transparency_unittest_sources': [
       'certificate_transparency/log_proof_fetcher_unittest.cc',
     ],
+    'child_trace_message_filter_unittest_sources': [
+      'tracing/child_trace_message_filter_unittest.cc',
+    ],
     'cloud_devices_unittest_sources': [
       'cloud_devices/common/cloud_devices_urls_unittest.cc',
       'cloud_devices/common/printer_description_unittest.cc',
@@ -859,6 +862,7 @@
         '<@(bubble_unittest_sources)',
         '<@(captive_portal_unittest_sources)',
         '<@(certificate_reporting_unittest_sources)',
+        '<@(child_trace_message_filter_unittest_sources)',
         '<@(cloud_devices_unittest_sources)',
         '<@(component_updater_unittest_sources)',
         '<@(compression_unittest_sources)',

components/tracing/child_trace_message_filter.cc

@@ -34,6 +34,10 @@ void ChildTraceMessageFilter::OnFilterAdded(IPC::Sender* sender) {
       this);
 }
 
+void ChildTraceMessageFilter::SetSenderForTesting(IPC::Sender* sender) {
+  sender_ = sender;
+}
+
 void ChildTraceMessageFilter::OnFilterRemoved() {
   ChildMemoryDumpManagerDelegateImpl::GetInstance()->SetChildTraceMessageFilter(
       nullptr);
@@ -306,10 +310,6 @@ void ChildTraceMessageFilter::OnSetUMACallback(
     base::HistogramBase::Sample max;
     base::HistogramBase::Count count;
     sample_iterator->Get(&min, &max, &count);
-    if (count == 0) {
-      sample_iterator->Next();
-      continue;
-    }
 
     if (min >= histogram_lower_value && max <= histogram_upper_value) {
       ipc_task_runner_->PostTask(
@@ -322,7 +322,10 @@ void ChildTraceMessageFilter::OnSetUMACallback(
           base::Bind(
               &ChildTraceMessageFilter::SendAbortBackgroundTracingMessage,
               this));
+      break;
     }
+
+    sample_iterator->Next();
   }
 }
 

components/tracing/child_trace_message_filter.h

@@ -42,6 +42,8 @@ class TRACING_EXPORT ChildTraceMessageFilter : public IPC::MessageFilter {
   ~ChildTraceMessageFilter() override;
 
  private:
+  friend class ChildTraceMessageFilterTest;
+
   // Message handlers.
   void OnBeginTracing(const std::string& trace_config_str,
                       base::TimeTicks browser_time,
@@ -84,6 +86,8 @@ class TRACING_EXPORT ChildTraceMessageFilter : public IPC::MessageFilter {
 
   void OnProcessMemoryDumpDone(uint64 dump_guid, bool success);
 
+  void SetSenderForTesting(IPC::Sender* sender);
+
   IPC::Sender* sender_;
   base::SingleThreadTaskRunner* ipc_task_runner_;
 

components/tracing/child_trace_message_filter_unittest.cc

@@ -0,0 +1,87 @@
+// Copyright 2015 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 "components/tracing/child_trace_message_filter.h"
+
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/message_loop/message_loop.h"
+#include "components/tracing/tracing_messages.h"
+#include "ipc/ipc_message.h"
+#include "ipc/ipc_sender.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace tracing {
+
+class FakeSender : public IPC::Sender {
+ public:
+  FakeSender() {}
+
+  ~FakeSender() override {}
+
+  bool Send(IPC::Message* msg) override {
+    last_message_.reset(msg);
+    return true;
+  }
+
+  scoped_ptr<IPC::Message> last_message_;
+};
+
+class ChildTraceMessageFilterTest : public testing::Test {
+ public:
+  ChildTraceMessageFilterTest() {
+    message_filter_ =
+        new tracing::ChildTraceMessageFilter(message_loop_.task_runner().get());
+    message_filter_->SetSenderForTesting(&fake_sender_);
+  }
+
+  void OnSetUMACallback(const std::string& histogram,
+                        int low,
+                        int high,
+                        bool repeat) {
+    fake_sender_.last_message_.reset();
+    message_filter_->OnSetUMACallback(histogram, low, high, repeat);
+  }
+
+  base::MessageLoop message_loop_;
+  FakeSender fake_sender_;
+  scoped_refptr<tracing::ChildTraceMessageFilter> message_filter_;
+};
+
+TEST_F(ChildTraceMessageFilterTest, TestHistogramDoesNotTrigger) {
+  LOCAL_HISTOGRAM_COUNTS("foo1", 10);
+
+  OnSetUMACallback("foo1", 20000, 25000, true);
+
+  message_loop_.RunUntilIdle();
+
+  EXPECT_FALSE(fake_sender_.last_message_);
+}
+
+TEST_F(ChildTraceMessageFilterTest, TestHistogramTriggers) {
+  LOCAL_HISTOGRAM_COUNTS("foo2", 2);
+
+  OnSetUMACallback("foo2", 1, 3, true);
+
+  message_loop_.RunUntilIdle();
+
+  EXPECT_TRUE(fake_sender_.last_message_);
+  EXPECT_EQ(fake_sender_.last_message_->type(),
+            TracingHostMsg_TriggerBackgroundTrace::ID);
+}
+
+TEST_F(ChildTraceMessageFilterTest, TestHistogramAborts) {
+  LOCAL_HISTOGRAM_COUNTS("foo3", 10);
+
+  OnSetUMACallback("foo3", 1, 3, false);
+
+  message_loop_.RunUntilIdle();
+
+  EXPECT_TRUE(fake_sender_.last_message_);
+  EXPECT_EQ(fake_sender_.last_message_->type(),
+            TracingHostMsg_AbortBackgroundTrace::ID);
+}
+
+}  // namespace tracing