Improve error messages for bad SocketParams.

Make it a compile-time error instead of link-time.
Add a check to make sure you don't try to use it with a pointer.
BUG=41016

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

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

6 files changed, 61 insertions, 14 deletions

diff --git a/base/base.gypi b/base/base.gypi
index f9d025c..42c1c47 100644
--- a/base/base.gypi
+++ b/base/base.gypi
@@ -235,6 +235,7 @@
           'sys_string_conversions_mac.mm',
           'sys_string_conversions_win.cc',
           'task.h',
+          'template_util.h',
           'thread.cc',
           'thread.h',
           'thread_collision_warner.cc',
diff --git a/base/template_util.h b/base/template_util.h
new file mode 100644
index 0000000..69702f3
--- /dev/null
+++ b/base/template_util.h
@@ -0,0 +1,29 @@
+// Copyright (c) 2010 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 BASE_TEMPLATE_UTIL_H_
+#define BASE_TEMPLATE_UTIL_H_
+
+namespace base {
+
+// template definitions from tr1
+
+template<class T, T v>
+struct integral_constant {
+  static const T value = v;
+  typedef T value_type;
+  typedef integral_constant<T, v> type;
+};
+
+template <class T, T v> const T integral_constant<T, v>::value;
+
+typedef integral_constant<bool, true> true_type;
+typedef integral_constant<bool, false> false_type;
+
+template <class T> struct is_pointer : false_type {};
+template <class T> struct is_pointer<T*> : true_type {};
+
+}  // namespace base
+
+#endif
diff --git a/net/socket/client_socket_pool.h b/net/socket/client_socket_pool.h
index e631980..c5f6f16 100644
--- a/net/socket/client_socket_pool.h
+++ b/net/socket/client_socket_pool.h
@@ -9,7 +9,9 @@
 #include <map>
 #include <string>
 
+#include "base/basictypes.h"
 #include "base/ref_counted.h"
+#include "base/template_util.h"
 #include "net/base/completion_callback.h"
 #include "net/base/host_resolver.h"
 #include "net/base/load_states.h"
@@ -119,16 +121,25 @@ class ClientSocketPool : public base::RefCounted<ClientSocketPool> {
 // will provide a definition of CheckIsValidSocketParamsForPool for the
 // ClientSocketPool subtype and SocketParams pair.  Trying to use a SocketParams
 // type that has not been registered with the corresponding ClientSocketPool
-// subtype will result in a link time error stating that
-// CheckIsValidSocketParamsForPool with those template parameters is undefined.
+// subtype will result in a compile-time error.
 template <typename PoolType, typename SocketParams>
-void CheckIsValidSocketParamsForPool();
+struct SocketParamTraits : public base::false_type {
+};
+
+template <typename PoolType, typename SocketParams>
+void CheckIsValidSocketParamsForPool() {
+  COMPILE_ASSERT(!base::is_pointer<SocketParams>::value,
+                 socket_params_cannot_be_pointer);
+  COMPILE_ASSERT((SocketParamTraits<PoolType, SocketParams>::value),
+                 invalid_socket_params_for_pool);
+}
 
 // Provides an empty definition for CheckIsValidSocketParamsForPool() which
 // should be optimized out by the compiler.
-#define REGISTER_SOCKET_PARAMS_FOR_POOL(pool_type, socket_params)         \
-template<>                                                                \
-inline void CheckIsValidSocketParamsForPool<pool_type, socket_params>() {}
+#define REGISTER_SOCKET_PARAMS_FOR_POOL(pool_type, socket_params)             \
+template<>                                                                    \
+struct SocketParamTraits<pool_type, socket_params> : public base::true_type { \
+}
 
 }  // namespace net
 
diff --git a/net/socket/client_socket_pool_base_unittest.cc b/net/socket/client_socket_pool_base_unittest.cc
index 8c0e6ea..f195d2d 100644
--- a/net/socket/client_socket_pool_base_unittest.cc
+++ b/net/socket/client_socket_pool_base_unittest.cc
@@ -30,7 +30,7 @@ const int kDefaultMaxSockets = 4;
 const int kDefaultMaxSocketsPerGroup = 2;
 const net::RequestPriority kDefaultPriority = MEDIUM;
 
