Eliminate usage of InterfacePtr::WaitForIncomingResponse.

net/proxy/proxy_resolver_factory_mojo_unittest.cc

 // Copyright 2015 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 "net/proxy/proxy_resolver_factory_mojo.h"
 
 #include <list>
 #include <map>
 #include <memory>
 #include <queue>
@@ skipping 249 matching lines @@
     }
     case GetProxyForUrlAction::DROP: {
       client.reset();
       break;
     }
     case GetProxyForUrlAction::DISCONNECT: {
       binding_.Close();
       break;
     }
     case GetProxyForUrlAction::WAIT_FOR_CLIENT_DISCONNECT: {
-      ASSERT_FALSE(client.WaitForIncomingResponse());
+      base::RunLoop run_loop;
+      client.set_connection_error_handler(run_loop.QuitClosure());
+      run_loop.Run();
+      ASSERT_TRUE(client.encountered_error());

[leonhsl(Using Gerrit), 2016/06/27 11:03:16]

I think InterfacePtr would detect the connection error immediately once
interface impl binding got closed, but here the thread always got hung when I
ran the test case. 
Seems the connection error handler does not get called. Am I doing something
wrong here?

[yzshen1, 2016/06/29 21:36:20]

Please look at base::MessageLoop::ScopedNestableTaskAllower
The code has multiple layers of base::RunLoop, and the connection error handler
won't be run on this layer if nestable task is not allowed, so it will wait
forever.

[leonhsl(Using Gerrit), 2016/06/29 22:53:49]

Yeah I noticed the multiple layers of base::RunLoop but I did not go down
deeply. Thanks a lot for the hints! Now I know where should I dip down, firstly
I'll try to understand base::MessageLoop::ScopedNestableTaskAllower there.

[leonhsl(Using Gerrit), 2016/07/01 08:53:50]

Done.

       break;
     }
     case GetProxyForUrlAction::MAKE_DNS_REQUEST: {
       interfaces::HostResolverRequestInfoPtr request(
           interfaces::HostResolverRequestInfo::New());
       request->host = url.spec();
       request->port = 12345;
       interfaces::HostResolverRequestClientPtr dns_client;
       mojo::GetProxy(&dns_client);
       client->ResolveDns(std::move(request), std::move(dns_client));
@@ skipping 142 matching lines @@
       blocked_clients_.push_back(
           base::WrapUnique(new interfaces::ProxyResolverFactoryRequestClientPtr(
               std::move(client))));
       break;
     }
     case CreateProxyResolverAction::DROP_BOTH: {
       // Both |request| and |client| will be closed.
       break;
     }
     case CreateProxyResolverAction::WAIT_FOR_CLIENT_DISCONNECT: {
-      ASSERT_FALSE(client.WaitForIncomingResponse());
+      base::RunLoop run_loop;
+      client.set_connection_error_handler(run_loop.QuitClosure());
+      run_loop.RunUntilIdle();

[yzshen1, 2016/06/29 21:36:20]

Please note that RunUntilIdle is incorrect. The message loop is not guaranteed
to be busy while we are waiting for the client to disconnect.

[leonhsl(Using Gerrit), 2016/06/29 22:53:49]

Understood.

[leonhsl(Using Gerrit), 2016/07/01 08:53:50]

Done.

+      ASSERT_TRUE(client.encountered_error());
       break;
     }
     case CreateProxyResolverAction::MAKE_DNS_REQUEST: {
       interfaces::HostResolverRequestInfoPtr request(
           interfaces::HostResolverRequestInfo::New());
       request->host = pac_script;
       request->port = 12345;
       interfaces::HostResolverRequestClientPtr dns_client;
       mojo::GetProxy(&dns_client);
       client->ResolveDns(std::move(request), std::move(dns_client));
@@ skipping 447 matching lines @@
   mock_proxy_resolver_.ClearBlockedClients();
   request->WaitForResult();
 }
 
 TEST_F(ProxyResolverFactoryMojoTest, DeleteResolver) {
   CreateProxyResolver();
   proxy_resolver_mojo_.reset();
   on_delete_callback_.WaitForResult();
 }
 }  // namespace net