Refactoring: Task to get the session name moved to separate file

Moved task to get the session name and its dependencies from chrome/browser/sync/glue/session_model_associator to chrome/browser/sync/util/get_session_name_task

BUG=103970
TEST=SessionModelAssociatorTest.*,GetSessionNameTaskTest.*

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

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

7 files changed, 176 insertions, 92 deletions

diff --git a/chrome/browser/sync/glue/session_model_associator.cc b/chrome/browser/sync/glue/session_model_associator.cc
index cafa74d..4b4e83e 100644
--- a/chrome/browser/sync/glue/session_model_associator.cc
+++ b/chrome/browser/sync/glue/session_model_associator.cc
@@ -8,6 +8,7 @@
 #include <set>
 #include <utility>
 
+#include "base/bind.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/sys_info.h"
@@ -25,6 +26,7 @@
 #include "chrome/browser/sync/internal_api/write_transaction.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/syncable/syncable.h"
+#include "chrome/browser/sync/util/get_session_name_task.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/url_constants.h"
 #include "content/browser/tab_contents/navigation_entry.h"
@@ -554,52 +556,22 @@ void SessionModelAssociator::InitializeCurrentMachineTag(
 void SessionModelAssociator::OnSessionNameInitialized(const std::string name) {
   DCHECK(CalledOnValidThread());
   // Only use the default machine name if it hasn't already been set.
-  if (current_session_name_.empty()) {
+  if (current_session_name_.empty())
     current_session_name_ = name;
-  }
 }
 
-// Task which runs on the file thread because it runs system calls which can
-// block while retrieving sytem information.
-class GetSessionNameTask : public Task {
- public:
-  explicit GetSessionNameTask(
-      const WeakHandle<SessionModelAssociator> associator) :
-    associator_(associator) {}
-
-  virtual void Run() {
-#if defined(OS_LINUX)
-    std::string session_name = base::GetLinuxDistro();
-#elif defined(OS_MACOSX)
-    std::string session_name = SessionModelAssociator::GetHardwareModelName();
-#elif defined(OS_WIN)
-    std::string session_name = SessionModelAssociator::GetComputerName();
-#else
-    std::string session_name;
-#endif
-    if (session_name == "Unknown" || session_name.empty()) {
-      session_name = base::SysInfo::OperatingSystemName();
-    }
-    associator_.Call(FROM_HERE,
-                     &SessionModelAssociator::OnSessionNameInitialized,
-                     session_name);
-  }
-  const WeakHandle<SessionModelAssociator> associator_;
-
-  DISALLOW_COPY_AND_ASSIGN(GetSessionNameTask);
-};
-
 void SessionModelAssociator::InitializeCurrentSessionName() {
   DCHECK(CalledOnValidThread());
   if (setup_for_test_) {
     OnSessionNameInitialized("TestSessionName");
   } else {
-#if defined(OS_CHROMEOS)
-    OnSessionNameInitialized("Chromebook");
-#else
-    BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
-        new GetSessionNameTask(MakeWeakHandle(AsWeakPtr())));
-#endif
+    scoped_refptr<GetSessionNameTask> task = new GetSessionNameTask(
+        base::Bind(&SessionModelAssociator::OnSessionNameInitialized,
+                   AsWeakPtr()));
+    BrowserThread::PostTask(
+        BrowserThread::FILE,
+        FROM_HERE,
+        base::Bind(&GetSessionNameTask::GetSessionNameAsync, task.get()));
   }
 }
 
@@ -1278,18 +1250,4 @@ bool SessionModelAssociator::CryptoReadyIfNecessary() {
          sync_service_->IsCryptographerReady(&trans);
 }
 
-#if defined(OS_WIN)
-// Static
-// TODO(nzea): This isn't safe to call on the UI-thread. Move it out to a util
-// or object that lives on the FILE thread.
-std::string SessionModelAssociator::GetComputerName() {
-  char computer_name[MAX_COMPUTERNAME_LENGTH + 1];
-  DWORD size = sizeof(computer_name);
-  if (GetComputerNameA(computer_name, &size)) {
-    return computer_name;
-  }
-  return std::string();
-}
-#endif
-
 }  // namespace browser_sync
diff --git a/chrome/browser/sync/glue/session_model_associator.h b/chrome/browser/sync/glue/session_model_associator.h
index 3fca14f..7b41ada 100644
--- a/chrome/browser/sync/glue/session_model_associator.h
+++ b/chrome/browser/sync/glue/session_model_associator.h
@@ -203,19 +203,6 @@ class SessionModelAssociator
   // Callback for when the session name has been computed.
   void OnSessionNameInitialized(const std::string name);
 
