[GCM] Fix gcm network change handling before connect

google_apis/gcm/engine/connection_factory_impl_unittest.cc

 // Copyright (c) 2013 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 "google_apis/gcm/engine/connection_factory_impl.h"
 
 #include <cmath>
 
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
@@ skipping 130 matching lines @@
   int connect_result_;
   // The number of expected connection attempts;
   int num_expected_attempts_;
   // Whether all expected connection attempts have been fulfilled since an
   // expectation was last set.
   bool connections_fulfilled_;
   // Whether to delay a login handshake completion or not.
   bool delay_login_;
   // Callback to invoke when all connection attempts have been made.
   base::Closure finished_callback_;
+  // A temporary scoped pointer to make sure we don't leak the handler in the
+  // cases it's never consumed by the ConnectionFactory.
+  scoped_ptr<FakeConnectionHandler> scoped_handler_;
   // The current fake connection handler..
   FakeConnectionHandler* fake_handler_;
+  // Dummy GCM Stats recorder.
   FakeGCMStatsRecorder dummy_recorder_;
 };
 
 TestConnectionFactoryImpl::TestConnectionFactoryImpl(
     const base::Closure& finished_callback)
     : ConnectionFactoryImpl(BuildEndpoints(),
                             net::BackoffEntry::Policy(),
                             NULL,
                             NULL,
                             NULL,
                             &dummy_recorder_),
       connect_result_(net::ERR_UNEXPECTED),
       num_expected_attempts_(0),
       connections_fulfilled_(true),
       delay_login_(false),
       finished_callback_(finished_callback),
-      fake_handler_(NULL) {
+      scoped_handler_(
+          new FakeConnectionHandler(base::Bind(&ReadContinuation),
+                                    base::Bind(&WriteContinuation))),
+      fake_handler_(scoped_handler_.get()) {
   // Set a non-null time.
   tick_clock_.Advance(base::TimeDelta::FromMilliseconds(1));
 }
 
 TestConnectionFactoryImpl::~TestConnectionFactoryImpl() {
   EXPECT_EQ(0, num_expected_attempts_);
 }
 
 void TestConnectionFactoryImpl::ConnectImpl() {
   ASSERT_GT(num_expected_attempts_, 0);
@@ skipping 24 matching lines @@
     const net::BackoffEntry::Policy* const policy) {
   return scoped_ptr<net::BackoffEntry>(new TestBackoffEntry(&tick_clock_));
 }
 
 scoped_ptr<ConnectionHandler>
 TestConnectionFactoryImpl::CreateConnectionHandler(
     base::TimeDelta read_timeout,
     const ConnectionHandler::ProtoReceivedCallback& read_callback,
     const ConnectionHandler::ProtoSentCallback& write_callback,
     const ConnectionHandler::ConnectionChangedCallback& connection_callback) {
-  fake_handler_ = new FakeConnectionHandler(
-      base::Bind(&ReadContinuation),
-      base::Bind(&WriteContinuation));
-  return make_scoped_ptr<ConnectionHandler>(fake_handler_);
+  return scoped_handler_.Pass();
 }
 
 base::TimeTicks TestConnectionFactoryImpl::NowTicks() {
   return tick_clock_.NowTicks();
 }
 
 void TestConnectionFactoryImpl::SetConnectResult(int connect_result) {
   DCHECK_NE(connect_result, net::ERR_UNEXPECTED);
   ASSERT_EQ(0, num_expected_attempts_);
   connections_fulfilled_ = false;
@@ skipping 80 matching lines @@
     const net::IPEndPoint& ip_endpoint) {
   connected_server_ = current_server;
 }
 
 void ConnectionFactoryImplTest::OnDisconnected() {
   connected_server_ = GURL();
 }
 
 // Verify building a connection handler works.
 TEST_F(ConnectionFactoryImplTest, Initialize) {
-  ConnectionHandler* handler = factory()->GetConnectionHandler();
-  ASSERT_TRUE(handler);
+  ASSERT_FALSE(factory()->GetConnectionHandler());
   EXPECT_FALSE(factory()->IsEndpointReachable());
   EXPECT_FALSE(connected_server().is_valid());
 }
 
 // An initial successful connection should not result in backoff.
 TEST_F(ConnectionFactoryImplTest, ConnectSuccess) {
   factory()->SetConnectResult(net::OK);
   factory()->Connect();
+  ASSERT_TRUE(factory()->GetConnectionHandler());
   EXPECT_TRUE(factory()->NextRetryAttempt().is_null());
   EXPECT_EQ(factory()->GetCurrentEndpoint(), BuildEndpoints()[0]);
   EXPECT_TRUE(factory()->IsEndpointReachable());
   EXPECT_TRUE(connected_server().is_valid());
 }
 
 // A connection failure should result in backoff, and attempting the fallback
 // endpoint next.
 TEST_F(ConnectionFactoryImplTest, ConnectFail) {
   factory()->SetConnectResult(net::ERR_CONNECTION_FAILED);
@@ skipping 202 matching lines @@
 
   // When the network returns, attempt to connect.
   factory()->SetConnectResult(net::OK);
   factory()->OnNetworkChanged(net::NetworkChangeNotifier::CONNECTION_4G);
   WaitForConnections();
 
   EXPECT_TRUE(factory()->IsEndpointReachable());
   EXPECT_TRUE(factory()->NextRetryAttempt().is_null());
 }
 
+// Receiving a network change event before the initial connection should have
+// no effect.
+TEST_F(ConnectionFactoryImplTest, NetworkChangeBeforeFirstConnection) {
+  factory()->OnNetworkChanged(net::NetworkChangeNotifier::CONNECTION_4G);
+  factory()->SetConnectResult(net::OK);
+  factory()->Connect();
+  EXPECT_TRUE(factory()->NextRetryAttempt().is_null());
+  EXPECT_TRUE(factory()->IsEndpointReachable());
+}
+
 }  // namespace gcm