-typedef const void* TestSocketParams;
+struct TestSocketParams {};
 typedef ClientSocketPoolBase<TestSocketParams> TestClientSocketPoolBase;
 
 class MockClientSocket : public ClientSocket {
@@ -299,8 +299,10 @@ class TestClientSocketPool : public ClientSocketPool {
       ClientSocketHandle* handle,
       CompletionCallback* callback,
       const BoundNetLog& net_log) {
+    const TestSocketParams* casted_socket_params =
+        static_cast<const TestSocketParams*>(params);
     return base_.RequestSocket(
-        group_name, params, priority, handle, callback, net_log);
+        group_name, *casted_socket_params, priority, handle, callback, net_log);
   }
 
   virtual void CancelRequest(
@@ -438,8 +440,9 @@ class ClientSocketPoolBaseTest : public ClientSocketPoolTest {
 
   int StartRequest(const std::string& group_name,
                    net::RequestPriority priority) {
+    TestSocketParams params;
     return StartRequestUsingPool<TestClientSocketPool, TestSocketParams>(
-        pool_, group_name, priority, NULL);
+        pool_, group_name, priority, params);
   }
 
   virtual void TearDown() {
@@ -474,8 +477,9 @@ int InitHandle(ClientSocketHandle* handle,
                CompletionCallback* callback,
                const scoped_refptr<TestClientSocketPool>& pool,
                const BoundNetLog& net_log) {
+  TestSocketParams params;
   return handle->Init<TestSocketParams, TestClientSocketPool>(
-      group_name, NULL, priority, callback, pool, net_log);
+      group_name, params, priority, callback, pool, net_log);
 }
 
 // Even though a timeout is specified, it doesn't time out on a synchronous
@@ -483,8 +487,9 @@ int InitHandle(ClientSocketHandle* handle,
 TEST_F(ClientSocketPoolBaseTest, ConnectJob_NoTimeoutOnSynchronousCompletion) {
   TestConnectJobDelegate delegate;
   ClientSocketHandle ignored;
+  TestSocketParams params;
   TestClientSocketPoolBase::Request request(
-      &ignored, NULL, kDefaultPriority, NULL, BoundNetLog());
+      &ignored, NULL, kDefaultPriority, params, BoundNetLog());
   scoped_ptr<TestConnectJob> job(
       new TestConnectJob(TestConnectJob::kMockJob,
                          "a",
@@ -501,8 +506,9 @@ TEST_F(ClientSocketPoolBaseTest, ConnectJob_TimedOut) {
   ClientSocketHandle ignored;
   CapturingNetLog log(CapturingNetLog::kUnbounded);
 
+  TestSocketParams params;
   TestClientSocketPoolBase::Request request(
-      &ignored, NULL, kDefaultPriority, NULL, BoundNetLog());
+      &ignored, NULL, kDefaultPriority, params, BoundNetLog());
   // Deleted by TestConnectJobDelegate.
   TestConnectJob* job =
       new TestConnectJob(TestConnectJob::kMockPendingJob,
diff --git a/net/socket/socks_client_socket_pool.h b/net/socket/socks_client_socket_pool.h
index c202ce1..b0b9b4b 100644
--- a/net/socket/socks_client_socket_pool.h
+++ b/net/socket/socks_client_socket_pool.h
@@ -188,7 +188,7 @@ class SOCKSClientSocketPool : public ClientSocketPool {
   DISALLOW_COPY_AND_ASSIGN(SOCKSClientSocketPool);
 };
 
-REGISTER_SOCKET_PARAMS_FOR_POOL(SOCKSClientSocketPool, SOCKSSocketParams)
+REGISTER_SOCKET_PARAMS_FOR_POOL(SOCKSClientSocketPool, SOCKSSocketParams);
 
 }  // namespace net
 
diff --git a/net/socket/tcp_client_socket_pool.h b/net/socket/tcp_client_socket_pool.h
index 8eee3c7..07ce3fa 100644
--- a/net/socket/tcp_client_socket_pool.h
+++ b/net/socket/tcp_client_socket_pool.h
@@ -198,7 +198,7 @@ class TCPClientSocketPool : public ClientSocketPool {
   DISALLOW_COPY_AND_ASSIGN(TCPClientSocketPool);
 };
 
-REGISTER_SOCKET_PARAMS_FOR_POOL(TCPClientSocketPool, TCPSocketParams)
+REGISTER_SOCKET_PARAMS_FOR_POOL(TCPClientSocketPool, TCPSocketParams);
 
 }  // namespace net