-#if defined(OS_WIN)
-  // Returns the computer name or the empty string an error occurred.
-  static std::string GetComputerName();
-#endif
-
-#if defined(OS_MACOSX)
-  // Returns the Hardware model name, without trailing numbers, if possible.
-  // See http://www.cocoadev.com/index.pl?MacintoshModels for an example list of
-  // models. If an error occurs trying to read the model, this simply returns
-  // "Unknown".
-  static std::string GetHardwareModelName();
-#endif
-
  private:
   FRIEND_TEST_ALL_PREFIXES(ProfileSyncServiceSessionTest, WriteSessionToNode);
   FRIEND_TEST_ALL_PREFIXES(ProfileSyncServiceSessionTest,
diff --git a/chrome/browser/sync/glue/session_model_associator_unittest.cc b/chrome/browser/sync/glue/session_model_associator_unittest.cc
index 7e47e61..3928958 100644
--- a/chrome/browser/sync/glue/session_model_associator_unittest.cc
+++ b/chrome/browser/sync/glue/session_model_associator_unittest.cc
@@ -124,23 +124,6 @@ TEST_F(SessionModelAssociatorTest, PopulateSessionTab) {
   ASSERT_EQ(GURL("http://foo/1"), tab.navigations[0].virtual_url());
 }
 
-#if defined(OS_WIN)
-// The test is somewhat silly, and just verifies that we return a computer name.
-TEST(SessionModelAssociatorTest, TestGetComputerName) {
-  std::string computer_name = SessionModelAssociator::GetComputerName();
-  EXPECT_TRUE(!computer_name.empty());
-}
-#endif
-
-#if defined(OS_MACOSX)
-// The test is somewhat silly, and just verifies that we return a hardware
-// model name.
-TEST_F(SessionModelAssociatorTest, GetHardwareModelName) {
-  std::string hardware_model = SessionModelAssociator::GetHardwareModelName();
-  EXPECT_TRUE(!hardware_model.empty());
-}
-#endif
-
 TEST_F(SessionModelAssociatorTest, TabNodePool) {
   SessionModelAssociator::TabNodePool pool(NULL);
   pool.set_machine_tag("tag");
diff --git a/chrome/browser/sync/util/get_session_name_task.cc b/chrome/browser/sync/util/get_session_name_task.cc
new file mode 100644
index 0000000..7dd6ebf
--- /dev/null
+++ b/chrome/browser/sync/util/get_session_name_task.cc
@@ -0,0 +1,65 @@
+// Copyright (c) 2011 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.
+
+#include "chrome/browser/sync/util/get_session_name_task.h"
+
+#include "base/bind.h"
+#include "base/location.h"
+#include "base/message_loop_proxy.h"
+#include "base/sys_info.h"
+
+#if defined(OS_LINUX)
+#include "base/linux_util.h"
+#elif defined(OS_WIN)
+#include <windows.h>
+#endif
+
+namespace browser_sync {
+
+GetSessionNameTask::GetSessionNameTask(
+    const GetSessionNameTask::OnSessionNameInitialized& callback)
+    : callback_(callback),
+      thread_(base::MessageLoopProxy::current()) {
+}
+
+GetSessionNameTask::~GetSessionNameTask() {}
+
+void GetSessionNameTask::GetSessionNameAsync() {
+#if defined(OS_CHROMEOS)
+  std::string session_name = "Chromebook";
+#elif defined(OS_LINUX)
+  std::string session_name = base::GetLinuxDistro();
+#elif defined(OS_MACOSX)
+  std::string session_name = GetSessionNameTask::GetHardwareModelName();
+#elif defined(OS_WIN)
+  std::string session_name = GetSessionNameTask::GetComputerName();
+#else
+  std::string session_name;
+#endif
+
+  if (session_name == "Unknown" || session_name.empty())
+    session_name = base::SysInfo::OperatingSystemName();
+
+  thread_->PostTask(FROM_HERE,
+                    base::Bind(&GetSessionNameTask::InvokeCallback,
+                               this,
+                               session_name));
+}
+
+void GetSessionNameTask::InvokeCallback(const std::string& session_name) {
+  callback_.Run(session_name);
+}
+
+#if defined(OS_WIN)
+// static
+std::string GetSessionNameTask::GetComputerName() {
+  char computer_name[MAX_COMPUTERNAME_LENGTH + 1];
+  DWORD size = sizeof(computer_name);
+  if (GetComputerNameA(computer_name, &size))
+    return computer_name;
+  return std::string();
+}
+#endif
+
+}  // namespace browser_sync
diff --git a/chrome/browser/sync/util/get_session_name_task.h b/chrome/browser/sync/util/get_session_name_task.h
new file mode 100644
index 0000000..1de5bde
--- /dev/null
+++ b/chrome/browser/sync/util/get_session_name_task.h
@@ -0,0 +1,62 @@
+// Copyright (c) 2011 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.
+
+#ifndef CHROME_BROWSER_SYNC_UTIL_GET_SESSION_NAME_TASK_H_
+#define CHROME_BROWSER_SYNC_UTIL_GET_SESSION_NAME_TASK_H_
+#pragma once
+
+#include <string>
+
+#include "base/basictypes.h"
+#include "base/callback.h"
+#include "base/gtest_prod_util.h"
+#include "base/memory/ref_counted.h"
+
+namespace base {
+class MessageLoopProxy;
+}  // namespace base
+
+namespace browser_sync {
+
+class GetSessionNameTask
+    : public base::RefCountedThreadSafe<GetSessionNameTask> {
+ public:
+  typedef base::Callback<void(const std::string& name)>
+      OnSessionNameInitialized;
+
+  explicit GetSessionNameTask(const OnSessionNameInitialized& callback);
+  virtual ~GetSessionNameTask();
+
+  void GetSessionNameAsync();
+
+ private:
+  friend class base::RefCountedThreadSafe<GetSessionNameTask>;
+
+  FRIEND_TEST_ALL_PREFIXES(GetSessionNameTaskTest, GetHardwareModelName);
+  FRIEND_TEST_ALL_PREFIXES(GetSessionNameTaskTest, GetComputerName);
+
+  void InvokeCallback(const std::string& session_name);
+
+#if defined(OS_MACOSX)
+  // Returns the Hardware model name, without trailing numbers, if possible.
+  // See http://www.cocoadev.com/index.pl?MacintoshModels for an example list of
+  // models. If an error occurs trying to read the model, this simply returns
+  // "Unknown".
+  static std::string GetHardwareModelName();
+#endif
+
+#if defined(OS_WIN)
+  // Returns the computer name or the empty string if an error occured.
+  static std::string GetComputerName();
+#endif
+
+  const OnSessionNameInitialized callback_;
+  const scoped_refptr<base::MessageLoopProxy> thread_;
+
+  DISALLOW_COPY_AND_ASSIGN(GetSessionNameTask);
+};
+
+}  // namespace browser_sync
+
+#endif  // CHROME_BROWSER_SYNC_UTIL_GET_SESSION_NAME_TASK_H__
diff --git a/chrome/browser/sync/glue/session_model_associator_mac.mm b/chrome/browser/sync/util/get_session_name_task_mac.mm
index 16635b2..76f115a 100644
--- a/chrome/browser/sync/glue/session_model_associator_mac.mm
+++ b/chrome/browser/sync/util/get_session_name_task_mac.mm
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/sync/glue/session_model_associator.h"
+#include "chrome/browser/sync/util/get_session_name_task.h"
 
 #import <Foundation/Foundation.h>
 #import <SystemConfiguration/SCDynamicStoreCopySpecific.h>
@@ -21,18 +21,17 @@
 
 namespace browser_sync {
 
-// Static
-std::string SessionModelAssociator::GetHardwareModelName() {
+// static
+std::string GetSessionNameTask::GetHardwareModelName() {
   NSHost* myHost = [NSHost currentHost];
-  if ([myHost respondsToSelector:@selector(localizedName)]) {
+  if ([myHost respondsToSelector:@selector(localizedName)])
     return base::SysNSStringToUTF8([myHost localizedName]);
-  }
+
   // Fallback for 10.5
   scoped_nsobject<NSString> computerName(base::mac::CFToNSCast(
-        SCDynamicStoreCopyComputerName(NULL, NULL)));
-  if (computerName.get() != NULL) {
+      SCDynamicStoreCopyComputerName(NULL, NULL)));
+  if (computerName.get() != NULL)
     return base::SysNSStringToUTF8(computerName.get());
-  }
 
   // If all else fails, return to using a slightly nicer version of the
   // hardware model.
@@ -40,9 +39,8 @@ std::string SessionModelAssociator::GetHardwareModelName() {
   size_t length = sizeof(modelBuffer);
   if (!sysctlbyname("hw.model", modelBuffer, &length, NULL, 0)) {
     for (size_t i = 0; i < length; i++) {
-      if (IsAsciiDigit(modelBuffer[i])) {
+      if (IsAsciiDigit(modelBuffer[i]))
         return std::string(modelBuffer, 0, i);
-      }
     }
     return std::string(modelBuffer, 0, length);
   }
diff --git a/chrome/browser/sync/util/get_session_name_task_unittest.cc b/chrome/browser/sync/util/get_session_name_task_unittest.cc
new file mode 100644
index 0000000..9683014
--- /dev/null
+++ b/chrome/browser/sync/util/get_session_name_task_unittest.cc
@@ -0,0 +1,31 @@
+// Copyright (c) 2011 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.
+
+#include <string>
+
+#include "chrome/browser/sync/util/get_session_name_task.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace browser_sync {
+
+typedef testing::Test GetSessionNameTaskTest;
+
+#if defined(OS_WIN)
+// The test is somewhat silly, and just verifies that we return a computer name.
+TEST_F(GetSessionNameTaskTest, GetComputerName) {
+  std::string computer_name = GetSessionNameTask::GetComputerName();
+  EXPECT_TRUE(!computer_name.empty());
+}
+#endif
+
+#if defined(OS_MACOSX)
+// The test is somewhat silly, and just verifies that we return a hardware
+// model name.
+TEST_F(GetSessionNameTaskTest, GetHardwareModelName) {
+  std::string hardware_model = GetSessionNameTask::GetHardwareModelName();
+  EXPECT_TRUE(!hardware_model.empty());
+}
+#endif
+
+}