Commit d67aff2c authored by grt's avatar grt Committed by Commit bot

Product cleanups in the wake of multi-install removal.

- Remove |options| set.
- Remove InitializeFrom* methods.
- Remove unused SetChannelFlags method.

BUG=577816

Review-Url: https://codereview.chromium.org/2621923002
Cr-Commit-Position: refs/heads/master@{#442870}
parent dddbf3df
......@@ -112,7 +112,7 @@ void InstallerState::set_level(Level level) {
// debug builds. See the log messages for details.
bool InstallerState::CanAddProduct(const base::FilePath* product_dir) const {
if (product_) {
LOG(DFATAL) << "Cannot process more than one single-install product.";
LOG(DFATAL) << "Cannot process more than one product.";
return false;
}
return true;
......@@ -127,7 +127,6 @@ Product* InstallerState::AddProductInDirectory(
std::unique_ptr<Product> product) {
DCHECK(product);
const Product& the_product = *product;
DCHECK(!the_product.HasOption(kOptionMultiInstall));
if (!CanAddProduct(product_dir))
return nullptr;
......@@ -158,7 +157,6 @@ Product* InstallerState::AddProductFromPreferences(
const InstallationState& machine_state) {
std::unique_ptr<Product> product_ptr(
new Product(BrowserDistribution::GetDistribution()));
product_ptr->InitializeFromPreferences(prefs);
Product* product = AddProductInDirectory(nullptr, std::move(product_ptr));
......@@ -176,7 +174,6 @@ Product* InstallerState::AddProductFromState(
const ProductState& state) {
std::unique_ptr<Product> product_ptr(
new Product(BrowserDistribution::GetDistribution()));
product_ptr->InitializeFromUninstallCommand(state.uninstall_command());
// Strip off <version>/Installer/setup.exe; see GetInstallerDirectory().
base::FilePath product_dir =
......
......@@ -6,91 +6,27 @@
#include "base/command_line.h"
#include "base/files/file_path.h"
#include "base/files/file_util.h"
#include "base/logging.h"
#include "base/strings/string_util.h"
#include "chrome/installer/util/browser_distribution.h"
#include "chrome/installer/util/channel_info.h"
#include "chrome/installer/util/helper.h"
#include "chrome/installer/util/install_util.h"
#include "chrome/installer/util/master_preferences.h"
#include "chrome/installer/util/master_preferences_constants.h"
#include "chrome/installer/util/shell_util.h"
#include "chrome/installer/util/user_experiment.h"
#include "chrome/installer/util/util_constants.h"
namespace installer {
void ChromeBrowserOperations::ReadOptions(const MasterPreferences& prefs,
std::set<base::string16>* options)
const {
DCHECK(options);
bool pref_value;
if (prefs.GetBool(master_preferences::kMultiInstall, &pref_value) &&
pref_value) {
options->insert(kOptionMultiInstall);
}
}
void ChromeBrowserOperations::ReadOptions(
const base::CommandLine& uninstall_command,
std::set<base::string16>* options) const {
DCHECK(options);
if (uninstall_command.HasSwitch(switches::kMultiInstall))
options->insert(kOptionMultiInstall);
}
void ChromeBrowserOperations::AddKeyFiles(
const std::set<base::string16>& options,
std::vector<base::FilePath>* key_files) const {
DCHECK(key_files);
key_files->push_back(base::FilePath(installer::kChromeDll));
}
void ChromeBrowserOperations::AppendProductFlags(
const std::set<base::string16>& options,
base::CommandLine* cmd_line) const {
DCHECK(cmd_line);
if (options.find(kOptionMultiInstall) != options.end()) {
// Add --multi-install if it isn't already there.
if (!cmd_line->HasSwitch(switches::kMultiInstall))
cmd_line->AppendSwitch(switches::kMultiInstall);
// --chrome is only needed in multi-install.
cmd_line->AppendSwitch(switches::kChrome);
}
}
void ChromeBrowserOperations::AppendRenameFlags(
const std::set<base::string16>& options,
base::CommandLine* cmd_line) const {
DCHECK(cmd_line);
// Add --multi-install if it isn't already there.
if (options.find(kOptionMultiInstall) != options.end() &&
!cmd_line->HasSwitch(switches::kMultiInstall)) {
cmd_line->AppendSwitch(switches::kMultiInstall);
}
}
bool ChromeBrowserOperations::SetChannelFlags(
const std::set<base::string16>& options,
bool set,
ChannelInfo* channel_info) const {
#if defined(GOOGLE_CHROME_BUILD)
DCHECK(channel_info);
bool chrome_changed = channel_info->SetChrome(set);
// Remove App Launcher's channel flags, since App Launcher does not exist as
// an independent product, and is a part of Chrome.
bool app_launcher_changed = channel_info->SetAppLauncher(false);
return chrome_changed || app_launcher_changed;
#else
return false;
#endif
}
// Modifies a ShortcutProperties object by adding default values to
......@@ -121,11 +57,10 @@ void ChromeBrowserOperations::AddDefaultShortcutProperties(
void ChromeBrowserOperations::LaunchUserExperiment(
const base::FilePath& setup_path,
const std::set<base::string16>& options,
InstallStatus status,
bool system_level) const {
base::CommandLine base_command(setup_path);
AppendProductFlags(options, &base_command);
AppendProductFlags(&base_command);
installer::LaunchBrowserUserExperiment(base_command, status, system_level);
}
......
......@@ -5,7 +5,6 @@
#ifndef CHROME_INSTALLER_UTIL_CHROME_BROWSER_OPERATIONS_H_
#define CHROME_INSTALLER_UTIL_CHROME_BROWSER_OPERATIONS_H_
#include "base/compiler_specific.h"
#include "base/macros.h"
#include "chrome/installer/util/product_operations.h"
......@@ -16,24 +15,11 @@ class ChromeBrowserOperations : public ProductOperations {
public:
ChromeBrowserOperations() {}
void ReadOptions(const MasterPreferences& prefs,
std::set<base::string16>* options) const override;
void AddKeyFiles(std::vector<base::FilePath>* key_files) const override;
void ReadOptions(const base::CommandLine& uninstall_command,
std::set<base::string16>* options) const override;
void AppendProductFlags(base::CommandLine* cmd_line) const override;
void AddKeyFiles(const std::set<base::string16>& options,
std::vector<base::FilePath>* key_files) const override;
void AppendProductFlags(const std::set<base::string16>& options,
base::CommandLine* cmd_line) const override;
void AppendRenameFlags(const std::set<base::string16>& options,
base::CommandLine* cmd_line) const override;
bool SetChannelFlags(const std::set<base::string16>& options,
bool set,
ChannelInfo* channel_info) const override;
void AppendRenameFlags(base::CommandLine* cmd_line) const override;
void AddDefaultShortcutProperties(
BrowserDistribution* dist,
......@@ -41,7 +27,6 @@ class ChromeBrowserOperations : public ProductOperations {
ShellUtil::ShortcutProperties* properties) const override;
void LaunchUserExperiment(const base::FilePath& setup_path,
const std::set<base::string16>& options,
InstallStatus status,
bool system_level) const override;
......
......@@ -11,21 +11,19 @@
namespace installer {
void ChromeBrowserSxSOperations::AppendProductFlags(
const std::set<base::string16>& options,
base::CommandLine* cmd_line) const {
DCHECK(cmd_line);
cmd_line->AppendSwitch(switches::kChromeSxS);
ChromeBrowserOperations::AppendProductFlags(options, cmd_line);
ChromeBrowserOperations::AppendProductFlags(cmd_line);
}
void ChromeBrowserSxSOperations::AppendRenameFlags(
const std::set<base::string16>& options,
base::CommandLine* cmd_line) const {
DCHECK(cmd_line);
cmd_line->AppendSwitch(switches::kChromeSxS);
ChromeBrowserOperations::AppendRenameFlags(options, cmd_line);
ChromeBrowserOperations::AppendRenameFlags(cmd_line);
}
} // namespace installer
......@@ -5,7 +5,6 @@
#ifndef CHROME_INSTALLER_UTIL_CHROME_BROWSER_SXS_OPERATIONS_H_
#define CHROME_INSTALLER_UTIL_CHROME_BROWSER_SXS_OPERATIONS_H_
#include "base/compiler_specific.h"
#include "base/macros.h"
#include "chrome/installer/util/chrome_browser_operations.h"
......@@ -16,11 +15,9 @@ class ChromeBrowserSxSOperations : public ChromeBrowserOperations {
public:
ChromeBrowserSxSOperations() {}
void AppendProductFlags(const std::set<base::string16>& options,
base::CommandLine* cmd_line) const override;
void AppendProductFlags(base::CommandLine* cmd_line) const override;
void AppendRenameFlags(const std::set<base::string16>& options,
base::CommandLine* cmd_line) const override;
void AppendRenameFlags(base::CommandLine* cmd_line) const override;
private:
DISALLOW_COPY_AND_ASSIGN(ChromeBrowserSxSOperations);
......
......@@ -164,7 +164,6 @@ void ElevateIfNeededToReenableUpdates() {
base::CommandLine cmd(exe_path);
cmd.AppendSwitch(installer::switches::kReenableAutoupdates);
installer::Product product(BrowserDistribution::GetDistribution());
product.InitializeFromUninstallCommand(product_state.uninstall_command());
product.AppendProductFlags(&cmd);
if (system_install)
cmd.AppendSwitch(installer::switches::kSystemLevel);
......
......@@ -15,12 +15,9 @@
#include "chrome/installer/util/chrome_browser_sxs_operations.h"
#include "chrome/installer/util/google_update_constants.h"
#include "chrome/installer/util/install_util.h"
#include "chrome/installer/util/master_preferences.h"
#include "chrome/installer/util/master_preferences_constants.h"
#include "chrome/installer/util/product_operations.h"
using base::win::RegKey;
using installer::MasterPreferences;
namespace installer {
......@@ -33,15 +30,6 @@ Product::Product(BrowserDistribution* distribution)
Product::~Product() {
}
void Product::InitializeFromPreferences(const MasterPreferences& prefs) {
operations_->ReadOptions(prefs, &options_);
}
void Product::InitializeFromUninstallCommand(
const base::CommandLine& uninstall_command) {
operations_->ReadOptions(uninstall_command, &options_);
}
bool Product::LaunchChrome(const base::FilePath& application_path) const {
bool success = !application_path.empty();
if (success) {
......@@ -112,19 +100,15 @@ bool Product::SetMsiMarker(bool system_install, bool set) const {
}
void Product::AddKeyFiles(std::vector<base::FilePath>* key_files) const {
operations_->AddKeyFiles(options_, key_files);
operations_->AddKeyFiles(key_files);
}
void Product::AppendProductFlags(base::CommandLine* command_line) const {
operations_->AppendProductFlags(options_, command_line);
operations_->AppendProductFlags(command_line);
}
void Product::AppendRenameFlags(base::CommandLine* command_line) const {
operations_->AppendRenameFlags(options_, command_line);
}
bool Product::SetChannelFlags(bool set, ChannelInfo* channel_info) const {
return operations_->SetChannelFlags(options_, set, channel_info);
operations_->AppendRenameFlags(command_line);
}
void Product::AddDefaultShortcutProperties(
......@@ -141,8 +125,7 @@ void Product::LaunchUserExperiment(const base::FilePath& setup_path,
VLOG(1) << "LaunchUserExperiment status: " << status << " product: "
<< distribution_->GetDisplayName()
<< " system_level: " << system_level;
operations_->LaunchUserExperiment(
setup_path, options_, status, system_level);
operations_->LaunchUserExperiment(setup_path, status, system_level);
}
}
......
......@@ -8,8 +8,6 @@
#include <stdint.h>
#include <memory>
#include <set>
#include <string>
#include <vector>
#include "base/macros.h"
......@@ -24,8 +22,6 @@ class CommandLine;
namespace installer {
class ChannelInfo;
class MasterPreferences;
class Product;
class ProductOperations;
......@@ -44,27 +40,10 @@ class Product {
~Product();
void InitializeFromPreferences(const MasterPreferences& prefs);
void InitializeFromUninstallCommand(
const base::CommandLine& uninstall_command);
BrowserDistribution* distribution() const {
return distribution_;
}
bool HasOption(const std::wstring& option) const {
return options_.find(option) != options_.end();
}
// Returns true if the set of options is mutated by this operation.
bool SetOption(const std::wstring& option, bool set) {
if (set)
return options_.insert(option).second;
else
return options_.erase(option) != 0;
}
// Launches Chrome without waiting for it to exit.
bool LaunchChrome(const base::FilePath& application_path) const;
......@@ -92,9 +71,6 @@ class Product {
// See ProductOperations::AppendRenameFlags.
void AppendRenameFlags(base::CommandLine* command_line) const;
// See Productoperations::SetChannelFlags.
bool SetChannelFlags(bool set, ChannelInfo* channel_info) const;
// See ProductOperations::AddDefaultShortcutProperties.
void AddDefaultShortcutProperties(
const base::FilePath& target_exe,
......@@ -111,7 +87,6 @@ class Product {
BrowserDistribution* const distribution_;
const std::unique_ptr<ProductOperations> operations_;
std::set<std::wstring> options_;
private:
DISALLOW_COPY_AND_ASSIGN(Product);
......
......@@ -5,12 +5,9 @@
#ifndef CHROME_INSTALLER_UTIL_PRODUCT_OPERATIONS_H_
#define CHROME_INSTALLER_UTIL_PRODUCT_OPERATIONS_H_
#include <set>
#include <string>
#include <vector>
#include "base/files/file_path.h"
#include "base/strings/string16.h"
#include "chrome/installer/util/shell_util.h"
#include "chrome/installer/util/util_constants.h"
......@@ -22,51 +19,30 @@ class CommandLine;
namespace installer {
class ChannelInfo;
class MasterPreferences;
// An interface to product-specific operations that depend on product
// configuration. Implementations are expected to be stateless. Configuration
// can be read from a MasterPreferences instance or from a product's uninstall
// command.
// configuration. Implementations are expected to be stateless.
class ProductOperations {
public:
virtual ~ProductOperations() {}
// Reads product-specific options from |prefs|, adding them to |options|.
virtual void ReadOptions(const MasterPreferences& prefs,
std::set<base::string16>* options) const = 0;
// Reads product-specific options from |command|, adding them to |options|.
virtual void ReadOptions(const base::CommandLine& command,
std::set<base::string16>* options) const = 0;
// A key-file is a file such as a DLL on Windows that is expected to be in use
// when the product is being used. For example "chrome.dll" for Chrome.
// when the product is being used. For example "chrome.dll" for Chrome.
// Before attempting to delete an installation directory during an
// uninstallation, the uninstaller will check if any one of a potential set of
// key files is in use and if they are, abort the delete operation. Only if
// none of the key files are in use, can the folder be deleted. Note that
// this function does not return a full path to the key file(s), only (a) file
// key files is in use and if they are, abort the delete operation. Only if
// none of the key files are in use, can the folder be deleted. Note that this
// function does not return a full path to the key file(s), only (a) file
// name(s).
virtual void AddKeyFiles(const std::set<base::string16>& options,
std::vector<base::FilePath>* key_files) const = 0;
virtual void AddKeyFiles(std::vector<base::FilePath>* key_files) const = 0;
// Given a command line, appends the set of product-specific flags. These are
// Given a command line, appends the set of product-specific flags. These are
// required for product-specific uninstall commands, but are of use for any
// invocation of setup.exe for the product.
virtual void AppendProductFlags(const std::set<base::string16>& options,
base::CommandLine* cmd_line) const = 0;
virtual void AppendProductFlags(base::CommandLine* cmd_line) const = 0;
// Given a command line, appends the set of product-specific rename flags.
virtual void AppendRenameFlags(const std::set<base::string16>& options,
base::CommandLine* cmd_line) const = 0;
// Adds or removes product-specific flags in |channel_info|. Returns true if
// |channel_info| is modified.
virtual bool SetChannelFlags(const std::set<base::string16>& options,
bool set,
ChannelInfo* channel_info) const = 0;
virtual void AppendRenameFlags(base::CommandLine* cmd_line) const = 0;
// Modifies a ShellUtil::ShortcutProperties object by assigning default values
// to unintialized members.
......@@ -79,7 +55,6 @@ class ProductOperations {
// experiment. This function determines if the user qualifies and if so it
// sets the wheels in motion or in simple cases does the experiment itself.
virtual void LaunchUserExperiment(const base::FilePath& setup_path,
const std::set<base::string16>& options,
InstallStatus status,
bool system_level) const = 0;
};
......
......@@ -16,29 +16,20 @@
#include "chrome/installer/util/browser_distribution.h"
#include "chrome/installer/util/google_update_constants.h"
#include "chrome/installer/util/installation_state.h"
#include "chrome/installer/util/master_preferences.h"
#include "testing/gtest/include/gtest/gtest.h"
using base::win::RegKey;
using installer::Product;
using installer::MasterPreferences;
using registry_util::RegistryOverrideManager;
TEST(ProductTest, ProductInstallBasic) {
// TODO(tommi): We should mock this and use our mocked distribution.
const bool multi_install = false;
const bool system_level = true;
base::CommandLine cmd_line = base::CommandLine::FromString(
std::wstring(L"setup.exe") +
(multi_install ? L" --multi-install --chrome" : L"") +
(system_level ? L" --system-level" : L""));
installer::MasterPreferences prefs(cmd_line);
installer::InstallationState machine_state;
machine_state.Initialize();
std::unique_ptr<Product> product =
base::MakeUnique<Product>(BrowserDistribution::GetDistribution());
product->InitializeFromPreferences(prefs);
BrowserDistribution* distribution = product->distribution();
base::FilePath user_data_dir;
......
......@@ -231,8 +231,6 @@ const wchar_t kInstallerResultUIString[] = L"InstallerResultUIString";
const wchar_t kInstallerSuccessLaunchCmdLine[] =
L"InstallerSuccessLaunchCmdLine";
const wchar_t kOptionMultiInstall[] = L"multi-install";
// Chrome channel display names.
const wchar_t kChromeChannelUnknown[] = L"unknown";
const wchar_t kChromeChannelCanary[] = L"canary";
......
......@@ -229,9 +229,6 @@ extern const wchar_t kInstallerResult[];
extern const wchar_t kInstallerResultUIString[];
extern const wchar_t kInstallerSuccessLaunchCmdLine[];
// Product options.
extern const wchar_t kOptionMultiInstall[];
// Chrome channel display names.
// NOTE: Canary is not strictly a 'channel', but rather a separate product
// installed side-by-side. However, GoogleUpdateSettings::GetChromeChannel
......
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