Refactor chromedriver command execution for less coupling and better testability.

No additional functionality is added except proper cleanup and exception handling. This includes adding an Init and Shutdown method for ChromeDriver, that is called when the DLL is loaded/unloaded.
BUG=none


Review URL: https://chromiumcodereview.appspot.com/11312065

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

26 files changed, 1040 insertions, 175 deletions

diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 18d8857..4714a2b 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -961,11 +961,20 @@
       'sources': [
         'test/chromedriver/chromedriver.cc',
         'test/chromedriver/chromedriver.h',
+        'test/chromedriver/command.h',
         'test/chromedriver/command_executor.h',
         'test/chromedriver/command_executor_impl.cc',
         'test/chromedriver/command_executor_impl.h',
+        'test/chromedriver/commands.cc',
+        'test/chromedriver/commands.h',
+        'test/chromedriver/session.cc',
+        'test/chromedriver/session.h',
+        'test/chromedriver/session_command.cc',
+        'test/chromedriver/session_command.h',
+        'test/chromedriver/session_map.h',
         'test/chromedriver/status.cc',
         'test/chromedriver/status.h',
+        'test/chromedriver/synchronized_map.h',
       ],
     },
     {
@@ -983,7 +992,13 @@
       'sources': [
         'test/chromedriver/chromedriver_unittest.cc',
         'test/chromedriver/command_executor_impl_unittest.cc',
+        'test/chromedriver/commands_unittest.cc',
+        'test/chromedriver/fake_session_accessor.cc',
+        'test/chromedriver/fake_session_accessor.h',
+        'test/chromedriver/session_command_unittest.cc',
+        'test/chromedriver/session_unittest.cc',
         'test/chromedriver/status_unittest.cc',
+        'test/chromedriver/synchronized_map_unittest.cc',
       ],
     },
     # This is the new ChromeDriver based on DevTools.
diff --git a/chrome/test/chromedriver/chromedriver.cc b/chrome/test/chromedriver/chromedriver.cc
index 8d9b41b..9f66847 100644
--- a/chrome/test/chromedriver/chromedriver.cc
+++ b/chrome/test/chromedriver/chromedriver.cc
@@ -6,27 +6,16 @@
 
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
-#include "base/lazy_instance.h"
+#include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
-#include "base/synchronization/lock.h"
 #include "base/values.h"
 #include "chrome/test/chromedriver/command_executor.h"
