Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(8397)

Unified Diff: chrome/installer/setup/setup_util.cc

Issue 2507753002: Install the chrome event log provider together with the browser. (Closed)
Patch Set: Address comments. Created 4 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/installer/setup/setup_util.cc
diff --git a/chrome/installer/setup/setup_util.cc b/chrome/installer/setup/setup_util.cc
index 233287bf519375396cf4082baa66791711886c63..88c6d22aa2b00692ef240c59952ad8e4666038e1 100644
--- a/chrome/installer/setup/setup_util.cc
+++ b/chrome/installer/setup/setup_util.cc
@@ -11,6 +11,8 @@
#include <algorithm>
#include <iterator>
+#include <limits>
+#include <memory>
#include <set>
#include <string>
@@ -36,6 +38,7 @@
#include "chrome/installer/setup/user_hive_visitor.h"
#include "chrome/installer/util/app_registration_data.h"
#include "chrome/installer/util/google_update_constants.h"
+#include "chrome/installer/util/install_util.h"
#include "chrome/installer/util/installation_state.h"
#include "chrome/installer/util/master_preferences.h"
#include "chrome/installer/util/master_preferences_constants.h"
@@ -47,6 +50,10 @@ namespace installer {
namespace {
+// Event log provider registry location and value names.
+constexpr wchar_t kEventLogProvidersRegPath[] =
grt (UTC plus 2) 2016/11/18 11:38:12 nit: kEventLogProviderRegPath
pastarmovj 2016/11/21 15:48:40 Done.
+ L"\\SYSTEM\\CurrentControlSet\\Services\\EventLog\\Application\\chrome";
grt (UTC plus 2) 2016/11/18 11:38:12 can "chrome" be any string, or does it need to be
grt (UTC plus 2) 2016/11/18 12:10:34 Maybe use BrowserDistribution::GetBaseAppName() fo
pastarmovj 2016/11/21 15:48:40 Unfortunately this has to coincide with the costan
grt (UTC plus 2) 2016/11/22 10:41:25 ah, this is pretty bad. any Chromium-based browser
grt (UTC plus 2) 2016/11/24 07:49:34 Also anything else built using Chromium's base, wh
+
// Returns true if product |type| cam be meaningfully installed without the
// --multi-install flag.
bool SupportsSingleInstall(BrowserDistribution::Type type) {
@@ -660,6 +667,46 @@ void RecordUnPackMetrics(UnPackStatus unpack_status,
->Add(status);
}
+void RegisterEventLogProvider(const base::FilePath& chrome_exe) {
+ VLOG(1) << "Registering Chrome's event log provider at "
+ << kEventLogProvidersRegPath;
+
+ std::unique_ptr<WorkItemList> work_item_list(WorkItem::CreateWorkItemList());
+
+ work_item_list->AddCreateRegKeyWorkItem(
+ HKEY_LOCAL_MACHINE, kEventLogProvidersRegPath, WorkItem::kWow64Default);
+ work_item_list->AddSetRegValueWorkItem(
+ HKEY_LOCAL_MACHINE, kEventLogProvidersRegPath, WorkItem::kWow64Default,
+ L"CategoryCount", static_cast<DWORD>(1), true);
+ work_item_list->AddSetRegValueWorkItem(
grt (UTC plus 2) 2016/11/18 11:38:12 nit: could you put a simple comment above each of
pastarmovj 2016/11/21 15:48:40 Done.
+ HKEY_LOCAL_MACHINE, kEventLogProvidersRegPath, WorkItem::kWow64Default,
+ L"TypesSupported", static_cast<DWORD>(7), true);
+
+ base::FilePath provider(chrome_exe.DirName());
+ provider.Append(FILE_PATH_LITERAL("eventlog_provider.dll"));
grt (UTC plus 2) 2016/11/18 11:38:12 the provider is in the version directory, so you n
pastarmovj 2016/11/21 15:48:40 Done.
+
+ static constexpr wchar_t* file_keys[] = {
+ L"CategoryMessageFile",
+ L"EventMessageFile",
+ L"ParameterMessageFile",
+ };
+ for (const wchar_t* file_key : file_keys) {
+ work_item_list->AddSetRegValueWorkItem(
+ HKEY_LOCAL_MACHINE, kEventLogProvidersRegPath, WorkItem::kWow64Default,
+ file_key, provider.value(), true);
+ }
+
+ // if the operation fails we log the error but still continue because none of
+ // these are critical for the proper operation of the browser.
+ if (!work_item_list->Do())
grt (UTC plus 2) 2016/11/18 11:38:12 are you intentionally not calling Rollback? if so,
pastarmovj 2016/11/21 15:48:41 No sorry I mistakenly assumed that the rollback is
+ LOG(ERROR) << "Could not register Chrome's event log provider.";
+}
+
+void DeRegisterEventLogProvider() {
+ InstallUtil::DeleteRegistryKey(HKEY_LOCAL_MACHINE, kEventLogProvidersRegPath,
+ WorkItem::kWow64Default);
+}
+
ScopedTokenPrivilege::ScopedTokenPrivilege(const wchar_t* privilege_name)
: is_enabled_(false) {
HANDLE temp_handle;

Powered by Google App Engine
This is Rietveld 408576698