Move net-export thread-hopping code into NetLogFileWriter and add IO polled data.

components/net_log/net_log_file_writer_unittest.cc

 // Copyright (c) 2013 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/net_log/net_log_file_writer.h"
 
 #include <stdint.h>
 
 #include <memory>
 
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_file.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/json/json_reader.h"
 #include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "base/test/test_simple_task_runner.h"
+#include "base/threading/thread.h"
 #include "base/values.h"
 #include "build/build_config.h"
 #include "components/net_log/chrome_net_log.h"
+#include "net/http/http_network_session.h"
 #include "net/log/net_log_capture_mode.h"
 #include "net/log/net_log_event_type.h"
 #include "net/log/write_to_file_net_log_observer.h"
+#include "net/url_request/url_request_context_getter.h"
+#include "net/url_request/url_request_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
 
 const char kChannelString[] = "SomeChannel";
 
+// Keep this in sync with kLogRelativePath defined in net_log_file_writer.cc.
+base::FilePath::CharType kLogRelativePath[] =
+    FILE_PATH_LITERAL("net-export/chrome-net-export-log.json");
+
+const char kCaptureModeDefaultString[] = "STRIP_PRIVATE_DATA";
+const char kCaptureModeIncludeCookiesAndCredentialsString[] = "NORMAL";
+const char kCaptureModeIncludeSocketBytesString[] = "LOG_BYTES";
+
+const char kStateUninitializedString[] = "UNINITIALIZED";
+const char kStateInitializingString[] = "INITIALIZING";
+const char kStateNotLoggingString[] = "NOT_LOGGING";
+const char kStateLoggingString[] = "LOGGING";
+const char kStateStoppingLogString[] = "STOPPING_LOG";
+
 }  // namespace
 
 namespace net_log {
 
-class TestNetLogFileWriter : public NetLogFileWriter {
+void VerifyFileExistsAndNotEmpty(const base::FilePath& path) {
+  DCHECK(!path.empty());
+
+  EXPECT_TRUE(base::PathExists(path));
+
+  int64_t file_size;
+  // base::GetFileSize returns proper file size on open handles.
+  ASSERT_TRUE(base::GetFileSize(path, &file_size));
+  EXPECT_GT(file_size, 0);
+}
+
+void VerifyFileIsValidJSON(const base::FilePath& path) {
+  DCHECK(!path.empty());
+
+  VerifyFileExistsAndNotEmpty(path);
+
+  std::string file_string;
+  ASSERT_TRUE(ReadFileToString(path, &file_string));
+  std::unique_ptr<base::Value> json = base::JSONReader::Read(file_string);
+  EXPECT_TRUE(json);
+}
+
+// Helper function to run |task_runner_a|, |task_runner_b|, and the main message
+// loop until all are idle. Assume that either task runner can only post replies
+// to the main thread (i.e. they cannot post tasks directly to each other).
+void RunTaskRunnersUntilIdle(

[eroman, 2017/01/24 20:01:36]

I think a better approach is to use separate threads for the network task runner
and file task runner.

In other words instead of:
  scoped_refptr<base::TestSimpleTaskRunner> file_task_runner_;
  scoped_refptr<base::TestSimpleTaskRunner> net_task_runner_;

You can do:
  base::Thread file_thread_;
  base::Thread net_thread_;
  
The problem with using two TestSimpleTaskRunner is it linearizes execution to
being single-threaded.
This creates an awkward coupling that is exposed by this function (needing to
drain tasks on both until things are quiescent). It also will mask any races
that the code has.

When using separate threads there are the benefits that:

 * The test includes coverage of possible race conditions (especially when being
run under TSAN)
 * Can leverage TestCompletionCallback
 * More closely matches production runtime environment

I don't think the helper threads need to be blocked for these tests, but if you
do find the need you can use WaitableEvent to block their message loops.

[wangyix1, 2017/01/25 22:48:27]

Done.

Many tests have been modified to become whitebox tests. Specifically,
StartThenVerifyFileAndState() has been modified to no longer check the file
after StartNetLog(), since that requires knowledge about when NetLogFileWriter
actually writes the constants. This change eliminates the need to explicitly
wait until the file thread is idle.

+    scoped_refptr<base::TestSimpleTaskRunner> task_runner_a,
+    scoped_refptr<base::TestSimpleTaskRunner> task_runner_b) {
+  // First, run the main message loop to handle any async tasks posted to the
+  // main thread.
+  base::RunLoop().RunUntilIdle();
+
+  // Repeatedly try to run both task runners until they're simultaneously idle.
+  // After running the pending tasks on a task runner, run the main message loop
+  // until idle since those tasks could've posted replies to the main thread.
+  // Now, the replies on the main thread could possibly post tasks to either
+  // task runner. Therefore, only stop if the two task runners are
+  // simultaneously idle.
+  bool keepRunning;
+  do {
+    keepRunning = false;
+
+    if (task_runner_a->HasPendingTask()) {
+      task_runner_a->RunUntilIdle();
+      base::RunLoop().RunUntilIdle();
+      keepRunning = true;
+      continue;
+    }
+
+    if (task_runner_b->HasPendingTask()) {
+      task_runner_b->RunUntilIdle();
+      base::RunLoop().RunUntilIdle();
+      keepRunning = true;
+    }
+  } while (keepRunning);
+}
+
+// A class similar to TestCompletionCallbackTemplate, but specialized for
+// StateCallbacks. One difference is that WaitUntilIdle() is used here instead
+// of WaitForResult(). WaitUntilIdle() will run the main thread and two
+// TestSimpleTaskRunners until they're all idle before storing the callback
+// result.
+class TestStateCallback {
  public:
-  explicit TestNetLogFileWriter(ChromeNetLog* chrome_net_log)
-      : NetLogFileWriter(
-            chrome_net_log,
-            base::CommandLine::ForCurrentProcess()->GetCommandLineString(),
-            kChannelString),
-        lie_about_net_export_log_directory_(false) {
-    EXPECT_TRUE(net_log_temp_dir_.CreateUniqueTempDir());
-  }
-
-  ~TestNetLogFileWriter() override { EXPECT_TRUE(net_log_temp_dir_.Delete()); }
-
-  // NetLogFileWriter implementation:
-  bool GetNetExportLogBaseDirectory(base::FilePath* path) const override {
-    if (lie_about_net_export_log_directory_)
-      return false;
-    *path = net_log_temp_dir_.GetPath();
-    return true;
-  }
-
-  void set_lie_about_net_export_log_directory(
-      bool lie_about_net_export_log_directory) {
-    lie_about_net_export_log_directory_ = lie_about_net_export_log_directory;
+  TestStateCallback()
+      : have_result_(false),
+        callback_(base::Bind(&TestStateCallback::SetResult,
+                             base::Unretained(this))) {}
+
+  const base::Callback<void(std::unique_ptr<base::DictionaryValue>)>&
+  Callback() {
+    return callback_;
+  }
+
+  std::unique_ptr<base::DictionaryValue> WaitUntilIdle(
+      scoped_refptr<base::TestSimpleTaskRunner> task_runner_a,
+      scoped_refptr<base::TestSimpleTaskRunner> task_runner_b) {
+    RunTaskRunnersUntilIdle(task_runner_a, task_runner_b);
+    DCHECK(have_result_);
+    have_result_ = false;
+    return std::move(result_);
   }
 
  private:
-  bool lie_about_net_export_log_directory_;
-
-  base::ScopedTempDir net_log_temp_dir_;
+  void SetResult(std::unique_ptr<base::DictionaryValue> result) {
+    result_ = std::move(result);
+    have_result_ = true;
+  }
+
+  bool have_result_;
+  std::unique_ptr<base::DictionaryValue> result_;
+  base::Callback<void(std::unique_ptr<base::DictionaryValue>)> callback_;
+};
+
+// A class similar to TestCompletionCallbackTemplate, but specialized for
+// FilePathCallbacks. One difference is that WaitUntilIdle() is used here
+// instead of WaitForResult(). WaitUntilIdle() will run the main thread and two
+// TestSimpleTaskRunners until they're all idle before storing the callback
+// result.
+class TestFilePathCallback {
+ public:
+  TestFilePathCallback()
+      : have_result_(false),
+        callback_(base::Bind(&TestFilePathCallback::SetResult,
+                             base::Unretained(this))) {}
+
+  const base::Callback<void(const base::FilePath&)>& Callback() {
+    return callback_;
+  }
+
+  const base::FilePath& WaitUntilIdle(
+      scoped_refptr<base::TestSimpleTaskRunner> task_runner_a,
+      scoped_refptr<base::TestSimpleTaskRunner> task_runner_b) {
+    RunTaskRunnersUntilIdle(task_runner_a, task_runner_b);
+    DCHECK(have_result_);
+    have_result_ = false;
+    return result_;
+  }
+
+ private:
+  void SetResult(const base::FilePath& result) {
+    result_ = result;
+    have_result_ = true;
+  }
+
+  bool have_result_;
+  base::FilePath result_;
+  base::Callback<void(const base::FilePath&)> callback_;
 };
 
 class NetLogFileWriterTest : public ::testing::Test {
  public:
   NetLogFileWriterTest()
-      : net_log_(new ChromeNetLog(
-            base::FilePath(),
-            net::NetLogCaptureMode::Default(),
+      : net_log_(base::FilePath(),
+                 net::NetLogCaptureMode::Default(),
+                 base::CommandLine::ForCurrentProcess()->GetCommandLineString(),
+                 kChannelString),
+        net_log_file_writer_(
+            &net_log_,
             base::CommandLine::ForCurrentProcess()->GetCommandLineString(),
-            kChannelString)),
-        net_log_file_writer_(new TestNetLogFileWriter(net_log_.get())) {}
-
-  std::string GetStateString() const {
-    std::unique_ptr<base::DictionaryValue> dict(
-        net_log_file_writer_->GetState());
-    std::string state;
-    EXPECT_TRUE(dict->GetString("state", &state));
-    return state;
-  }
-
-  std::string GetLogTypeString() const {
-    std::unique_ptr<base::DictionaryValue> dict(
-        net_log_file_writer_->GetState());
-    std::string log_type;
-    EXPECT_TRUE(dict->GetString("logType", &log_type));
-    return log_type;
-  }
-
-  // Make sure the export file has been created and is non-empty, as net
-  // constants will always be written to it on creation.
-  void VerifyNetExportLogExists() {
-    net_export_log_ = net_log_file_writer_->log_path_;
-    ASSERT_TRUE(base::PathExists(net_export_log_));
-
-    int64_t file_size;
-    // base::GetFileSize returns proper file size on open handles.
-    ASSERT_TRUE(base::GetFileSize(net_export_log_, &file_size));
-    EXPECT_GT(file_size, 0);
-  }
-
-  // Make sure the export file has been created and a valid JSON file.  This
-  // should always be the case once logging has been stopped.
-  void VerifyNetExportLogComplete() {
-    VerifyNetExportLogExists();
-
-    std::string log;
-    ASSERT_TRUE(ReadFileToString(net_export_log_, &log));
-    base::JSONReader reader;
-    std::unique_ptr<base::Value> json = base::JSONReader::Read(log);
-    EXPECT_TRUE(json);
-  }
-
-  // Verify state and GetFilePath return correct values if EnsureInit() fails.
-  void VerifyFilePathAndStateAfterEnsureInitFailure() {
-    EXPECT_EQ("UNINITIALIZED", GetStateString());
-    EXPECT_EQ(NetLogFileWriter::STATE_UNINITIALIZED,
-              net_log_file_writer_->state());
-
-    base::FilePath net_export_file_path;
-    EXPECT_FALSE(net_log_file_writer_->GetFilePath(&net_export_file_path));
-  }
-
-  // When we lie in NetExportLogExists, make sure state and GetFilePath return
-  // correct values.
-  void VerifyFilePathAndStateAfterEnsureInit() {
-    EXPECT_EQ("NOT_LOGGING", GetStateString());
-    EXPECT_EQ(NetLogFileWriter::STATE_NOT_LOGGING,
-              net_log_file_writer_->state());
-    EXPECT_EQ("NONE", GetLogTypeString());
-    EXPECT_EQ(NetLogFileWriter::LOG_TYPE_NONE,
-              net_log_file_writer_->log_type());
-
-    base::FilePath net_export_file_path;
-    EXPECT_FALSE(net_log_file_writer_->GetFilePath(&net_export_file_path));
-    EXPECT_FALSE(net_log_file_writer_->NetExportLogExists());
-  }
-
-  // The following methods make sure the export file has been successfully
-  // initialized by a DO_START command of the given type.
-
-  void VerifyFileAndStateAfterDoStart() {
-    VerifyFileAndStateAfterStart(
-        NetLogFileWriter::LOG_TYPE_NORMAL, "NORMAL",
-        net::NetLogCaptureMode::IncludeCookiesAndCredentials());
-  }
-
-  void VerifyFileAndStateAfterDoStartStripPrivateData() {
-    VerifyFileAndStateAfterStart(NetLogFileWriter::LOG_TYPE_STRIP_PRIVATE_DATA,
-                                 "STRIP_PRIVATE_DATA",
-                                 net::NetLogCaptureMode::Default());
-  }
-
-  void VerifyFileAndStateAfterDoStartLogBytes() {
-    VerifyFileAndStateAfterStart(NetLogFileWriter::LOG_TYPE_LOG_BYTES,
-                                 "LOG_BYTES",
-                                 net::NetLogCaptureMode::IncludeSocketBytes());
-  }
-
-  // Make sure the export file has been successfully initialized after DO_STOP
-  // command following a DO_START command of the given type.
-
-  void VerifyFileAndStateAfterDoStop() {
-    VerifyFileAndStateAfterDoStop(NetLogFileWriter::LOG_TYPE_NORMAL, "NORMAL");
-  }
-
-  void VerifyFileAndStateAfterDoStopWithStripPrivateData() {
-    VerifyFileAndStateAfterDoStop(NetLogFileWriter::LOG_TYPE_STRIP_PRIVATE_DATA,
-                                  "STRIP_PRIVATE_DATA");
-  }
-
-  void VerifyFileAndStateAfterDoStopWithLogBytes() {
-    VerifyFileAndStateAfterDoStop(NetLogFileWriter::LOG_TYPE_LOG_BYTES,
-                                  "LOG_BYTES");
-  }
-
-  std::unique_ptr<ChromeNetLog> net_log_;
+            kChannelString),
+        file_task_runner_(new base::TestSimpleTaskRunner()),
+        net_task_runner_(new base::TestSimpleTaskRunner()) {}
+
+  // ::testing::Test implementation
+  void SetUp() override {
+    ASSERT_TRUE(log_temp_dir_.CreateUniqueTempDir());
+
+    // Override |net_log_file_writer_|'s default-log-base-directory-getter to
+    // a getter that returns the temp dir created for the test.
+    net_log_file_writer_.SetDefaultLogBaseDirectoryGetterForTest(base::Bind(
+        &NetLogFileWriterTest::GetLogTempDirPath, log_temp_dir_.GetPath()));
+
+    default_log_path_ = log_temp_dir_.GetPath().Append(kLogRelativePath);
+
+    net_log_file_writer_.SetTaskRunners(file_task_runner_, net_task_runner_);
+  }
+
+  void TearDown() override {
+    DCHECK(!(file_task_runner_->HasPendingTask()));
+    DCHECK(!(net_task_runner_->HasPendingTask()));
+    ASSERT_TRUE(log_temp_dir_.Delete());
+  }
+
+  // A getter used to override NetLogFileWriter's usual getter that's used to
+  // retrieve the default base directory for the log file.
+  static bool GetLogTempDirPath(const base::FilePath& path_to_return,
+                                base::FilePath* path) {
+    *path = path_to_return;
+    return true;
+  }
+
+  std::unique_ptr<base::DictionaryValue> FileWriterGetState() {
+    TestStateCallback test_callback;
+    net_log_file_writer_.GetState(test_callback.Callback());
+    return test_callback.WaitUntilIdle(file_task_runner_, net_task_runner_);
+  }
+
+  std::unique_ptr<base::DictionaryValue>
+  FileWriterGetStateWithoutRunningTaskRunners() {
+    // By passing two new temporary TestSimpleTaskRunners to WaitUntilIdle()
+    // instead of |file_task_runner_| and |net_task_runner_|, it allows
+    // GetState() to complete without allowing any tasks on |file_task_runner_|
+    // or |net_task_runner_| to run.
+    // This only works if |net_log_file_writer_| is already initialized.
+    // Otherwise, GetState() would trigger initialization, which requires
+    // running tasks on |file_task_runner_|.
+    TestStateCallback test_callback;
+    net_log_file_writer_.GetState(test_callback.Callback());
+    return test_callback.WaitUntilIdle(new base::TestSimpleTaskRunner(),
+                                       new base::TestSimpleTaskRunner());
+  }
+
+  base::FilePath FileWriterGetFilePathToCompletedLog() {
+    TestFilePathCallback test_callback;
+    net_log_file_writer_.GetFilePathToCompletedLog(test_callback.Callback());
+    return test_callback.WaitUntilIdle(file_task_runner_, net_task_runner_);
+  }
+
+  // Checks the fields of the state returned by NetLogFileWriter's callbacks.
+  // |expected_log_capture_mode_string| is only checked if
+  // |expected_log_capture_mode_known| is true.
+  void VerifyState(std::unique_ptr<base::DictionaryValue> state,
+                   const std::string& expected_state_string,
+                   bool expected_log_exists,
+                   bool expected_log_capture_mode_known,
+                   const std::string& expected_log_capture_mode_string) {
+    std::string state_string;
+    ASSERT_TRUE(state->GetString("state", &state_string));
+    EXPECT_EQ(state_string, expected_state_string);
+
+    bool log_exists;
+    ASSERT_TRUE(state->GetBoolean("logExists", &log_exists));
+    EXPECT_EQ(log_exists, expected_log_exists);
+
+    bool log_capture_mode_known;
+    ASSERT_TRUE(
+        state->GetBoolean("logCaptureModeKnown", &log_capture_mode_known));
+    EXPECT_EQ(log_capture_mode_known, expected_log_capture_mode_known);
+
+    if (expected_log_capture_mode_known) {
+      std::string log_capture_mode_string;
+      ASSERT_TRUE(state->GetString("captureMode", &log_capture_mode_string));
+      EXPECT_EQ(log_capture_mode_string, expected_log_capture_mode_string);
+    }
+  }
+
+  // If |custom_log_path| is empty path, |net_log_file_writer_| will use its
+  // default log path.
+  void StartThenVerifyFileAndState(
+      const base::FilePath& custom_log_path,
+      net::NetLogCaptureMode capture_mode,
+      const std::string& expected_capture_mode_string) {
+    TestStateCallback test_callback;
+    net_log_file_writer_.StartNetLog(custom_log_path, capture_mode,
+                                     test_callback.Callback());
+    VerifyState(
+        test_callback.WaitUntilIdle(file_task_runner_, net_task_runner_),
+        kStateLoggingString, true, true, expected_capture_mode_string);
+
+    // Make sure NetLogFileWriter::GetFilePath() returns empty path when
+    // logging.
+    EXPECT_EQ(FileWriterGetFilePathToCompletedLog(), base::FilePath());
+
+    // The log file should exist and be non-empty since StartnetLog() always

[eroman, 2017/01/24 20:01:36]

StartNetLog

[wangyix1, 2017/01/25 22:48:27]

Comment no longer exists.

+    // writes constants to the log file.
+    DCHECK(!file_task_runner_->HasPendingTask());
+    VerifyFileExistsAndNotEmpty(custom_log_path.empty() ? default_log_path_
+                                                        : custom_log_path);
+  }
+
+  // If |custom_log_path| is empty path, it's assumed the log file with be at
+  // |default_path_|.
+  void StopThenVerifyFileAndState(
+      const base::FilePath& custom_log_path,
+      std::unique_ptr<base::DictionaryValue> polled_data,
+      scoped_refptr<net::URLRequestContextGetter> context_getter,
+      const std::string& expected_capture_mode_string) {
+    TestStateCallback test_callback;
+    net_log_file_writer_.StopNetLog(std::move(polled_data), context_getter,
+                                    test_callback.Callback());
+    VerifyState(
+        test_callback.WaitUntilIdle(file_task_runner_, net_task_runner_),
+        kStateNotLoggingString, true, true, expected_capture_mode_string);
+
+    const base::FilePath& log_path =
+        custom_log_path.empty() ? default_log_path_ : custom_log_path;
+    EXPECT_EQ(FileWriterGetFilePathToCompletedLog(), log_path);
+
+    DCHECK(!file_task_runner_->HasPendingTask());
+    VerifyFileIsValidJSON(log_path);
+  }
+
+ protected:
+  ChromeNetLog net_log_;
+
   // |net_log_file_writer_| is initialized after |net_log_| so that it can stop
   // obvserving on destruction.
-  std::unique_ptr<TestNetLogFileWriter> net_log_file_writer_;
-  base::FilePath net_export_log_;
+  NetLogFileWriter net_log_file_writer_;
+
+  base::ScopedTempDir log_temp_dir_;
+
+  // The default log path that |net_log_file_writer_| will use is cached here.
+  base::FilePath default_log_path_;
+
+  scoped_refptr<base::TestSimpleTaskRunner> file_task_runner_;
+  scoped_refptr<base::TestSimpleTaskRunner> net_task_runner_;
 
  private:
-  // Checks state after one of the DO_START* commands.
-  void VerifyFileAndStateAfterStart(
-      NetLogFileWriter::LogType expected_log_type,
-      const std::string& expected_log_type_string,
-      net::NetLogCaptureMode expected_capture_mode) {
-    EXPECT_EQ(NetLogFileWriter::STATE_LOGGING, net_log_file_writer_->state());
-    EXPECT_EQ("LOGGING", GetStateString());
-    EXPECT_EQ(expected_log_type, net_log_file_writer_->log_type());
-    EXPECT_EQ(expected_log_type_string, GetLogTypeString());
-    EXPECT_EQ(expected_capture_mode,
-              net_log_file_writer_->write_to_file_observer_->capture_mode());
-
-    // Check GetFilePath returns false when still writing to the file.
-    base::FilePath net_export_file_path;
-    EXPECT_FALSE(net_log_file_writer_->GetFilePath(&net_export_file_path));
-
-    VerifyNetExportLogExists();
-  }
-
-  void VerifyFileAndStateAfterDoStop(
-      NetLogFileWriter::LogType expected_log_type,
-      const std::string& expected_log_type_string) {
-    EXPECT_EQ(NetLogFileWriter::STATE_NOT_LOGGING,
-              net_log_file_writer_->state());
-    EXPECT_EQ("NOT_LOGGING", GetStateString());
-    EXPECT_EQ(expected_log_type, net_log_file_writer_->log_type());
-    EXPECT_EQ(expected_log_type_string, GetLogTypeString());
-
-    base::FilePath net_export_file_path;
-    EXPECT_TRUE(net_log_file_writer_->GetFilePath(&net_export_file_path));
-    EXPECT_EQ(net_export_log_, net_export_file_path);
-
-    VerifyNetExportLogComplete();
-  }
-
+  // Allows tasks to be posted to the main thread.
   base::MessageLoop message_loop_;
 };
 
-TEST_F(NetLogFileWriterTest, EnsureInitFailure) {
-  net_log_file_writer_->set_lie_about_net_export_log_directory(true);
-
-  EXPECT_FALSE(net_log_file_writer_->EnsureInit());
-  VerifyFilePathAndStateAfterEnsureInitFailure();
-
-  net_log_file_writer_->ProcessCommand(NetLogFileWriter::DO_START);
-  VerifyFilePathAndStateAfterEnsureInitFailure();
-}
-
-TEST_F(NetLogFileWriterTest, EnsureInitAllowStart) {
-  EXPECT_TRUE(net_log_file_writer_->EnsureInit());
-  VerifyFilePathAndStateAfterEnsureInit();
-
-  // Calling EnsureInit() second time should be a no-op.
-  EXPECT_TRUE(net_log_file_writer_->EnsureInit());
-  VerifyFilePathAndStateAfterEnsureInit();
-
-  // GetFilePath() should failed when there's no file.
-  base::FilePath net_export_file_path;
-  EXPECT_FALSE(net_log_file_writer_->GetFilePath(&net_export_file_path));
-}
-
-TEST_F(NetLogFileWriterTest, EnsureInitAllowStartOrSend) {
-  net_log_file_writer_->SetUpDefaultNetExportLogPath();
-  net_export_log_ = net_log_file_writer_->log_path_;
-
-  // Create and close an empty log file, to simulate an old log file already
-  // existing.
-  base::ScopedFILE created_file(base::OpenFile(net_export_log_, "w"));
-  ASSERT_TRUE(created_file.get());
-  created_file.reset();
-
-  EXPECT_TRUE(net_log_file_writer_->EnsureInit());
-
-  EXPECT_EQ("NOT_LOGGING", GetStateString());
-  EXPECT_EQ(NetLogFileWriter::STATE_NOT_LOGGING, net_log_file_writer_->state());
-  EXPECT_EQ("UNKNOWN", GetLogTypeString());
-  EXPECT_EQ(NetLogFileWriter::LOG_TYPE_UNKNOWN,
-            net_log_file_writer_->log_type());
-  EXPECT_EQ(net_export_log_, net_log_file_writer_->log_path_);
-  EXPECT_TRUE(base::PathExists(net_export_log_));
-
-  base::FilePath net_export_file_path;
-  EXPECT_TRUE(net_log_file_writer_->GetFilePath(&net_export_file_path));
-  EXPECT_TRUE(base::PathExists(net_export_file_path));
-  EXPECT_EQ(net_export_log_, net_export_file_path);
-}
-
-TEST_F(NetLogFileWriterTest, ProcessCommandDoStartAndStop) {
-  net_log_file_writer_->ProcessCommand(NetLogFileWriter::DO_START);
-  VerifyFileAndStateAfterDoStart();
-
-  // Calling a second time should be a no-op.
-  net_log_file_writer_->ProcessCommand(NetLogFileWriter::DO_START);
-  VerifyFileAndStateAfterDoStart();
-
-  // starting with other log levels should also be no-ops.
-  net_log_file_writer_->ProcessCommand(
-      NetLogFileWriter::DO_START_STRIP_PRIVATE_DATA);
-  VerifyFileAndStateAfterDoStart();
-  net_log_file_writer_->ProcessCommand(NetLogFileWriter::DO_START_LOG_BYTES);
-  VerifyFileAndStateAfterDoStart();
-
-  net_log_file_writer_->ProcessCommand(NetLogFileWriter::DO_STOP);
-  VerifyFileAndStateAfterDoStop();
-
-  // Calling DO_STOP second time should be a no-op.
-  net_log_file_writer_->ProcessCommand(NetLogFileWriter::DO_STOP);
-  VerifyFileAndStateAfterDoStop();
-}
-
-TEST_F(NetLogFileWriterTest,
-       ProcessCommandDoStartAndStopWithPrivateDataStripping) {
-  net_log_file_writer_->ProcessCommand(
-      NetLogFileWriter::DO_START_STRIP_PRIVATE_DATA);
-  VerifyFileAndStateAfterDoStartStripPrivateData();
-
-  // Calling a second time should be a no-op.
-  net_log_file_writer_->ProcessCommand(
-      NetLogFileWriter::DO_START_STRIP_PRIVATE_DATA);
-  VerifyFileAndStateAfterDoStartStripPrivateData();
-
-  net_log_file_writer_->ProcessCommand(NetLogFileWriter::DO_STOP);
-  VerifyFileAndStateAfterDoStopWithStripPrivateData();
-
-  // Calling DO_STOP second time should be a no-op.
-  net_log_file_writer_->ProcessCommand(NetLogFileWriter::DO_STOP);
-  VerifyFileAndStateAfterDoStopWithStripPrivateData();
-}
-
-TEST_F(NetLogFileWriterTest, ProcessCommandDoStartAndStopWithByteLogging) {
-  net_log_file_writer_->ProcessCommand(NetLogFileWriter::DO_START_LOG_BYTES);
-  VerifyFileAndStateAfterDoStartLogBytes();
-
-  // Calling a second time should be a no-op.
-  net_log_file_writer_->ProcessCommand(NetLogFileWriter::DO_START_LOG_BYTES);
-  VerifyFileAndStateAfterDoStartLogBytes();
-
-  net_log_file_writer_->ProcessCommand(NetLogFileWriter::DO_STOP);
-  VerifyFileAndStateAfterDoStopWithLogBytes();
-
-  // Calling DO_STOP second time should be a no-op.
-  net_log_file_writer_->ProcessCommand(NetLogFileWriter::DO_STOP);
-  VerifyFileAndStateAfterDoStopWithLogBytes();
-}
-
-TEST_F(NetLogFileWriterTest, DoStartClearsFile) {
-  // Verify file sizes after two consecutive starts/stops are the same (even if
-  // we add some junk data in between).
-  net_log_file_writer_->ProcessCommand(NetLogFileWriter::DO_START);
-  VerifyFileAndStateAfterDoStart();
+TEST_F(NetLogFileWriterTest, InitFail) {
+  // Override net_log_file_writer_'s default log base directory getter to always
+  // fail.
+  net_log_file_writer_.SetDefaultLogBaseDirectoryGetterForTest(
+      base::Bind([](base::FilePath* path) -> bool { return false; }));
+
+  // GetState() will cause |net_log_file_writer_| to initialize. In this case,
+  // initialization will fail since |net_log_file_writer_| will fail to retrive

[eroman, 2017/01/24 20:01:36]

retrieve

[wangyix1, 2017/01/25 22:48:27]

Done.

+  // the default log base directory.
+  VerifyState(FileWriterGetState(), kStateUninitializedString, false, false,
+              "");
+
+  // NetLogFileWriter::GetFilePath() should return empty path if uninitialized.
+  EXPECT_TRUE(FileWriterGetFilePathToCompletedLog().empty());
+}
+
+TEST_F(NetLogFileWriterTest, InitWithoutExistingLog) {
+  // GetState() will cause |net_log_file_writer_| to initialize.
+  VerifyState(FileWriterGetState(), kStateNotLoggingString, false, false, "");
+
+  // NetLogFileWriter::GetFilePathToCompletedLog() should return empty path when
+  // no log file exists.
+  EXPECT_TRUE(FileWriterGetFilePathToCompletedLog().empty());
+}
+
+TEST_F(NetLogFileWriterTest, InitWithExistingLog) {
+  // Create and close an empty log file to simulate existence of a previous log
+  // file.
+  ASSERT_TRUE(
+      base::CreateDirectoryAndGetError(default_log_path_.DirName(), nullptr));
+  base::ScopedFILE empty_file(base::OpenFile(default_log_path_, "w"));
+  ASSERT_TRUE(empty_file.get());
+  empty_file.reset();
+
+  // GetState() will cause |net_log_file_writer_| to initialize.
+  VerifyState(FileWriterGetState(), kStateNotLoggingString, true, false, "");
+
+  EXPECT_EQ(FileWriterGetFilePathToCompletedLog(), default_log_path_);
+}
+
+TEST_F(NetLogFileWriterTest, StartAndStopWithAllCaptureModes) {
+  const net::NetLogCaptureMode captureModes[3] = {

[eroman, 2017/01/24 20:01:36]

either kCaptureModes, or capture_modes

[wangyix1, 2017/01/25 22:48:27]

Done.

+      net::NetLogCaptureMode::Default(),
+      net::NetLogCaptureMode::IncludeCookiesAndCredentials(),
+      net::NetLogCaptureMode::IncludeSocketBytes()};
+
+  const std::string captureModeStrings[3] = {

[eroman, 2017/01/24 20:01:36]

same here

[wangyix1, 2017/01/25 22:48:27]

Done.

+      kCaptureModeDefaultString, kCaptureModeIncludeCookiesAndCredentialsString,
+      kCaptureModeIncludeSocketBytesString};
+
+  // For each capture mode, start and stop |net_log_file_writer_| in that mode.
+  for (int i = 0; i < 3; ++i) {

[eroman, 2017/01/24 20:01:36]

[optional] Could use a parameterized test via TEST_P()

[wangyix1, 2017/01/25 22:48:27]

I'm not sure there's a clean way to do this. Each generated test would need to
know the NetLogCaptureMode being tested, its string representation, and the
other NetLogCaptureModes that aren't being tested. The only way I can think of
is to store all NetLogCaptureModes and their string representations in two
arrays somewhere, and then the test class would accept an int parameter to be
used as an index to select which NetLogCaptureMode to test. But it seems weird
for this test's parameter to be an index.

[eroman, 2017/01/27 02:23:23]

Had we gone this route, I would envision having the parameter be a struct that
binds the capture mode with the capture mode string:

struct CaptureModeParam {
  net::NetLogCaptureMode capture_mode;
  const char* capture_mode_string;
};

So inside of the TEST_P() you would use |GetParam().capture_mode| in place of
captureModes[i], and |GetParam().capture_mode_string| in place of
captureModeStrings[i], and do those tests outside of a loop.

For the two redundant calls to StartThenVerifyFileAndState(), rather than
referencing captureModes[(i+1)%3] and captureModes[(i+2)%3] you could use a
loop:

const net::NetLogCaptureMode kModes[] = {
  net::NetLogCaptureMode::Default(),
  net::NetLogCaptureMode::IncludeCookiesAndCredentials(),
  net::NetLogCaptureMode::IncludeSocketBytes()
};

// Starting with other capture modes should also be no-ops.
for (const auto& mode : kModes) {
  if (GetParam().capture_mode != mode) {
    StartThenVerifyFileAndState(base::FilePath(), mode,
GetParam().capture_mode_string);
  }
}

The only duplication would be kModes which renames modes of interest to call
StartThenVerifyFileAndState() with.

+    StartThenVerifyFileAndState(base::FilePath(), captureModes[i],
+                                captureModeStrings[i]);
+
+    // Starting a second time should be a no-op.
+    StartThenVerifyFileAndState(base::FilePath(), captureModes[i],
+                                captureModeStrings[i]);
+
+    // Starting with other capture modes should also be no-ops. This should also
+    // not affect the capture mode reported by |net_log_file_writer_|.
+    StartThenVerifyFileAndState(base::FilePath(), captureModes[(i + 1) % 3],
+                                captureModeStrings[i]);
+
+    StartThenVerifyFileAndState(base::FilePath(), captureModes[(i + 2) % 3],
+                                captureModeStrings[i]);
+
+    StopThenVerifyFileAndState(base::FilePath(), nullptr, nullptr,
+                               captureModeStrings[i]);
+
+    // Stopping a second time should be a no-op.
+    StopThenVerifyFileAndState(base::FilePath(), nullptr, nullptr,
+                               captureModeStrings[i]);
+  }
+}
+
+// Verify the file sizes after two consecutive starts/stops are the same (even
+// if some junk data is added in between).
+TEST_F(NetLogFileWriterTest, StartClearsFile) {
+  StartThenVerifyFileAndState(base::FilePath(),
+                              net::NetLogCaptureMode::Default(),
+                              kCaptureModeDefaultString);
 
   int64_t start_file_size;
-  EXPECT_TRUE(base::GetFileSize(net_export_log_, &start_file_size));
-
-  net_log_file_writer_->ProcessCommand(NetLogFileWriter::DO_STOP);
-  VerifyFileAndStateAfterDoStop();
+  EXPECT_TRUE(base::GetFileSize(default_log_path_, &start_file_size));
+
+  StopThenVerifyFileAndState(base::FilePath(), nullptr, nullptr,
+                             kCaptureModeDefaultString);
 
   int64_t stop_file_size;
-  EXPECT_TRUE(base::GetFileSize(net_export_log_, &stop_file_size));
+  EXPECT_TRUE(base::GetFileSize(default_log_path_, &stop_file_size));
   EXPECT_GE(stop_file_size, start_file_size);
 
   // Add some junk at the end of the file.
   std::string junk_data("Hello");
+  EXPECT_TRUE(base::AppendToFile(default_log_path_, junk_data.c_str(),
+                                 junk_data.size()));
+
+  int64_t junk_file_size;
+  EXPECT_TRUE(base::GetFileSize(default_log_path_, &junk_file_size));
+  EXPECT_GT(junk_file_size, stop_file_size);
+
+  // Start and stop again and make sure the file is back to the size it was
+  // before adding the junk data.
+  StartThenVerifyFileAndState(base::FilePath(),
+                              net::NetLogCaptureMode::Default(),
+                              kCaptureModeDefaultString);
+
+  int64_t new_start_file_size;
+  EXPECT_TRUE(base::GetFileSize(default_log_path_, &new_start_file_size));
+  EXPECT_EQ(new_start_file_size, start_file_size);
+
+  StopThenVerifyFileAndState(base::FilePath(), nullptr, nullptr,
+                             kCaptureModeDefaultString);
+
+  int64_t new_stop_file_size;
+  EXPECT_TRUE(base::GetFileSize(default_log_path_, &new_stop_file_size));
+
+  EXPECT_EQ(new_stop_file_size, stop_file_size);
+}
+
+// Adds an event to the log file, then checks that the file is larger than
+// the file created without that event.
+TEST_F(NetLogFileWriterTest, AddEvent) {
+  StartThenVerifyFileAndState(base::FilePath(),
+                              net::NetLogCaptureMode::Default(),
+                              kCaptureModeDefaultString);
+
+  StopThenVerifyFileAndState(base::FilePath(), nullptr, nullptr,
+                             kCaptureModeDefaultString);
+
+  // Get file size without the event.
+  int64_t stop_file_size;
+  EXPECT_TRUE(base::GetFileSize(default_log_path_, &stop_file_size));
+
+  StartThenVerifyFileAndState(base::FilePath(),
+                              net::NetLogCaptureMode::Default(),
+                              kCaptureModeDefaultString);
+
+  net_log_.AddGlobalEntry(net::NetLogEventType::CANCELLED);
+
+  StopThenVerifyFileAndState(base::FilePath(), nullptr, nullptr,
+                             kCaptureModeDefaultString);
+
+  int64_t new_stop_file_size;
+  EXPECT_TRUE(base::GetFileSize(default_log_path_, &new_stop_file_size));
+
+  EXPECT_GE(new_stop_file_size, stop_file_size);
+}
+
+// Using a custom path to make sure logging can still occur when
+// the path has changed.
+TEST_F(NetLogFileWriterTest, AddEventCustomPath) {
+  base::FilePath::CharType kCustomRelativePath[] =
+      FILE_PATH_LITERAL("custom/custom/chrome-net-export-log.json");
+  base::FilePath custom_log_path =
+      log_temp_dir_.GetPath().Append(kCustomRelativePath);
   EXPECT_TRUE(
-      base::AppendToFile(net_export_log_, junk_data.c_str(), junk_data.size()));
-
-  int64_t junk_file_size;
-  EXPECT_TRUE(base::GetFileSize(net_export_log_, &junk_file_size));
-  EXPECT_GT(junk_file_size, stop_file_size);
-
-  // Execute DO_START/DO_STOP commands and make sure the file is back to the
-  // size before addition of junk data.
-  net_log_file_writer_->ProcessCommand(NetLogFileWriter::DO_START);
-  VerifyFileAndStateAfterDoStart();
-
-  int64_t new_start_file_size;
-  EXPECT_TRUE(base::GetFileSize(net_export_log_, &new_start_file_size));
-  EXPECT_EQ(new_start_file_size, start_file_size);
-
-  net_log_file_writer_->ProcessCommand(NetLogFileWriter::DO_STOP);
-  VerifyFileAndStateAfterDoStop();
+      base::CreateDirectoryAndGetError(custom_log_path.DirName(), nullptr));
+
+  StartThenVerifyFileAndState(custom_log_path,
+                              net::NetLogCaptureMode::Default(),
+                              kCaptureModeDefaultString);
+
+  StopThenVerifyFileAndState(custom_log_path, nullptr, nullptr,
+                             kCaptureModeDefaultString);
+
+  // Get file size without the event.
+  int64_t stop_file_size;
+  EXPECT_TRUE(base::GetFileSize(custom_log_path, &stop_file_size));
+
+  StartThenVerifyFileAndState(custom_log_path,
+                              net::NetLogCaptureMode::Default(),
+                              kCaptureModeDefaultString);
+
+  net_log_.AddGlobalEntry(net::NetLogEventType::CANCELLED);
+
+  StopThenVerifyFileAndState(custom_log_path, nullptr, nullptr,
+                             kCaptureModeDefaultString);
 
   int64_t new_stop_file_size;
-  EXPECT_TRUE(base::GetFileSize(net_export_log_, &new_stop_file_size));
-  EXPECT_EQ(new_stop_file_size, stop_file_size);
-}
-
-TEST_F(NetLogFileWriterTest, CheckAddEvent) {
-  // Add an event to |net_log_| and then test to make sure that, after we stop
-  // logging, the file is larger than the file created without that event.
-  net_log_file_writer_->ProcessCommand(NetLogFileWriter::DO_START);
-  VerifyFileAndStateAfterDoStart();
-
-  // Get file size without the event.
-  net_log_file_writer_->ProcessCommand(NetLogFileWriter::DO_STOP);
-  VerifyFileAndStateAfterDoStop();
-
-  int64_t stop_file_size;
-  EXPECT_TRUE(base::GetFileSize(net_export_log_, &stop_file_size));
-
-  // Perform DO_START and add an Event and then DO_STOP and then compare
-  // file sizes.
-  net_log_file_writer_->ProcessCommand(NetLogFileWriter::DO_START);
-  VerifyFileAndStateAfterDoStart();
-
-  // Log an event.
-  net_log_->AddGlobalEntry(net::NetLogEventType::CANCELLED);
-
-  net_log_file_writer_->ProcessCommand(NetLogFileWriter::DO_STOP);
-  VerifyFileAndStateAfterDoStop();
-
-  int64_t new_stop_file_size;
-  EXPECT_TRUE(base::GetFileSize(net_export_log_, &new_stop_file_size));
+  EXPECT_TRUE(base::GetFileSize(custom_log_path, &new_stop_file_size));
   EXPECT_GE(new_stop_file_size, stop_file_size);
 }
 
-TEST_F(NetLogFileWriterTest, CheckAddEventWithCustomPath) {
-  // Using a custom path to make sure logging can still occur when
-  // the path has changed.
-  base::FilePath path;
-  net_log_file_writer_->GetNetExportLogBaseDirectory(&path);
-
-  base::FilePath::CharType kCustomPath[] =
-      FILE_PATH_LITERAL("custom/custom/chrome-net-export-log.json");
-  base::FilePath custom_path = path.Append(kCustomPath);
-
-  EXPECT_TRUE(base::CreateDirectoryAndGetError(custom_path.DirName(), nullptr));
-
-  net_log_file_writer_->SetUpNetExportLogPath(custom_path);
-  net_export_log_ = net_log_file_writer_->log_path_;
-  EXPECT_EQ(custom_path, net_export_log_);
-
-  // Add an event to |net_log_| and then test to make sure that, after we stop
-  // logging, the file is larger than the file created without that event.
-  net_log_file_writer_->ProcessCommand(NetLogFileWriter::DO_START);
-  VerifyFileAndStateAfterDoStart();
-
-  // Get file size without the event.
-  net_log_file_writer_->ProcessCommand(NetLogFileWriter::DO_STOP);
-  VerifyFileAndStateAfterDoStop();
-
-  int64_t stop_file_size;
-  EXPECT_TRUE(base::GetFileSize(net_export_log_, &stop_file_size));
-
-  // Perform DO_START and add an Event and then DO_STOP and then compare
-  // file sizes.
-  net_log_file_writer_->ProcessCommand(NetLogFileWriter::DO_START);
-  VerifyFileAndStateAfterDoStart();
-
-  // Log an event.
-  net_log_->AddGlobalEntry(net::NetLogEventType::CANCELLED);
-
-  net_log_file_writer_->ProcessCommand(NetLogFileWriter::DO_STOP);
-  VerifyFileAndStateAfterDoStop();
-
-  int64_t new_stop_file_size;
-  EXPECT_TRUE(base::GetFileSize(net_export_log_, &new_stop_file_size));
-  EXPECT_GE(new_stop_file_size, stop_file_size);
+TEST_F(NetLogFileWriterTest, StartAndStopWithPolledDataAndContextGetter) {
+  // Create dummy polled data
+  const char kDummyPolledDataPath[] = "dummy_path";
+  const char kDummyPolledDataString[] = "dummy_info";
+  std::unique_ptr<base::DictionaryValue> dummy_polled_data =
+      base::MakeUnique<base::DictionaryValue>();
+  dummy_polled_data->SetString(kDummyPolledDataPath, kDummyPolledDataString);
+
+  // Create test context getter
+  std::unique_ptr<net::TestURLRequestContext> context(
+      new net::TestURLRequestContext(true));
+  context->set_net_log(&net_log_);
+  const int kDummyParam = 75;
+  std::unique_ptr<net::HttpNetworkSession::Params> params(
+      new net::HttpNetworkSession::Params);
+  params->quic_idle_connection_timeout_seconds = kDummyParam;
+  context->set_http_network_session_params(std::move(params));
+  context->Init();
+  scoped_refptr<net::TestURLRequestContextGetter> context_getter(
+      new net::TestURLRequestContextGetter(net_task_runner_,
+                                           std::move(context)));
+
+  StartThenVerifyFileAndState(base::FilePath(),
+                              net::NetLogCaptureMode::Default(),
+                              kCaptureModeDefaultString);
+
+  StopThenVerifyFileAndState(base::FilePath(), std::move(dummy_polled_data),
+                             context_getter, kCaptureModeDefaultString);
+
+  // Read polledData from log file.
+  std::string log_file_string;
+  ASSERT_TRUE(ReadFileToString(default_log_path_, &log_file_string));
+  std::unique_ptr<base::DictionaryValue> dict =
+      base::DictionaryValue::From(base::JSONReader::Read(log_file_string));
+  ASSERT_TRUE(dict);
+  base::DictionaryValue* polled_data;
+  ASSERT_TRUE(dict->GetDictionary("polledData", &polled_data));
+
+  // Check that it contains the field from the polled data that was passed in.
+  std::string dummy_string;
+  ASSERT_TRUE(polled_data->GetString(kDummyPolledDataPath, &dummy_string));
+  EXPECT_EQ(dummy_string, kDummyPolledDataString);
+
+  // Check that it also contains the field from the URLRequestContext that was
+  // passed in.
+  base::DictionaryValue* quic_info;
+  ASSERT_TRUE(polled_data->GetDictionary("quicInfo", &quic_info));
+  base::Value* timeout_value = nullptr;
+  int timeout;
+  ASSERT_TRUE(
+      quic_info->Get("idle_connection_timeout_seconds", &timeout_value));
+  ASSERT_TRUE(timeout_value->GetAsInteger(&timeout));
+  EXPECT_EQ(timeout, kDummyParam);
+}
+
+TEST_F(NetLogFileWriterTest, ReceiveCommandsWhileInitializing) {
+  // Trigger initialization, but don't run any of the posted initialization
+  // tasks.
+  TestStateCallback init_callback;
+  net_log_file_writer_.GetState(init_callback.Callback());
+
+  // Use GetState() to retrive the state without running any of the tasks posted

[eroman, 2017/01/24 20:01:36]

retrieve throughtout

[wangyix1, 2017/01/25 22:48:27]

Comment no longer exists.

+  // to |file_task_runner_| or |net_task_runner_|.
+  VerifyState(FileWriterGetStateWithoutRunningTaskRunners(),
+              kStateInitializingString, false, false, "");
+
+  // Tell |net_log_file_writer_| to start logging while it's initializing. This
+  // should be a no-op.
+  TestStateCallback start_during_init_callback;
+  net_log_file_writer_.StartNetLog(base::FilePath(),
+                                   net::NetLogCaptureMode::Default(),
+                                   start_during_init_callback.Callback());
+
+  // Use GetState() to retrive the state without running any of the tasks posted
+  // to |file_task_runner_| or |net_task_runner_|.
+  VerifyState(FileWriterGetStateWithoutRunningTaskRunners(),
+              kStateInitializingString, false, false, "");
+
+  // Now, run the pending tasks on |file_task_runner_| and |net_task_runner_|.
+  // This allows |net_log_file_writer_| to fully initialize.
+
+  // The state returned by the ignored StartNetLog() call should be the state
+  // after the initialization triggered by the first GetState() finishes.
+  VerifyState(start_during_init_callback.WaitUntilIdle(file_task_runner_,
+                                                       net_task_runner_),
+              kStateNotLoggingString, false, false, "");
+
+  // Make sure the state returned by the first GetState() makes sense.
+  VerifyState(init_callback.WaitUntilIdle(file_task_runner_, net_task_runner_),
+              kStateNotLoggingString, false, false, "");
+
+  // Get the state with anoter GetState() call after everything's done to
+  // confirm |net_log_file_writer_| is not logging.
+  VerifyState(FileWriterGetState(), kStateNotLoggingString, false, false, "");
+}
+
+TEST_F(NetLogFileWriterTest, ReceiveCommandsWhileStopLogging) {
+  // Tell |net_log_file_writer_| to start logging and let it complete the
+  // command.
+  StartThenVerifyFileAndState(base::FilePath(),
+                              net::NetLogCaptureMode::IncludeSocketBytes(),
+                              kCaptureModeIncludeSocketBytesString);
+
+  // Tell |net_log_file_writer_| to stop logging, but don't run any of the
+  // tasks posted by StopNetLog().
+  TestStateCallback stop_callback;
+  net_log_file_writer_.StopNetLog(nullptr, nullptr, stop_callback.Callback());
+
+  // Use GetState() to retrive the state without running any of the tasks posted

[eroman, 2017/01/24 20:01:36]

retrieve

[wangyix1, 2017/01/25 22:48:27]

Comment no longer exists.

+  // to |file_task_runner_| or |net_task_runner_|.
+  VerifyState(FileWriterGetStateWithoutRunningTaskRunners(),
+              kStateStoppingLogString, true, true,
+              kCaptureModeIncludeSocketBytesString);
+
+  // Tell |net_log_file_writer_| to start logging while it's stopping logging.
+  // This should be a no-op.
+  TestStateCallback start_during_stop_callback;
+  net_log_file_writer_.StartNetLog(base::FilePath(),
+                                   net::NetLogCaptureMode::IncludeSocketBytes(),
+                                   start_during_stop_callback.Callback());
+
+  // Use GetState() to retrive the state without running any of the tasks posted

[eroman, 2017/01/24 20:01:36]

same typo

[wangyix1, 2017/01/25 22:48:27]

Comment no longer exists.

+  // to |file_task_runner_| or |net_task_runner_|.
+  VerifyState(FileWriterGetStateWithoutRunningTaskRunners(),
+              kStateStoppingLogString, true, true,
+              kCaptureModeIncludeSocketBytesString);
+
+  // Now, run the pending tasks on |file_task_runner_| and |net_task_runner_|.
+  // This allows |net_log_file_writer_| to fully stop logging.
+
+  // The state returned by the callback of the ignored StartNetLog() call should
+  // still makes sense.
+  std::string start_during_stop_state_string;
+  ASSERT_TRUE(start_during_stop_callback
+                  .WaitUntilIdle(file_task_runner_, net_task_runner_)
+                  ->GetString("state", &start_during_stop_state_string));
+  if (start_during_stop_state_string != kStateStoppingLogString &&
+      start_during_stop_state_string != kStateNotLoggingString) {
+    ADD_FAILURE() << "State returned by ignored StartNetLog() should indicate "
+                     "either stopping-log or not-logging";
+  }
+
+  // Make sure the state returned by the StopNetLog() makes sense.
+  VerifyState(stop_callback.WaitUntilIdle(file_task_runner_, net_task_runner_),
+              kStateNotLoggingString, true, true,
+              kCaptureModeIncludeSocketBytesString);
+
+  // Get the state with another GetState() call after everything's done to
+  // confirm |net_log_file_writer_| is not logging.
+  VerifyState(FileWriterGetState(), kStateNotLoggingString, true, true,
+              kCaptureModeIncludeSocketBytesString);
 }
 
 }  // namespace net_log