 Chromium Code Reviews
 Chromium Code Reviews Issue 1913943003:
  Remove dependencies on chrome\installer from the ChromeCrashReporterClient class on Windows.  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@master
    
  
    Issue 1913943003:
  Remove dependencies on chrome\installer from the ChromeCrashReporterClient class on Windows.  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@master| Index: chrome/install_static/install_util.cc | 
| diff --git a/chrome/install_static/install_util.cc b/chrome/install_static/install_util.cc | 
| index 6fbef47569c23475978e43550443272fd66e21a5..cf95bb70925b37829273a8f9463ef4e27029ad18 100644 | 
| --- a/chrome/install_static/install_util.cc | 
| +++ b/chrome/install_static/install_util.cc | 
| @@ -5,14 +5,34 @@ | 
| #include "chrome/install_static/install_util.h" | 
| #include <assert.h> | 
| -#include <windows.h> | 
| +#include <shlobj.h> | 
| #include <stddef.h> | 
| +#include <memory> | 
| +#include <windows.h> | 
| +#include "base/files/file_util.h" | 
| #include "base/macros.h" | 
| -#include "base/strings/string16.h" | 
| +#include "base/memory/scoped_ptr.h" | 
| +#include "base/strings/pattern.h" | 
| +#include "base/strings/utf_string_conversions.h" | 
| +#include "base/win/scoped_co_mem.h" | 
| +#include "build/build_config.h" | 
| + | 
| +namespace install_static { | 
| ProcessType g_process_type = ProcessType::UNINITIALIZED; | 
| +// Chrome channel display names. | 
| +// TODO(ananta) | 
| +// These constants are defined in the chrome/installer directory as well. We | 
| +// need to unify them. | 
| +const wchar_t kChromeChannelUnknown[] = L"unknown"; | 
| +const wchar_t kChromeChannelCanary[] = L"canary"; | 
| +const wchar_t kChromeChannelDev[] = L"dev"; | 
| +const wchar_t kChromeChannelBeta[] = L"beta"; | 
| +const wchar_t kChromeChannelStable[] = L""; | 
| +const wchar_t kChromeChannelStableExplicit[] = L"stable"; | 
| + | 
| namespace { | 
| const wchar_t kRegPathClientState[] = L"Software\\Google\\Update\\ClientState"; | 
| @@ -28,6 +48,9 @@ const wchar_t kRegValueUsageStats[] = L"usagestats"; | 
| const wchar_t kUninstallArgumentsField[] = L"UninstallArguments"; | 
| const wchar_t kMetricsReportingEnabled[] = L"MetricsReportingEnabled"; | 
| +const wchar_t kRegPathGoogleUpdate[] = L"Software\\Google\\Update"; | 
| +const wchar_t kRegGoogleUpdateVersion[] = L"version"; | 
| + | 
| const wchar_t kAppGuidCanary[] = | 
| L"{4ea16ac7-fd5a-47c3-875b-dbf4a2008c20}"; | 
| const wchar_t kAppGuidGoogleChrome[] = | 
| @@ -35,6 +58,17 @@ const wchar_t kAppGuidGoogleChrome[] = | 
| const wchar_t kAppGuidGoogleBinaries[] = | 
| L"{4DC8B4CA-1BDA-483e-B5FA-D3C12E15B62D}"; | 
| +// TODO(ananta) | 
| +// These constants are defined in the chrome/installer directory as well. We | 
| +// need to unify them. | 
| +const wchar_t kSxSSuffix[] = L" SxS"; | 
| +const wchar_t kGoogleChromeInstallSubDir1[] = L"Google"; | 
| +const wchar_t kGoogleChromeInstallSubDir2[] = L"Chrome"; | 
| +const wchar_t kChromiumInstallSubDir[] = L"Chromium"; | 
| +const wchar_t kUserDataDirname[] = L"User Data"; | 
| +const wchar_t kRegApField[] = L"ap"; | 
| +const wchar_t kBrowserCrashDumpMetricsSubKey[] = L"\\BrowserCrashDumpAttempts"; | 
| + | 
| bool ReadKeyValueString(bool system_install, const wchar_t* key_path, | 
| const wchar_t* guid, const wchar_t* value_to_read, | 
| base::string16* value_out) { | 
| @@ -42,8 +76,10 @@ bool ReadKeyValueString(bool system_install, const wchar_t* key_path, | 
| value_out->clear(); | 
| base::string16 full_key_path(key_path); | 
| - full_key_path.append(1, L'\\'); | 
| - full_key_path.append(guid); | 
| + if (wcslen(guid) > 0) { | 
| + full_key_path.append(1, L'\\'); | 
| + full_key_path.append(guid); | 
| + } | 
| if (::RegOpenKeyEx(system_install ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER, | 
| full_key_path.c_str(), 0, | 
| @@ -107,6 +143,83 @@ bool ReadKeyValueDW(bool system_install, const wchar_t* key_path, | 
| return result == ERROR_SUCCESS && size == sizeof(*value_out); | 
| } | 
| +struct LanguageAndCodePage { | 
| + WORD language; | 
| + WORD code_page; | 
| +}; | 
| + | 
| +bool GetLanguageAndCodePageFromVersionResource(const char* version_resource, | 
| + WORD* language, | 
| + WORD* code_page) { | 
| + if (!version_resource) | 
| + return false; | 
| + | 
| + LanguageAndCodePage* translation_info = nullptr; | 
| + uint32_t page_count = 0; | 
| + BOOL query_result = VerQueryValue(version_resource, | 
| + L"\\VarFileInfo\\Translation", | 
| + reinterpret_cast<void**>(&translation_info), | 
| + &page_count); | 
| + if (!query_result) | 
| + return false; | 
| + | 
| + *language = translation_info->language; | 
| + *code_page = translation_info->code_page; | 
| + return true; | 
| +} | 
| + | 
| +bool GetValueFromVersionResource(const char* version_resource, | 
| + const base::string16& name, | 
| + base::string16* value_str) { | 
| 
robertshield
2016/04/26 18:56:33
DCHECK or return on value_str == nullptr
 
ananta
2016/04/26 20:23:16
Added DCHECK
 | 
| + WORD language = 0; | 
| + WORD code_page = 0; | 
| + if (!GetLanguageAndCodePageFromVersionResource(version_resource, | 
| + &language, | 
| + &code_page)) { | 
| + return false; | 
| + } | 
| + | 
| + WORD lang_codepage[8]; | 
| 
robertshield
2016/04/26 18:56:33
= {}
 
ananta
2016/04/26 20:23:15
Done.
 | 
| + size_t i = 0; | 
| + // Use the language and codepage | 
| + lang_codepage[i++] = language; | 
| + lang_codepage[i++] = code_page; | 
| + // Use the default language and codepage from the resource. | 
| + lang_codepage[i++] = ::GetUserDefaultLangID(); | 
| + lang_codepage[i++] = code_page; | 
| + // Use the language from the resource and Latin codepage (most common). | 
| + lang_codepage[i++] = language; | 
| + lang_codepage[i++] = 1252; | 
| + // Use the default language and Latin codepage (most common). | 
| + lang_codepage[i++] = ::GetUserDefaultLangID(); | 
| + lang_codepage[i++] = 1252; | 
| + | 
| + i = 0; | 
| + while (i < arraysize(lang_codepage)) { | 
| + wchar_t sub_block[MAX_PATH] = {}; | 
| + WORD language = lang_codepage[i++]; | 
| + WORD code_page = lang_codepage[i++]; | 
| 
robertshield
2016/04/26 18:56:33
might want to COMPILE_ASSERT that arraysize(lang_c
 | 
| + _snwprintf_s(sub_block, MAX_PATH, MAX_PATH, | 
| + L"\\StringFileInfo\\%04x%04x\\%ls", language, code_page, name.c_str()); | 
| + LPVOID value = NULL; | 
| 
robertshield
2016/04/26 18:56:33
void*?
 
ananta
2016/04/26 20:23:15
Done.
 | 
| + uint32_t size; | 
| 
robertshield
2016/04/26 18:56:33
= 0
 
ananta
2016/04/26 20:23:16
Done.
 | 
| + BOOL r = ::VerQueryValue(version_resource, sub_block, &value, &size); | 
| + if (r && value) { | 
| + value_str->assign(static_cast<wchar_t*>(value)); | 
| + return true; | 
| + } | 
| + } | 
| + return false; | 
| +} | 
| + | 
| +// Returns the executable path for the current process. | 
| +base::string16 GetCurrentProcessExePath() { | 
| + wchar_t exe_path[MAX_PATH] = {}; | 
| + if (GetModuleFileName(NULL, exe_path, arraysize(exe_path)) == 0) | 
| + return base::string16(); | 
| + return exe_path; | 
| +} | 
| + | 
| } // namespace | 
| bool IsCanary(const wchar_t* exe_path) { | 
| @@ -138,17 +251,19 @@ bool IsMultiInstall(bool is_system_install) { | 
| return args.find(L"--multi-install") != base::string16::npos; | 
| } | 
| -bool AreUsageStatsEnabled(const wchar_t* exe_path) { | 
| +bool AreUsageStatsEnabled() { | 
| 
scottmg
2016/04/26 18:38:02
"UsageStats" sounds a bit vague, I think it'd be g
 
ananta
2016/04/26 20:23:16
Done.
 | 
| bool enabled = true; | 
| bool controlled_by_policy = ReportingIsEnforcedByPolicy(&enabled); | 
| if (controlled_by_policy && !enabled) | 
| return false; | 
| - bool system_install = IsSystemInstall(exe_path); | 
| + base::string16 exe_path = GetCurrentProcessExePath(); | 
| + | 
| + bool system_install = IsSystemInstall(exe_path.c_str()); | 
| base::string16 app_guid; | 
| - if (IsCanary(exe_path)) { | 
| + if (IsCanary(exe_path.c_str())) { | 
| app_guid = kAppGuidCanary; | 
| } else { | 
| app_guid = IsMultiInstall(system_install) ? kAppGuidGoogleBinaries : | 
| @@ -222,3 +337,183 @@ bool IsNonBrowserProcess() { | 
| assert(g_process_type != ProcessType::UNINITIALIZED); | 
| return g_process_type == ProcessType::NON_BROWSER_PROCESS; | 
| } | 
| + | 
| +bool GetDefaultUserDataDirectory(base::FilePath* result) { | 
| + base::win::ScopedCoMem<wchar_t> app_data_path; | 
| + | 
| + if (FAILED(SHGetKnownFolderPath(FOLDERID_LocalAppData, 0, NULL, | 
| 
scottmg
2016/04/26 18:38:02
Isn't shell32 going to pull in user32?
 
robertshield
2016/04/26 18:56:33
indeed, this *should* cause this test to fail: htt
 
ananta
2016/04/26 20:23:15
Yeah. My bad. Changed to use ExpandEnvironmentStri
 | 
| + &app_data_path))) { | 
| + return false; | 
| + } | 
| + | 
| + base::string16 install_sub_directory = GetChromeInstallSubDirectory(); | 
| + | 
| + *result = base::FilePath(app_data_path.get()); | 
| + *result = result->Append(install_sub_directory); | 
| + *result = result->Append(kUserDataDirname); | 
| 
robertshield
2016/04/26 18:56:33
wouldn't *result = base::FilePath(...).Append().Ap
 
ananta
2016/04/26 20:23:15
Done.
 
ananta
2016/04/26 20:23:16
Done.
 | 
| + return true; | 
| +} | 
| + | 
| +bool GetDefaultCrashDumpLocation(base::FilePath* crash_dir) { | 
| + // In order to be able to start crash handling very early, we do not rely on | 
| + // chrome's PathService entries (for DIR_CRASH_DUMPS) being available on | 
| + // Windows. See https://crbug.com/564398. | 
| + if (!GetDefaultUserDataDirectory(crash_dir)) | 
| + return false; | 
| + // We have to make sure the user data dir exists on first run. See | 
| + // http://crbug.com/591504. | 
| + // TODO(ananta) | 
| + // Avoid using base file helpers here. | 
| + if (!base::PathExists(*crash_dir) && !base::CreateDirectory(*crash_dir)) | 
| + return false; | 
| + *crash_dir = crash_dir->Append(FILE_PATH_LITERAL("Crashpad")); | 
| + return true; | 
| +} | 
| + | 
| + | 
| +std::string GetEnvironmentString(const std::string& variable_name) { | 
| + DWORD value_length = ::GetEnvironmentVariable( | 
| + base::UTF8ToWide(variable_name).c_str(), NULL, 0); | 
| + if (value_length == 0) | 
| + return std::string(); | 
| + std::unique_ptr<wchar_t[]> value(new wchar_t[value_length]); | 
| + ::GetEnvironmentVariable(base::UTF8ToWide(variable_name).c_str(), | 
| + value.get(), value_length); | 
| + return base::WideToUTF8(value.get()); | 
| +} | 
| + | 
| +bool SetEnvironmentString(const std::string& variable_name, | 
| + const std::string& new_value) { | 
| + return !!SetEnvironmentVariable(base::UTF8ToWide(variable_name).c_str(), | 
| + base::UTF8ToWide(new_value).c_str()); | 
| +} | 
| + | 
| +bool HasEnvironmentVariable(const std::string& variable_name) { | 
| + return !!::GetEnvironmentVariable(base::UTF8ToWide(variable_name).c_str(), | 
| + NULL, 0); | 
| +} | 
| + | 
| +bool GetExecutableVersionDetails(const base::string16& exe_path, | 
| + base::string16* product_name, | 
| + base::string16* version, | 
| + base::string16* special_build, | 
| + base::string16* channel_name) { | 
| + DCHECK(product_name); | 
| + DCHECK(version); | 
| + DCHECK(special_build); | 
| + DCHECK(channel_name); | 
| + | 
| + // Default values in case we don't find a version resource. | 
| + *product_name = L"Chrome"; | 
| + *version = L"0.0.0.0-devel"; | 
| + | 
| + DWORD dummy = 0; | 
| + DWORD length = ::GetFileVersionInfoSize(exe_path.c_str(), &dummy); | 
| + if (length) { | 
| + scoped_ptr<char> data(new char[length]); | 
| + if (!::GetFileVersionInfo(exe_path.c_str(), dummy, length, | 
| + data.get())) { | 
| + return false; | 
| 
scottmg
2016/04/26 18:51:30
If this returns false, then special_build and chan
 
ananta
2016/04/26 20:23:15
Fixed. The special_build parameter was uninitializ
 | 
| + } | 
| + GetValueFromVersionResource(data.get(), L"ProductVersion", version); | 
| + base::string16 official_build; | 
| + GetValueFromVersionResource(data.get(), L"Official Build", | 
| + &official_build); | 
| + bool is_official_build = (official_build.compare(L"1") == 0); | 
| 
scottmg
2016/04/26 18:51:30
nit; remove unnecessary () around the expression o
 
scottmg
2016/04/26 18:51:30
Or how about just official_build == "1"?
 
ananta
2016/04/26 20:23:16
Ignore this. Changed to == 1
 
ananta
2016/04/26 20:23:16
Done.
 | 
| + if (!is_official_build) | 
| + version->append(L"-devel"); | 
| + GetValueFromVersionResource(data.get(), L"ProductShortName", product_name); | 
| + GetValueFromVersionResource(data.get(), L"SpecialBuild", special_build); | 
| + } | 
| + GetChromeChannelName(!IsSystemInstall(exe_path.c_str()), channel_name); | 
| + return true; | 
| +} | 
| + | 
| +void GetChromeChannelName(bool is_per_user_install, | 
| + base::string16* channel_name) { | 
| +#if !defined(GOOGLE_CHROME_BUILD) | 
| + *channel_name = kChromeChannelUnknown; | 
| + return; | 
| +#else | 
| + // TODO(ananta) | 
| + // Unify this with the chrome/installer/util/channel_info.h/.cc" | 
| 
scottmg
2016/04/26 18:38:02
nit; . not " at end of line.
 
ananta
2016/04/26 20:23:16
Done.
 | 
| + if (IsCanary(GetCurrentProcessExePath().c_str())) { | 
| + *channel_name = L"canary"; | 
| + } else { | 
| + base::string16 value; | 
| + if (ReadKeyValueString(!is_per_user_install, | 
| + kRegPathClientState, | 
| + kAppGuidGoogleChrome, | 
| + kRegApField, | 
| + &value)) { | 
| + static const wchar_t kChromeChannelBetaPattern[] = L"1.1-"; | 
| + static const wchar_t kChromeChannelBetaX64Pattern[] = L"x64-beta"; | 
| + static const wchar_t kChromeChannelDevPattern[] = L"2.0-d"; | 
| + static const wchar_t kChromeChannelDevX64Pattern[] = L"x64-dev"; | 
| + static const wchar_t kChromeStableMultiInstallPattern[] = L"*multi*"; | 
| + | 
| + // Channel names containing stable should be reported as an empty string. | 
| + if (value.find(kChromeChannelStableExplicit) != | 
| + base::string16::npos) { | 
| + return; | 
| 
scottmg
2016/04/26 18:51:30
channel_name->erase() here at the top of the funct
 
ananta
2016/04/26 20:23:15
Done.
 | 
| + } | 
| + | 
| + if (base::MatchPattern(value, kChromeChannelDevPattern) || | 
| 
scottmg
2016/04/26 18:51:30
Looks like channel_info is case-insensitive too, i
 
scottmg
2016/04/26 18:51:30
In channel_info.cc, the 2.0-d has to be at the beg
 
ananta
2016/04/26 20:23:15
Its not. Thanks for finding that. Changed value to
 
ananta
2016/04/26 20:23:15
The FindSubstringMatch function just does a patter
 | 
| + base::MatchPattern(value, kChromeChannelDevX64Pattern)) { | 
| 
scottmg
2016/04/26 18:51:30
Do the patterns need to be "*x64-dev*", i.e. with
 
ananta
2016/04/26 20:23:15
Yeah. Done.
 | 
| + channel_name->assign(kChromeChannelDev); | 
| + } | 
| + | 
| + if (base::MatchPattern(value, kChromeChannelBetaPattern) || | 
| 
scottmg
2016/04/26 18:51:30
Same here as for dev above.
 | 
| + base::MatchPattern(value, kChromeChannelBetaPattern)) { | 
| + channel_name->assign(kChromeChannelBeta); | 
| + } | 
| + | 
| + if (value.find(kChromeStableMultiInstallPattern) != | 
| + base::string16::npos) { | 
| + *channel_name = L"-m"; | 
| + } | 
| + *channel_name = value; | 
| 
scottmg
2016/04/26 18:51:30
It looks like channel_info does some stuff with st
 
ananta
2016/04/26 20:23:16
Added a comment for investigation. At the moment w
 | 
| + } else { | 
| + *channel_name = kChromeChannelUnknown; | 
| + } | 
| + } | 
| +#endif // GOOGLE_CHROME_BUILD | 
| +} | 
| + | 
| +base::Version GetGoogleUpdateVersion() { | 
| + base::string16 update_version; | 
| + if (ReadKeyValueString(IsSystemInstall(GetCurrentProcessExePath().c_str()), | 
| + kRegPathGoogleUpdate, | 
| + L"", | 
| + kRegGoogleUpdateVersion, | 
| + &update_version)) { | 
| + return base::Version(base::UTF16ToUTF8(update_version)); | 
| + } | 
| + return base::Version(); | 
| +} | 
| + | 
| +base::string16 GetChromeInstallSubDirectory() { | 
| + base::FilePath result; | 
| +#if defined(GOOGLE_CHROME_BUILD) | 
| + base::string16 sub_directory = kGoogleChromeInstallSubDir1; | 
| + sub_directory += L"\\"; | 
| + sub_directory += kGoogleChromeInstallSubDir2; | 
| + if (IsCanary(GetCurrentProcessExePath().c_str())) | 
| + sub_directory += kSxSSuffix; | 
| + result = result.Append(sub_directory); | 
| +#else | 
| + result = result.Append(kChromiumInstallSubDir); | 
| +#endif | 
| + return result.value(); | 
| +} | 
| + | 
| +base::string16 GetBrowserCrashDumpAttemptsRegistryPath() { | 
| + base::string16 registry_path = L"Software"; | 
| + base::string16 install_sub_directory = GetChromeInstallSubDirectory(); | 
| + registry_path += install_sub_directory; | 
| + registry_path += kBrowserCrashDumpMetricsSubKey; | 
| + return registry_path; | 
| +} | 
| + | 
| + | 
| 
scottmg
2016/04/26 18:38:02
extra \n
 
ananta
2016/04/26 20:23:16
Done.
 | 
| +} // namespace install_static |