-#include "chrome/test/chromedriver/command_executor_impl.h"
 #include "chrome/test/chromedriver/status.h"
 
 namespace {
 
-// Guards initialization of |g_command_executor|.
-base::LazyInstance<base::Lock> g_lazy_lock = LAZY_INSTANCE_INITIALIZER;
-
 CommandExecutor* g_command_executor = NULL;
 
-CommandExecutor* CreateCommandExecutor() {
-  return new CommandExecutorImpl();
-}
-
-CommandExecutorFactoryFunc g_executor_factory = &CreateCommandExecutor;
-
 void SetResponse(StatusCode status,
                  const base::Value* value,
                  const std::string& session_id,
@@ -48,26 +37,12 @@ void SetError(const std::string& error_msg,
 
 }  // namespace
 
-void SetCommandExecutorFactoryForTesting(CommandExecutorFactoryFunc func) {
-  g_executor_factory = func;
+void Init(scoped_ptr<CommandExecutor> executor) {
+  g_command_executor = executor.release();
 }
 
-// Synchronously executes the given command. Thread safe.
-// Command must be a JSON object:
-// {
-//   "name": <string>,
-//   "parameters": <dictionary>,
-//   "sessionId": <string>
-// }
-// Response will always be a JSON object:
-// {
-//   "status": <integer>,
-//   "value": <object>,
-//   "sessionId": <string>
-// }
-// If "status" is non-zero, "value" will be an object with a string "message"
-// property which signifies the error message.
 void ExecuteCommand(const std::string& command, std::string* response) {
+  CHECK(g_command_executor);
   std::string error_msg;
   scoped_ptr<base::Value> value(base::JSONReader::ReadAndReturnError(
       command, 0, NULL, &error_msg));
@@ -100,12 +75,38 @@ void ExecuteCommand(const std::string& command, std::string* response) {
   StatusCode out_status = kOk;
   scoped_ptr<base::Value> out_value;
   std::string out_session_id;
-  {
-    base::AutoLock(g_lazy_lock.Get());
-    if (!g_command_executor)
-      g_command_executor = g_executor_factory();
-  }
   g_command_executor->ExecuteCommand(
       name, *params, session_id, &out_status, &out_value, &out_session_id);
   SetResponse(out_status, out_value.get(), out_session_id, response);
 }
+
+void Shutdown() {
+  // TODO: Move this out to a separate doc.
+  // On shutdown, it is nice to quit all running sessions so that we don't
+  // have leftover Chrome processes and temporary user data dirs.
+  // To do this, we execute the quitAll command.
+  // Alternative shutdown behaviors:
+  // 1. If the user doesn't quit the session, don't clean it up.
+  //    This is what the FF driver does, except the temp dir is
+  //    cleaned up in FF because the client side is responsible for creating
+  //    the temp directory, not the driver.
+  // 2. Separate helper process that we spawn that is in charge of
+  //    launching processes and creating temporary files. This process
+  //    communicates to the helper via a socket. The helper process
+  //    kills all processes and deletes temp files if it sees the parent
+  //    die and then exits itself. This is more complicated, but it guarantees
+  //    that the processes and temp dirs are cleaned up, even if this process
+  //    exits abnormally.
+  // 3. Add Chrome command-line switch for socket/pipe that Chrome listens
+  //    to and exits if the pipe is closed. This is how the old
+  //    TestingAutomationProvider worked. However, this doesn't clean up
+  //    temp directories, unless we make Chrome clean its own directory too.
+  //    If Chrome crashes the directory would be leaked.
+  base::DictionaryValue params;
+  StatusCode status_code;
+  scoped_ptr<base::Value> value;
+  std::string session_id;
+  g_command_executor->ExecuteCommand(
+      "quitAll", params, "", &status_code, &value, &session_id);
+  delete g_command_executor;
+}
diff --git a/chrome/test/chromedriver/chromedriver.h b/chrome/test/chromedriver/chromedriver.h
index fbad292..5252da1 100644
--- a/chrome/test/chromedriver/chromedriver.h
+++ b/chrome/test/chromedriver/chromedriver.h
@@ -7,13 +7,12 @@
 
 #include <string>
 
-class CommandExecutor;
+#include "base/memory/scoped_ptr.h"
 
-typedef CommandExecutor* (*CommandExecutorFactoryFunc)();
+class CommandExecutor;
 
-// Sets the function to use for creating new |CommandExecutor|s. Only
-// for testing purposes.
-void SetCommandExecutorFactoryForTesting(CommandExecutorFactoryFunc func);
+// Inits the command executor. Must be called before |ExecuteCommand|.
+void Init(scoped_ptr<CommandExecutor> executor);
 
 // Synchronously executes the given command. Thread safe.
 // Command must be a JSON object:
@@ -32,4 +31,7 @@ void SetCommandExecutorFactoryForTesting(CommandExecutorFactoryFunc func);
 // property which signifies the error message.
 void ExecuteCommand(const std::string& command, std::string* response);
 
+// Shuts down the command executor. No commands must be currently executing.
+void Shutdown();
+
 #endif  // CHROME_TEST_CHROMEDRIVER_CHROMEDRIVER_H_
diff --git a/chrome/test/chromedriver/chromedriver.py b/chrome/test/chromedriver/chromedriver.py
index 02d395a..06afea6 100644
--- a/chrome/test/chromedriver/chromedriver.py
+++ b/chrome/test/chromedriver/chromedriver.py
@@ -5,6 +5,27 @@
 import ctypes
 import json
 
+class ChromeDriverException(Exception):
+  pass
+class UnknownCommand(ChromeDriverException):
+  pass
+class UnknownError(ChromeDriverException):
+  pass
+class SessionNotCreatedException(ChromeDriverException):
+  pass
+class NoSuchSession(ChromeDriverException):
+  pass
+
+def _ExceptionForResponse(response):
+  exception_class_map = {
+    9: UnknownCommand,
+    13: UnknownError,
+    33: SessionNotCreatedException,
+    100: NoSuchSession
+  }
+  status = response['status']
+  msg = response['value']['message']
+  return exception_class_map.get(status, ChromeDriverException)(msg)
 
 class ChromeDriver(object):
   """Starts and controls a single Chrome instance on this machine."""
@@ -29,7 +50,10 @@ class ChromeDriver(object):
         ctypes.byref(response_size))
     response_json = ctypes.string_at(response_data, response_size.value)
     self._lib.Free(response_data)
-    return json.loads(response_json)['value']
+    response = json.loads(response_json)
+    if response['status'] != 0:
+      raise _ExceptionForResponse(response)
+    return response['value']
 
   def _ExecuteSessionCommand(self, name, params={}):
     return self._ExecuteCommand(name, params, self._session_id)
diff --git a/chrome/test/chromedriver/chromedriver_shared_library.cc b/chrome/test/chromedriver/chromedriver_shared_library.cc
index c097b00..ec7dcf5 100644
--- a/chrome/test/chromedriver/chromedriver_shared_library.cc
+++ b/chrome/test/chromedriver/chromedriver_shared_library.cc
@@ -10,6 +10,8 @@
 #include "base/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "chrome/test/chromedriver/chromedriver.h"
+#include "chrome/test/chromedriver/command_executor.h"
+#include "chrome/test/chromedriver/command_executor_impl.h"
 #include "chrome/test/chromedriver/third_party/jni/jni.h"
 
 #if defined(OS_WIN)
@@ -77,17 +79,23 @@ EXPORT jstring Java_org_openqa_selenium_chrome_NewCommandExecutor_execute(
 
 #if defined(OS_WIN)
 BOOL APIENTRY DllMain(HMODULE module, DWORD reason, void* reserved) {
-  if (reason == DLL_PROCESS_ATTACH)
+  if (reason == DLL_PROCESS_ATTACH) {
     g_at_exit = new base::AtExitManager();
-  if (reason == DLL_PROCESS_DETACH)
+    Init(scoped_ptr<CommandExecutor>(new CommandExecutorImpl()));
+  }
+  if (reason == DLL_PROCESS_DETACH) {
+    Shutdown();
     delete g_at_exit;
+  }
   return TRUE;
 }
 #else
 void __attribute__((constructor)) OnLoad(void) {
   g_at_exit = new base::AtExitManager();
+  Init(scoped_ptr<CommandExecutor>(new CommandExecutorImpl()));
 }
 void __attribute__((destructor)) OnUnload(void) {
+  Shutdown();
   delete g_at_exit;
 }
 #endif
diff --git a/chrome/test/chromedriver/chromedriver_unittest.cc b/chrome/test/chromedriver/chromedriver_unittest.cc
index 5b3e248..9c85383 100644
--- a/chrome/test/chromedriver/chromedriver_unittest.cc
+++ b/chrome/test/chromedriver/chromedriver_unittest.cc
@@ -10,6 +10,7 @@
 #include "base/json/json_reader.h"
 #include "base/location.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/scoped_vector.h"
 #include "base/message_loop.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/threading/thread.h"
@@ -33,9 +34,9 @@ void ExpectExecuteError(const std::string& command) {
   EXPECT_EQ(kUnknownError, status);
 }
 
-class ExecutorMock : public CommandExecutor {
+class DummyExecutor : public CommandExecutor {
  public:
-  virtual ~ExecutorMock() {}
+  virtual ~DummyExecutor() {}
 
   virtual void ExecuteCommand(const std::string& name,
                               const base::DictionaryValue& params,
@@ -43,42 +44,74 @@ class ExecutorMock : public CommandExecutor {
                               StatusCode* status,
                               scoped_ptr<base::Value>* value,
                               std::string* out_session_id) OVERRIDE {
-    EXPECT_STREQ("name", name.c_str());
-    int param = 0;
-    EXPECT_TRUE(params.GetInteger("param", &param));
-    EXPECT_EQ(100, param);
-    EXPECT_STREQ("id", session_id.c_str());
-    *status = kOk;
-    value->reset(new base::StringValue("stuff"));
-    *out_session_id = "session_id";
   }
+};
 
-  static void CheckExecuteCommand()  {
-    std::string response;
-    ::ExecuteCommand("{\"name\": \"name\", "
-                     " \"parameters\": {\"param\": 100}, "
-                     " \"sessionId\": \"id\"}",
-                     &response);
-    scoped_ptr<base::Value> value(base::JSONReader::Read(response));
-    ASSERT_TRUE(value.get());
-    base::DictionaryValue* dict;
-    ASSERT_TRUE(value->GetAsDictionary(&dict));
-    int status;
-    ASSERT_TRUE(dict->GetInteger("status", &status));
-    std::string value_str;
-    ASSERT_TRUE(dict->GetString("value", &value_str));
-    EXPECT_STREQ("stuff", value_str.c_str());
-    EXPECT_EQ(kOk, status);
+
+struct ExpectedCommand {
+  ExpectedCommand(
+      const std::string& name,
+      const base::DictionaryValue& in_params,
+      const std::string& session_id,
+      StatusCode return_status,
+      scoped_ptr<base::Value> return_value,
+      const std::string& return_session_id)
+      : name(name),
+        session_id(session_id),
+        return_status(return_status),
+        return_value(return_value.Pass()),
+        return_session_id(return_session_id) {
+    params.MergeDictionary(&in_params);
   }
+
+  ~ExpectedCommand() {}
+
+  std::string name;
+  base::DictionaryValue params;
+  std::string session_id;
+  StatusCode return_status;
+  scoped_ptr<base::Value> return_value;
+  std::string return_session_id;
 };
 
-CommandExecutor* CreateExecutorMock() {
-  return new ExecutorMock();
-}
+class ExecutorMock : public CommandExecutor {
+ public:
+  virtual ~ExecutorMock() {
+    EXPECT_TRUE(DidSatisfyExpectations());
+  }
+
+  virtual void ExecuteCommand(const std::string& name,
+                              const base::DictionaryValue& params,
+                              const std::string& session_id,
+                              StatusCode* status,
+                              scoped_ptr<base::Value>* value,
+                              std::string* out_session_id) OVERRIDE {
+    ASSERT_TRUE(expectations_.size());
+    ASSERT_STREQ(expectations_[0]->name.c_str(), name.c_str());
+    ASSERT_TRUE(expectations_[0]->params.Equals(&params));
+    ASSERT_STREQ(expectations_[0]->session_id.c_str(), session_id.c_str());
+    *status = expectations_[0]->return_status;
+    value->reset(expectations_[0]->return_value.release());
+    *out_session_id = expectations_[0]->return_session_id;
+    expectations_.erase(expectations_.begin());
+  }
+
+  void Expect(scoped_ptr<ExpectedCommand> expected) {
+    expectations_.push_back(expected.release());
+  }
+
+  bool DidSatisfyExpectations() const {
+    return expectations_.empty();
+  }
+
+ private:
+  ScopedVector<ExpectedCommand> expectations_;
+};
 
 }  // namespace
 
 TEST(ChromeDriver, InvalidCommands) {
+  Init(scoped_ptr<CommandExecutor>(new DummyExecutor()));
   ExpectExecuteError("hi[]");
   ExpectExecuteError("[]");
   ExpectExecuteError(
@@ -93,9 +126,43 @@ TEST(ChromeDriver, InvalidCommands) {
       "{\"name\": \"\", \"parameters\": {}}");
   ExpectExecuteError(
       "{\"name\": \"\", \"parameters\": {}, \"sessionId\": 1}");
+  Shutdown();
 }
 
 TEST(ChromeDriver, ExecuteCommand) {
-  SetCommandExecutorFactoryForTesting(&CreateExecutorMock);
-  ExecutorMock::CheckExecuteCommand();
+  scoped_ptr<ExecutorMock> scoped_mock(new ExecutorMock());
+  ExecutorMock* mock = scoped_mock.get();
+  Init(scoped_mock.PassAs<CommandExecutor>());
+  {
+    base::DictionaryValue params;
+    params.SetInteger("param", 100);
+    scoped_ptr<base::Value> value(new base::StringValue("stuff"));
+    mock->Expect(scoped_ptr<ExpectedCommand>(new ExpectedCommand(
+        "name", params, "id", kOk, value.Pass(), "session_id")));
+  }
+  std::string response;
+  ExecuteCommand("{\"name\": \"name\", "
+                 " \"parameters\": {\"param\": 100}, "
+                 " \"sessionId\": \"id\"}",
+                 &response);
+  ASSERT_TRUE(mock->DidSatisfyExpectations());
+  {
+    scoped_ptr<base::Value> value(base::JSONReader::Read(response));
+    ASSERT_TRUE(value.get());
+    base::DictionaryValue* dict;
+    ASSERT_TRUE(value->GetAsDictionary(&dict));
+    int status;
+    ASSERT_TRUE(dict->GetInteger("status", &status));
+    std::string value_str;
+    ASSERT_TRUE(dict->GetString("value", &value_str));
+    EXPECT_STREQ("stuff", value_str.c_str());
+    EXPECT_EQ(kOk, status);
+  }
+  {
+    base::DictionaryValue params;
+    scoped_ptr<base::Value> value(base::Value::CreateNullValue());
+    mock->Expect(scoped_ptr<ExpectedCommand>(new ExpectedCommand(
+        "quitAll", params, "", kOk, value.Pass(), "")));
+  }
+  Shutdown();
 }
diff --git a/chrome/test/chromedriver/command.h b/chrome/test/chromedriver/command.h
new file mode 100644
index 0000000..6654694
--- /dev/null
+++ b/chrome/test/chromedriver/command.h
@@ -0,0 +1,26 @@
+// Copyright (c) 2012 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_TEST_CHROMEDRIVER_COMMAND_H_
+#define CHROME_TEST_CHROMEDRIVER_COMMAND_H_
+
+#include <string>
+
+#include "base/callback_forward.h"
+#include "base/memory/scoped_ptr.h"
+
+namespace base {
+class DictionaryValue;
+class Value;
+}
+
+class Status;
+
+typedef base::Callback<Status(
+    const base::DictionaryValue&,
+    const std::string&,
+    scoped_ptr<base::Value>*,
+    std::string*)> Command;
+
+#endif  // CHROME_TEST_CHROMEDRIVER_COMMAND_H_
diff --git a/chrome/test/chromedriver/command_executor_impl.cc b/chrome/test/chromedriver/command_executor_impl.cc
index 422d0ea..7d21a5f 100644
--- a/chrome/test/chromedriver/command_executor_impl.cc
+++ b/chrome/test/chromedriver/command_executor_impl.cc
@@ -6,24 +6,31 @@
 
 #include "base/bind.h"
 #include "base/callback.h"
-#include "base/string_number_conversions.h"
 #include "base/values.h"
+#include "chrome/test/chromedriver/commands.h"
+#include "chrome/test/chromedriver/session.h"
+#include "chrome/test/chromedriver/session_command.h"
+#include "chrome/test/chromedriver/session_map.h"
 #include "chrome/test/chromedriver/status.h"
 
-namespace {
-
-void ExecuteQuit(
-    const base::DictionaryValue& params,
-    Status* status,
-    scoped_ptr<base::Value>* value) {
-}
-
-}  // namespace
-
-CommandExecutorImpl::CommandExecutorImpl()
-    : next_session_id_(1) {
-  session_command_map_.insert(std::make_pair("quit", base::Bind(
-      &ExecuteQuit)));
+CommandExecutorImpl::CommandExecutorImpl() {
+  base::Callback<Status(
+      const SessionCommand&,
+      const base::DictionaryValue&,
+      const std::string&,
+      scoped_ptr<base::Value>*,
+      std::string*)> execute_session_command = base::Bind(
+          &ExecuteSessionCommand,
+          &session_map_);
+  Command quit_command = base::Bind(execute_session_command,
+      base::Bind(&ExecuteQuit, &session_map_));
+  command_map_.Set("quit", quit_command);
+
+  // Non-session commands.
+  command_map_.Set("newSession",
+      base::Bind(&ExecuteNewSession, &session_map_));
+  command_map_.Set("quitAll",
+      base::Bind(&ExecuteQuitAll, quit_command, &session_map_));
 }
 
 CommandExecutorImpl::~CommandExecutorImpl() {}
@@ -35,46 +42,20 @@ void CommandExecutorImpl::ExecuteCommand(
     StatusCode* status_code,
     scoped_ptr<base::Value>* value,
     std::string* out_session_id) {
-  base::AutoLock auto_lock(lock_);
+  Command cmd;
   Status status(kOk);
-  ExecuteCommandInternal(name, params, session_id, &status, value,
-                         out_session_id);
+  if (command_map_.Get(name, &cmd)) {
+    status = cmd.Run(params, session_id, value, out_session_id);
+  } else {
+    status = Status(kUnknownCommand, name);
+    *out_session_id = session_id;
+  }
   *status_code = status.code();
   if (status.IsError()) {
-    base::DictionaryValue* error = new base::DictionaryValue();
+    scoped_ptr<base::DictionaryValue> error(new base::DictionaryValue());
     error->SetString("message", status.message());
-    value->reset(error);
+    value->reset(error.release());
   }
   if (!*value)
     value->reset(base::Value::CreateNullValue());
 }
-
-void CommandExecutorImpl::ExecuteCommandInternal(
-    const std::string& name,
-    const base::DictionaryValue& params,
-    const std::string& session_id,
-    Status* status,
-    scoped_ptr<base::Value>* value,
-    std::string* out_session_id) {
-  *out_session_id = session_id;
-  if (name == "newSession") {
-    ExecuteNewSession(out_session_id);
-    return;
-  }
-
-  // Although most versions of the C++ std lib supposedly guarantee
-  // thread safety for concurrent reads, better safe than sorry.
-  SessionCommandMap::const_iterator iter = session_command_map_.find(name);
-  if (iter != session_command_map_.end()) {
-    SessionCommandCallback command_callback = iter->second;
-    base::AutoUnlock auto_unlock(lock_);
-    command_callback.Run(params, status, value);
-    return;
-  }
-
-  *status = Status(kUnknownCommand, name);
-}
-
-void CommandExecutorImpl::ExecuteNewSession(std::string* session_id) {
-  *session_id = base::IntToString(next_session_id_++);
-}
diff --git a/chrome/test/chromedriver/command_executor_impl.h b/chrome/test/chromedriver/command_executor_impl.h
index 63cdc22..f7f5e5e 100644
--- a/chrome/test/chromedriver/command_executor_impl.h
+++ b/chrome/test/chromedriver/command_executor_impl.h
@@ -5,16 +5,18 @@
 #ifndef CHROME_TEST_CHROMEDRIVER_COMMAND_EXECUTOR_IMPL_H_
 #define CHROME_TEST_CHROMEDRIVER_COMMAND_EXECUTOR_IMPL_H_
 
-#include <map>
 #include <string>
 
 #include "base/basictypes.h"
-#include "base/callback.h"
+#include "base/callback_forward.h"
 #include "base/compiler_specific.h"
+#include "base/gtest_prod_util.h"
 #include "base/memory/scoped_ptr.h"
-#include "base/synchronization/lock.h"
+#include "chrome/test/chromedriver/command.h"
 #include "chrome/test/chromedriver/command_executor.h"
+#include "chrome/test/chromedriver/session_map.h"
 #include "chrome/test/chromedriver/status.h"
+#include "chrome/test/chromedriver/synchronized_map.h"
 
 namespace base {
 class DictionaryValue;
@@ -35,26 +37,13 @@ class CommandExecutorImpl : public CommandExecutor {
                               std::string* out_session_id) OVERRIDE;
 
  private:
-  typedef base::Callback<void(
-      const base::DictionaryValue& params,
-      Status* status,
-      scoped_ptr<base::Value>* value)>
-          SessionCommandCallback;
-  typedef std::map<std::string, SessionCommandCallback> SessionCommandMap;
-
-  void ExecuteCommandInternal(
-      const std::string& name,
-      const base::DictionaryValue& params,
-      const std::string& session_id,
-      Status* status,
-      scoped_ptr<base::Value>* value,
-      std::string* out_session_id);
-
-  void ExecuteNewSession(std::string* session_id);
-
-  base::Lock lock_;
-  int next_session_id_;
-  SessionCommandMap session_command_map_;
+  FRIEND_TEST_ALL_PREFIXES(CommandExecutorImplTest, SimpleCommand);
+  FRIEND_TEST_ALL_PREFIXES(
+      CommandExecutorImplTest, CommandThatDoesntSetValueOrSessionId);
+  FRIEND_TEST_ALL_PREFIXES(CommandExecutorImplTest, CommandThatReturnsError);
+
+  SessionMap session_map_;
+  SynchronizedMap<std::string, Command> command_map_;
 
   DISALLOW_COPY_AND_ASSIGN(CommandExecutorImpl);
 };
diff --git a/chrome/test/chromedriver/command_executor_impl_unittest.cc b/chrome/test/chromedriver/command_executor_impl_unittest.cc
index 0485656..61c7ffb 100644
--- a/chrome/test/chromedriver/command_executor_impl_unittest.cc
+++ b/chrome/test/chromedriver/command_executor_impl_unittest.cc
@@ -4,30 +4,15 @@
 
 #include <string>
 
+#include "base/bind.h"
+#include "base/callback.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/values.h"
 #include "chrome/test/chromedriver/command_executor_impl.h"
 #include "chrome/test/chromedriver/status.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-TEST(CommandExecutorTest, NewSession) {
-  CommandExecutorImpl executor;
-  base::DictionaryValue empty_dict;
-  StatusCode status;
-  scoped_ptr<base::Value> value;
-  std::string session_id;
-  executor.ExecuteCommand("newSession", empty_dict, "",
-                          &status, &value, &session_id);
-  ASSERT_EQ(kOk, status);
-  ASSERT_TRUE(value.get());
-  ASSERT_TRUE(value->IsType(base::Value::TYPE_NULL));
-  ASSERT_STREQ("1", session_id.c_str());
-  executor.ExecuteCommand("newSession", empty_dict, "random",
-                          &status, &value, &session_id);
-  ASSERT_STREQ("2", session_id.c_str());
-}
-
-TEST(CommandExecutorTest, UnknownCommand) {
+TEST(CommandExecutorImplTest, UnknownCommand) {
   CommandExecutorImpl executor;
   base::DictionaryValue empty_dict;
   StatusCode status;
@@ -44,16 +29,103 @@ TEST(CommandExecutorTest, UnknownCommand) {
   ASSERT_STREQ("session", session_id.c_str());
 }
 
-TEST(CommandExecutorTest, Quit) {
+namespace {
+
+Status ExecuteSimpleCommand(
+    const base::DictionaryValue* expected_params,
+    const std::string& expected_session_id,
+    const base::DictionaryValue& params,
+    const std::string& session_id,
+    scoped_ptr<base::Value>* value,
+    std::string* out_session_id) {
+  EXPECT_EQ(expected_params, &params);
+  EXPECT_STREQ(expected_session_id.c_str(), session_id.c_str());
+  value->reset(new base::StringValue("hi"));
+  *out_session_id = "out session id";
+  return Status(kOk);
+}
+
+}  // namespace
+
+TEST(CommandExecutorImplTest, SimpleCommand) {
   CommandExecutorImpl executor;
-  base::DictionaryValue empty_dict;
-  StatusCode status;
+  base::DictionaryValue params;
+  std::string session_id("some id");
+  executor.command_map_.Set("simpleCommand",
+      base::Bind(&ExecuteSimpleCommand, &params, session_id));
+
+  StatusCode status_code;
+  scoped_ptr<base::Value> value;
+  std::string out_session_id;
+  executor.ExecuteCommand("simpleCommand", params, session_id,
+                          &status_code, &value,
+                          &out_session_id);
+  ASSERT_EQ(kOk, status_code);
+  ASSERT_TRUE(value);
+  base::StringValue hi("hi");
+  ASSERT_TRUE(value->Equals(&hi));
+  ASSERT_STREQ("out session id", out_session_id.c_str());
+}
+
+namespace {
+
+Status ExecuteSimpleCommand2(
+    const base::DictionaryValue& params,
+    const std::string& session_id,
+    scoped_ptr<base::Value>* out_value,
+    std::string* out_session_id) {
+  return Status(kOk);
+}
+
+}  // namespace
+
+TEST(CommandExecutorImplTest, CommandThatDoesntSetValueOrSessionId) {
+  CommandExecutorImpl executor;
+  executor.command_map_.Set(
+      "simpleCommand",
+      base::Bind(&ExecuteSimpleCommand2));
+
+  base::DictionaryValue params;
+  StatusCode status_code;
   scoped_ptr<base::Value> value;
   std::string session_id;
-  executor.ExecuteCommand("quit", empty_dict, "id",
-                          &status, &value, &session_id);
-  ASSERT_EQ(kOk, status);
-  ASSERT_TRUE(value.get());
+  executor.ExecuteCommand("simpleCommand", params, "session",
+                          &status_code, &value, &session_id);
+
   ASSERT_TRUE(value->IsType(base::Value::TYPE_NULL));
-  ASSERT_STREQ("id", session_id.c_str());
+  ASSERT_STREQ("", session_id.c_str());
+}
+
+namespace {
+
+Status ExecuteSimpleCommand3(
+    const base::DictionaryValue& params,
+    const std::string& session_id,
+    scoped_ptr<base::Value>* value,
+    std::string* out_session_id) {
+  value->reset(new base::StringValue("hi"));
+  return Status(kUnknownError);
+}
+
+}  // namespace
+
+TEST(CommandExecutorImplTest, CommandThatReturnsError) {
+  CommandExecutorImpl executor;
+  executor.command_map_.Set("simpleCommand",
+                            base::Bind(&ExecuteSimpleCommand3));
+
+  base::DictionaryValue params;
+  StatusCode status_code;
+  scoped_ptr<base::Value> value;
+  std::string out_session_id;
+  executor.ExecuteCommand("simpleCommand", params, "",
+                          &status_code, &value,
+                          &out_session_id);
+  ASSERT_EQ(kUnknownError, status_code);
+  ASSERT_TRUE(value);
+  base::DictionaryValue* error;
+  ASSERT_TRUE(value->GetAsDictionary(&error));
+  std::string message;
+  ASSERT_TRUE(error->GetString("message", &message));
+  ASSERT_STREQ(Status(kUnknownError).message().c_str(), message.c_str());
 }
diff --git a/chrome/test/chromedriver/commands.cc b/chrome/test/chromedriver/commands.cc
new file mode 100644
index 0000000..9d62687
--- /dev/null
+++ b/chrome/test/chromedriver/commands.cc
@@ -0,0 +1,60 @@
+// Copyright (c) 2012 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/test/chromedriver/commands.h"
+
+#include "base/bind.h"
+#include "base/callback.h"
+#include "base/format_macros.h"
+#include "base/rand_util.h"
+#include "base/stringprintf.h"
+#include "base/values.h"
+#include "chrome/test/chromedriver/session.h"
+#include "chrome/test/chromedriver/status.h"
+
+Status ExecuteNewSession(
+    SessionMap* session_map,
+    const base::DictionaryValue& params,
+    const std::string& session_id,
+    scoped_ptr<base::Value>* out_value,
+    std::string* out_session_id) {
+  uint64 msb = base::RandUint64();
+  uint64 lsb = base::RandUint64();
+  std::string new_id =
+      base::StringPrintf("%016" PRIx64 "%016" PRIx64, msb, lsb);
+  scoped_ptr<Session> session(new Session(new_id));
+  scoped_refptr<SessionAccessor> accessor(
+      new SessionAccessorImpl(session.Pass()));
+  session_map->Set(new_id, accessor);
+
+  out_value->reset(new base::StringValue(new_id));
+  *out_session_id = new_id;
+  return Status(kOk);
+}
+
+Status ExecuteQuitAll(
+    Command quit_command,
+    SessionMap* session_map,
+    const base::DictionaryValue& params,
+    const std::string& session_id,
+    scoped_ptr<base::Value>* out_value,
+    std::string* out_session_id) {
+  std::vector<std::string> session_ids;
+  session_map->GetKeys(&session_ids);
+  for (size_t i = 0; i < session_ids.size(); ++i) {
+    scoped_ptr<base::Value> unused_value;
+    std::string unused_session_id;
+    quit_command.Run(params, session_ids[i], &unused_value, &unused_session_id);
+  }
+  return Status(kOk);
+}
+
+Status ExecuteQuit(
+    SessionMap* session_map,
+    Session* session,
+    const base::DictionaryValue& params,
+    scoped_ptr<base::Value>* value) {
+  CHECK(session_map->Remove(session->id));
+  return Status(kOk);
+}
diff --git a/chrome/test/chromedriver/commands.h b/chrome/test/chromedriver/commands.h
new file mode 100644
index 0000000..50752b8
--- /dev/null
+++ b/chrome/test/chromedriver/commands.h
@@ -0,0 +1,47 @@
+// Copyright (c) 2012 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_TEST_CHROMEDRIVER_COMMANDS_H_
+#define CHROME_TEST_CHROMEDRIVER_COMMANDS_H_
+
+#include <map>
+#include <string>
+
+#include "base/callback_forward.h"
+#include "base/memory/scoped_ptr.h"
+#include "chrome/test/chromedriver/command.h"
+#include "chrome/test/chromedriver/session_map.h"
+
+namespace base {
+class DictionaryValue;
+class Value;
+}
+
+struct Session;
+
+// Creates a new session.
+Status ExecuteNewSession(
+    SessionMap* session_map,
+    const base::DictionaryValue& params,
+    const std::string& session_id,
+    scoped_ptr<base::Value>* out_value,
+    std::string* out_session_id);
+
+// Quits all sessions.
+Status ExecuteQuitAll(
+    Command quit_command,
+    SessionMap* session_map,
+    const base::DictionaryValue& params,
+    const std::string& session_id,
+    scoped_ptr<base::Value>* out_value,
+    std::string* out_session_id);
+
+// Quits a particular session.
+Status ExecuteQuit(
+    SessionMap* session_map,
+    Session* session,
+    const base::DictionaryValue& params,
+    scoped_ptr<base::Value>* value);
+
+#endif  // CHROME_TEST_CHROMEDRIVER_COMMANDS_H_
diff --git a/chrome/test/chromedriver/commands_unittest.cc b/chrome/test/chromedriver/commands_unittest.cc
new file mode 100644
index 0000000..2149e32
--- /dev/null
+++ b/chrome/test/chromedriver/commands_unittest.cc
@@ -0,0 +1,89 @@
+// Copyright (c) 2012 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 "base/bind.h"
+#include "base/callback.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/synchronization/lock.h"
+#include "base/values.h"
+#include "chrome/test/chromedriver/command_executor_impl.h"
+#include "chrome/test/chromedriver/commands.h"
+#include "chrome/test/chromedriver/fake_session_accessor.h"
+#include "chrome/test/chromedriver/status.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+TEST(CommandsTest, NewSession) {
+  SessionMap map;
+  base::DictionaryValue params;
+  scoped_ptr<base::Value> value;
+  std::string session_id;
+  Status status =
+      ExecuteNewSession(&map, params, "", &value, &session_id);
+  ASSERT_EQ(kOk, status.code());
+  ASSERT_TRUE(value);
+  std::string id;
+  ASSERT_TRUE(value->GetAsString(&id));
+  ASSERT_EQ(32u, id.length());
+  ASSERT_STREQ(id.c_str(), session_id.c_str());
+  scoped_refptr<SessionAccessor> accessor;
+  ASSERT_TRUE(map.Get(id, &accessor));
+  scoped_ptr<base::AutoLock> lock;
+  Session* session = accessor->Access(&lock);
+  ASSERT_TRUE(session);
+  ASSERT_STREQ(id.c_str(), session->id.c_str());
+}
+
+namespace {
+
+Status ExecuteStubQuit(
+    int* count,
+    const base::DictionaryValue& params,
+    const std::string& session_id,
+    scoped_ptr<base::Value>* value,
+    std::string* out_session_id) {
+  if (*count == 0) {
+    EXPECT_STREQ("id", session_id.c_str());
+  } else {
+    EXPECT_STREQ("id2", session_id.c_str());
+  }
+  (*count)++;
+  return Status(kOk);
+}
+
+}  // namespace
+
+TEST(CommandsTest, QuitAll) {
+  SessionMap map;
+  Session session("id");
+  Session session2("id2");
+  map.Set(session.id,
+          scoped_refptr<SessionAccessor>(new FakeSessionAccessor(&session)));
+  map.Set(session2.id,
+          scoped_refptr<SessionAccessor>(new FakeSessionAccessor(&session2)));
+
+  int count = 0;
+  Command cmd = base::Bind(&ExecuteStubQuit, &count);
+  base::DictionaryValue params;
+  scoped_ptr<base::Value> value;
+  std::string session_id;
+  Status status =
+      ExecuteQuitAll(cmd, &map, params, "", &value, &session_id);
+  ASSERT_EQ(kOk, status.code());
+  ASSERT_FALSE(value.get());
+  ASSERT_EQ(2, count);
+}
+
+TEST(CommandsTest, Quit) {
+  SessionMap map;
+  Session session("id");
+  map.Set(session.id,
+          scoped_refptr<SessionAccessor>(new FakeSessionAccessor(&session)));
+  base::DictionaryValue params;
+  scoped_ptr<base::Value> value;
+  ASSERT_EQ(kOk, ExecuteQuit(&map, &session, params, &value).code());
+  ASSERT_FALSE(map.Has(session.id));
+  ASSERT_FALSE(value.get());
+}
diff --git a/chrome/test/chromedriver/fake_session_accessor.cc b/chrome/test/chromedriver/fake_session_accessor.cc
new file mode 100644
index 0000000..c076752
--- /dev/null
+++ b/chrome/test/chromedriver/fake_session_accessor.cc
@@ -0,0 +1,15 @@
+// Copyright (c) 2012 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/test/chromedriver/fake_session_accessor.h"
+
+FakeSessionAccessor::FakeSessionAccessor(Session* session)
+    : session_(session) {}
+
+Session* FakeSessionAccessor::Access(
+    scoped_ptr<base::AutoLock>* lock) {
+  return session_;
+}
+
+FakeSessionAccessor::~FakeSessionAccessor() {}
diff --git a/chrome/test/chromedriver/fake_session_accessor.h b/chrome/test/chromedriver/fake_session_accessor.h
new file mode 100644
index 0000000..da29aa2
--- /dev/null
+++ b/chrome/test/chromedriver/fake_session_accessor.h
@@ -0,0 +1,31 @@
+// Copyright (c) 2012 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_TEST_CHROMEDRIVER_FAKE_SESSION_ACCESSOR_H_
+#define CHROME_TEST_CHROMEDRIVER_FAKE_SESSION_ACCESSOR_H_
+
+#include "base/basictypes.h"
+#include "base/compiler_specific.h"
+#include "base/memory/scoped_ptr.h"
+#include "chrome/test/chromedriver/session.h"
+
+namespace base {
+class AutoLock;
+}
+
+// Fake session accessor that doesn't actually lock.
+class FakeSessionAccessor : public SessionAccessor {
+ public:
+  explicit FakeSessionAccessor(Session* session);
+
+  // Overriden from SessionAccessor:
+  virtual Session* Access(scoped_ptr<base::AutoLock>* lock) OVERRIDE;
+
+ private:
+  virtual ~FakeSessionAccessor();
+
+  Session* session_;
+};
+
+#endif  // CHROME_TEST_CHROMEDRIVER_FAKE_SESSION_ACCESSOR_H_
diff --git a/chrome/test/chromedriver/session.cc b/chrome/test/chromedriver/session.cc
new file mode 100644
index 0000000..ae35578
--- /dev/null
+++ b/chrome/test/chromedriver/session.cc
@@ -0,0 +1,19 @@
+// Copyright (c) 2012 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/test/chromedriver/session.h"
+
+Session::Session(const std::string& id) : id(id) {}
+
+Session::~Session() {}
+
+SessionAccessorImpl::SessionAccessorImpl(scoped_ptr<Session> session)
+    : session_(session.Pass()) {}
+
+Session* SessionAccessorImpl::Access(scoped_ptr<base::AutoLock>* lock) {
+  lock->reset(new base::AutoLock(session_lock_));
+  return session_.get();
+}
+
+SessionAccessorImpl::~SessionAccessorImpl() {}
diff --git a/chrome/test/chromedriver/session.h b/chrome/test/chromedriver/session.h
new file mode 100644
index 0000000..266fe72
--- /dev/null
+++ b/chrome/test/chromedriver/session.h
@@ -0,0 +1,46 @@
+// Copyright (c) 2012 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_TEST_CHROMEDRIVER_SESSION_H_
+#define CHROME_TEST_CHROMEDRIVER_SESSION_H_
+
+#include <string>
+
+#include "base/basictypes.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/synchronization/lock.h"
+
+struct Session {
+  explicit Session(const std::string& id);
+  ~Session();
+
+  const std::string id;
+};
+
+class SessionAccessor : public base::RefCountedThreadSafe<SessionAccessor> {
+ public:
+  virtual Session* Access(scoped_ptr<base::AutoLock>* lock) = 0;
+
+ protected:
+  friend class base::RefCountedThreadSafe<SessionAccessor>;
+  virtual ~SessionAccessor() {}
+};
+
+class SessionAccessorImpl : public SessionAccessor {
+ public:
+  explicit SessionAccessorImpl(scoped_ptr<Session> session);
+
+  virtual Session* Access(scoped_ptr<base::AutoLock>* lock) OVERRIDE;
+
+ private:
+  virtual ~SessionAccessorImpl();
+
+  base::Lock session_lock_;
+  scoped_ptr<Session> session_;
+
+  DISALLOW_COPY_AND_ASSIGN(SessionAccessorImpl);
+};
+
+#endif  // CHROME_TEST_CHROMEDRIVER_SESSION_H_
diff --git a/chrome/test/chromedriver/session_command.cc b/chrome/test/chromedriver/session_command.cc
new file mode 100644
index 0000000..beee00d
--- /dev/null
+++ b/chrome/test/chromedriver/session_command.cc
@@ -0,0 +1,31 @@
+// Copyright (c) 2012 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/test/chromedriver/session_command.h"
+
+#include "base/callback.h"
+#include "base/memory/ref_counted.h"
+#include "base/synchronization/lock.h"
+#include "base/values.h"
+#include "chrome/test/chromedriver/session.h"
+#include "chrome/test/chromedriver/session_map.h"
+#include "chrome/test/chromedriver/status.h"
+
+Status ExecuteSessionCommand(
+    SessionMap* session_map,
+    const SessionCommand& command,
+    const base::DictionaryValue& params,
+    const std::string& session_id,
+    scoped_ptr<base::Value>* out_value,
+    std::string* out_session_id) {
+  *out_session_id = session_id;
+  scoped_refptr<SessionAccessor> session_accessor;
+  if (!session_map->Get(session_id, &session_accessor))
+    return Status(kNoSuchSession, session_id);
+  scoped_ptr<base::AutoLock> session_lock;
+  Session* session = session_accessor->Access(&session_lock);
+  if (!session)
+    return Status(kNoSuchSession, session_id);
+  return command.Run(session, params, out_value);
+}
diff --git a/chrome/test/chromedriver/session_command.h b/chrome/test/chromedriver/session_command.h
new file mode 100644
index 0000000..f371339
--- /dev/null
+++ b/chrome/test/chromedriver/session_command.h
@@ -0,0 +1,37 @@
+// Copyright (c) 2012 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_TEST_CHROMEDRIVER_SESSION_COMMAND_H_
+#define CHROME_TEST_CHROMEDRIVER_SESSION_COMMAND_H_
+
+#include <string>
+
+#include "base/callback_forward.h"
+#include "base/memory/scoped_ptr.h"
+#include "chrome/test/chromedriver/session_map.h"
+
+namespace base {
+class DictionaryValue;
+class Value;
+}
+
+struct Session;
+class Status;
+
+typedef base::Callback<Status(
+    Session* session,
+    const base::DictionaryValue&,
+    scoped_ptr<base::Value>*)> SessionCommand;
+
+// Executes a given session command, after acquiring access to the appropriate
+// session.
+Status ExecuteSessionCommand(
+    SessionMap* session_map,
+    const SessionCommand& command,
+    const base::DictionaryValue& params,
+    const std::string& session_id,
+    scoped_ptr<base::Value>* out_value,
+    std::string* out_session_id);
+
+#endif  // CHROME_TEST_CHROMEDRIVER_SESSION_COMMAND_H_
diff --git a/chrome/test/chromedriver/session_command_unittest.cc b/chrome/test/chromedriver/session_command_unittest.cc
new file mode 100644
index 0000000..c766b47
--- /dev/null
+++ b/chrome/test/chromedriver/session_command_unittest.cc
@@ -0,0 +1,96 @@
+// Copyright (c) 2012 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 "base/bind.h"
+#include "base/callback.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/values.h"
+#include "chrome/test/chromedriver/fake_session_accessor.h"
+#include "chrome/test/chromedriver/session.h"
+#include "chrome/test/chromedriver/session_command.h"
+#include "chrome/test/chromedriver/session_map.h"
+#include "chrome/test/chromedriver/status.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+Status ExecuteSimpleCommand(
+    Session* expected_session,
+    base::DictionaryValue* expected_params,
+    base::Value* value,
+    Session* session,
+    const base::DictionaryValue& params,
+    scoped_ptr<base::Value>* return_value) {
+  EXPECT_EQ(expected_session, session);
+  EXPECT_TRUE(expected_params->Equals(&params));
+  return_value->reset(value->DeepCopy());
+  return Status(kOk);
+}
+
+}  // namespace
+
+TEST(SessionCommandTest, SimpleCommand) {
+  SessionMap map;
+  Session session("session");
+  scoped_refptr<SessionAccessor> accessor(new FakeSessionAccessor(&session));
+  map.Set(session.id, accessor);
+
+  base::DictionaryValue params;
+  params.SetInteger("param", 5);
+  base::FundamentalValue expected_value(6);
+  SessionCommand cmd = base::Bind(
+      &ExecuteSimpleCommand, &session, &params, &expected_value);
+
+  scoped_ptr<base::Value> value;
+  std::string session_id;
+  Status status = ExecuteSessionCommand(
+      &map, cmd, params,
+      session.id, &value, &session_id);
+  ASSERT_EQ(kOk, status.code());
+  ASSERT_TRUE(expected_value.Equals(value.get()));
+  ASSERT_STREQ(session.id.c_str(), session_id.c_str());
+}
+
+namespace {
+
+Status ShouldNotBeCalled(
+    Session* session,
+    const base::DictionaryValue& params,
+    scoped_ptr<base::Value>* value) {
+  EXPECT_TRUE(false);
+  return Status(kOk);
+}
+
+}  // namespace
+
+TEST(SessionCommandTest, NoSuchSession) {
+  SessionMap map;
+  base::DictionaryValue params;
+  scoped_ptr<base::Value> value;
+  std::string session_id;
+  Status status = ExecuteSessionCommand(
+      &map, base::Bind(&ShouldNotBeCalled), params,
+      "session", &value, &session_id);
+  ASSERT_EQ(kNoSuchSession, status.code());
+  ASSERT_FALSE(value.get());
+  ASSERT_STREQ("session", session_id.c_str());
+}
+
+TEST(SessionCommandTest, SessionDeletedWhileWaiting) {
+  SessionMap map;
+  scoped_refptr<SessionAccessor> accessor(new FakeSessionAccessor(NULL));
+  map.Set("session", accessor);
+
+  base::DictionaryValue params;
+  scoped_ptr<base::Value> value;
+  std::string session_id;
+  Status status = ExecuteSessionCommand(
+      &map, base::Bind(&ShouldNotBeCalled), params,
+      "session", &value, &session_id);
+  ASSERT_EQ(kNoSuchSession, status.code());
+  ASSERT_FALSE(value.get());
+  ASSERT_STREQ("session", session_id.c_str());
+}
diff --git a/chrome/test/chromedriver/session_map.h b/chrome/test/chromedriver/session_map.h
new file mode 100644
index 0000000..1af8884
--- /dev/null
+++ b/chrome/test/chromedriver/session_map.h
@@ -0,0 +1,18 @@
+// Copyright (c) 2012 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_TEST_CHROMEDRIVER_SESSION_MAP_H_
+#define CHROME_TEST_CHROMEDRIVER_SESSION_MAP_H_
+
+#include <string>
+
+#include "base/memory/ref_counted.h"
+#include "chrome/test/chromedriver/synchronized_map.h"
+
+class SessionAccessor;
+
+typedef SynchronizedMap<std::string, scoped_refptr<SessionAccessor> >
+    SessionMap;
+
+#endif  // CHROME_TEST_CHROMEDRIVER_SESSION_MAP_H_
diff --git a/chrome/test/chromedriver/session_unittest.cc b/chrome/test/chromedriver/session_unittest.cc
new file mode 100644
index 0000000..8d2e228
--- /dev/null
+++ b/chrome/test/chromedriver/session_unittest.cc
@@ -0,0 +1,21 @@
+// Copyright (c) 2012 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 "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/synchronization/lock.h"
+#include "chrome/test/chromedriver/session.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+TEST(SessionAccessorTest, LocksSession) {
+  scoped_ptr<Session> scoped_session(new Session("id"));
+  Session* session = scoped_session.get();
+  scoped_refptr<SessionAccessor> accessor(
+      new SessionAccessorImpl(scoped_session.Pass()));
+  scoped_ptr<base::AutoLock> lock;
+  ASSERT_EQ(session, accessor->Access(&lock));
+  ASSERT_TRUE(lock.get());
+}
diff --git a/chrome/test/chromedriver/status.cc b/chrome/test/chromedriver/status.cc
index 60e09f0..bd50279 100644
--- a/chrome/test/chromedriver/status.cc
+++ b/chrome/test/chromedriver/status.cc
@@ -11,10 +11,14 @@ const char* DefaultMessageForStatusCode(StatusCode code) {
   switch (code) {
     case kOk:
       return "ok";
-    case kUnknownError:
-      return "unknown error";
     case kUnknownCommand:
       return "unknown command";
+    case kUnknownError:
+      return "unknown error";
+    case kSessionNotCreatedException:
+      return "session not created exception";
+    case kNoSuchSession:
+      return "no such session";
     default:
       return "<unknown>";
   }
diff --git a/chrome/test/chromedriver/status.h b/chrome/test/chromedriver/status.h
index 8f5a75b..a52d6e4 100644
--- a/chrome/test/chromedriver/status.h
+++ b/chrome/test/chromedriver/status.h
@@ -11,7 +11,9 @@
 enum StatusCode {
   kOk = 0,
   kUnknownCommand = 9,
-  kUnknownError = 13
+  kUnknownError = 13,
+  kSessionNotCreatedException = 33,
+  kNoSuchSession = 100
 };
 
 // Represents a WebDriver status, which may be an error or ok.
diff --git a/chrome/test/chromedriver/synchronized_map.h b/chrome/test/chromedriver/synchronized_map.h
new file mode 100644
index 0000000..a7bbb78
--- /dev/null
+++ b/chrome/test/chromedriver/synchronized_map.h
@@ -0,0 +1,86 @@
+// Copyright (c) 2012 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_TEST_CHROMEDRIVER_SYNCHRONIZED_MAP_H_
+#define CHROME_TEST_CHROMEDRIVER_SYNCHRONIZED_MAP_H_
+
+#include <map>
+#include <utility>
+#include <vector>
+
+#include "base/basictypes.h"
+#include "base/synchronization/lock.h"
+
+template <typename K, typename V>
+class SynchronizedMap {
+ public:
+  SynchronizedMap();
+  ~SynchronizedMap();
+
+  void Set(const K& key, const V& value);
+  bool Get(const K& key, V* value) const;
+  bool Has(const K& key) const;
+  bool Remove(const K& key);
+
+  void GetKeys(std::vector<K>* keys) const;
+
+ private:
+  typedef std::map<K, V> Map;
+  Map map_;
+  mutable base::Lock map_lock_;
+
+  DISALLOW_COPY_AND_ASSIGN(SynchronizedMap);
+};
+
+template <typename K, typename V>
+SynchronizedMap<K, V>::SynchronizedMap() {}
+
+template <typename K, typename V>
+SynchronizedMap<K, V>::~SynchronizedMap() {}
+
+template <typename K, typename V>
+void SynchronizedMap<K, V>::Set(const K& key, const V& value) {
+  base::AutoLock lock(map_lock_);
+  typename Map::iterator iter = map_.find(key);
+  if (iter != map_.end())
+    map_.erase(iter);
+  map_.insert(std::make_pair(key, value));
+}
+
+template <typename K, typename V>
+bool SynchronizedMap<K, V>::Get(const K& key, V* value) const {
+  base::AutoLock lock(map_lock_);
+  typename Map::const_iterator iter = map_.find(key);
+  if (iter == map_.end())
+    return false;
+  *value = iter->second;
+  return true;
+}
+
+template <typename K, typename V>
+bool SynchronizedMap<K, V>::Has(const K& key) const {
+  base::AutoLock lock(map_lock_);
+  return map_.find(key) != map_.end();
+}
+
+template <typename K, typename V>
+bool SynchronizedMap<K, V>::Remove(const K& key) {
+  base::AutoLock lock(map_lock_);
+  typename Map::iterator iter = map_.find(key);
+  if (iter == map_.end())
+    return false;
+  map_.erase(iter);
+  return true;
+}
+
+template <typename K, typename V>
+void SynchronizedMap<K, V>::GetKeys(std::vector<K>* keys) const {
+  keys->clear();
+  base::AutoLock lock(map_lock_);
+  typename Map::const_iterator iter;
+  for (iter = map_.begin(); iter != map_.end(); iter++)
+    keys->push_back(iter->first);
+}
+
+#endif  // CHROME_TEST_CHROMEDRIVER_SYNCHRONIZED_MAP_H_
diff --git a/chrome/test/chromedriver/synchronized_map_unittest.cc b/chrome/test/chromedriver/synchronized_map_unittest.cc
new file mode 100644
index 0000000..4e52395
--- /dev/null
+++ b/chrome/test/chromedriver/synchronized_map_unittest.cc
@@ -0,0 +1,78 @@
+// Copyright (c) 2012 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 <vector>
+
+#include "chrome/test/chromedriver/synchronized_map.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+TEST(SynchronizedMapTest, Set) {
+  SynchronizedMap<int, int> map;
+  map.Set(1, 2);
+  ASSERT_TRUE(map.Has(1));
+  int val = 0;
+  ASSERT_TRUE(map.Get(1, &val));
+  ASSERT_EQ(2, val);
+
+  map.Set(1, 3);
+  ASSERT_TRUE(map.Has(1));
+  ASSERT_TRUE(map.Get(1, &val));
+  ASSERT_EQ(3, val);
+
+  map.Set(3, 1);
+  ASSERT_TRUE(map.Has(1));
+  ASSERT_TRUE(map.Get(1, &val));
+  ASSERT_EQ(3, val);
+  ASSERT_TRUE(map.Has(3));
+  ASSERT_TRUE(map.Get(3, &val));
+  ASSERT_EQ(1, val);
+}
+
+TEST(SynchronizedMapTest, Get) {
+  SynchronizedMap<int, int> map;
+  int val = 0;
+  ASSERT_FALSE(map.Get(1, &val));
+  map.Set(1, 2);
+  ASSERT_TRUE(map.Get(1, &val));
+  ASSERT_EQ(2, val);
+
+  ASSERT_TRUE(map.Remove(1));
+  val = 100;
+  ASSERT_FALSE(map.Get(1, &val));
+  ASSERT_EQ(100, val);
+}
+
+TEST(SynchronizedMapTest, Has) {
+  SynchronizedMap<int, int> map;
+  ASSERT_FALSE(map.Has(1));
+  map.Set(1, 2);
+  ASSERT_TRUE(map.Has(1));
+  ASSERT_FALSE(map.Has(2));
+
+  ASSERT_TRUE(map.Remove(1));
+  ASSERT_FALSE(map.Has(1));
+}
+
+TEST(SynchronizedMapTest, GetKeys) {
+  SynchronizedMap<int, int> map;
+
+  std::vector<int> keys;
+  map.GetKeys(&keys);
+  ASSERT_EQ(0u, keys.size());
+
+  keys.push_back(100);
+  map.GetKeys(&keys);
+  ASSERT_EQ(0u, keys.size());
+
+  map.Set(1, 2);
+  map.GetKeys(&keys);
+  ASSERT_EQ(1u, keys.size());
+  ASSERT_EQ(1, keys[0]);
+
+  map.Set(2, 4);
+  map.GetKeys(&keys);
+  ASSERT_EQ(2u, keys.size());
+  ASSERT_EQ(1, keys[0]);
+  ASSERT_EQ(2, keys[1]);
+}