Retrial of the first step to port file_util::CountFilesCreatedAfter()

Submitting http://codereview.chromium.org/75033 on behalf of hamaji


Review URL: http://codereview.chromium.org/67276

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@13993 0039d316-1c4b-4281-b951-d872f2087c98

base/file_util.h

@@ -27,6 +27,10 @@
 #include "base/scoped_ptr.h"
 #include "base/file_path.h"
 
+namespace base {
+class Time;
+}
+
 namespace file_util {
 
 //-----------------------------------------------------------------------------
@@ -135,10 +139,9 @@ void ReplaceIllegalCharacters(std::wstring* file_name, int replace_char);
 
 #if defined(OS_WIN)
 // Returns the number of files matching the current path that were
-// created on or after the given FILETIME.  Doesn't count ".." or ".".
-// Filetime is UTC filetime, not LocalFiletime.
-int CountFilesCreatedAfter(const std::wstring& path,
-                           const FILETIME& file_time);
+// created on or after the given |file_time|.  Doesn't count ".." or ".".
+int CountFilesCreatedAfter(const FilePath& path,
+                           const base::Time& file_time);
 #endif  // defined(OS_WIN)
 
 // Deletes the given path, whether it's a file or a directory.

base/file_util_unittest.cc

@@ -20,6 +20,7 @@
 #include "base/logging.h"
 #include "base/path_service.h"
 #include "base/string_util.h"
+#include "base/time.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
 
@@ -316,30 +317,26 @@ TEST_F(FileUtilTest, GetDirectoryFromPath) {
 #if defined OS_WIN
 TEST_F(FileUtilTest, CountFilesCreatedAfter) {
   // Create old file (that we don't want to count)
-  FilePath old_file_name = test_dir_.Append(L"Old File.txt");
+  FilePath old_file_name = test_dir_.Append(FILE_PATH_LITERAL("Old File.txt"));
   CreateTextFile(old_file_name, L"Just call me Mr. Creakybits");
 
   // Age to perfection
   Sleep(100);
 
   // Establish our cutoff time
-  FILETIME test_start_time;
-  GetSystemTimeAsFileTime(&test_start_time);
-  EXPECT_EQ(0, file_util::CountFilesCreatedAfter(test_dir_.value(),
-                                                 test_start_time));
+  base::Time now(base::Time::NowFromSystemTime());
+  EXPECT_EQ(0, file_util::CountFilesCreatedAfter(test_dir_, now));
 
   // Create a new file (that we do want to count)
-  FilePath new_file_name = test_dir_.Append(L"New File.txt");
+  FilePath new_file_name = test_dir_.Append(FILE_PATH_LITERAL("New File.txt"));
   CreateTextFile(new_file_name, L"Waaaaaaaaaaaaaah.");
 
   // We should see only the new file.
-  EXPECT_EQ(1, file_util::CountFilesCreatedAfter(test_dir_.value(),
-                                                 test_start_time));
+  EXPECT_EQ(1, file_util::CountFilesCreatedAfter(test_dir_, now));
 
   // Delete new file, we should see no files after cutoff now
   EXPECT_TRUE(file_util::Delete(new_file_name, false));
-  EXPECT_EQ(0, file_util::CountFilesCreatedAfter(test_dir_.value(),
-                                                 test_start_time));
+  EXPECT_EQ(0, file_util::CountFilesCreatedAfter(test_dir_, now));
 }
 #endif
 

base/file_util_win.cc

@@ -14,6 +14,7 @@
 #include "base/logging.h"
 #include "base/scoped_handle.h"
 #include "base/string_util.h"
+#include "base/time.h"
 #include "base/win_util.h"
 
 namespace file_util {
@@ -39,12 +40,14 @@ bool AbsolutePath(FilePath* path) {
   return true;
 }
 
-int CountFilesCreatedAfter(const std::wstring& path,
-                           const FILETIME& comparison_time) {
+int CountFilesCreatedAfter(const FilePath& path,
+                           const base::Time& comparison_time) {
   int file_count = 0;
+  FILETIME comparison_filetime(comparison_time.ToFileTime());
 
   WIN32_FIND_DATA find_file_data;
-  std::wstring filename_spec = path + L"\\*";  // All files in given dir
+  // All files in given dir
+  std::wstring filename_spec = path.Append(L"*").value();
   HANDLE find_handle = FindFirstFile(filename_spec.c_str(), &find_file_data);
   if (find_handle != INVALID_HANDLE_VALUE) {
     do {
@@ -54,7 +57,7 @@ int CountFilesCreatedAfter(const std::wstring& path,
         continue;
 
       long result = CompareFileTime(&find_file_data.ftCreationTime,
-                                    &comparison_time);
+                                    &comparison_filetime);
       // File was created after or on comparison time
       if ((result == 1) || (result == 0))
         ++file_count;

base/time.h

@@ -207,6 +207,12 @@ class Time {
   // times are increasing, or that two calls to Now() won't be the same.
   static Time Now();
 
+  // Returns the current time. Same as Now() except that this function always
+  // uses system time so that there are no discrepancies between the returned
+  // time and system time even on virtual environments including our test bot.
+  // For timing sensitive unittests, this function should be used.
+  static Time NowFromSystemTime();
+
   // Converts to/from time_t in UTC and a Time class.
   // TODO(brettw) this should be removed once everybody starts using the |Time|
   // class.

base/time_mac.cc

@@ -40,6 +40,12 @@ Time Time::Now() {
   return Time(static_cast<int64>(now * kMicrosecondsPerSecond));
 }
 
+// static
+Time Time::NowFromSystemTime() {
+  // Just use Now() because Now() returns the system time.
+  return Now();
+}
+
 // static
 Time Time::FromExploded(bool is_local, const Exploded& exploded) {
   CFGregorianDate date;

base/time_posix.cc

@@ -40,6 +40,12 @@ Time Time::Now() {
   return tv.tv_sec * kMicrosecondsPerSecond + tv.tv_usec;
 }
 
+// static
+Time Time::NowFromSystemTime() {
+  // Just use Now() because Now() returns the system time.
+  return Now();
+}
+
 // static
 Time Time::FromExploded(bool is_local, const Exploded& exploded) {
   struct tm timestruct;

base/time_win.cc

@@ -126,10 +126,17 @@ Time Time::Now() {
       continue;
     }
 
-    return elapsed + initial_time;
+    return Time(elapsed + initial_time);
   }
 }
 
+// static
+Time Time::NowFromSystemTime() {
+  // Force resync.
+  InitializeClock();
+  return Time(initial_time);
+}
+
 // static
 Time Time::FromFileTime(FILETIME ft) {
   return Time(FileTimeToMicroseconds(ft));

chrome/test/automated_ui_tests/automated_ui_tests.cc

@@ -11,6 +11,7 @@
 #include "base/rand_util.h"
 #include "base/string_util.h"
 #include "base/sys_info.h"
+#include "base/time.h"
 #include "chrome/app/chrome_dll_resource.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/character_encoding.h"
@@ -81,11 +82,11 @@ const std::string kDialogs[] = {
 };
 
 AutomatedUITest::AutomatedUITest()
-    : total_crashes_(0),
+    : test_start_time_(base::Time::NowFromSystemTime()),
+      total_crashes_(0),
       debug_logging_enabled_(false),
       post_action_delay_(0) {
   show_window_ = true;
-  GetSystemTimeAsFileTime(&test_start_time_);
   const CommandLine& parsed_command_line = *CommandLine::ForCurrentProcess();
   if (parsed_command_line.HasSwitch(kDebugModeSwitch))
     debug_logging_enabled_ = true;
@@ -941,7 +942,7 @@ std::wstring AutomatedUITest::GetMostRecentCrashDump() {
 }
 
 bool AutomatedUITest::DidCrash(bool update_total_crashes) {
-  std::wstring crash_dump_path;
+  FilePath crash_dump_path;
   PathService::Get(chrome::DIR_CRASH_DUMPS, &crash_dump_path);
   // Each crash creates two dump files, so we divide by two here.
   int actual_crashes = file_util::CountFilesCreatedAfter(

chrome/test/automated_ui_tests/automated_ui_tests.h

@@ -104,6 +104,10 @@
 #include "chrome/test/automated_ui_tests/automated_ui_test_base.h"
 #include "chrome/test/ui/ui_test.h"
 
+namespace base {
+class Time;
+}
+
 class AutomatedUITest : public AutomatedUITestBase {
  protected:
   AutomatedUITest();
@@ -456,7 +460,7 @@ class AutomatedUITest : public AutomatedUITestBase {
   XmlWriter xml_writer_;
 
   // Time the test was started. Used to find crash dumps.
-  FILETIME test_start_time_;
+  base::Time test_start_time_;
 
   // Number of times the browser has crashed during this run.
   // Used to check for new crashes.

chrome/test/ui/ui_test.cc

@@ -110,17 +110,13 @@ UITest::UITest()
       include_testing_id_(true),
       use_existing_browser_(default_use_existing_browser_),
       enable_file_cookies_(true),
+      test_start_time_(base::Time::NowFromSystemTime()),
       command_execution_timeout_ms_(kMaxTestExecutionTime),
       action_timeout_ms_(kWaitForActionMsec),
       action_max_timeout_ms_(kWaitForActionMaxMsec),
       sleep_timeout_ms_(kWaitForActionMsec) {
   PathService::Get(chrome::DIR_APP, &browser_directory_);
   PathService::Get(chrome::DIR_TEST_DATA, &test_data_directory_);
-#if defined(OS_WIN)
-  GetSystemTimeAsFileTime(&test_start_time_);
-#else
-  // http://code.google.com/p/chromium/issues/detail?id=9833
-#endif
 }
 
 void UITest::SetUp() {
@@ -165,7 +161,7 @@ void UITest::TearDown() {
 
 #if defined(OS_WIN)
   // Check for crashes during the test
-  std::wstring crash_dump_path;
+  FilePath crash_dump_path;
   PathService::Get(chrome::DIR_CRASH_DUMPS, &crash_dump_path);
   // Each crash creates two dump files, so we divide by two here.
   int actual_crashes =

chrome/test/ui/ui_test.h

@@ -444,13 +444,8 @@ class UITest : public testing::Test {
   bool enable_file_cookies_;            // Enable file cookies, default is true.
 
  private:
-#if defined(OS_WIN)
-  // TODO(port): make this use base::Time instead.  It would seem easy, but
-  // the code also depends on file_util::CountFilesCreatedAfter which hasn't
-  // yet been made portable.
-  FILETIME test_start_time_;            // Time the test was started
+  base::Time test_start_time_;          // Time the test was started
                                         // (so we can check for new crash dumps)
-#endif
   static bool no_sandbox_;
   static bool safe_plugins_;
   static bool full_memory_dump_;        // If true, write full memory dump