Move Mac LaunchOnStartup enable/disable to File thread

chrome/browser/background_mode_manager.cc

Index: chrome/browser/background_mode_manager.cc
diff --git a/chrome/browser/background_mode_manager.cc b/chrome/browser/background_mode_manager.cc
index 8a5678235f7ba085284273c4865f93758f857a4b..b5c111e18161353d1e71f01264eccb9cb43dd9c3 100644
--- a/chrome/browser/background_mode_manager.cc
+++ b/chrome/browser/background_mode_manager.cc
@@ -6,9 +6,7 @@
 #include "app/resource_bundle.h"
 #include "base/base_paths.h"
 #include "base/command_line.h"
-#include "base/file_path.h"
 #include "base/logging.h"
-#include "base/path_service.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/background_application_list_model.h"
@@ -18,7 +16,6 @@
 #include "chrome/browser/metrics/user_metrics.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profile.h"
-#include "chrome/browser/shell_integration.h"
 #include "chrome/browser/status_icons/status_icon.h"
 #include "chrome/browser/status_icons/status_tray.h"
 #include "chrome/common/chrome_switches.h"
@@ -26,147 +23,10 @@
 #include "chrome/common/notification_service.h"
 #include "chrome/common/notification_type.h"
 #include "chrome/common/pref_names.h"
-#include "grit/browser_resources.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
 
-#if defined(OS_LINUX)
-#include <unistd.h>
-#include "base/environment.h"
-#include "base/file_util.h"
-#include "base/nix/xdg_util.h"
-#include "base/task.h"
-#include "base/utf_string_conversions.h"
-#include "chrome/common/chrome_version_info.h"
-#endif
-
-#if defined(OS_MACOSX)
-#include "base/mac_util.h"
-#endif
-
-#if defined(TOOLKIT_GTK)
-#include "chrome/browser/gtk/gtk_util.h"
-#endif
-
-#if defined(OS_LINUX)
-static const FilePath::CharType kAutostart[] = "autostart";
-static const FilePath::CharType kConfig[] = ".config";
-static const char kXdgConfigHome[] = "XDG_CONFIG_HOME";
-#endif
-
-#if defined(OS_WIN)
-#include "base/win/registry.h"
-const HKEY kBackgroundModeRegistryRootKey = HKEY_CURRENT_USER;
-const wchar_t* kBackgroundModeRegistrySubkey =
-    L"Software\\Microsoft\\Windows\\CurrentVersion\\Run";
-const wchar_t* kBackgroundModeRegistryKeyName = L"chromium";
-#endif
-
-#if defined(OS_LINUX)
-namespace {
-
-FilePath GetAutostartDirectory(base::Environment* environment) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-  FilePath result =
-      base::nix::GetXDGDirectory(environment, kXdgConfigHome, kConfig);
-  result = result.Append(kAutostart);
-  return result;
-}
-
-FilePath GetAutostartFilename(base::Environment* environment) {
-  FilePath directory = GetAutostartDirectory(environment);
-  return directory.Append(ShellIntegration::GetDesktopName(environment));
-}
-
-}  // namespace
-#endif  // defined(OS_LINUX)
-
-#if defined(OS_WIN) || defined(OS_LINUX)
-class DisableLaunchOnStartupTask : public Task {
- public:
-  virtual void Run() {
-#if defined(OS_LINUX)
-    scoped_ptr<base::Environment> environment(base::Environment::Create());
-    if (!file_util::Delete(GetAutostartFilename(environment.get()), false)) {
-      LOG(WARNING) << "Failed to deregister launch on login.";
-    }
-#elif defined(OS_WIN)
-  const wchar_t* key_name = kBackgroundModeRegistryKeyName;
-  base::win::RegKey read_key(kBackgroundModeRegistryRootKey,
-                             kBackgroundModeRegistrySubkey, KEY_READ);
-  base::win::RegKey write_key(kBackgroundModeRegistryRootKey,
-                              kBackgroundModeRegistrySubkey, KEY_WRITE);
-  if (read_key.ValueExists(key_name) && !write_key.DeleteValue(key_name))
-    LOG(WARNING) << "Failed to deregister launch on login.";
-#endif
-  }
-};
-#endif  // defined(OS_WIN) || defined(OS_LINUX)
-
-// TODO(rickcam): Bug 56280: Share implementation with ShellIntegration
-#if defined(OS_WIN) || defined(OS_LINUX)
-class EnableLaunchOnStartupTask : public Task {
- public:
-  virtual void Run() {
-#if defined(OS_LINUX)
-    scoped_ptr<base::Environment> environment(base::Environment::Create());
-    scoped_ptr<chrome::VersionInfo> version_info(new chrome::VersionInfo());
-    FilePath autostart_directory = GetAutostartDirectory(environment.get());
-    FilePath autostart_file = GetAutostartFilename(environment.get());
-    if (!file_util::DirectoryExists(autostart_directory) &&
-        !file_util::CreateDirectory(autostart_directory)) {
-      LOG(WARNING)
-        << "Failed to register launch on login.  No autostart directory.";
-      return;
-    }
-    std::string wrapper_script;
-    if (!environment->GetVar("CHROME_WRAPPER", &wrapper_script)) {
-      LOG(WARNING)
-        << "Failed to register launch on login.  CHROME_WRAPPER not set.";
-      return;
-    }
-    std::string autostart_file_contents =
-        "[Desktop Entry]\n"
-        "Type=Application\n"
-        "Terminal=false\n"
-        "Exec=" + wrapper_script +
-        " --enable-background-mode --no-startup-window\n"
-        "Name=" + version_info->Name() + "\n";
-    std::string::size_type content_length = autostart_file_contents.length();
-    if (file_util::WriteFile(autostart_file, autostart_file_contents.c_str(),
-                             content_length) !=
-        static_cast<int>(content_length)) {
-      LOG(WARNING) << "Failed to register launch on login.  Failed to write "
-                   << autostart_file.value();
-      file_util::Delete(GetAutostartFilename(environment.get()), false);
-    }
-#elif defined(OS_WIN)
-  // TODO(rickcam): Bug 53597: Make RegKey mockable.
-  // TODO(rickcam): Bug 53600: Use distinct registry keys per flavor+profile.
-  const wchar_t* key_name = kBackgroundModeRegistryKeyName;
-  base::win::RegKey read_key(kBackgroundModeRegistryRootKey,
-                             kBackgroundModeRegistrySubkey, KEY_READ);
-  base::win::RegKey write_key(kBackgroundModeRegistryRootKey,
-                              kBackgroundModeRegistrySubkey, KEY_WRITE);
-  FilePath executable;
-  if (!PathService::Get(base::FILE_EXE, &executable))
-    return;
-  std::wstring new_value = executable.value() + L" --no-startup-window";
-  if (read_key.ValueExists(key_name)) {
-    std::wstring current_value;
-    if (read_key.ReadValue(key_name, &current_value) &&
-        (current_value == new_value)) {
-      return;
-    }
-  }
-  if (!write_key.WriteValue(key_name, new_value.c_str()))
-    LOG(WARNING) << "Failed to register launch on login.";
-#endif
-  }
-};
-#endif  // defined(OS_WIN) || defined(OS_LINUX)
-
 void BackgroundModeManager::OnApplicationDataChanged(
     const Extension* extension) {
   UpdateContextMenuEntryIcon(extension);
@@ -384,39 +244,13 @@ void BackgroundModeManager::EnableLaunchOnStartup(bool should_launch) {
   // This functionality is only defined for default profile, currently.
   if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kUserDataDir))
     return;
