netlog: Don't enable recording if the "netlog" tracing category is disabled.

net/log/trace_net_log_observer_unittest.cc

 // Copyright 2014 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 "net/log/trace_net_log_observer.h"
 
 #include <memory>
 #include <string>
 #include <vector>
 
@@ skipping 329 matching lines @@
   net_log()->AddGlobalEntry(NetLogEventType::URL_REQUEST_START_JOB);
 
   EndTraceAndFlush();
 
   TestNetLogEntry::List entries;
   net_log()->GetEntries(&entries);
   EXPECT_EQ(3u, entries.size());
   EXPECT_EQ(0u, trace_events()->GetSize());
 }
 
+TEST_F(TraceNetLogObserverTest, DisabledCategoryDoesntEnableNetLog) {

[xunjieli, 2016/12/05 14:55:01]

Thanks for working on this. The change looks good. 
We should test whether NetLog::IsCapturing() is set accordingly. If
IsCapturing(), it means that we have at least one observer. However, since the
TraceNetLogObserverTest fixture always has a TestNetLog so it will always have
an observer.

Could you use TEST instead of TEST_P (to avoid instantiating a TestNetLog) and
do something along the following lines?

TEST(TraceNetLogEnableTest, DisabledCategory) {
  TraceNetLogObserver observer;
  NetLog net_log;  
  observer.WatchForTraceStart(&net_log);

  EXPECT_FALSE(net_log.IsCapturing());
  std::string disabled_netlog_category =
      std::string("-") + kNetLogTracingCategory;
  TraceLog::GetInstance()->SetEnabled(
      base::trace_event::TraceConfig(disabled_netlog_category, ""),
      TraceLog::RECORDING_MODE);
  net_log.AddGlobalEntry(NetLogEventType::REQUEST_ALIVE);

  EXPECT_FALSE(net_log.IsCapturing());

  observer.StopWatchForTraceStart();

  EXPECT_FALSE(net_log.IsCapturing());
}


And another test where netlog category is enabled and we have
net_log.IsCapturing() equals to true before StopWatchForTraceStart().

+  set_trace_net_log_observer(new TraceNetLogObserver());
+  trace_net_log_observer()->WatchForTraceStart(net_log());
+
+  std::string disabled_netlog_category =
+      std::string("-") + kNetLogTracingCategory;
+  TraceLog::GetInstance()->SetEnabled(
+      base::trace_event::TraceConfig(disabled_netlog_category, ""),
+      TraceLog::RECORDING_MODE);
+
+  net_log()->AddGlobalEntry(NetLogEventType::REQUEST_ALIVE);
+
+  trace_net_log_observer()->StopWatchForTraceStart();
+
+  EndTraceAndFlush();
+  TestNetLogEntry::List entries;
+  net_log()->GetEntries(&entries);
+  EXPECT_EQ(1u, entries.size());
+  EXPECT_EQ(0u, trace_events()->GetSize());
+}
+
 TEST_F(TraceNetLogObserverTest, EventsWithAndWithoutParameters) {
   trace_net_log_observer()->WatchForTraceStart(net_log());
   EnableTraceLog();
   NetLogParametersCallback net_log_callback;
   std::string param = "bar";
   net_log_callback = NetLog::StringCallback("foo", &param);
 
   net_log()->AddGlobalEntry(NetLogEventType::CANCELLED, net_log_callback);
   net_log()->AddGlobalEntry(NetLogEventType::REQUEST_ALIVE);
 
@@ skipping 34 matching lines @@
   EXPECT_TRUE(item1->GetString("args.params.foo", &item1_params));
   EXPECT_EQ("bar", item1_params);
 
   EXPECT_TRUE(item2->GetString("args.params", &item2_params));
   EXPECT_TRUE(item2_params.empty());
 }
 
 }  // namespace
 
 }  // namespace net