Add an API test to exersize the open error case. This is a regression test for

chrome/browser/extensions/api/cast_channel/cast_channel_apitest.cc

Index: chrome/browser/extensions/api/cast_channel/cast_channel_apitest.cc
===================================================================
--- chrome/browser/extensions/api/cast_channel/cast_channel_apitest.cc	(revision 250590)
+++ chrome/browser/extensions/api/cast_channel/cast_channel_apitest.cc	(working copy)
@@ -16,14 +16,17 @@
 #include "net/base/completion_callback.h"
 #include "net/base/net_errors.h"
 #include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gmock_mutant.h"
 
 namespace cast_channel =  extensions::api::cast_channel;
 using cast_channel::CastSocket;
+using cast_channel::ChannelError;
 using cast_channel::ChannelInfo;
 using cast_channel::MessageInfo;
 
+using ::testing::_;
 using ::testing::A;
-using ::testing::_;
+using ::testing::DoAll;
 using ::testing::Invoke;
 using ::testing::Return;
 
@@ -46,6 +49,18 @@
   ::std::tr1::get<k>(args).Run(result);
 }
 
+// mfoltz,wez: Please see this and suggest how to make ACTION_TEMPLATE
+// to work. I have currently used testing::CreateFunctor, but it will be
+// better to use ACTION_TEMPLATE for consistency with InvokeCompletionCallback

[mark a. foltz, 2014/02/24 06:14:05]

This looks like it should work, but I am far from the expert on ACTION_TEMPLATE.
 What error do you get?

[Munjal (Google), 2014/02/26 20:46:33]

I figured out a way to use ACTION_P macro.

+/*
+ACTION_TEMPLATE(InvokeDelegateOnError,
+                HAS_2_TEMPLATE_PARAMS(extensions::CastChannelAPI*, api,
+                                      MockCastSocket*, cast_socket),
+                AND_1_VALUE_PARAMS(callback)) {
+  ::std::tr1::get<api, cast_socket>(args).Run(callback);
+}
+*/
+
 class MockCastSocket : public CastSocket {
  public:
   explicit MockCastSocket(CastSocket::Delegate* delegate,
@@ -103,21 +118,38 @@
     channel_info->error_state = cast_channel::CHANNEL_ERROR_NONE;
   }
 
+  void CallOnError(extensions::CastChannelAPI* api,
+                   const net::CompletionCallback& callback) {
+    api->OnError(mock_cast_socket_, cast_channel::CHANNEL_ERROR_CONNECT_ERROR);

[mark a. foltz, 2014/02/24 06:14:05]

Will api ever != GetApi()?  Can we use the GetApi() method to be consistent if
not?

[Munjal (Google), 2014/02/26 20:46:33]

The issue is that if GetAPI is called here then a DCHECk of CalledOnValidThread
fails somewhere on the stack of GetAPI call. So I have to call it outside (on
the right thread) and pass it in.

+
+    /*

[mark a. foltz, 2014/02/24 06:14:05]

Delete commented out code and method it calls

[Munjal (Google), 2014/02/26 20:46:33]

Done.

+    content::BrowserThread::PostTask(
+        content::BrowserThread::IO,
+        FROM_HERE,
+        base::Bind(&CastChannelAPITest::DoCallOnError, this,
+                   api, mock_cast_socket_));
+    */
+  }
+
+  void DoCallOnError(extensions::CastChannelAPI* api,
+                     MockCastSocket* cast_socket) {
+    api->OnError(cast_socket, cast_channel::CHANNEL_ERROR_CONNECT_ERROR);
+  }
+
  protected:
   void CallOnMessage(const std::string& message) {
     content::BrowserThread::PostTask(
         content::BrowserThread::IO,
         FROM_HERE,
         base::Bind(&CastChannelAPITest::DoCallOnMessage, this,
-                   GetApi(), mock_cast_socket_, message));
+                   mock_cast_socket_, message));
   }
 
-  void DoCallOnMessage(extensions::CastChannelAPI* api,
-                       MockCastSocket* cast_socket,
+  void DoCallOnMessage(MockCastSocket* cast_socket,
                        const std::string& message) {
     MessageInfo message_info;
     FillMessageInfo(&message_info, message);
-    api->OnMessage(cast_socket, message_info);
+    GetApi()->OnMessage(cast_socket, message_info);
   }
 
   MockCastSocket* mock_cast_socket_;
@@ -182,3 +214,31 @@
   CallOnMessage("some-message");
   EXPECT_TRUE(catcher.GetNextResult()) << catcher.message();
 }
+
+// TODO(munjal): Win Dbg has a workaround that makes RunExtensionSubtest
+// always return true without actually running the test. Remove when fixed.
+#if defined(OS_WIN) && !defined(NDEBUG)
+#define MAYBE_TestOpenError DISABLED_TestOpenError
+#else
+#define MAYBE_TestOpenError TestOpenError
+#endif
+// Test the case when socket open results in an error.
+IN_PROC_BROWSER_TEST_F(CastChannelAPITest, MAYBE_TestOpenError) {
+  SetUpMockCastSocket();
+
+  EXPECT_CALL(*mock_cast_socket_, Connect(_))
+        .WillOnce(DoAll(
+            Invoke(testing::CreateFunctor(
+                this, &CastChannelAPITest::CallOnError, GetApi())),
+            InvokeCompletionCallback<0>(net::ERR_FAILED)));
+  EXPECT_CALL(*mock_cast_socket_, FillChannelInfo(_))
+      .WillOnce(Invoke(FillChannelInfoForClosedState))
+      .WillOnce(Invoke(FillChannelInfoForClosedState));

[mark a. foltz, 2014/02/24 06:14:05]

Is it a typo to invoke FIllChannelInfoForClosedState twice?

[Munjal (Google), 2014/02/26 20:46:33]

No, it is actually called twice.

+  EXPECT_CALL(*mock_cast_socket_, Close(_));
+
+  EXPECT_TRUE(RunExtensionSubtest("cast_channel/api",

[mark a. foltz, 2014/02/24 06:14:05]

Can we convert these to use RunExtensionTest instead of RunExtensionSubtest? 
According to https://code.google.com/p/chromium/issues/detail?id=177163 it seems
to improve reliability - but no one knows exactly why.

[Munjal (Google), 2014/02/26 20:46:33]

Let me handle this separately.

+                                  "test_open_error.html"));
+
+  ResultCatcher catcher;
+  EXPECT_TRUE(catcher.GetNextResult()) << catcher.message();
+}