First cut at UI for saving net_logs data into a temporary file on

chrome/browser/net/net_log_temp_file_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 "chrome/browser/net/net_log_temp_file.h"
+
+#include "base/basictypes.h"
+#include "base/file_path.h"
+#include "base/file_util.h"
+#include "base/message_loop.h"
+#include "base/values.h"
+#include "build/build_config.h"
+#include "chrome/browser/net/chrome_net_log.h"
+#include "content/public/test/test_browser_thread.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using content::BrowserThread;
+
+class TestNetLogTempFile : public NetLogTempFile {
+ public:
+  explicit TestNetLogTempFile(ChromeNetLog* chrome_net_log)
+      : NetLogTempFile(chrome_net_log),
+        lie_about_file_existence_(false) {
+  }
+
+  // NetLogTempFile implementation:
+  virtual bool NetExportLogExists() OVERRIDE {
+    if (lie_about_file_existence_)
+      return false;
+    return NetLogTempFile::NetExportLogExists();
+  }
+
+  void set_lie_about_file_existence(bool lie_about_file_existence) {
+    lie_about_file_existence_ = lie_about_file_existence;
+  }
+
+ private:
+  bool lie_about_file_existence_;
+};
+
+class NetLogTempFileTest : public ::testing::Test {
+ protected:
+  NetLogTempFileTest()
+      : file_user_blocking_thread_(BrowserThread::FILE_USER_BLOCKING,
+                                   &message_loop_) {
+    net_log_.reset(new ChromeNetLog);
+    net_log_temp_file_ = new TestNetLogTempFile(net_log_.get());

[mmenke, 2013/01/25 17:39:12]

Can put these in the initializer list.

[ramant (doing other things), 2013/01/25 20:05:24]

Done.

+  }
+
+  // ::testing::Test implementation:
+  virtual void SetUp() OVERRIDE {
+    // Get a temporary file name for unit tests.
+    FilePath net_log_dir;
+    ASSERT_TRUE(net_log_temp_file_->GetNetExportLogDirectory(&net_log_dir));
+    ASSERT_TRUE(file_util::CreateTemporaryFileInDir(net_log_dir,
+                                                    &net_export_log_));
+
+    net_log_temp_file_->log_filename_ = net_export_log_.BaseName().value();
+
+    // We can't compare entire paths because GetTemporaryFile on Windows may
+    // convert the path to include legacy 8.3 filenames. Get
+    // net_log_temp_file_->log_path_ to test the path during unit tests.
+    net_log_temp_file_->GetNetExportLog();
+    net_export_log_ = net_log_temp_file_->log_path_;
+  }
+
+  // ::testing::Test implementation:

[mmenke, 2013/01/25 17:39:12]

nit:  Not needed.  Generally just have one comment above the first function of
the implementation, and all functions together.

[ramant (doing other things), 2013/01/25 20:05:24]

Done.

+  virtual void TearDown() OVERRIDE {
+    // Delete the temporary file we have created.
+    ASSERT_TRUE(file_util::Delete(net_export_log_, false));
+  }
+
+  std::string GetStateString() const {
+    scoped_ptr<base::DictionaryValue> dict(net_log_temp_file_->GetState());
+    std::string state;
+    EXPECT_TRUE(dict->GetString("state", &state));
+    return state;
+  }
+
+  void VerifyNetExportLog() {

[mmenke, 2013/01/25 17:39:12]

Think the three "verify" functions are worth commenting.  The non-empty file
checks, in particular, are non-obvious.

For this one, for instance, I suggest "Make sure the export file has been
created and is non-empty, as net constants will always be written to it on
creation."

For the others, just mention successful export file initialization.

[ramant (doing other things), 2013/01/25 20:05:24]

Done.

+    // We can't compare entire paths because GetTemporaryFile on Windows may
+    // convert the path to include legacy 8.3 filenames.

[mmenke, 2013/01/25 17:39:12]

Looks like we are comparing entire paths here now.

[ramant (doing other things), 2013/01/25 20:05:24]

Deleted the comment.
Done.

+    EXPECT_EQ(net_export_log_, net_log_temp_file_->log_path_);
+    EXPECT_TRUE(file_util::PathExists(net_export_log_));
+    int64 file_size;
+    // file_util::GetFileSize returns proper file size on open handles.
+    EXPECT_TRUE(file_util::GetFileSize(net_export_log_, &file_size));
+    EXPECT_GT(file_size, 0);
+  }
+
+  void VerifyStateAfterDoStart() {

[mmenke, 2013/01/25 17:39:12]

Suggest "VertifyFileAndStateAfterDoStart()", and the corresponding change to the
other.

[ramant (doing other things), 2013/01/25 20:05:24]

Done.

+    // Verfiy that we have transitioned to STATE_ALLOW_STOP state.
+    EXPECT_EQ("ALLOW_STOP", GetStateString());
+    EXPECT_EQ(NetLogTempFile::STATE_ALLOW_STOP, net_log_temp_file_->state());
+    VerifyNetExportLog();
+  }
+
+  void VerifyStateAfterDoStop() {
+    // Verfiy that we have transitioned to STATE_ALLOW_START_SEND state.
+    EXPECT_EQ("ALLOW_START_SEND", GetStateString());
+    EXPECT_EQ(NetLogTempFile::STATE_ALLOW_START_SEND,
+              net_log_temp_file_->state());
+    VerifyNetExportLog();
+  }
+
+  scoped_ptr<ChromeNetLog> net_log_;
+  TestNetLogTempFile* net_log_temp_file_;

[mmenke, 2013/01/25 17:39:12]

Should have a note that this must go after the net_log_, so it can stop
obvserving on destruction.

[mmenke, 2013/01/25 17:39:12]

This should be a scoped_ptr.  Currently, it leaks.

[ramant (doing other things), 2013/01/25 20:05:24]

Done.

[ramant (doing other things), 2013/01/25 20:05:24]

Done.

+  FilePath net_export_log_;
+
+ private:
+  MessageLoop message_loop_;
+  content::TestBrowserThread file_user_blocking_thread_;
+};
+
+TEST_F(NetLogTempFileTest, InitAllowStart) {
+  net_log_temp_file_->set_lie_about_file_existence(true);
+  EXPECT_TRUE(net_log_temp_file_->Init());
+
+  EXPECT_EQ("ALLOW_START", GetStateString());
+  EXPECT_EQ(NetLogTempFile::STATE_ALLOW_START, net_log_temp_file_->state());
+
+  FilePath net_export_file_path;
+  EXPECT_FALSE(net_log_temp_file_->GetFilePath(&net_export_file_path));
+  EXPECT_FALSE(net_log_temp_file_->NetExportLogExists());
+}
+
+TEST_F(NetLogTempFileTest, InitAllowStartOrSend) {
+  EXPECT_TRUE(net_log_temp_file_->Init());
+
+  EXPECT_EQ("ALLOW_START_SEND", GetStateString());
+  EXPECT_EQ(NetLogTempFile::STATE_ALLOW_START_SEND,
+            net_log_temp_file_->state());
+  EXPECT_EQ(net_export_log_, net_log_temp_file_->log_path_);
+  EXPECT_TRUE(file_util::PathExists(net_export_log_));
+
+  FilePath net_export_file_path;
+  EXPECT_TRUE(net_log_temp_file_->GetFilePath(&net_export_file_path));
+  EXPECT_TRUE(file_util::PathExists(net_export_file_path));
+}
+
+TEST_F(NetLogTempFileTest, ProcessCommandDoStartAndStop) {
+  net_log_temp_file_->ProcessCommand(NetLogTempFile::DO_START);
+  VerifyStateAfterDoStart();
+
+  // Check GetFilePath returns false, if we are still writing to file.
+  FilePath net_export_file_path;
+  EXPECT_FALSE(net_log_temp_file_->GetFilePath(&net_export_file_path));
+
+  net_log_temp_file_->ProcessCommand(NetLogTempFile::DO_STOP);
+  VerifyStateAfterDoStop();
+
+  EXPECT_TRUE(net_log_temp_file_->GetFilePath(&net_export_file_path));
+  EXPECT_TRUE(file_util::PathExists(net_export_file_path));
+
+  // TODO(rtenneti): Add check for the contents of the file, by adding a random
+  // NetLog event and verify it is there.
+}
+
+TEST_F(NetLogTempFileTest, DoStartClearsFile) {
+  // Verify file sizes after two consecutives start/stop are the same (even if
+  // we add some junk data in between).
+  net_log_temp_file_->ProcessCommand(NetLogTempFile::DO_START);
+  VerifyStateAfterDoStart();
+
+  int64 start_file_size;
+  EXPECT_TRUE(file_util::GetFileSize(net_export_log_, &start_file_size));
+
+  net_log_temp_file_->ProcessCommand(NetLogTempFile::DO_STOP);
+  VerifyStateAfterDoStop();
+
+  int64 stop_file_size;
+  EXPECT_TRUE(file_util::GetFileSize(net_export_log_, &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_GT(file_util::AppendToFile(
+      net_export_log_, junk_data.c_str(), junk_data.size()), 0);
+
+  int64 junk_file_size;
+  EXPECT_TRUE(file_util::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_temp_file_->ProcessCommand(NetLogTempFile::DO_START);
+  VerifyStateAfterDoStart();
+
+  int64 new_start_file_size;
+  EXPECT_TRUE(file_util::GetFileSize(net_export_log_, &new_start_file_size));
+  EXPECT_EQ(new_start_file_size, start_file_size);
+
+  net_log_temp_file_->ProcessCommand(NetLogTempFile::DO_STOP);
+  VerifyStateAfterDoStop();
+
+  int64 new_stop_file_size;
+  EXPECT_TRUE(file_util::GetFileSize(net_export_log_, &new_stop_file_size));
+  EXPECT_EQ(new_stop_file_size, stop_file_size);
+}
+
+TEST_F(NetLogTempFileTest, 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_temp_file_->ProcessCommand(NetLogTempFile::DO_START);
+  VerifyStateAfterDoStart();
+
+  // Get file size without the event.
+  net_log_temp_file_->ProcessCommand(NetLogTempFile::DO_STOP);
+  VerifyStateAfterDoStop();
+
+  int64 stop_file_size;
+  EXPECT_TRUE(file_util::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_temp_file_->ProcessCommand(NetLogTempFile::DO_START);
+  VerifyStateAfterDoStart();
+
+  // Log an event.
+  net_log_->AddGlobalEntry(net::NetLog::TYPE_CANCELLED);
+
+  net_log_temp_file_->ProcessCommand(NetLogTempFile::DO_STOP);
+  VerifyStateAfterDoStop();
+
+  int64 new_stop_file_size;
+  EXPECT_TRUE(file_util::GetFileSize(net_export_log_, &new_stop_file_size));
+  EXPECT_GE(new_stop_file_size, stop_file_size);
+}