Revert 58639 -

This seems to have caused some ui_tests to fail (AllowCookies, BlockCookies)
Original comment:
Write the outcome of the Toast Experiment for system-level installs to the right registry key.

BUG=44378
TEST=None

Review URL: http://codereview.chromium.org/3308003

TBR=finnur@chromium.org
Review URL: http://codereview.chromium.org/3369002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@58667 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 24 insertions, 146 deletions

diff --git a/base/process_util.h b/base/process_util.h
index 4699c9e2..dae3d46 100644
--- a/base/process_util.h
+++ b/base/process_util.h
@@ -174,13 +174,6 @@ bool GetProcessIntegrityLevel(ProcessHandle process, IntegrityLevel *level);
 bool LaunchApp(const std::wstring& cmdline,
                bool wait, bool start_hidden, ProcessHandle* process_handle);
 
-// Same as LaunchApp, except allows the new process to inherit handles of the
-// parent process.
-bool LaunchAppWithHandleInheritance(const std::wstring& cmdline,
-                                    bool wait,
-                                    bool start_hidden,
-                                    ProcessHandle* process_handle);
-
 // Runs the given application name with the given command line as if the user
 // represented by |token| had launched it. The caveats about |cmdline| and
 // |process_handle| explained for LaunchApp above apply as well.
@@ -195,11 +188,10 @@ bool LaunchAppAsUser(UserTokenHandle token, const std::wstring& cmdline,
                      bool start_hidden, ProcessHandle* process_handle);
 
 // Has the same behavior as LaunchAppAsUser, but offers the boolean option to
-// use an empty string for the desktop name and a boolean for allowing the
-// child process to inherit handles from its parent.
+// use an empty string for the desktop name.
 bool LaunchAppAsUser(UserTokenHandle token, const std::wstring& cmdline,
                      bool start_hidden, ProcessHandle* process_handle,
-                     bool empty_desktop_name, bool inherit_handles);
+                     bool empty_desktop_name);
 
 
 #elif defined(OS_POSIX)
diff --git a/base/process_util_win.cc b/base/process_util_win.cc
index 044960e..fb7bdb8 100644
--- a/base/process_util_win.cc
+++ b/base/process_util_win.cc
@@ -191,9 +191,8 @@ bool GetProcessIntegrityLevel(ProcessHandle process, IntegrityLevel *level) {
   return true;
 }
 
