diff options
author | robertshield@google.com <robertshield@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-03-04 03:22:32 +0000 |
---|---|---|
committer | robertshield@google.com <robertshield@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-03-04 03:22:32 +0000 |
commit | 0bb1a6204af17f50ad0577f811a2c044b2bf62ff (patch) | |
tree | a300fc9b01eaddd694d6fb58322862719c5661e7 /chrome/installer | |
parent | bff6ef77683a85e61c5a916ade6c2f85b29e6f62 (diff) | |
download | chromium_src-0bb1a6204af17f50ad0577f811a2c044b2bf62ff.zip chromium_src-0bb1a6204af17f50ad0577f811a2c044b2bf62ff.tar.gz chromium_src-0bb1a6204af17f50ad0577f811a2c044b2bf62ff.tar.bz2 |
Add a set of long-running metrics to Chrome that are sent up only at uninstall time via the uninstall survey page.These uninstall metrics are collected according to the same opt-in policy as the existing UMA code. They are stored along with other prefs in the browser's Local State file. At uninstall time, the Local State file is copied to a temporary location during the file deletion stage and then read to extract the uninstall metrics. If the user selected to have metrics reported, the uninstall metrics are then sent up to the uninstall survey page that is currently opened.
Review URL: http://codereview.chromium.org/27092
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@10859 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/installer')
-rw-r--r-- | chrome/installer/setup/uninstall.cc | 35 | ||||
-rw-r--r-- | chrome/installer/util/browser_distribution.cc | 2 | ||||
-rw-r--r-- | chrome/installer/util/browser_distribution.h | 3 | ||||
-rw-r--r-- | chrome/installer/util/google_chrome_distribution.cc | 99 | ||||
-rw-r--r-- | chrome/installer/util/google_chrome_distribution.h | 30 | ||||
-rw-r--r-- | chrome/installer/util/google_chrome_distribution_unittest.cc | 49 | ||||
-rw-r--r-- | chrome/installer/util/util.vcproj | 8 | ||||
-rw-r--r-- | chrome/installer/util/util_constants.cc | 4 | ||||
-rw-r--r-- | chrome/installer/util/util_constants.h | 2 |
9 files changed, 213 insertions, 19 deletions
diff --git a/chrome/installer/setup/uninstall.cc b/chrome/installer/setup/uninstall.cc index 69b5a5d..2fb0815 100644 --- a/chrome/installer/setup/uninstall.cc +++ b/chrome/installer/setup/uninstall.cc @@ -11,6 +11,8 @@ #include "base/registry.h" #include "base/string_util.h" #include "chrome/common/result_codes.h" +#include "chrome/common/chrome_constants.h" +#include "chrome/common/chrome_paths_internal.h" #include "chrome/installer/setup/setup.h" #include "chrome/installer/setup/setup_constants.h" #include "chrome/installer/util/browser_distribution.h" @@ -45,7 +47,8 @@ void CloseAllChromeProcesses() { } // If asking politely didn't work, wait for 15 seconds and then kill all - // chrome.exe. This check is just in case Chrome is ignoring WM_CLOSE messages. + // chrome.exe. This check is just in case Chrome is ignoring WM_CLOSE + // messages. base::CleanupProcesses(installer_util::kChromeExe, 15000, ResultCodes::HUNG, NULL); } @@ -83,15 +86,17 @@ void DeleteChromeShortcut(bool system_uninstall) { // installation folder, in all other cases it returns true even in case // of error (only logs the error). bool DeleteFilesAndFolders(const std::wstring& exe_path, bool system_uninstall, - const installer::Version& installed_version) { + const installer::Version& installed_version, + std::wstring* local_state_path) { std::wstring install_path(installer::GetChromeInstallPath(system_uninstall)); if (install_path.empty()) { LOG(ERROR) << "Could not get installation destination path."; - return false; // Nothing else we can do for uninstall, so we return. + return false; // Nothing else we can do for uninstall, so we return. } else { LOG(INFO) << "install destination path: " << install_path; } + // Move setup.exe to the temp path. std::wstring setup_exe(installer::GetInstallerPathUnderChrome( install_path, installed_version.GetString())); file_util::AppendToPath(&setup_exe, file_util::GetFilenameFromPath(exe_path)); @@ -100,6 +105,16 @@ bool DeleteFilesAndFolders(const std::wstring& exe_path, bool system_uninstall, file_util::CreateTemporaryFileName(&temp_file); file_util::Move(setup_exe, temp_file); + // Move the browser's persisted local state + FilePath user_local_state; + if (chrome::GetDefaultUserDataDirectory(&user_local_state)) { + std::wstring user_local_file( + user_local_state.Append(chrome::kLocalStateFilename).value()); + + file_util::CreateTemporaryFileName(local_state_path); + file_util::CopyFile(user_local_file, *local_state_path); + } + LOG(INFO) << "Deleting install path " << install_path; if (!file_util::Delete(install_path, true)) { LOG(ERROR) << "Failed to delete folder (1st try): " << install_path; @@ -269,13 +284,21 @@ installer_util::InstallStatus installer_setup::UninstallChrome( } // Finally delete all the files from Chrome folder after moving setup.exe - // to a temp location. - if (!DeleteFilesAndFolders(exe_path, system_uninstall, installed_version)) + // and the user's Local State to a temp location. + std::wstring local_state_path; + if (!DeleteFilesAndFolders(exe_path, system_uninstall, installed_version, + &local_state_path)) return installer_util::UNINSTALL_FAILED; if (!force_uninstall) { LOG(INFO) << "Uninstallation complete. Launching Uninstall survey."; - dist->DoPostUninstallOperations(installed_version); + dist->DoPostUninstallOperations(installed_version, local_state_path); } + + // Try and delete the preserved local state once the post-install + // operations are complete. + if (!local_state_path.empty()) + file_util::Delete(local_state_path, false); + return installer_util::UNINSTALL_SUCCESSFUL; } diff --git a/chrome/installer/util/browser_distribution.cc b/chrome/installer/util/browser_distribution.cc index 64a5ef3..4d52c8c 100644 --- a/chrome/installer/util/browser_distribution.cc +++ b/chrome/installer/util/browser_distribution.cc @@ -23,7 +23,7 @@ BrowserDistribution* BrowserDistribution::GetDistribution() { } void BrowserDistribution::DoPostUninstallOperations( - const installer::Version& version) { + const installer::Version& version, const std::wstring& local_data_path) { } std::wstring BrowserDistribution::GetApplicationName() { diff --git a/chrome/installer/util/browser_distribution.h b/chrome/installer/util/browser_distribution.h index 3ce15da..f48dcd8 100644 --- a/chrome/installer/util/browser_distribution.h +++ b/chrome/installer/util/browser_distribution.h @@ -17,7 +17,8 @@ class BrowserDistribution { static BrowserDistribution* GetDistribution(); - virtual void DoPostUninstallOperations(const installer::Version& version); + virtual void DoPostUninstallOperations(const installer::Version& version, + const std::wstring& local_data_path); virtual std::wstring GetApplicationName(); diff --git a/chrome/installer/util/google_chrome_distribution.cc b/chrome/installer/util/google_chrome_distribution.cc index d477623..d70bcd2 100644 --- a/chrome/installer/util/google_chrome_distribution.cc +++ b/chrome/installer/util/google_chrome_distribution.cc @@ -16,9 +16,12 @@ #include "base/registry.h" #include "base/string_util.h" #include "base/wmi_util.h" +#include "chrome/common/json_value_serializer.h" +#include "chrome/common/pref_names.h" #include "chrome/installer/util/l10n_string_util.h" #include "chrome/installer/util/google_update_constants.h" #include "chrome/installer/util/google_update_settings.h" +#include "chrome/installer/util/util_constants.h" #include "installer_util_strings.h" @@ -27,7 +30,8 @@ namespace { // Google Update tells us is the locale. In case we fail to find // the locale, we use US English. std::wstring GetUninstallSurveyUrl() { - std::wstring kSurveyUrl = L"http://www.google.com/support/chrome/bin/request.py?hl=$1&contact_type=uninstall"; + std::wstring kSurveyUrl = L"http://www.google.com/support/chrome/bin/" + L"request.py?hl=$1&contact_type=uninstall"; std::wstring language; if (!GoogleUpdateSettings::GetLanguage(&language)) @@ -37,8 +41,85 @@ std::wstring GetUninstallSurveyUrl() { } } +bool GoogleChromeDistribution::BuildUninstallMetricsString( + DictionaryValue* uninstall_metrics_dict, std::wstring* metrics) { + DCHECK(NULL != metrics); + bool has_values = false; + + DictionaryValue::key_iterator iter(uninstall_metrics_dict->begin_keys()); + for (; iter != uninstall_metrics_dict->end_keys(); ++iter) { + has_values = true; + metrics->append(L"&"); + metrics->append(*iter); + metrics->append(L"="); + + std::wstring value; + uninstall_metrics_dict->GetString(*iter, &value); + metrics->append(value); + } + + return has_values; +} + +bool GoogleChromeDistribution::ExtractUninstallMetricsFromFile( + const std::wstring& file_path, std::wstring* uninstall_metrics_string) { + + JSONFileValueSerializer json_serializer(file_path); + + std::string json_error_string; + scoped_ptr<Value> root(json_serializer.Deserialize(NULL)); + + // Preferences should always have a dictionary root. + if (!root->IsType(Value::TYPE_DICTIONARY)) + return false; + + return ExtractUninstallMetrics(*static_cast<DictionaryValue*>(root.get()), + uninstall_metrics_string); +} + +bool GoogleChromeDistribution::ExtractUninstallMetrics( + const DictionaryValue& root, std::wstring* uninstall_metrics_string) { + // Make sure that the user wants us reporting metrics. If not, don't + // add our uninstall metrics. + bool metrics_reporting_enabled = false; + if (!root.GetBoolean(prefs::kMetricsReportingEnabled, + &metrics_reporting_enabled) || + !metrics_reporting_enabled) { + return false; + } + + DictionaryValue* uninstall_metrics_dict; + if (!root.HasKey(installer_util::kUninstallMetricsName) || + !root.GetDictionary(installer_util::kUninstallMetricsName, + &uninstall_metrics_dict)) { + return false; + } + + if (!BuildUninstallMetricsString(uninstall_metrics_dict, + uninstall_metrics_string)) { + return false; + } + + // We use the creation date of the metric's client id as the installation + // date, as this is fairly close to the first-run date for those who + // opt-in to metrics from the get-go. Slightly more accurate would be to + // have setup.exe write something somewhere at installation time. + // As is, the absence of this field implies that the browser was never + // run with an opt-in to metrics selection. + std::wstring installation_date; + if (root.GetString(prefs::kMetricsClientIDTimestamp, &installation_date)) { + uninstall_metrics_string->append(L"&"); + uninstall_metrics_string->append( + installer_util::kUninstallInstallationDate); + uninstall_metrics_string->append(L"="); + uninstall_metrics_string->append(installation_date); + } + + return true; +} + void GoogleChromeDistribution::DoPostUninstallOperations( - const installer::Version& version) { + const installer::Version& version, const std::wstring& local_data_path) { // Send the Chrome version and OS version as params to the form. // It would be nice to send the locale, too, but I don't see an // easy way to get that in the existing code. It's something we @@ -67,6 +148,12 @@ void GoogleChromeDistribution::DoPostUninstallOperations( std::wstring command = iexplore + L" " + GetUninstallSurveyUrl() + L"&" + kVersionParam + L"=" + kVersion + L"&" + kOSParam + L"=" + os_version; + + std::wstring uninstall_metrics; + if (ExtractUninstallMetricsFromFile(local_data_path, &uninstall_metrics)) { + command += uninstall_metrics; + } + int pid = 0; WMIProcessUtil::Launch(command, &pid); } @@ -81,8 +168,9 @@ std::wstring GoogleChromeDistribution::GetInstallSubDir() { return L"Google\\Chrome"; } -std::wstring GoogleChromeDistribution::GetNewGoogleUpdateApKey(bool diff_install, - installer_util::InstallStatus status, const std::wstring& value) { +std::wstring GoogleChromeDistribution::GetNewGoogleUpdateApKey( + bool diff_install, installer_util::InstallStatus status, + const std::wstring& value) { // Magic suffix that we need to add or remove to "ap" key value. const std::wstring kMagicSuffix = L"-full"; @@ -171,7 +259,7 @@ void GoogleChromeDistribution::UpdateDiffInstallStatus(bool system_install, std::wstring reg_key(google_update::kRegPathClientState); reg_key.append(L"\\"); reg_key.append(google_update::kChromeGuid); - if (!key.Open(reg_root, reg_key.c_str(), KEY_ALL_ACCESS) || + if (!key.Open(reg_root, reg_key.c_str(), KEY_ALL_ACCESS) || !key.ReadValue(google_update::kRegApField, &ap_key_value)) { LOG(INFO) << "Application key not found."; if (!incremental_install || !GetInstallReturnCode(install_status)) { @@ -198,4 +286,3 @@ void GoogleChromeDistribution::UpdateDiffInstallStatus(bool system_install, } key.Close(); } - diff --git a/chrome/installer/util/google_chrome_distribution.h b/chrome/installer/util/google_chrome_distribution.h index 69227d4..f0bc8bd 100644 --- a/chrome/installer/util/google_chrome_distribution.h +++ b/chrome/installer/util/google_chrome_distribution.h @@ -11,9 +11,14 @@ #include "chrome/installer/util/browser_distribution.h" #include "chrome/installer/util/util_constants.h" +#include "testing/gtest/include/gtest/gtest_prod.h" // for FRIEND_TEST + +class DictionaryValue; + class GoogleChromeDistribution : public BrowserDistribution { public: - virtual void DoPostUninstallOperations(const installer::Version& version); + virtual void DoPostUninstallOperations(const installer::Version& version, + const std::wstring& local_data_path); virtual std::wstring GetApplicationName(); @@ -54,7 +59,28 @@ class GoogleChromeDistribution : public BrowserDistribution { private: friend class BrowserDistribution; - + FRIEND_TEST(GoogleChromeDistributionTest, TestExtractUninstallMetrics); + + // Extracts uninstall metrics from the JSON file located at file_path. + // Returns them in a form suitable for appending to a url that already + // has GET parameters, i.e. &metric1=foo&metric2=bar. + // Returns true if uninstall_metrics has been successfully populated with + // the uninstall metrics, false otherwise. + virtual bool ExtractUninstallMetricsFromFile( + const std::wstring& file_path, std::wstring* uninstall_metrics); + + // Extracts uninstall metrics from the given JSON value. + virtual bool ExtractUninstallMetrics(const DictionaryValue& root, + std::wstring* uninstall_metrics); + + // Given a DictionaryValue containing a set of uninstall metrics, + // this builds a URL parameter list of all the contained metrics. + // Returns true if at least one uninstall metric was found in + // uninstall_metrics_dict, false otherwise. + virtual bool BuildUninstallMetricsString( + DictionaryValue* uninstall_metrics_dict, std::wstring* metrics); + + // Disallow construction from non-friends. GoogleChromeDistribution() {} }; diff --git a/chrome/installer/util/google_chrome_distribution_unittest.cc b/chrome/installer/util/google_chrome_distribution_unittest.cc index 063377a..de437f4 100644 --- a/chrome/installer/util/google_chrome_distribution_unittest.cc +++ b/chrome/installer/util/google_chrome_distribution_unittest.cc @@ -9,6 +9,7 @@ #include "base/registry.h" #include "base/scoped_ptr.h" #include "base/file_util.h" +#include "chrome/common/json_value_serializer.h" #include "chrome/installer/util/browser_distribution.h" #include "chrome/installer/util/google_update_constants.h" #include "chrome/installer/util/google_chrome_distribution.h" @@ -28,7 +29,7 @@ class GoogleChromeDistributionTest : public testing::Test { } // Creates "ap" key with the value given as parameter. Also adds work - // items to work_item_list given so that they can be rollbed back later. + // items to work_item_list given so that they can be rolled back later. bool CreateApKey(WorkItemList* work_item_list, std::wstring value) { HKEY reg_root = HKEY_CURRENT_USER; std::wstring reg_key = GetApKeyPath(); @@ -155,7 +156,7 @@ TEST_F(GoogleChromeDistributionTest, UpdateDiffInstallStatusTest) { HKEY reg_root = HKEY_CURRENT_USER; bool ap_key_deleted = false; RegKey key; - if (!key.Open(HKEY_CURRENT_USER, reg_key.c_str(), KEY_ALL_ACCESS)){ + if (!key.Open(HKEY_CURRENT_USER, reg_key.c_str(), KEY_ALL_ACCESS)) { work_item_list->AddCreateRegKeyWorkItem(reg_root, reg_key); if (!work_item_list->Do()) FAIL() << "Failed to create ClientState key."; @@ -180,8 +181,52 @@ TEST_F(GoogleChromeDistributionTest, UpdateDiffInstallStatusTest) { if (!CreateApKey(work_item_list.get(), ap_key_value)) FAIL() << "Failed to restore ap key."; } +} + +TEST_F(GoogleChromeDistributionTest, TestExtractUninstallMetrics) { + // A make-believe JSON preferences file. + std::string pref_string( + "{ \n" + " \"foo\": \"bar\",\n" + " \"uninstall_metrics\": { \n" + " \"last_launch_time_sec\": \"1235341118\"," + " \"last_observed_running_time_sec\": \"1235341183\"," + " \"launch_count\": \"11\"," + " \"page_load_count\": \"68\"," + " \"uptime_sec\": \"809\"\n" + " },\n" + " \"blah\": {\n" + " \"this_sentence_is_true\": false\n" + " },\n" + " \"user_experience_metrics\": { \n" + " \"client_id_timestamp\": \"1234567890\"," + " \"reporting_enabled\": true\n" + " }\n" + "} \n"); + + // The URL string we expect to be generated from said make-believe file. + std::wstring expected_url_string( + L"&last_launch_time_sec=1235341118" + L"&last_observed_running_time_sec=1235341183" + L"&launch_count=11&page_load_count=68&uptime_sec=809&"); + expected_url_string += installer_util::kUninstallInstallationDate; + expected_url_string += L"=1234567890"; + + JSONStringValueSerializer json_deserializer(pref_string); + std::string error_message; + scoped_ptr<Value> root(json_deserializer.Deserialize(&error_message)); + ASSERT_TRUE(root.get()); + + std::wstring uninstall_metrics_string; + GoogleChromeDistribution* dist = static_cast<GoogleChromeDistribution*>( + BrowserDistribution::GetDistribution()); + EXPECT_TRUE( + dist->ExtractUninstallMetrics(*static_cast<DictionaryValue*>(root.get()), + &uninstall_metrics_string)); + EXPECT_EQ(expected_url_string, uninstall_metrics_string); } + #endif TEST(MasterPreferences, ParseDistroParams) { diff --git a/chrome/installer/util/util.vcproj b/chrome/installer/util/util.vcproj index 9c7547f..99c7fe3 100644 --- a/chrome/installer/util/util.vcproj +++ b/chrome/installer/util/util.vcproj @@ -225,6 +225,10 @@ > </File> <File + RelativePath="..\..\common\json_value_serializer.cc" + > + </File> + <File RelativePath=".\l10n_string_util.cc" > </File> @@ -257,6 +261,10 @@ > </File> <File + RelativePath="..\..\common\pref_names.cc" + > + </File> + <File RelativePath=".\set_reg_value_work_item.cc" > </File> diff --git a/chrome/installer/util/util_constants.cc b/chrome/installer/util/util_constants.cc index ab695e1..e10a284 100644 --- a/chrome/installer/util/util_constants.cc +++ b/chrome/installer/util/util_constants.cc @@ -41,7 +41,7 @@ const wchar_t kLogFile[] = L"log-file"; // Register Chrome as default browser on the system. Usually this will require // that setup is running as admin. If running as admin we try to register // as default browser at system level, if running as non-admin we try to -// register as default browser only for the current user. +// register as default browser only for the current user. const wchar_t kMakeChromeDefault[] = L"make-chrome-default"; // Register Chrome as a valid browser on the current sytem. This option @@ -81,4 +81,6 @@ const wchar_t kInstallerDir[] = L"Installer"; const wchar_t kUninstallStringField[] = L"UninstallString"; const wchar_t kUninstallDisplayNameField[] = L"DisplayName"; +const wchar_t kUninstallMetricsName[] = L"uninstall_metrics"; +const wchar_t kUninstallInstallationDate[] = L"installation_date"; } // namespace installer_util diff --git a/chrome/installer/util/util_constants.h b/chrome/installer/util/util_constants.h index 049fdac..246a50f 100644 --- a/chrome/installer/util/util_constants.h +++ b/chrome/installer/util/util_constants.h @@ -93,6 +93,8 @@ extern const wchar_t kInstallerDir[]; extern const wchar_t kUninstallStringField[]; extern const wchar_t kUninstallDisplayNameField[]; +extern const wchar_t kUninstallMetricsName[]; +extern const wchar_t kUninstallInstallationDate[]; } // namespace installer_util #endif // CHROME_INSTALLER_UTIL_UTIL_CONSTANTS_H__ |