Move Mac LaunchOnStartup enable/disable to File thread

chrome/browser/background_mode_manager.h

Index: chrome/browser/background_mode_manager.h
diff --git a/chrome/browser/background_mode_manager.h b/chrome/browser/background_mode_manager.h
index 6da40ece9317ce24b0482c82d0a2303bb2a95ad3..f0870219d3dbb5dff06ad22b503a6f9379d79491 100644
--- a/chrome/browser/background_mode_manager.h
+++ b/chrome/browser/background_mode_manager.h
@@ -8,6 +8,7 @@
 
 #include "app/menus/simple_menu_model.h"
 #include "base/gtest_prod_util.h"
+#include "base/task.h"
 #include "chrome/browser/background_application_list_model.h"
 #include "chrome/browser/prefs/pref_change_registrar.h"
 #include "chrome/browser/status_icons/status_icon.h"
@@ -49,6 +50,26 @@ class BackgroundModeManager
   static bool IsBackgroundModeEnabled(const CommandLine* command_line);
 
  private:
+  class DisableLaunchOnStartupTask : public Task {
+   public:
+    explicit DisableLaunchOnStartupTask(BackgroundModeManager* manager)
+      : manager_(manager) {

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

I'm really nervous about passing the BackgroundModeManager in to the task - it's
not a thread-safe object, and I suspect the APIs we're calling (which access
prefs) will fail if called from the file thread.

On a related note, I don't think that the Task object should be the interface
point for cross-platform behavior. For one reason, you end up having to pollute
the object with things like manager_ which isn't needed across all platforms
(for that matter, the Tasks themselves aren't really needed on chromeos since we
don't do anything).

I think a cleaner way to go is to define to instance methods:
DisableLaunchOnStartup() and EnableLaunchOnStartup(), and you can implement
those on each platform. This will let you run platform specific code (like
IsLaunchOnStartupResetAllowed()) on the correct thread, and also let you omit
the Tasks entirely if the platform doesn't need them for some reason (like
chromeos). The downside is the PostTask call needs to be duplicated on each
platform, but I don't think that's a big deal.

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

Done.  I've pushed the EnableLaunchOnStartup definition down into the
platform-specific files, and then reorganized and cleaned-up those files.  A
positive side-effect is that I was also able to move the Task class definitions
out of the header file.

+    }
+    virtual void Run();
+   private:
+    BackgroundModeManager* manager_;
+  };
+
+  class EnableLaunchOnStartupTask : public Task {
+   public:
+    explicit EnableLaunchOnStartupTask(BackgroundModeManager* manager)
+      : manager_(manager) {
+    }
+    virtual void Run();
+   private:
+    BackgroundModeManager* manager_;
+  };
+
   friend class TestBackgroundModeManager;
   friend class BackgroundModeManagerTest;
   FRIEND_TEST_ALL_PREFIXES(BackgroundModeManagerTest,
@@ -119,6 +140,10 @@ class BackgroundModeManager
   // manually, or all apps have been loaded).
   void EndKeepAliveForStartup();
 
+  // Add a Preferences (or Options, or Settings) entry to the Status Icon's
+  // context menu.
+  void AddPreferencesItem(menus::SimpleMenuModel* menu);
+
   // Create a status tray icon to allow the user to shutdown Chrome when running
   // in background mode. Virtual to enable testing.
   virtual void CreateStatusTrayIcon();