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 | |
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')
-rw-r--r-- | chrome/browser/metrics/metrics_service.cc | 57 | ||||
-rw-r--r-- | chrome/browser/metrics/metrics_service.h | 4 | ||||
-rw-r--r-- | chrome/common/pref_names.cc | 12 | ||||
-rw-r--r-- | chrome/common/pref_names.h | 7 | ||||
-rw-r--r-- | chrome/common/pref_service.cc | 47 | ||||
-rw-r--r-- | chrome/common/pref_service.h | 7 | ||||
-rw-r--r-- | chrome/common/pref_service_unittest.cc | 8 | ||||
-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 | ||||
-rw-r--r-- | chrome/test/data/pref_service/write.golden.json | 3 |
17 files changed, 339 insertions, 38 deletions
diff --git a/chrome/browser/metrics/metrics_service.cc b/chrome/browser/metrics/metrics_service.cc index e081bd9..220102f 100644 --- a/chrome/browser/metrics/metrics_service.cc +++ b/chrome/browser/metrics/metrics_service.cc @@ -303,10 +303,10 @@ class MetricsService::GetPluginListTask : public Task { void MetricsService::RegisterPrefs(PrefService* local_state) { DCHECK(IsSingleThreaded()); local_state->RegisterStringPref(prefs::kMetricsClientID, L""); - local_state->RegisterStringPref(prefs::kMetricsClientIDTimestamp, L"0"); - local_state->RegisterStringPref(prefs::kStabilityLaunchTimeSec, L"0"); - local_state->RegisterStringPref(prefs::kStabilityLastTimestampSec, L"0"); - local_state->RegisterStringPref(prefs::kStabilityUptimeSec, L"0"); + local_state->RegisterInt64Pref(prefs::kMetricsClientIDTimestamp, 0); + local_state->RegisterInt64Pref(prefs::kStabilityLaunchTimeSec, 0); + local_state->RegisterInt64Pref(prefs::kStabilityLastTimestampSec, 0); + local_state->RegisterInt64Pref(prefs::kStabilityUptimeSec, 0); local_state->RegisterStringPref(prefs::kStabilityStatsVersion, L""); local_state->RegisterBooleanPref(prefs::kStabilityExitedCleanly, true); local_state->RegisterBooleanPref(prefs::kStabilitySessionEndCompleted, true); @@ -336,6 +336,12 @@ void MetricsService::RegisterPrefs(PrefService* local_state) { local_state->RegisterIntegerPref(prefs::kNumKeywords, 0); local_state->RegisterListPref(prefs::kMetricsInitialLogs); local_state->RegisterListPref(prefs::kMetricsOngoingLogs); + + local_state->RegisterInt64Pref(prefs::kUninstallMetricsPageLoadCount, 0); + local_state->RegisterInt64Pref(prefs::kUninstallLaunchCount, 0); + local_state->RegisterInt64Pref(prefs::kUninstallMetricsUptimeSec, 0); + local_state->RegisterInt64Pref(prefs::kUninstallLastLaunchTimeSec, 0); + local_state->RegisterInt64Pref(prefs::kUninstallLastObservedRunTimeSec, 0); } // static @@ -592,8 +598,7 @@ void MetricsService::InitializeMetricsState() { pref->SetString(prefs::kMetricsClientID, UTF8ToWide(client_id_)); // Might as well make a note of how long this ID has existed - pref->SetString(prefs::kMetricsClientIDTimestamp, - Int64ToWString(Time::Now().ToTimeT())); + pref->SetInt64(prefs::kMetricsClientIDTimestamp, Time::Now().ToTimeT()); } // Update session ID @@ -617,20 +622,29 @@ void MetricsService::InitializeMetricsState() { // This is marked false when we get a WM_ENDSESSION. pref->SetBoolean(prefs::kStabilitySessionEndCompleted, true); - int64 last_start_time = StringToInt64( - WideToUTF16Hack(pref->GetString(prefs::kStabilityLaunchTimeSec))); - int64 last_end_time = StringToInt64( - WideToUTF16Hack(pref->GetString(prefs::kStabilityLastTimestampSec))); - int64 uptime = StringToInt64( - WideToUTF16Hack(pref->GetString(prefs::kStabilityUptimeSec))); + int64 last_start_time = pref->GetInt64(prefs::kStabilityLaunchTimeSec); + int64 last_end_time = pref->GetInt64(prefs::kStabilityLastTimestampSec); + int64 uptime = pref->GetInt64(prefs::kStabilityUptimeSec); + + // Same idea as uptime, except this one never gets reset and is used at + // uninstallation. + int64 uninstall_metrics_uptime = + pref->GetInt64(prefs::kUninstallMetricsUptimeSec); if (last_start_time && last_end_time) { // TODO(JAR): Exclude sleep time. ... which must be gathered in UI loop. - uptime += last_end_time - last_start_time; - pref->SetString(prefs::kStabilityUptimeSec, Int64ToWString(uptime)); + int64 uptime_increment = last_end_time - last_start_time; + uptime += uptime_increment; + pref->SetInt64(prefs::kStabilityUptimeSec, uptime); + + uninstall_metrics_uptime += uptime_increment; + pref->SetInt64(prefs::kUninstallMetricsUptimeSec, + uninstall_metrics_uptime); } - pref->SetString(prefs::kStabilityLaunchTimeSec, - Int64ToWString(Time::Now().ToTimeT())); + pref->SetInt64(prefs::kStabilityLaunchTimeSec, Time::Now().ToTimeT()); + + // Bookkeeping for the uninstall metrics. + IncrementLongPrefsValue(prefs::kUninstallLaunchCount); // Save profile metrics. PrefService* prefs = g_browser_process->local_state(); @@ -1562,8 +1576,16 @@ void MetricsService::IncrementPrefValue(const wchar_t* path) { pref->SetInteger(path, value + 1); } +void MetricsService::IncrementLongPrefsValue(const wchar_t* path) { + PrefService* pref = g_browser_process->local_state(); + DCHECK(pref); + int64 value = pref->GetInt64(path); + pref->SetInt64(path, value+1); +} + void MetricsService::LogLoadStarted() { IncrementPrefValue(prefs::kStabilityPageLoadCount); + IncrementLongPrefsValue(prefs::kUninstallMetricsPageLoadCount); // We need to save the prefs, as page load count is a critical stat, and it // might be lost due to a crash :-(. } @@ -1750,8 +1772,7 @@ void MetricsService::RecordBooleanPrefValue(const wchar_t* path, bool value) { } void MetricsService::RecordCurrentState(PrefService* pref) { - pref->SetString(prefs::kStabilityLastTimestampSec, - Int64ToWString(Time::Now().ToTimeT())); + pref->SetInt64(prefs::kStabilityLastTimestampSec, Time::Now().ToTimeT()); RecordPluginChanges(pref); } diff --git a/chrome/browser/metrics/metrics_service.h b/chrome/browser/metrics/metrics_service.h index 58c491a..d5fa3c1 100644 --- a/chrome/browser/metrics/metrics_service.h +++ b/chrome/browser/metrics/metrics_service.h @@ -303,6 +303,10 @@ class MetricsService : public NotificationObserver, // Reads, increments and then sets the specified integer preference. void IncrementPrefValue(const wchar_t* path); + // Reads, increments and then sets the specified long preference that is + // stored as a string. + void IncrementLongPrefsValue(const wchar_t* path); + // Records a renderer process crash. void LogRendererCrash(); diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc index bda65f9..e120347 100644 --- a/chrome/common/pref_names.cc +++ b/chrome/common/pref_names.cc @@ -369,6 +369,18 @@ const wchar_t kStabilityPluginLaunches[] = L"launches"; const wchar_t kStabilityPluginInstances[] = L"instances"; const wchar_t kStabilityPluginCrashes[] = L"crashes"; +// The keys below are strictly increasing counters over the lifetime of +// a chrome installation. They are (optionally) sent up to the uninstall +// survey in the event of uninstallation. +const wchar_t kUninstallMetricsPageLoadCount[] = + L"uninstall_metrics.page_load_count"; +const wchar_t kUninstallLaunchCount[] = L"uninstall_metrics.launch_count"; +const wchar_t kUninstallMetricsUptimeSec[] = L"uninstall_metrics.uptime_sec"; +const wchar_t kUninstallLastLaunchTimeSec[] = + L"uninstall_metrics.last_launch_time_sec"; +const wchar_t kUninstallLastObservedRunTimeSec[] = + L"uninstall_metrics.last_observed_running_time_sec"; + // If true, the user will be prompted to manually launch renderer processes. const wchar_t kStartRenderersManually[] = L"renderer.start_manually"; diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h index 49367fd..718fb9a 100644 --- a/chrome/common/pref_names.h +++ b/chrome/common/pref_names.h @@ -123,6 +123,13 @@ extern const wchar_t kStabilityPluginLaunches[]; extern const wchar_t kStabilityPluginInstances[]; extern const wchar_t kStabilityPluginCrashes[]; +extern const wchar_t kUninstallMetricsPageLoadCount[]; +extern const wchar_t kUninstallLaunchCount[]; + +extern const wchar_t kUninstallMetricsUptimeSec[]; +extern const wchar_t kUninstallLastLaunchTimeSec[]; +extern const wchar_t kUninstallLastObservedRunTimeSec[]; + extern const wchar_t kStartRenderersManually[]; extern const wchar_t kBrowserWindowPlacement[]; extern const wchar_t kTaskManagerWindowPlacement[]; diff --git a/chrome/common/pref_service.cc b/chrome/common/pref_service.cc index fd6cef7..b5bbb55 100644 --- a/chrome/common/pref_service.cc +++ b/chrome/common/pref_service.cc @@ -610,6 +610,53 @@ void PrefService::SetFilePath(const wchar_t* path, const FilePath& value) { FireObserversIfChanged(path, old_value.get()); } +void PrefService::SetInt64(const wchar_t* path, int64 value) { + DCHECK(CalledOnValidThread()); + + const Preference* pref = FindPreference(path); + if (!pref) { + DCHECK(false) << "Trying to write an unregistered pref: " << path; + return; + } + if (pref->type() != Value::TYPE_STRING) { + DCHECK(false) << "Wrong type for SetInt64: " << path; + return; + } + + scoped_ptr<Value> old_value(GetPrefCopy(path)); + bool rv = persistent_->SetString(path, Int64ToWString(value)); + DCHECK(rv); + + FireObserversIfChanged(path, old_value.get()); +} + +int64 PrefService::GetInt64(const wchar_t* path) const { + DCHECK(CalledOnValidThread()); + + std::wstring result; + if (transient_->GetString(path, &result)) + return StringToInt64(WideToUTF16Hack(result)); + + const Preference* pref = FindPreference(path); + if (!pref) { +#if defined(OS_WIN) + DCHECK(false) << "Trying to read an unregistered pref: " << path; +#else + // TODO(port): remove this exception +#endif + return StringToInt64(WideToUTF16Hack(result)); + } + bool rv = pref->GetValue()->GetAsString(&result); + DCHECK(rv); + return StringToInt64(WideToUTF16Hack(result)); +} + +void PrefService::RegisterInt64Pref(const wchar_t* path, int64 default_value) { + Preference* pref = new Preference(persistent_.get(), path, + Value::CreateStringValue(Int64ToWString(default_value))); + RegisterPreference(pref); +} + DictionaryValue* PrefService::GetMutableDictionary(const wchar_t* path) { DCHECK(CalledOnValidThread()); diff --git a/chrome/common/pref_service.h b/chrome/common/pref_service.h index 0a7690c..f5377d2 100644 --- a/chrome/common/pref_service.h +++ b/chrome/common/pref_service.h @@ -156,6 +156,13 @@ class PrefService : public NonThreadSafe { void SetString(const wchar_t* path, const std::wstring& value); void SetFilePath(const wchar_t* path, const FilePath& value); + // Int64 helper methods that actually store the given value as a string. + // Note that if obtaining the named value via GetDictionary or GetList, the + // Value type will be TYPE_STRING. + void SetInt64(const wchar_t* path, int64 value); + int64 GetInt64(const wchar_t* path) const; + void RegisterInt64Pref(const wchar_t* path, int64 default_value); + // Used to set the value of dictionary or list values in the pref tree. This // will create a dictionary or list if one does not exist in the pref tree. // This method returns NULL only if you're requesting an unregistered pref or diff --git a/chrome/common/pref_service_unittest.cc b/chrome/common/pref_service_unittest.cc index 2a43e69..e690579a 100644 --- a/chrome/common/pref_service_unittest.cc +++ b/chrome/common/pref_service_unittest.cc @@ -16,7 +16,7 @@ namespace { class PrefServiceTest : public testing::Test { -protected: + protected: virtual void SetUp() { // Name a subdirectory of the temp directory. ASSERT_TRUE(PathService::Get(base::DIR_TEMP, &test_dir_)); @@ -98,9 +98,11 @@ TEST_F(PrefServiceTest, Basic) { // Register test prefs. const wchar_t kNewWindowsInTabs[] = L"tabs.new_windows_in_tabs"; const wchar_t kMaxTabs[] = L"tabs.max_tabs"; + const wchar_t kLongIntPref[] = L"long_int.pref"; prefs.RegisterStringPref(prefs::kHomePage, L""); prefs.RegisterBooleanPref(kNewWindowsInTabs, false); prefs.RegisterIntegerPref(kMaxTabs, 0); + prefs.RegisterStringPref(kLongIntPref, L"2147483648"); std::wstring microsoft(L"http://www.microsoft.com"); std::wstring cnn(L"http://www.cnn.com"); @@ -130,6 +132,10 @@ TEST_F(PrefServiceTest, Basic) { prefs.SetInteger(kMaxTabs, 10); EXPECT_EQ(10, prefs.GetInteger(kMaxTabs)); + EXPECT_EQ(2147483648LL, prefs.GetInt64(kLongIntPref)); + prefs.SetInt64(kLongIntPref, 214748364842LL); + EXPECT_EQ(214748364842LL, prefs.GetInt64(kLongIntPref)); + EXPECT_EQ(FilePath::StringType(FILE_PATH_LITERAL("/usr/local/")), prefs.GetFilePath(kSomeDirectory).value()); prefs.SetFilePath(kSomeDirectory, some_path); 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__ diff --git a/chrome/test/data/pref_service/write.golden.json b/chrome/test/data/pref_service/write.golden.json index 362e32d..6e635cc 100644 --- a/chrome/test/data/pref_service/write.golden.json +++ b/chrome/test/data/pref_service/write.golden.json @@ -1,6 +1,9 @@ { "homepage": "http://www.cnn.com", "some_directory": "/usr/sbin/", + "long_int": { + "pref": "214748364842" + }, "tabs": { "max_tabs": 10, "new_windows_in_tabs": false |