Add a PartialPreReadImage function to file_util (on Windows).

base/file_util_unittest.cc

 // Copyright (c) 2012 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 "build/build_config.h"
 
 #if defined(OS_WIN)
 #include <windows.h>
 #include <shellapi.h>
 #include <shlobj.h>
@@ skipping 15 matching lines @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
 
 #if defined(OS_WIN)
 #include "base/win/scoped_handle.h"
 #endif
 
 // This macro helps avoid wrapped lines in the test structs.
 #define FPL(x) FILE_PATH_LITERAL(x)
 
+// Declare some internal functions that get exercised below.
+#if defined(OS_WIN)
+namespace file_util {
+namespace internal {
+
+bool PartialPreReadPeFile(const wchar_t* file_path,

[Sigurður Ásgeirsson, 2012/01/26 18:59:20]

IMHO it's preferable to declare these in the file_util header. There's plenty of
precedent in other headers for declaring and exporting functions from internal
namespace:

http://code.google.com/p/chromium/source/search?q=%22namespace+internal%22+fi...

[Roger McFarlane (Google), 2012/01/27 05:24:08]

Done.

+                           size_t percentage,

[chrisha, 2012/01/26 18:23:09]

indent - 1

[Roger McFarlane (Google), 2012/01/27 05:24:08]

Done.

+                           size_t max_chunk_size);
+bool PartialPreReadPeImage(const wchar_t* file_path,
+                           size_t percentage,
+                           size_t step_size);
+
+}  // namespace file_util::internal
+}  // namepsace file_util
+#endif  // defined(OS_WIN)
+
 namespace {
 
 // To test that file_util::Normalize FilePath() deals with NTFS reparse points
 // correctly, we need functions to create and delete reparse points.
 #if defined(OS_WIN)
 typedef struct _REPARSE_DATA_BUFFER {
   ULONG  ReparseTag;
   USHORT  ReparseDataLength;
   USHORT  Reserved;
   union {
@@ skipping 616 matching lines @@
                    FILE_FLAG_BACKUP_SEMANTICS,  // Needed to open a directory.
                    NULL));
   ASSERT_TRUE(dir.IsValid());
   base::PlatformFileInfo info;
   EXPECT_TRUE(base::GetPlatformFileInfo(dir.Get(), &info));
   EXPECT_TRUE(info.is_directory);
   EXPECT_FALSE(info.is_symbolic_link);
   EXPECT_EQ(0, info.size);
 }
 
+TEST_F(FileUtilTest, PreReadImage) {
+  using file_util::PreReadImage;
+
+  const size_t kStepSize = 2 * 1024 * 1024;
+  wchar_t module_path[MAX_PATH];
+  ASSERT_TRUE(::GetModuleFileName(NULL,
+                                  &module_path[0],
+                                  arraysize(module_path)));
+
+  int64 file_size_64 = 0;
+  ASSERT_TRUE(file_util::GetFileSize(FilePath(module_path), &file_size_64));
+  ASSERT_TRUE(file_size_64 < std::numeric_limits<std::size_t>::max());
+  size_t file_size = static_cast<size_t>(file_size_64);
+
+  ASSERT_TRUE(PreReadImage(module_path, 0, kStepSize));
+  ASSERT_TRUE(PreReadImage(module_path, file_size / 4, kStepSize));
+  ASSERT_TRUE(PreReadImage(module_path, file_size / 2, kStepSize));
+  ASSERT_TRUE(PreReadImage(module_path, file_size, kStepSize));
+  ASSERT_TRUE(PreReadImage(module_path, file_size * 2, kStepSize));

[chrisha, 2012/01/26 18:23:09]

Is there any reason why these specific parameter values are being tested? Is
there any other way to test the behaviour of the of the function besides simply
blasting parameters at it and checking true? (Can you mock out some inner
function and make sure it's being called appropriately?)

[Roger McFarlane (Google), 2012/01/27 05:24:08]

All I'm really testing is that the function returns without error for coverage
of some "interesting" cases. Where I define interesting as (expressed in
percentages).

   - valid edge cases (0%, 100%)
   - a valid case somewhere in the middle (25%, 50%)... chosen arbitrarily
   - a case that's otherwise invalid but should be handled gracefully
      (some value greater than 100%).

Two slight wrinkles for this particular case is (1) there's no way to skip
reading the file altogether and (2) specifying to read the full size by giving
the file size can results in slightly different paging behaviour on XP vs
Vista+, but shouldn't fail.

Re: mocking out some interface exercised internally.

A recent TOTT (or was it a LOTL?) highlighted this a testing anti-pattern,
advocating testing of externally visible state changes instead of, more brittle,
internal control-flow/algorithm validation.

What I really want to test is the subsequent paging behavior with and without
the pre-read. I'll have to investigate how to do that from the unittest (added
as a TODO).

+}
+
+TEST_F(FileUtilTest, PartialPreReadImage) {
+  using file_util::PartialPreReadImage;
+
+  const size_t kStepSize = 2 * 1024 * 1024;
+  wchar_t module_path[MAX_PATH];
+  ASSERT_TRUE(::GetModuleFileName(NULL,
+                                  &module_path[0],
+                                  arraysize(module_path)));
+
+  ASSERT_TRUE(PartialPreReadImage(module_path, 0, kStepSize));

[Sigurður Ásgeirsson, 2012/01/26 18:59:20]

For these tests, it would be more possible, and more useful to see what effect
they have on the process working set, when loading and unloading a DLL that's
otherwise not in memory.

[Roger McFarlane (Google), 2012/01/27 05:24:08]

I'll look at doing that. Added a TODO for now.

+  ASSERT_TRUE(PartialPreReadImage(module_path, 25, kStepSize));
+  ASSERT_TRUE(PartialPreReadImage(module_path, 50, kStepSize));
+  ASSERT_TRUE(PartialPreReadImage(module_path, 100, kStepSize));
+  ASSERT_TRUE(PartialPreReadImage(module_path, 150, kStepSize));

[chrisha, 2012/01/26 18:23:09]

Ditto for these tests and others.

[Roger McFarlane (Google), 2012/01/27 05:24:08]

Ditto for my previous reply.

+}
+
+TEST_F(FileUtilTest, PartialPreReadPeFile) {
+  using file_util::internal::PartialPreReadPeFile;
+
+  const size_t kStepSize = 2 * 1024 * 1024;
+  wchar_t module_path[MAX_PATH];
+  ASSERT_TRUE(::GetModuleFileName(NULL,
+                                  &module_path[0],
+                                  arraysize(module_path)));
+
+  ASSERT_TRUE(PartialPreReadPeFile(module_path, 0, kStepSize));
+  ASSERT_TRUE(PartialPreReadPeFile(module_path, 25, kStepSize));
+  ASSERT_TRUE(PartialPreReadPeFile(module_path, 50, kStepSize));
+  ASSERT_TRUE(PartialPreReadPeFile(module_path, 100, kStepSize));
+  ASSERT_TRUE(PartialPreReadPeFile(module_path, 150, kStepSize));
+}
+
+TEST_F(FileUtilTest, PartialPreReadPeImage) {
+  using file_util::internal::PartialPreReadPeImage;
+
+  const size_t kStepSize = 4096;
+  wchar_t module_path[MAX_PATH];
+  ASSERT_TRUE(::GetModuleFileName(NULL,
+                                  &module_path[0],
+                                  arraysize(module_path)));
+
+  ASSERT_TRUE(PartialPreReadPeImage(module_path, 0, kStepSize));
+  ASSERT_TRUE(PartialPreReadPeImage(module_path, 25, kStepSize));
+  ASSERT_TRUE(PartialPreReadPeImage(module_path, 50, kStepSize));
+  ASSERT_TRUE(PartialPreReadPeImage(module_path, 100, kStepSize));
+  ASSERT_TRUE(PartialPreReadPeImage(module_path, 150, kStepSize));
+}
+
 #endif  // defined(OS_WIN)
 
 #if defined(OS_POSIX)
 
 TEST_F(FileUtilTest, CreateAndReadSymlinks) {
   FilePath link_from = temp_dir_.path().Append(FPL("from_file"));
   FilePath link_to = temp_dir_.path().Append(FPL("to_file"));
   CreateTextFile(link_to, bogus_content);
 
   ASSERT_TRUE(file_util::CreateSymbolicLink(link_to, link_from))
@@ skipping 1604 matching lines @@
       file_util::VerifyPathControlledByUser(
           base_dir_, text_file_, uid_, ok_gids_));
   EXPECT_TRUE(
       file_util::VerifyPathControlledByUser(
           sub_dir_, text_file_, uid_, ok_gids_));
 }
 
 #endif  // defined(OS_POSIX)
 
 }  // namespace