Commit 5b7de9e9 authored by fdoray's avatar fdoray Committed by Commit bot

Don't use ::GetFileVersionInfo() in CreateFileVersionInfoForModule()

Currently, base::FileVersionInfo::CreateFileVersionInfoForModule()
calls ::GetModuleFileName and ::GetFileVersionInfo, grabs the loader
lock and potentially touches the disk to obtain the VS_VERSION_INFO
of the module. This is gratuitous for a module that is already loaded.

With this CL, base::FileVersionInfo::CreateFileVersionInfoForModule()
uses base::win::GetResourceFromModule() to get the VS_VERSION_INFO
resource from memory.

First version of this CL: https://codereview.chromium.org/2046583002/

TBR=thestig@chromium.org
BUG=609709

Review-Url: https://codereview.chromium.org/2111613002
Cr-Commit-Position: refs/heads/master@{#402985}
parent 7982a71b
......@@ -1752,7 +1752,7 @@ test("base_unittests") {
"deferred_sequenced_task_runner_unittest.cc",
"environment_unittest.cc",
"feature_list_unittest.cc",
"file_version_info_unittest.cc",
"file_version_info_win_unittest.cc",
"files/dir_reader_posix_unittest.cc",
"files/file_locking_unittest.cc",
"files/file_path_unittest.cc",
......@@ -2058,8 +2058,6 @@ test("base_unittests") {
}
if (is_linux) {
sources -= [ "file_version_info_unittest.cc" ]
if (is_desktop_linux) {
sources += [ "nix/xdg_util_unittest.cc" ]
}
......
......@@ -407,7 +407,7 @@
'deferred_sequenced_task_runner_unittest.cc',
'environment_unittest.cc',
'feature_list_unittest.cc',
'file_version_info_unittest.cc',
'file_version_info_win_unittest.cc',
'files/dir_reader_posix_unittest.cc',
'files/file_locking_unittest.cc',
'files/file_path_unittest.cc',
......@@ -684,9 +684,6 @@
'defines': [
'USE_SYMBOLIZE',
],
'sources!': [
'file_version_info_unittest.cc',
],
'conditions': [
[ 'desktop_linux==1', {
'sources': [
......
......@@ -6,48 +6,63 @@
#include <windows.h>
#include <stddef.h>
#include <stdint.h>
#include "base/file_version_info.h"
#include "base/files/file_path.h"
#include "base/logging.h"
#include "base/macros.h"
#include "base/threading/thread_restrictions.h"
#include "base/win/resource_util.h"
using base::FilePath;
FileVersionInfoWin::FileVersionInfoWin(void* data,
WORD language,
WORD code_page)
: language_(language), code_page_(code_page) {
base::ThreadRestrictions::AssertIOAllowed();
data_.reset((char*) data);
fixed_file_info_ = NULL;
UINT size;
::VerQueryValue(data_.get(), L"\\", (LPVOID*)&fixed_file_info_, &size);
namespace {
struct LanguageAndCodePage {
WORD language;
WORD code_page;
};
// Returns the \\VarFileInfo\\Translation value extracted from the
// VS_VERSION_INFO resource in |data|.
LanguageAndCodePage* GetTranslate(const void* data) {
LanguageAndCodePage* translate = nullptr;
UINT length;
if (::VerQueryValue(data, L"\\VarFileInfo\\Translation",
reinterpret_cast<void**>(&translate), &length)) {
return translate;
}
return nullptr;
}
FileVersionInfoWin::~FileVersionInfoWin() {
DCHECK(data_.get());
VS_FIXEDFILEINFO* GetVsFixedFileInfo(const void* data) {
VS_FIXEDFILEINFO* fixed_file_info = nullptr;
UINT length;
if (::VerQueryValue(data, L"\\", reinterpret_cast<void**>(&fixed_file_info),
&length)) {
return fixed_file_info;
}
return nullptr;
}
typedef struct {
WORD language;
WORD code_page;
} LanguageAndCodePage;
} // namespace
FileVersionInfoWin::~FileVersionInfoWin() = default;
// static
FileVersionInfo* FileVersionInfo::CreateFileVersionInfoForModule(
HMODULE module) {
// Note that the use of MAX_PATH is basically in line with what we do for
// all registered paths (PathProviderWin).
wchar_t system_buffer[MAX_PATH];
system_buffer[0] = 0;
if (!GetModuleFileName(module, system_buffer, MAX_PATH))
return NULL;
void* data;
size_t version_info_length;
const bool has_version_resource = base::win::GetResourceFromModule(
module, VS_VERSION_INFO, RT_VERSION, &data, &version_info_length);
if (!has_version_resource)
return nullptr;
const LanguageAndCodePage* translate = GetTranslate(data);
if (!translate)
return nullptr;
FilePath app_path(system_buffer);
return CreateFileVersionInfo(app_path);
return new FileVersionInfoWin(data, translate->language,
translate->code_page);
}
// static
......@@ -57,32 +72,21 @@ FileVersionInfo* FileVersionInfo::CreateFileVersionInfo(
DWORD dummy;
const wchar_t* path = file_path.value().c_str();
DWORD length = ::GetFileVersionInfoSize(path, &dummy);
const DWORD length = ::GetFileVersionInfoSize(path, &dummy);
if (length == 0)
return NULL;
return nullptr;
void* data = calloc(length, 1);
if (!data)
return NULL;
std::vector<uint8_t> data(length, 0);
if (!::GetFileVersionInfo(path, dummy, length, data)) {
free(data);
return NULL;
}
LanguageAndCodePage* translate = NULL;
uint32_t page_count;
BOOL query_result = VerQueryValue(data, L"\\VarFileInfo\\Translation",
(void**) &translate, &page_count);
if (!::GetFileVersionInfo(path, dummy, length, data.data()))
return nullptr;
if (query_result && translate) {
return new FileVersionInfoWin(data, translate->language,
translate->code_page);
const LanguageAndCodePage* translate = GetTranslate(data.data());
if (!translate)
return nullptr;
} else {
free(data);
return NULL;
}
return new FileVersionInfoWin(std::move(data), translate->language,
translate->code_page);
}
base::string16 FileVersionInfoWin::company_name() {
......@@ -175,7 +179,7 @@ bool FileVersionInfoWin::GetValue(const wchar_t* name,
L"\\StringFileInfo\\%04x%04x\\%ls", language, code_page, name);
LPVOID value = NULL;
uint32_t size;
BOOL r = ::VerQueryValue(data_.get(), sub_block, &value, &size);
BOOL r = ::VerQueryValue(data_, sub_block, &value, &size);
if (r && value) {
value_str->assign(static_cast<wchar_t*>(value));
return true;
......@@ -191,3 +195,24 @@ std::wstring FileVersionInfoWin::GetStringValue(const wchar_t* name) {
else
return L"";
}
FileVersionInfoWin::FileVersionInfoWin(std::vector<uint8_t>&& data,
WORD language,
WORD code_page)
: owned_data_(std::move(data)),
data_(owned_data_.data()),
language_(language),
code_page_(code_page),
fixed_file_info_(GetVsFixedFileInfo(data_)) {
DCHECK(!owned_data_.empty());
}
FileVersionInfoWin::FileVersionInfoWin(void* data,
WORD language,
WORD code_page)
: data_(data),
language_(language),
code_page_(code_page),
fixed_file_info_(GetVsFixedFileInfo(data)) {
DCHECK(data_);
}
......@@ -5,20 +5,23 @@
#ifndef BASE_FILE_VERSION_INFO_WIN_H_
#define BASE_FILE_VERSION_INFO_WIN_H_
#include <windows.h>
#include <stdint.h>
#include <memory>
#include <string>
#include <vector>
#include "base/base_export.h"
#include "base/file_version_info.h"
#include "base/macros.h"
#include "base/memory/free_deleter.h"
struct tagVS_FIXEDFILEINFO;
typedef tagVS_FIXEDFILEINFO VS_FIXEDFILEINFO;
class BASE_EXPORT FileVersionInfoWin : public FileVersionInfo {
public:
FileVersionInfoWin(void* data, WORD language, WORD code_page);
~FileVersionInfoWin() override;
// Accessors to the different version properties.
......@@ -48,14 +51,25 @@ class BASE_EXPORT FileVersionInfoWin : public FileVersionInfo {
std::wstring GetStringValue(const wchar_t* name);
// Get the fixed file info if it exists. Otherwise NULL
VS_FIXEDFILEINFO* fixed_file_info() { return fixed_file_info_; }
const VS_FIXEDFILEINFO* fixed_file_info() const { return fixed_file_info_; }
private:
std::unique_ptr<char, base::FreeDeleter> data_;
WORD language_;
WORD code_page_;
// This is a pointer into the data_ if it exists. Otherwise NULL.
VS_FIXEDFILEINFO* fixed_file_info_;
friend FileVersionInfo;
// |data| is a VS_VERSION_INFO resource. |language| and |code_page| are
// extracted from the \VarFileInfo\Translation value of |data|.
FileVersionInfoWin(std::vector<uint8_t>&& data,
WORD language,
WORD code_page);
FileVersionInfoWin(void* data, WORD language, WORD code_page);
const std::vector<uint8_t> owned_data_;
const void* const data_;
const WORD language_;
const WORD code_page_;
// This is a pointer into |data_| if it exists. Otherwise nullptr.
const VS_FIXEDFILEINFO* const fixed_file_info_;
DISALLOW_COPY_AND_ASSIGN(FileVersionInfoWin);
};
......
......@@ -2,27 +2,26 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "base/file_version_info.h"
#include "base/file_version_info_win.h"
#include <windows.h>
#include <stddef.h>
#include <memory>
#include "base/file_version_info.h"
#include "base/files/file_path.h"
#include "base/macros.h"
#include "base/memory/ptr_util.h"
#include "base/path_service.h"
#include "build/build_config.h"
#include "base/scoped_native_library.h"
#include "testing/gtest/include/gtest/gtest.h"
#if defined(OS_WIN)
#include "base/file_version_info_win.h"
#endif
using base::FilePath;
namespace {
#if defined(OS_WIN)
FilePath GetTestDataPath() {
FilePath path;
PathService::Get(base::DIR_SOURCE_ROOT, &path);
......@@ -32,12 +31,54 @@ FilePath GetTestDataPath() {
path = path.AppendASCII("file_version_info_unittest");
return path;
}
#endif
class FileVersionInfoFactory {
public:
explicit FileVersionInfoFactory(const FilePath& path) : path_(path) {}
std::unique_ptr<FileVersionInfo> Create() const {
return base::WrapUnique(FileVersionInfo::CreateFileVersionInfo(path_));
}
private:
const FilePath path_;
DISALLOW_COPY_AND_ASSIGN(FileVersionInfoFactory);
};
class FileVersionInfoForModuleFactory {
public:
explicit FileVersionInfoForModuleFactory(const FilePath& path)
// Load the library with LOAD_LIBRARY_AS_IMAGE_RESOURCE since it shouldn't
// be executed.
: library_(::LoadLibraryEx(path.value().c_str(),
nullptr,
LOAD_LIBRARY_AS_IMAGE_RESOURCE)) {
EXPECT_TRUE(library_.is_valid());
}
std::unique_ptr<FileVersionInfo> Create() const {
return base::WrapUnique(
FileVersionInfo::CreateFileVersionInfoForModule(library_.get()));
}
private:
const base::ScopedNativeLibrary library_;
DISALLOW_COPY_AND_ASSIGN(FileVersionInfoForModuleFactory);
};
template <typename T>
class FileVersionInfoTest : public testing::Test {};
using FileVersionInfoFactories =
::testing::Types<FileVersionInfoFactory, FileVersionInfoForModuleFactory>;
} // namespace
#if defined(OS_WIN)
TEST(FileVersionInfoTest, HardCodedProperties) {
TYPED_TEST_CASE(FileVersionInfoTest, FileVersionInfoFactories);
TYPED_TEST(FileVersionInfoTest, HardCodedProperties) {
const wchar_t kDLLName[] = {L"FileVersionInfoTest1.dll"};
const wchar_t* const kExpectedValues[15] = {
......@@ -62,8 +103,9 @@ TEST(FileVersionInfoTest, HardCodedProperties) {
FilePath dll_path = GetTestDataPath();
dll_path = dll_path.Append(kDLLName);
std::unique_ptr<FileVersionInfo> version_info(
FileVersionInfo::CreateFileVersionInfo(dll_path));
TypeParam factory(dll_path);
std::unique_ptr<FileVersionInfo> version_info(factory.Create());
ASSERT_TRUE(version_info);
int j = 0;
EXPECT_EQ(kExpectedValues[j++], version_info->company_name());
......@@ -82,62 +124,52 @@ TEST(FileVersionInfoTest, HardCodedProperties) {
EXPECT_EQ(kExpectedValues[j++], version_info->legal_trademarks());
EXPECT_EQ(kExpectedValues[j++], version_info->last_change());
}
#endif
#if defined(OS_WIN)
TEST(FileVersionInfoTest, IsOfficialBuild) {
const wchar_t* kDLLNames[] = {
L"FileVersionInfoTest1.dll",
L"FileVersionInfoTest2.dll"
TYPED_TEST(FileVersionInfoTest, IsOfficialBuild) {
constexpr struct {
const wchar_t* const dll_name;
const bool is_official_build;
} kTestItems[]{
{L"FileVersionInfoTest1.dll", true}, {L"FileVersionInfoTest2.dll", false},
};
const bool kExpected[] = {
true,
false,
};
// Test consistency check.
ASSERT_EQ(arraysize(kDLLNames), arraysize(kExpected));
for (size_t i = 0; i < arraysize(kDLLNames); ++i) {
FilePath dll_path = GetTestDataPath();
dll_path = dll_path.Append(kDLLNames[i]);
for (const auto& test_item : kTestItems) {
const FilePath dll_path = GetTestDataPath().Append(test_item.dll_name);
std::unique_ptr<FileVersionInfo> version_info(
FileVersionInfo::CreateFileVersionInfo(dll_path));
TypeParam factory(dll_path);
std::unique_ptr<FileVersionInfo> version_info(factory.Create());
ASSERT_TRUE(version_info);
EXPECT_EQ(kExpected[i], version_info->is_official_build());
EXPECT_EQ(test_item.is_official_build, version_info->is_official_build());
}
}
#endif
#if defined(OS_WIN)
TEST(FileVersionInfoTest, CustomProperties) {
TYPED_TEST(FileVersionInfoTest, CustomProperties) {
FilePath dll_path = GetTestDataPath();
dll_path = dll_path.AppendASCII("FileVersionInfoTest1.dll");
std::unique_ptr<FileVersionInfo> version_info(
FileVersionInfo::CreateFileVersionInfo(dll_path));
TypeParam factory(dll_path);
std::unique_ptr<FileVersionInfo> version_info(factory.Create());
ASSERT_TRUE(version_info);
// Test few existing properties.
std::wstring str;
FileVersionInfoWin* version_info_win =
static_cast<FileVersionInfoWin*>(version_info.get());
EXPECT_TRUE(version_info_win->GetValue(L"Custom prop 1", &str));
EXPECT_TRUE(version_info_win->GetValue(L"Custom prop 1", &str));
EXPECT_EQ(L"Un", str);
EXPECT_EQ(L"Un", version_info_win->GetStringValue(L"Custom prop 1"));
EXPECT_TRUE(version_info_win->GetValue(L"Custom prop 2", &str));
EXPECT_TRUE(version_info_win->GetValue(L"Custom prop 2", &str));
EXPECT_EQ(L"Deux", str);
EXPECT_EQ(L"Deux", version_info_win->GetStringValue(L"Custom prop 2"));
EXPECT_TRUE(version_info_win->GetValue(L"Custom prop 3", &str));
EXPECT_TRUE(version_info_win->GetValue(L"Custom prop 3", &str));
EXPECT_EQ(L"1600 Amphitheatre Parkway Mountain View, CA 94043", str);
EXPECT_EQ(L"1600 Amphitheatre Parkway Mountain View, CA 94043",
version_info_win->GetStringValue(L"Custom prop 3"));
// Test an non-existing property.
EXPECT_FALSE(version_info_win->GetValue(L"Unknown property", &str));
EXPECT_FALSE(version_info_win->GetValue(L"Unknown property", &str));
EXPECT_EQ(L"", version_info_win->GetStringValue(L"Unknown property"));
}
#endif
......@@ -15,7 +15,7 @@
#include "base/bind.h"
#include "base/command_line.h"
#include "base/environment.h"
#include "base/file_version_info_win.h"
#include "base/file_version_info.h"
#include "base/files/file_path.h"
#include "base/i18n/case_conversion.h"
#include "base/macros.h"
......@@ -630,16 +630,10 @@ void ModuleEnumerator::PopulateModuleInformation(Module* module) {
module->recommended_action = NONE;
std::unique_ptr<FileVersionInfo> version_info(
FileVersionInfo::CreateFileVersionInfo(base::FilePath(module->location)));
if (version_info.get()) {
FileVersionInfoWin* version_info_win =
static_cast<FileVersionInfoWin*>(version_info.get());
VS_FIXEDFILEINFO* fixed_file_info = version_info_win->fixed_file_info();
if (fixed_file_info) {
module->description = version_info_win->file_description();
module->version = version_info_win->file_version();
module->product_name = version_info_win->product_name();
}
if (version_info) {
module->description = version_info->file_description();
module->version = version_info->file_version();
module->product_name = version_info->product_name();
}
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment