[ChromeDriver] Added CommandListener interface

chrome/test/chromedriver/session_unittest.cc

 // 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 <list>
 #include <string>
 
 #include "base/memory/scoped_ptr.h"
 #include "chrome/test/chromedriver/chrome/status.h"
 #include "chrome/test/chromedriver/chrome/stub_chrome.h"
@@ skipping 14 matching lines @@
       *web_view = &web_view_;
       return Status(kOk);
     }
     return Status(kUnknownError);
   }
 
  private:
   StubWebView web_view_;
 };
 
+class MockCommandListener : public CommandListener {

[stgao, 2014/06/15 23:54:41]

Duplicate code as the one in commands_unittest.cc

You could create a stub_command_listener.h/cc.
In the constructor, you could pass in a cmd_name that will be notified, instead
of a hard-code one.

[johnmoore, 2014/06/16 17:28:34]

Would it make sense for the stub class to perform the ASSERTs too? It seems that
would make it more of a mock class than a stub class. If I had to define a
MockCommandListener that inherits from StubCommandListener in both unittest .cc
files, there would be just as much duplicated code, plus the separate stub .h
and .cc files. I'm not sure what the best practice would be here.

+ public:
+  MockCommandListener() : called_(false) {}
+  virtual ~MockCommandListener() {}
+
+  virtual Status OnCommand(const std::string& command_name) OVERRIDE {
+    called_ = true;
+    EXPECT_STREQ("cmd", command_name.c_str());
+    return Status(kOk);
+  }
+
+  void VerifyCalled() {
+    EXPECT_TRUE(called_);
+  }
+
+  void VerifyNotCalled() {
+    EXPECT_FALSE(called_);
+  }
+
+ private:
+  bool called_;
+};
+
 }  // namespace
 
 TEST(Session, GetTargetWindowNoChrome) {
   Session session("1");
   WebView* web_view;
   ASSERT_EQ(kNoSuchWindow, session.GetTargetWindow(&web_view).code());
 }
 
 TEST(Session, GetTargetWindowTargetWindowClosed) {
   scoped_ptr<Chrome> chrome(new MockChrome());
@@ skipping 49 matching lines @@
   session.SwitchToTopFrame();
   ASSERT_EQ(std::string(), session.GetCurrentFrameId());
 
   session.SwitchToSubFrame("3.1", std::string());
   ASSERT_EQ("3.1", session.GetCurrentFrameId());
   session.SwitchToSubFrame("3.2", std::string());
   ASSERT_EQ("3.2", session.GetCurrentFrameId());
   session.SwitchToTopFrame();
   ASSERT_EQ(std::string(), session.GetCurrentFrameId());
 }
+
+TEST(Session, OnCommand) {
+  Session session("1");
+  MockCommandListener* listener1 = new MockCommandListener();

[stgao, 2014/06/15 23:57:38]

Memory leak: please use a scoped_ptr.

[johnmoore, 2014/06/16 20:49:18]

session.AddListener should take care of transferring ownership to session, which
then destroys the listeners if I am not mistaken

+  MockCommandListener* listener2 = new MockCommandListener();
+  MockCommandListener* listener3 = new MockCommandListener();
+  session.AddListener(listener1);
+  session.AddListener(listener2);
+  session.AddListener(listener3);
+  listener1->VerifyNotCalled();
+  listener2->VerifyNotCalled();
+  listener3->VerifyNotCalled();
+  session.OnCommand("cmd");
+  listener1->VerifyCalled();
+  listener2->VerifyCalled();
+  listener3->VerifyCalled();
+}