Skip to content

Commit

Permalink
Windows install_static refactor.
Browse files Browse the repository at this point in the history
This change introduces install_static::InstallDetails, a module-global
holder of details relating to the current install. It also more clearly
defines the concept of a primary install mode and one or more secondary
install modes (e.g., "Chrome SxS" for Google Chrome's canary channel).
Furthermore, it creates a clear boundary for brand-specific constants so
that Chromium and Google Chrome don't bleed into one another. See
chrome/install_static/README.md for more information.

BUG=373987
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng

Review-Url: https://codereview.chromium.org/2422643002
Cr-Commit-Position: refs/heads/master@{#430728}
  • Loading branch information
GregTho authored and Commit bot committed Nov 8, 2016
1 parent 316a0c0 commit 92bc27b
Show file tree
Hide file tree
Showing 36 changed files with 1,930 additions and 960 deletions.
20 changes: 9 additions & 11 deletions chrome/app/chrome_crash_reporter_client_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "base/debug/leak_annotations.h"
#include "base/format_macros.h"
#include "chrome/common/chrome_result_codes.h"
#include "chrome/install_static/install_details.h"
#include "chrome/install_static/install_util.h"
#include "components/crash/content/app/crashpad.h"
#include "components/crash/core/common/crash_keys.h"
Expand Down Expand Up @@ -327,23 +328,20 @@ bool ChromeCrashReporterClient::GetDeferredUploadsSupported(
return false;
}

// TODO(grt): Remove |exe_path| from crash_reporter::CrashReporterClient.
bool ChromeCrashReporterClient::GetIsPerUserInstall(
const base::string16& exe_path) {
return !install_static::IsSystemInstall(exe_path.c_str());
return !install_static::InstallDetails::Get().system_level();
}

// TODO(grt): Remove |is_per_user_install| from
// crash_reporter::CrashReporterClient.
bool ChromeCrashReporterClient::GetShouldDumpLargerDumps(
bool is_per_user_install) {
base::string16 channel_name;
install_static::GetChromeChannelName(is_per_user_install,
false, // !add_modifier
&channel_name);
// Capture more detail in crash dumps for Beta, Dev, Canary channels and
// if channel is unknown (e.g. Chromium or developer builds).
return (channel_name == install_static::kChromeChannelBeta ||
channel_name == install_static::kChromeChannelDev ||
channel_name == install_static::kChromeChannelCanary ||
channel_name == install_static::kChromeChannelUnknown);
// Capture larger dumps for Google Chrome "beta", "dev", and "canary"
// channels. Stable channel and Chromium builds are on channel "", and use
// smaller dumps.
return !install_static::InstallDetails::Get().channel().empty();
}

int ChromeCrashReporterClient::GetResultCodeRespawnFailed() {
Expand Down
6 changes: 4 additions & 2 deletions chrome/app/chrome_exe_main_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include "base/at_exit.h"
#include "base/command_line.h"
#include "base/files/file_path.h"
#include "base/lazy_instance.h"
#include "base/logging.h"
#include "base/path_service.h"
#include "base/strings/string16.h"
Expand All @@ -26,9 +25,10 @@
#include "chrome/app/main_dll_loader_win.h"
#include "chrome/browser/policy/policy_path_parser.h"
#include "chrome/browser/win/chrome_process_finder.h"
#include "chrome/common/chrome_constants.h"
#include "chrome/common/chrome_paths_internal.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/installer/util/browser_distribution.h"
#include "chrome/install_static/install_details.h"
#include "chrome_elf/chrome_elf_main.h"
#include "components/crash/content/app/crash_switches.h"
#include "components/crash/content/app/crashpad.h"
Expand Down Expand Up @@ -204,6 +204,8 @@ int APIENTRY wWinMain(HINSTANCE instance, HINSTANCE prev, wchar_t*, int) {
int main() {
HINSTANCE instance = GetModuleHandle(nullptr);
#endif
install_static::InstallDetails::InitializeFromPrimaryModule(
chrome::kChromeElfDllName);
SignalInitializeCrashReporting();

// Initialize the CommandLine singleton from the environment.
Expand Down
13 changes: 12 additions & 1 deletion chrome/app/chrome_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "base/debug/dump_without_crashing.h"
#include "base/win/win_util.h"
#include "chrome/common/chrome_constants.h"
#include "chrome/install_static/install_details.h"

#define DLLEXPORT __declspec(dllexport)

Expand Down Expand Up @@ -54,6 +55,11 @@ int ChromeMain(int argc, const char** argv) {
_set_FMA3_enable(0);
#endif // WIN && ARCH_CPU_X86_64

#if defined(OS_WIN)
install_static::InstallDetails::InitializeFromPrimaryModule(
chrome::kChromeElfDllName);
#endif

ChromeMainDelegate chrome_main_delegate(
base::TimeTicks::FromInternalValue(exe_entry_point_ticks));
content::ContentMainParams params(&chrome_main_delegate);
Expand All @@ -66,14 +72,19 @@ int ChromeMain(int argc, const char** argv) {
params.sandbox_info = sandbox_info;

// SetDumpWithoutCrashingFunction must be passed the DumpProcess function
// from the EXE and not from the DLL in order for DumpWithoutCrashing to
// from chrome_elf and not from the DLL in order for DumpWithoutCrashing to
// function correctly.
typedef void (__cdecl *DumpProcessFunction)();
DumpProcessFunction DumpProcess = reinterpret_cast<DumpProcessFunction>(
::GetProcAddress(::GetModuleHandle(chrome::kChromeElfDllName),
"DumpProcessWithoutCrash"));
CHECK(DumpProcess);
base::debug::SetDumpWithoutCrashingFunction(DumpProcess);

// Verify that chrome_elf and this module (chrome.dll and chrome_child.dll)
// have the same version.
if (install_static::InstallDetails::Get().VersionMismatch())
base::debug::DumpWithoutCrashing();
#else
params.argc = argc;
params.argv = argv;
Expand Down
8 changes: 4 additions & 4 deletions chrome/app/main_dll_loader_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,10 @@ int MainDllLoader::Launch(HINSTANCE instance,
ChromeWatcherMainFunction watcher_main =
reinterpret_cast<ChromeWatcherMainFunction>(
::GetProcAddress(watcher_dll, kChromeWatcherDLLEntrypoint));
return watcher_main(
chrome::GetBrowserExitCodesRegistryPath().c_str(),
parent_process.Take(), main_thread_id, on_initialized_event.Take(),
watcher_data_directory.value().c_str(), channel_name.c_str());
return watcher_main(chrome::GetBrowserExitCodesRegistryPath().c_str(),
parent_process.Take(), main_thread_id,
on_initialized_event.Take(),
watcher_data_directory.value().c_str());
}

// Initialize the sandbox services.
Expand Down
1 change: 0 additions & 1 deletion chrome/chrome_watcher/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ shared_library("chrome_watcher") {
"//base:base_static",
"//build/config/sanitizers:deps",
"//chrome/common:metrics_constants_util_win",
"//chrome/installer/util:with_no_strings",
"//components/browser_watcher",
]
ldflags = [ "/DEF:" + rebase_path("chrome_watcher.def", root_build_dir) ]
Expand Down
2 changes: 1 addition & 1 deletion chrome/chrome_watcher/DEPS
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
include_rules = [
"+base",
"+chrome/installer/util",
"+chrome/install_static",
"+components/browser_watcher",
"+components/crash",
"+components/memory_pressure",
Expand Down
9 changes: 5 additions & 4 deletions chrome/chrome_watcher/chrome_watcher_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@
#include "base/time/time.h"
#include "base/win/scoped_handle.h"
#include "base/win/win_util.h"

#include "chrome/chrome_watcher/chrome_watcher_main_api.h"
#include "chrome/installer/util/util_constants.h"
#include "chrome/common/chrome_constants.h"
#include "chrome/install_static/install_details.h"
#include "components/browser_watcher/endsession_watcher_window_win.h"
#include "components/browser_watcher/exit_code_watcher_win.h"
#include "components/browser_watcher/window_hang_monitor_win.h"
Expand Down Expand Up @@ -187,8 +187,9 @@ extern "C" int WatcherMain(const base::char16* registry_path,
HANDLE process_handle,
DWORD main_thread_id,
HANDLE on_initialized_event_handle,
const base::char16* browser_data_directory,
const base::char16* channel_name) {
const base::char16* browser_data_directory) {
install_static::InstallDetails::InitializeFromPrimaryModule(
chrome::kChromeElfDllName);
base::Process process(process_handle);
base::win::ScopedHandle on_initialized_event(on_initialized_event_handle);

Expand Down
6 changes: 2 additions & 4 deletions chrome/chrome_watcher/chrome_watcher_main_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,13 @@ extern const base::FilePath::CharType kPermanentlyFailedReportsSubdir[];
// enabled, a Kasko reporter process is also instantiated, using
// |browser_data_directory| to store crash reports. |on_initialized_event| will
// be signaled once the watcher process is fully initialized. Takes ownership of
// |parent_process| and |on_initialized_event|. |channel_name| is the current
// Chrome distribution channel (one of installer::kChromeChannelXXX).
// |parent_process| and |on_initialized_event|.
typedef int (*ChromeWatcherMainFunction)(
const base::char16* registry_path,
HANDLE parent_process,
DWORD main_thread_id,
HANDLE on_initialized_event,
const base::char16* browser_data_directory,
const base::char16* channel_name);
const base::char16* browser_data_directory);

// Returns an RPC endpoint name for the identified client process. This method
// may be invoked in both the client and the watcher process with the PID of the
Expand Down
26 changes: 26 additions & 0 deletions chrome/install_static/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,38 @@ assert(is_win)
# This file only contains utility functions which must only depend on kernel32.
# Please don't add dependencies on other system libraries.
static_library("install_static_util") {
deps = [
"//components/version_info:generate_version_info",
]

public_deps = [
"//chrome_elf:nt_registry",
]

sources = [
"install_constants.h",
"install_details.cc",
"install_details.h",
"install_modes.cc",
"install_modes.h",
"install_util.cc",
"install_util.h",
"product_install_details.cc",
"product_install_details.h",
]

if (is_chrome_branded) {
sources += [
"google_chrome_install_modes.cc",
"google_chrome_install_modes.h",
]
} else {
sources += [
"chromium_install_modes.cc",
"chromium_install_modes.h",
]
}

libs = [ "kernel32.lib" ]

configs += [
Expand All @@ -30,7 +53,10 @@ static_library("install_static_util") {
test("install_static_unittests") {
output_name = "install_static_unittests"
sources = [
"install_details_unittest.cc",
"install_modes_unittest.cc",
"install_util_unittest.cc",
"product_install_details_unittest.cc",
]
include_dirs = [ "$target_gen_dir" ]
deps = [
Expand Down
11 changes: 10 additions & 1 deletion chrome/install_static/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,16 @@ include_rules = [
"-base",
# Nothing from chrome.
"-chrome",
"+chrome/install_static/install_util.h",
# Anything from this dir.
"+chrome/install_static",
# All registry access should go through nt_registry.
"+chrome_elf/nt_registry/nt_registry.h",
# For the compile-time PRODUCT_VERSION macro.
"+components/version_info/version_info_values.h",
]

specific_include_rules = {
".*_unittest\.cc": [
"+base",
],
}
10 changes: 10 additions & 0 deletions chrome/install_static/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Install Static Library

The install_static library for Windows contains the minimum functionality to
determine the fundamental properties of Chrome's installation plus various
installation-related utility functions. It has no dependencies beyond
kernel32.dll and version.dll, thereby making it suitable for use within
chrome_elf.

Key concepts for the library are documented in `install_modes.h`,
`install_constants.h`, and `install_details.h`.
41 changes: 41 additions & 0 deletions chrome/install_static/chromium_install_modes.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright 2016 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.

// Brand-specific constants and install modes for Chromium.

#include <stdlib.h>

#include "chrome/install_static/install_modes.h"

namespace install_static {

const wchar_t kCompanyPathName[] = L"";

const wchar_t kProductPathName[] = L"Chromium";

const size_t kProductPathNameLength = _countof(kProductPathName) - 1;

// No integration with Google Update, so no app GUID.
const wchar_t kBinariesAppGuid[] = L"";

const wchar_t kBinariesPathName[] = L"Chromium Binaries";

const InstallConstants kInstallModes[] = {
// The primary (and only) install mode for Chromium.
{
sizeof(kInstallModes[0]),
CHROMIUM_INDEX,
L"", // Empty install_suffix for the primary install mode.
L"", // Empty app_guid since no integraion with Google Update.
L"", // Empty default channel name as above.
ChannelStrategy::UNSUPPORTED,
true, // Supports system-level installs.
true, // Supports multi-install.
},
};

static_assert(_countof(kInstallModes) == NUM_INSTALL_MODES,
"Imbalance between kInstallModes and InstallConstantIndex");

} // namespace install_static
23 changes: 23 additions & 0 deletions chrome/install_static/chromium_install_modes.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2016 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.

// Brand-specific types and constants for Chromium.

#ifndef CHROME_INSTALL_STATIC_CHROMIUM_INSTALL_MODES_H_
#define CHROME_INSTALL_STATIC_CHROMIUM_INSTALL_MODES_H_

namespace install_static {

enum : bool {
kUseGoogleUpdateIntegration = false,
};

enum InstallConstantIndex {
CHROMIUM_INDEX,
NUM_INSTALL_MODES,
};

} // namespace install_static

#endif // CHROME_INSTALL_STATIC_CHROMIUM_INSTALL_MODES_H_
51 changes: 51 additions & 0 deletions chrome/install_static/google_chrome_install_modes.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright 2016 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.

// Brand-specific constants and install modes for Google Chrome.

#include <stdlib.h>

#include "chrome/install_static/install_modes.h"

namespace install_static {

const wchar_t kCompanyPathName[] = L"Google";

const wchar_t kProductPathName[] = L"Chrome";

const size_t kProductPathNameLength = _countof(kProductPathName) - 1;

const wchar_t kBinariesAppGuid[] = L"{4DC8B4CA-1BDA-483e-B5FA-D3C12E15B62D}";

// Google Chrome integrates with Google Update, so the app GUID above is used.
const wchar_t kBinariesPathName[] = L"";

const InstallConstants kInstallModes[] = {
// The primary install mode for stable Google Chrome.
{
sizeof(kInstallModes[0]),
STABLE_INDEX,
L"", // Empty install_suffix for the primary install mode.
L"{8A69D345-D564-463c-AFF1-A69D9E530F96}",
L"", // The empty string means "stable".
ChannelStrategy::ADDITIONAL_PARAMETERS,
true, // Supports system-level installs.
true, // Supports multi-install.
},
{
sizeof(kInstallModes[0]),
CANARY_INDEX,
L" SxS",
L"{4ea16ac7-fd5a-47c3-875b-dbf4a2008c20}",
L"canary",
ChannelStrategy::FIXED,
true, // Does not support system-level installs.
false, // Does not support multi-install.
},
};

static_assert(_countof(kInstallModes) == NUM_INSTALL_MODES,
"Imbalance between kInstallModes and InstallConstantIndex");

} // namespace install_static
Loading

0 comments on commit 92bc27b

Please sign in to comment.