Convert net apptests to net unittests using mojo::ShellTest.

R=yzshen@chromium.org
BUG=

Review URL: https://codereview.chromium.org/1796313002

Cr-Commit-Position: refs/heads/master@{#381183}

13 files changed, 98 insertions, 60 deletions

diff --git a/mojo/BUILD.gn b/mojo/BUILD.gn
index e18e93d..62bb452 100644
--- a/mojo/BUILD.gn
+++ b/mojo/BUILD.gn
@@ -41,7 +41,7 @@ group("tests") {
     "//mojo/edk/test:mojo_public_bindings_unittests",
     "//mojo/edk/test:mojo_public_system_perftests",
     "//mojo/edk/test:mojo_public_system_unittests",
-    "//mojo/services/network:apptests",
+    "//mojo/services/network:unittests",
     "//mojo/shell/public/cpp/tests:mojo_public_application_unittests",
     "//mojo/shell/runner/host:mojo_runner_host_unittests",
     "//mojo/shell/tests",
diff --git a/mojo/services/network/BUILD.gn b/mojo/services/network/BUILD.gn
index aad26845..7d54a96 100644
--- a/mojo/services/network/BUILD.gn
+++ b/mojo/services/network/BUILD.gn
@@ -3,6 +3,8 @@
 # found in the LICENSE file.
 
 import("//mojo/public/mojo_application.gni")
+import("//mojo/public/mojo_application_manifest.gni")
+import("//testing/test.gni")
 
 if (is_android) {
   import("//build/config/android/rules.gni")
@@ -123,14 +125,20 @@ source_set("sources") {
   ]
 }
 
-mojo_native_application("apptests") {
-  output_name = "network_service_apptests"
+group("network_service_unittests") {
   testonly = true
+  deps = [
+    ":unittests",
+  ]
+}
+
+test("unittests") {
+  output_name = "network_service_unittests"
 
   sources = [
-    "http_server_apptest.cc",
-    "udp_socket_apptest.cc",
-    "url_loader_impl_apptest.cc",
+    "http_server_unittest.cc",
+    "udp_socket_unittest.cc",
+    "url_loader_impl_unittest.cc",
   ]
 
   deps = [
@@ -142,10 +150,21 @@ mojo_native_application("apptests") {
     "//mojo/public/cpp/bindings:callback",
     "//mojo/services/network/public/cpp",
     "//mojo/services/network/public/interfaces",
+    "//mojo/shell/public/cpp:shell_test_support",
     "//mojo/shell/public/cpp:sources",
-    "//mojo/shell/public/cpp:test_support",
+    "//mojo/shell/public/cpp/test:run_all_shelltests",
     "//net",
     "//net:test_support",
     "//testing/gtest",
   ]
+
+  data_deps = [
+    ":test_manifest",
+  ]
+}
+
+mojo_application_manifest("test_manifest") {
+  type = "exe"
+  application_name = "network_service_unittests"
+  source = "test_manifest.json"
 }
diff --git a/mojo/services/network/http_server_apptest.cc b/mojo/services/network/http_server_unittest.cc
index 24b070a..dc9769b 100644
--- a/mojo/services/network/http_server_apptest.cc
+++ b/mojo/services/network/http_server_unittest.cc
@@ -26,7 +26,7 @@
 #include "mojo/services/network/public/interfaces/network_service.mojom.h"
 #include "mojo/services/network/public/interfaces/web_socket.mojom.h"
 #include "mojo/services/network/public/interfaces/web_socket_factory.mojom.h"
-#include "mojo/shell/public/cpp/application_test_base.h"
+#include "mojo/shell/public/cpp/shell_test.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 #include "net/base/test_completion_callback.h"
@@ -545,16 +545,15 @@ class HttpServerDelegateImpl : public HttpServerDelegate {
   DISALLOW_COPY_AND_ASSIGN(HttpServerDelegateImpl);
 };
 
-class HttpServerAppTest : public test::ApplicationTestBase {
+class HttpServerTest : public test::ShellTest {
  public:
-  HttpServerAppTest() : message_loop_(base::MessageLoop::TYPE_IO) {}
-  ~HttpServerAppTest() override {}
+  HttpServerTest()
+      : ShellTest("exe:network_service_unittests") {}
+  ~HttpServerTest() override {}
 
  protected:
-  bool ShouldCreateDefaultRunLoop() override { return false; }
-
   void SetUp() override {
-    ApplicationTestBase::SetUp();
+    ShellTest::SetUp();
 
     scoped_ptr<Connection> connection =
         connector()->Connect("mojo:network_service");
@@ -562,30 +561,34 @@ class HttpServerAppTest : public test::ApplicationTestBase {
     connection->GetInterface(&web_socket_factory_);
   }
 
+  scoped_ptr<base::MessageLoop> CreateMessageLoop() override {
+    return make_scoped_ptr(new base::MessageLoop(base::MessageLoop::TYPE_IO));
+  }
+
   void CreateHttpServer(HttpServerDelegatePtr delegate,
                         NetAddressPtr* out_bound_to) {
+    base::RunLoop loop;
     network_service_->CreateHttpServer(
         GetLocalHostWithAnyPort(), std::move(delegate),
-        [out_bound_to](NetworkErrorPtr result, NetAddressPtr bound_to) {
+        [out_bound_to, &loop](NetworkErrorPtr result, NetAddressPtr bound_to) {
           ASSERT_EQ(net::OK, result->code);
           EXPECT_NE(0u, bound_to->ipv4->port);
           *out_bound_to = std::move(bound_to);
+          loop.Quit();
         });
-    network_service_.WaitForIncomingResponse();
+    loop.Run();
   }
 
   NetworkServicePtr network_service_;
   WebSocketFactoryPtr web_socket_factory_;
 
  private:
-  base::MessageLoop message_loop_;
-
-  DISALLOW_COPY_AND_ASSIGN(HttpServerAppTest);
+  DISALLOW_COPY_AND_ASSIGN(HttpServerTest);
 };
 
 }  // namespace
 
-TEST_F(HttpServerAppTest, BasicHttpRequestResponse) {
+TEST_F(HttpServerTest, BasicHttpRequestResponse) {
   NetAddressPtr bound_to;
   HttpServerDelegatePtr server_delegate_ptr;
   HttpServerDelegateImpl server_delegate_impl(&server_delegate_ptr);
@@ -618,7 +621,7 @@ TEST_F(HttpServerAppTest, BasicHttpRequestResponse) {
   CheckResponse(response_data, response_message);
 }
 
-TEST_F(HttpServerAppTest, HttpRequestResponseWithBody) {
+TEST_F(HttpServerTest, HttpRequestResponseWithBody) {
   NetAddressPtr bound_to;
   HttpServerDelegatePtr server_delegate_ptr;
   HttpServerDelegateImpl server_delegate_impl(&server_delegate_ptr);
@@ -657,7 +660,7 @@ TEST_F(HttpServerAppTest, HttpRequestResponseWithBody) {
   CheckResponse(response_data, response_message);
 }
 
-TEST_F(HttpServerAppTest, WebSocket) {
+TEST_F(HttpServerTest, WebSocket) {
   NetAddressPtr bound_to;
   HttpServerDelegatePtr server_delegate_ptr;
   HttpServerDelegateImpl server_delegate_impl(&server_delegate_ptr);
diff --git a/mojo/services/network/manifest.json b/mojo/services/network/manifest.json
new file mode 100644
index 0000000..d4bf307
--- /dev/null
+++ b/mojo/services/network/manifest.json
@@ -0,0 +1,5 @@
+{
+  "name": "mojo:network_service",
+  "display_name": "Network Service",
+  "capabilities": { "*": [ "*" ] }
+}
diff --git a/mojo/services/network/network_service_delegate.cc b/mojo/services/network/network_service_delegate.cc
index b257331..1970928 100644
--- a/mojo/services/network/network_service_delegate.cc
+++ b/mojo/services/network/network_service_delegate.cc
@@ -78,6 +78,10 @@ bool NetworkServiceDelegate::AcceptConnection(Connection* connection) {
   return true;
 }
 
+void NetworkServiceDelegate::ShellConnectionLost() {
+  base::MessageLoop::current()->QuitWhenIdle();
+}
+
 void NetworkServiceDelegate::Create(Connection* connection,
                                     InterfaceRequest<NetworkService> request) {
   new NetworkServiceImpl(ref_factory_.CreateRef(), std::move(request));
diff --git a/mojo/services/network/network_service_delegate.h b/mojo/services/network/network_service_delegate.h
index 7098939..8997097 100644
--- a/mojo/services/network/network_service_delegate.h
+++ b/mojo/services/network/network_service_delegate.h
@@ -38,6 +38,7 @@ class NetworkServiceDelegate : public ShellClient,
   void Initialize(Connector* connector, const Identity& identity,
                   uint32_t id) override;
   bool AcceptConnection(Connection* connection) override;
+  void ShellConnectionLost() override;
 
   // InterfaceFactory<NetworkService> implementation.
   void Create(Connection* connection,
diff --git a/mojo/services/network/test_manifest.json b/mojo/services/network/test_manifest.json
new file mode 100644
index 0000000..17a78cc
--- /dev/null
+++ b/mojo/services/network/test_manifest.json
@@ -0,0 +1,5 @@
+{
+  "name": "exe:network_service_unittests",
+  "display_name": "Network Service Unittests",
+  "capabilities": { "*": [ "*" ] }
+}
diff --git a/mojo/services/network/udp_socket_apptest.cc b/mojo/services/network/udp_socket_unittest.cc
index 075a29c..9aaf1f2 100644
--- a/mojo/services/network/udp_socket_apptest.cc
+++ b/mojo/services/network/udp_socket_unittest.cc
@@ -14,7 +14,7 @@
 #include "mojo/services/network/public/cpp/udp_socket_wrapper.h"
 #include "mojo/services/network/public/interfaces/network_service.mojom.h"
 #include "mojo/services/network/public/interfaces/udp_socket.mojom.h"
-#include "mojo/shell/public/cpp/application_test_base.h"
+#include "mojo/shell/public/cpp/shell_test.h"
 #include "net/base/net_errors.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -317,13 +317,14 @@ class UDPSocketReceiverImpl : public UDPSocketReceiver {
   DISALLOW_COPY_AND_ASSIGN(UDPSocketReceiverImpl);
 };
 
-class UDPSocketAppTest : public test::ApplicationTestBase {
+class UDPSocketTest : public test::ShellTest {
  public:
-  UDPSocketAppTest() : receiver_binding_(&receiver_) {}
-  ~UDPSocketAppTest() override {}
+  UDPSocketTest() : ShellTest("exe:network_service_unittests"),
+                    receiver_binding_(&receiver_) {}
+  ~UDPSocketTest() override {}
 
   void SetUp() override {
-    ApplicationTestBase::SetUp();
+    ShellTest::SetUp();
     connector()->ConnectToInterface("mojo:network_service", &network_service_);
     network_service_->CreateUDPSocket(GetProxy(&socket_));
   }
@@ -334,12 +335,12 @@ class UDPSocketAppTest : public test::ApplicationTestBase {
   UDPSocketReceiverImpl receiver_;
   Binding<UDPSocketReceiver> receiver_binding_;
 
-  DISALLOW_COPY_AND_ASSIGN(UDPSocketAppTest);
+  DISALLOW_COPY_AND_ASSIGN(UDPSocketTest);
 };
 
 }  // namespace
 
-TEST_F(UDPSocketAppTest, Settings) {
+TEST_F(UDPSocketTest, Settings) {
   TestCallback callback1;
   socket_->AllowAddressReuse(callback1.callback());
   callback1.WaitForResult();
@@ -390,7 +391,7 @@ TEST_F(UDPSocketAppTest, Settings) {
   EXPECT_GT(callback9.result(), 0u);
 }
 
-TEST_F(UDPSocketAppTest, TestReadWrite) {
+TEST_F(UDPSocketTest, TestReadWrite) {
   TestCallbackWithAddressAndReceiver callback1;
   socket_->Bind(GetLocalHostWithAnyPort(), callback1.callback());
   callback1.WaitForResult();
@@ -438,7 +439,7 @@ TEST_F(UDPSocketAppTest, TestReadWrite) {
   }
 }
 
-TEST_F(UDPSocketAppTest, TestConnectedReadWrite) {
+TEST_F(UDPSocketTest, TestConnectedReadWrite) {
   TestCallbackWithAddressAndReceiver callback1;
   socket_->Bind(GetLocalHostWithAnyPort(), callback1.callback());
   callback1.WaitForResult();
@@ -516,7 +517,7 @@ TEST_F(UDPSocketAppTest, TestConnectedReadWrite) {
   }
 }
 
-TEST_F(UDPSocketAppTest, TestWrapperReadWrite) {
+TEST_F(UDPSocketTest, TestWrapperReadWrite) {
   UDPSocketWrapper socket(std::move(socket_), 4, 4);
 
   TestCallbackWithAddress callback1;
@@ -568,7 +569,7 @@ TEST_F(UDPSocketAppTest, TestWrapperReadWrite) {
   }
 }
 
-TEST_F(UDPSocketAppTest, TestWrapperConnectedReadWrite) {
+TEST_F(UDPSocketTest, TestWrapperConnectedReadWrite) {
   UDPSocketWrapper socket(std::move(socket_), 4, 4);
 
   TestCallbackWithAddress callback1;
diff --git a/mojo/services/network/url_loader_impl_apptest.cc b/mojo/services/network/url_loader_impl_unittest.cc
index fe7bd06..bbabc3e 100644
--- a/mojo/services/network/url_loader_impl_apptest.cc
+++ b/mojo/services/network/url_loader_impl_unittest.cc
@@ -13,8 +13,8 @@
 #include "mojo/public/cpp/bindings/interface_request.h"
 #include "mojo/services/network/network_context.h"
 #include "mojo/services/network/url_loader_impl.h"
-#include "mojo/shell/public/cpp/application_test_base.h"
 #include "mojo/shell/public/cpp/message_loop_ref.h"
+#include "mojo/shell/public/cpp/shell_test.h"
 #include "net/base/net_errors.h"
 #include "net/url_request/url_request_job.h"
 #include "net/url_request/url_request_job_factory_impl.h"
@@ -107,23 +107,21 @@ class TestProtocolHandler : public net::URLRequestJobFactory::ProtocolHandler {
   base::Closure quit_closure_;
 };
 
-class UrlLoaderImplTest : public test::ApplicationTestBase {
+class UrlLoaderImplTest : public test::ShellTest {
  public:
-  UrlLoaderImplTest() : message_loop_(common::MessagePumpMojo::Create()) {}
+  UrlLoaderImplTest() : ShellTest("exe:network_service_unittests") {}
 
  protected:
-  bool ShouldCreateDefaultRunLoop() override {
-    return false;
-  }
-
   void SetUp() override {
-    ApplicationTestBase::SetUp();
+    ShellTest::SetUp();
+
+    wait_for_request_.reset(new base::RunLoop);
 
     scoped_ptr<net::TestURLRequestContext> url_request_context(
         new net::TestURLRequestContext(true));
     ASSERT_TRUE(url_request_job_factory_.SetProtocolHandler(
         "http", make_scoped_ptr(new TestProtocolHandler(
-            wait_for_request_.QuitClosure()))));
+            wait_for_request_->QuitClosure()))));
     url_request_context->set_job_factory(&url_request_job_factory_);
     url_request_context->Init();
     network_context_.reset(new NetworkContext(std::move(url_request_context)));
@@ -138,12 +136,11 @@ class UrlLoaderImplTest : public test::ApplicationTestBase {
     return network_context_->GetURLLoaderCountForTesting() > 0u;
   }
 
-  base::MessageLoop message_loop_;
   net::TestJobInterceptor* job_interceptor_;
   net::URLRequestJobFactoryImpl url_request_job_factory_;
   scoped_ptr<NetworkContext> network_context_;
   URLLoaderPtr url_loader_proxy_;
-  base::RunLoop wait_for_request_;
+  scoped_ptr<base::RunLoop> wait_for_request_;
 };
 
 TEST_F(UrlLoaderImplTest, ClosedBeforeAnyCall) {
@@ -160,7 +157,7 @@ TEST_F(UrlLoaderImplTest, ClosedWhileWaitingOnTheNetwork) {
   URLResponsePtr response;
   url_loader_proxy_->Start(std::move(request),
                            base::Bind(&PassA<URLResponsePtr>, &response));
-  wait_for_request_.Run();
+  wait_for_request_->Run();
 
   EXPECT_TRUE(IsUrlLoaderValid());
   EXPECT_FALSE(response);
@@ -191,7 +188,7 @@ TEST_F(UrlLoaderImplTest, ClosedWhileWaitingOnThePipeToBeWriteable) {
   URLResponsePtr response;
   url_loader_proxy_->Start(std::move(request),
                            base::Bind(&PassA<URLResponsePtr>, &response));
-  wait_for_request_.Run();
+  wait_for_request_->Run();
 
   EXPECT_TRUE(IsUrlLoaderValid());
   EXPECT_FALSE(response);
@@ -232,7 +229,7 @@ TEST_F(UrlLoaderImplTest, RequestCompleted) {
   URLResponsePtr response;
   url_loader_proxy_->Start(std::move(request),
                            base::Bind(&PassA<URLResponsePtr>, &response));
-  wait_for_request_.Run();
+  wait_for_request_->Run();
 
   EXPECT_TRUE(IsUrlLoaderValid());
   EXPECT_FALSE(response);
@@ -263,7 +260,7 @@ TEST_F(UrlLoaderImplTest, RequestFailed) {
   URLResponsePtr response;
   url_loader_proxy_->Start(std::move(request),
                            base::Bind(&PassA<URLResponsePtr>, &response));
-  wait_for_request_.Run();
+  wait_for_request_->Run();
 
   EXPECT_TRUE(IsUrlLoaderValid());
   EXPECT_FALSE(response);
diff --git a/mojo/services/tracing/public/cpp/trace_provider_impl.cc b/mojo/services/tracing/public/cpp/trace_provider_impl.cc
index 12842e5..ea6c8f4 100644
--- a/mojo/services/tracing/public/cpp/trace_provider_impl.cc
+++ b/mojo/services/tracing/public/cpp/trace_provider_impl.cc
@@ -19,7 +19,9 @@ namespace mojo {
 TraceProviderImpl::TraceProviderImpl()
     : binding_(this), tracing_forced_(false), weak_factory_(this) {}
 
-TraceProviderImpl::~TraceProviderImpl() {}
+TraceProviderImpl::~TraceProviderImpl() {
+  StopTracing();
+}
 
 void TraceProviderImpl::Bind(InterfaceRequest<tracing::TraceProvider> request) {
   if (!binding_.is_bound()) {
@@ -44,11 +46,12 @@ void TraceProviderImpl::StartTracing(const String& categories,
 }
 
 void TraceProviderImpl::StopTracing() {
-  DCHECK(recorder_);
-  base::trace_event::TraceLog::GetInstance()->SetDisabled();
+  if (recorder_) {
+    base::trace_event::TraceLog::GetInstance()->SetDisabled();
 
-  base::trace_event::TraceLog::GetInstance()->Flush(
-      base::Bind(&TraceProviderImpl::SendChunk, base::Unretained(this)));
+    base::trace_event::TraceLog::GetInstance()->Flush(
+        base::Bind(&TraceProviderImpl::SendChunk, base::Unretained(this)));
+  }
 }
 
 void TraceProviderImpl::ForceEnableTracing() {
diff --git a/mojo/shell/public/cpp/lib/shell_test.cc b/mojo/shell/public/cpp/lib/shell_test.cc
index 402d750..e5cb3c2 100644
--- a/mojo/shell/public/cpp/lib/shell_test.cc
+++ b/mojo/shell/public/cpp/lib/shell_test.cc
@@ -32,6 +32,10 @@ scoped_ptr<ShellClient> ShellTest::CreateShellClient() {
   return make_scoped_ptr(new ShellTestClient(this));
 }
 
+scoped_ptr<base::MessageLoop> ShellTest::CreateMessageLoop() {
+  return make_scoped_ptr(new base::MessageLoop);
+}
+
 void ShellTest::InitializeCalled(Connector* connector,
                                  const std::string& name,
                                  const std::string& user_id,
@@ -44,7 +48,7 @@ void ShellTest::InitializeCalled(Connector* connector,
 
 void ShellTest::SetUp() {
   shell_client_ = CreateShellClient();
-  message_loop_.reset(new base::MessageLoop);
+  message_loop_ = CreateMessageLoop();
   background_shell_.reset(new shell::BackgroundShell);
   background_shell_->Init(nullptr);
   shell_connection_.reset(new ShellConnection(
diff --git a/mojo/shell/public/cpp/shell_test.h b/mojo/shell/public/cpp/shell_test.h
index 9157b676..ee85d12 100644
--- a/mojo/shell/public/cpp/shell_test.h
+++ b/mojo/shell/public/cpp/shell_test.h
@@ -71,6 +71,8 @@ class ShellTest : public testing::Test {
   // work.
   virtual scoped_ptr<ShellClient> CreateShellClient();
 
+  virtual scoped_ptr<base::MessageLoop> CreateMessageLoop();
+
   // Call to set Initialize() metadata when GetShellClient() is overridden.
   void InitializeCalled(Connector* connector,
                         const std::string& name,
diff --git a/mojo/tools/data/apptests b/mojo/tools/data/apptests
index a8f0a35..6ffff1e 100644
--- a/mojo/tools/data/apptests
+++ b/mojo/tools/data/apptests
@@ -24,12 +24,6 @@
 # WARNING: If you add a test make sure the gn target mojo_apptests depends upon
 # it.
 tests = [
-    # TODO(rockot): Flaky. Failing with "Unrecognized --gtest_list_tests output"
-    # http://crbug.com/569367.
-#  {
-#    'test': 'mojo:network_service_apptests',
-#    'type': 'gtest_isolated',
-#  },
   {
     'test': 'mojo:resource_provider_apptests',
     'type': 'gtest_isolated',