-bool LaunchAppImpl(const std::wstring& cmdline,
-                   bool wait, bool start_hidden, bool inherit_handles,
-                   ProcessHandle* process_handle) {
+bool LaunchApp(const std::wstring& cmdline,
+               bool wait, bool start_hidden, ProcessHandle* process_handle) {
   STARTUPINFO startup_info = {0};
   startup_info.cb = sizeof(startup_info);
   startup_info.dwFlags = STARTF_USESHOWWINDOW;
@@ -201,7 +200,7 @@ bool LaunchAppImpl(const std::wstring& cmdline,
   PROCESS_INFORMATION process_info;
   if (!CreateProcess(NULL,
                      const_cast<wchar_t*>(cmdline.c_str()), NULL, NULL,
-                     inherit_handles, 0, NULL, NULL,
+                     FALSE, 0, NULL, NULL,
                      &startup_info, &process_info))
     return false;
 
@@ -220,26 +219,14 @@ bool LaunchAppImpl(const std::wstring& cmdline,
   return true;
 }
 
-bool LaunchApp(const std::wstring& cmdline,
-               bool wait, bool start_hidden, ProcessHandle* process_handle) {
-  return LaunchAppImpl(cmdline, wait, start_hidden, false, process_handle);
-}
-
-bool LaunchAppWithHandleInheritance(
-    const std::wstring& cmdline, bool wait, bool start_hidden,
-    ProcessHandle* process_handle) {
-  return LaunchAppImpl(cmdline, wait, start_hidden, true, process_handle);
-}
-
 bool LaunchAppAsUser(UserTokenHandle token, const std::wstring& cmdline,
                      bool start_hidden, ProcessHandle* process_handle) {
-  return LaunchAppAsUser(token, cmdline, start_hidden, process_handle,
-                         false, false);
+  return LaunchAppAsUser(token, cmdline, start_hidden, process_handle, false);
 }
 
 bool LaunchAppAsUser(UserTokenHandle token, const std::wstring& cmdline,
                      bool start_hidden, ProcessHandle* process_handle,
-                     bool empty_desktop_name, bool inherit_handles) {
+                     bool empty_desktop_name) {
   STARTUPINFO startup_info = {0};
   startup_info.cb = sizeof(startup_info);
   if (empty_desktop_name)
@@ -257,7 +244,7 @@ bool LaunchAppAsUser(UserTokenHandle token, const std::wstring& cmdline,
 
   BOOL launched =
       CreateProcessAsUser(token, NULL, const_cast<wchar_t*>(cmdline.c_str()),
-                          NULL, NULL, inherit_handles, flags, enviroment_block,
+                          NULL, NULL, FALSE, flags, enviroment_block,
                           NULL, &startup_info, &process_info);
 
   DestroyEnvironmentBlock(enviroment_block);
@@ -277,8 +264,8 @@ bool LaunchAppAsUser(UserTokenHandle token, const std::wstring& cmdline,
 
 bool LaunchApp(const CommandLine& cl,
                bool wait, bool start_hidden, ProcessHandle* process_handle) {
-  return LaunchAppImpl(cl.command_line_string(), wait,
-                       false, start_hidden, process_handle);
+  return LaunchApp(cl.command_line_string(), wait,
+                   start_hidden, process_handle);
 }
 
 // Attempts to kill the process identified by the given process
diff --git a/chrome/installer/util/google_chrome_distribution.cc b/chrome/installer/util/google_chrome_distribution.cc
index 22fe7e8..adb5b1a 100644
--- a/chrome/installer/util/google_chrome_distribution.cc
+++ b/chrome/installer/util/google_chrome_distribution.cc
@@ -128,37 +128,15 @@ int GetDirectoryWriteAgeInHours(const wchar_t* path) {
 
 // Launches again this same process with switch --|flag|=|value|.
 // If system_level_toast is true, appends --system-level-toast.
-// If handle to experiment result key was given at startup, re-add it.
 // Does not wait for the process to terminate.
 bool RelaunchSetup(const std::string& flag, int value,
                    bool system_level_toast) {
-  CommandLine new_cmd_line(CommandLine::ForCurrentProcess()->GetProgram());
-  new_cmd_line.AppendSwitchASCII(flag, base::IntToString(value));
-
-  // Re-add the system level toast flag.
-  if (system_level_toast) {
-    new_cmd_line.AppendSwitch(
+  CommandLine cmd_line(CommandLine::ForCurrentProcess()->GetProgram());
+  cmd_line.AppendSwitchASCII(flag, base::IntToString(value));
+  if (system_level_toast)
+    cmd_line.AppendSwitch(
       WideToASCII(installer_util::switches::kSystemLevelToast));
-
-    // Re-add the toast result key. We need to do this because Setup running as
-    // system passes the key to Setup running as user, but that child process
-    // does not perform the actual toasting, it launches another Setup (as user)
-    // to do so. That is the process that needs the key.
-    const CommandLine& current_cmd_line = *CommandLine::ForCurrentProcess();
-    std::string key = WideToASCII(installer_util::switches::kToastResultsKey);
-    std::string toast_key = current_cmd_line.GetSwitchValueASCII(key);
-    if (!toast_key.empty()) {
-      new_cmd_line.AppendSwitchASCII(key, toast_key);
-
-      // Use handle inheritance to make sure the duplicated toast results key
-      // gets inherited by the child process.
-      return base::LaunchAppWithHandleInheritance(
-          new_cmd_line.command_line_string(), false, false, NULL);
-    }
-  }
-
-  return base::LaunchApp(new_cmd_line.command_line_string(),
-                         false, false, NULL);
+  return base::LaunchApp(cmd_line, false, false, NULL);
 }
 
 // For System level installs, setup.exe lives in the system temp, which
@@ -220,18 +198,12 @@ bool RelaunchSetupAsConsoleUser(const std::wstring& flag) {
   CommandLine cmd_line(setup_exe);
   cmd_line.AppendSwitch(WideToASCII(flag));
 
-  // Get the Google Update results key, and pass it on the command line to
-  // the child process.
-  int key = GoogleUpdateSettings::DuplicateGoogleUpdateSystemClientKey();
-  cmd_line.AppendSwitchASCII(
-      WideToASCII(installer_util::switches::kToastResultsKey),
-      base::IntToString(key));
-
   if (win_util::GetWinVersion() > win_util::WINVERSION_XP) {
     // Make sure that in Vista and Above we have the proper DACLs so
     // the interactive user can launch it.
-    if (!FixDACLsForExecute(setup_exe.ToWStringHack().c_str()))
+    if (!FixDACLsForExecute(setup_exe.ToWStringHack().c_str())) {
       NOTREACHED();
+    }
   }
 
   DWORD console_id = ::WTSGetActiveConsoleSessionId();
@@ -240,11 +212,9 @@ bool RelaunchSetupAsConsoleUser(const std::wstring& flag) {
   HANDLE user_token;
   if (!::WTSQueryUserToken(console_id, &user_token))
     return false;
-  // Note: Handle inheritance must be true in order for the child process to be
-  // able to use the duplicated handle above (Google Update results).
   bool launched = base::LaunchAppAsUser(user_token,
                                         cmd_line.command_line_string(),
-                                        false, NULL, true, true);
+                                        false, NULL, true);
   ::CloseHandle(user_token);
   return launched;
 }
@@ -524,6 +494,7 @@ void GoogleChromeDistribution::UpdateDiffInstallStatus(bool system_install,
 void GoogleChromeDistribution::LaunchUserExperiment(
     installer_util::InstallStatus status, const installer::Version& version,
     bool system_install) {
+
   if (system_install) {
     if (installer_util::NEW_VERSION_UPDATED == status) {
       // We need to relaunch as the interactive user.
@@ -553,7 +524,7 @@ void GoogleChromeDistribution::LaunchUserExperiment(
     const int kThirtyDays = 3000 * 24;
     int dir_age_hours = GetDirectoryWriteAgeInHours(user_data_dir.c_str());
     if (dir_age_hours < 0) {
-      // This means that we failed to find the user data dir. The most likely
+      // This means that we failed to find the user data dir. The most likey
       // cause is that this user has not ever used chrome at all which can
       // happen in a system-level install.
       GoogleUpdateSettings::SetClient(
@@ -577,10 +548,8 @@ void GoogleChromeDistribution::LaunchUserExperiment(
   LOG(INFO) << "User drafted for toast experiment " << flavor;
   GoogleUpdateSettings::SetClient(
       GetExperimentGroup(kToastExpBaseGroup, flavor));
-  // User level: The experiment needs to be performed in a different process
-  // because google_update expects the upgrade process to be quick and nimble.
-  // System level: We have already been relaunched, so we don't need to be
-  // quick, but we relaunch to follow the exact same codepath.
+  // The experiment needs to be performed in a different process because
+  // google_update expects the upgrade process to be quick and nimble.
   RelaunchSetup(installer_util::switches::kInactiveUserToast, flavor,
                 system_install);
 }
@@ -617,28 +586,7 @@ void GoogleChromeDistribution::InactiveUserToastExperiment(int flavor,
     default:
       outcome = kToastExpTriesErrorGroup;
   };
-
-  // If a specific Toast Results key handle (presumably to our HKLM key) was
-  // passed in to the command line (such as for system level installs), we use
-  // it. Otherwise, we write to the key under HKCU.
-  const CommandLine& cmd_line = *CommandLine::ForCurrentProcess();
-  if (cmd_line.HasSwitch(installer_util::switches::kToastResultsKey)) {
-    // Get the handle to the key under HKLM.
-    int handle;
-    base::StringToInt(cmd_line.GetSwitchValueASCII(
-        WideToASCII(installer_util::switches::kToastResultsKey)).c_str(),
-        &handle);
-    // Use it to write the experiment results.
-    GoogleUpdateSettings::WriteGoogleUpdateSystemClientKey(
-        handle,
-        google_update::kRegClientField,
-        GetExperimentGroup(outcome, flavor));
-    CloseHandle((HANDLE) handle);
-  } else {
-    // Write to HKCU.
-    GoogleUpdateSettings::SetClient(GetExperimentGroup(outcome, flavor));
-  }
-
+  GoogleUpdateSettings::SetClient(GetExperimentGroup(outcome, flavor));
   if (outcome != kToastExpUninstallGroup)
     return;
   // The user wants to uninstall. This is a best effort operation. Note that
diff --git a/chrome/installer/util/google_update_settings.cc b/chrome/installer/util/google_update_settings.cc
index 91867f3..e56f1c6 100644
--- a/chrome/installer/util/google_update_settings.cc
+++ b/chrome/installer/util/google_update_settings.cc
@@ -244,33 +244,6 @@ std::wstring GoogleUpdateSettings::GetNewGoogleUpdateApKey(
   return new_value;
 }
 
-int GoogleUpdateSettings::DuplicateGoogleUpdateSystemClientKey() {
-  BrowserDistribution* dist = BrowserDistribution::GetDistribution();
-  std::wstring reg_path = dist->GetStateKey();
-
-  // Minimum access needed is to be able to write to this key.
-  RegKey reg_key(HKEY_LOCAL_MACHINE, reg_path.c_str(), KEY_SET_VALUE);
-  if (!reg_key.Valid())
-    return 0;
-
-  HANDLE target_handle = 0;
-  if (!DuplicateHandle(GetCurrentProcess(), reg_key.Handle(),
-                       GetCurrentProcess(), &target_handle, KEY_SET_VALUE,
-                       TRUE, DUPLICATE_SAME_ACCESS)) {
-    return 0;
-  }
-  return reinterpret_cast<int>(target_handle);
-}
-
-bool GoogleUpdateSettings::WriteGoogleUpdateSystemClientKey(
-    int handle, const std::wstring& key, const std::wstring& value) {
-  HKEY reg_key = reinterpret_cast<HKEY>(reinterpret_cast<void*>(handle));
-  DWORD size = static_cast<DWORD>(value.size()) * sizeof(wchar_t);
-  LSTATUS status = RegSetValueEx(reg_key, key.c_str(), 0, REG_SZ,
-      reinterpret_cast<const BYTE*>(value.c_str()), size);
-  return status == ERROR_SUCCESS;
-}
-
 bool GoogleUpdateSettings::IsOrganic(const std::wstring& brand) {
   const CommandLine& command_line = *CommandLine::ForCurrentProcess();
   if (command_line.HasSwitch(switches::kOrganicInstall))
diff --git a/chrome/installer/util/google_update_settings.h b/chrome/installer/util/google_update_settings.h
index 715ef9f..6e12cb66 100644
--- a/chrome/installer/util/google_update_settings.h
+++ b/chrome/installer/util/google_update_settings.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -105,23 +105,6 @@ class GoogleUpdateSettings {
                                               int install_return_code,
                                               const std::wstring& value);
 
-  // For system-level installs, we need to be able to communicate the results
-  // of the Toast Experiments back to Google Update. The problem is just that
-  // the experiment is run in the context of the user, which doesn't have
-  // write access to the HKLM key that Google Update expects the results in.
-  // However, when we are about to switch contexts from system to user, we can
-  // duplicate the handle to the registry key and pass it (through handle
-  // inheritance) to the newly created child process that is launched as the
-  // user, allowing the child process to write to the key, with the
-  // WriteGoogleUpdateSystemClientKey function below.
-  static int DuplicateGoogleUpdateSystemClientKey();
-
-  // Takes a |handle| to a registry key and writes |value| string into the
-  // specified |key|. See DuplicateGoogleUpdateSystemClientKey for details.
-  static bool WriteGoogleUpdateSystemClientKey(int handle,
-                                               const std::wstring& key,
-                                               const std::wstring& value);
-
   // True if a build is strictly organic, according to its brand code.
   static bool IsOrganic(const std::wstring& brand);
 
diff --git a/chrome/installer/util/util_constants.cc b/chrome/installer/util/util_constants.cc
index fe664c0..e22d2c3 100644
--- a/chrome/installer/util/util_constants.cc
+++ b/chrome/installer/util/util_constants.cc
@@ -114,10 +114,6 @@ const char kInactiveUserToast[] = "inactive-user-toast";
 // User toast experiment switch from system context to user context.
 const wchar_t kSystemLevelToast[] = L"system-level-toast";
 
-// A handle value of the key to write the results of the toast experiment
-// to. See DuplicateGoogleUpdateSystemClientKey for details.
-const wchar_t kToastResultsKey[] = L"toast-results-key";
-
 }  // namespace switches
 
 const wchar_t kGoogleChromeInstallSubDir1[] = L"Google";
diff --git a/chrome/installer/util/util_constants.h b/chrome/installer/util/util_constants.h
index 5f3359d..3a3d0f9 100644
--- a/chrome/installer/util/util_constants.h
+++ b/chrome/installer/util/util_constants.h
@@ -81,7 +81,6 @@ extern const char kShowEula[];
 extern const wchar_t kAltDesktopShortcut[];
 extern const char kInactiveUserToast[];
 extern const wchar_t kSystemLevelToast[];
-extern const wchar_t kToastResultsKey[];
 }  // namespace switches
 
 extern const wchar_t kGoogleChromeInstallSubDir1[];