-#if defined(OS_WIN) || defined(OS_LINUX)
   if (should_launch) {
     BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
-                            new EnableLaunchOnStartupTask());
+                            new EnableLaunchOnStartupTask(this));

[Andrew T Wilson (Slow), 2010/12/03 02:06:59]

See my comment about moving this entire statement down into the
platform-specific files.

[The wrong rickcam account, 2010/12/04 02:08:04]

Done.

   } else {
     BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
-                            new DisableLaunchOnStartupTask());
+                            new DisableLaunchOnStartupTask(this));
   }
-#elif defined(OS_MACOSX)
-  if (should_launch) {
-    // Return if Chrome is already a Login Item (avoid overriding user choice).
-    if (mac_util::CheckLoginItemStatus(NULL))
-      return;
-
-    mac_util::AddToLoginItems(true);  // Hide on startup.
-
-    // Remember we set Login Item, not the user - so we can reset it later.
-    SetLaunchOnStartupResetAllowed(true);
-  } else {
-    // If we didn't set Login Item, don't mess with it.
-    if (!IsLaunchOnStartupResetAllowed())
-      return;
-    SetLaunchOnStartupResetAllowed(false);
-
-    // Check if Chrome is not a login Item, or is a Login Item but w/o 'hidden'
-    // flag - most likely user has modified the setting, don't override it.
-    bool is_hidden = false;
-    if (!mac_util::CheckLoginItemStatus(&is_hidden) || !is_hidden)
-      return;
-
-    mac_util::RemoveFromLoginItems();
-  }
-#endif
 }
 
 void BackgroundModeManager::CreateStatusTrayIcon() {
@@ -462,19 +296,7 @@ void BackgroundModeManager::UpdateStatusTrayIconContextMenu() {
   // Add About item
   menu->AddItem(IDC_ABOUT, l10n_util::GetStringFUTF16(IDS_ABOUT,
       l10n_util::GetStringUTF16(IDS_PRODUCT_NAME)));
-
-  // Add Preferences item
-#if defined(OS_CHROMEOS)
-  menu->AddItemWithStringId(IDC_OPTIONS, IDS_SETTINGS);
-#elif defined(TOOLKIT_GTK)
-  string16 preferences = gtk_util::GetStockPreferencesMenuLabel();
-  if (!preferences.empty())
-    menu->AddItem(IDC_OPTIONS, preferences);
-  else
-    menu->AddItemWithStringId(IDC_OPTIONS, IDS_PREFERENCES);
-#else
-  menu->AddItemWithStringId(IDC_OPTIONS, IDS_OPTIONS);
-#endif
+  AddPreferencesItem(menu);

[Andrew T Wilson (Slow), 2010/12/03 02:06:59]

One other way to do this is to define:

string16 GetPreferencesMenuLabel()

and implement this on various platforms.

Then this code becomes:

menu->AddItem(IDC_OPTIONS, GetPreferencesMenuLabel());

I don't actually know if that's any cleaner, though, so what you have here is
just fine too.

[The wrong rickcam account, 2010/12/04 02:08:04]

Done.  I'd argue that yours is cleaner.  It makes the API about data rather than
side-effect.  More RESTful :-)

   menu->AddItemWithStringId(IDC_TASK_MANAGER, IDS_TASK_MANAGER);
   menu->AddSeparator();
   int application_position = 0;